Add missing visibility annotation for __base

[yunlian]

Summary

This adds missing visibility annotation for __base.

[mclow.lists]

Sorry - what problem is this solving?

[pcc]

This change ensures that __base receives public LTO visibilty:
https://clang.llvm.org/docs/LTOVisibility.html
if a translation unit is compiled with -fvisibility=hidden and without _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS defined (i.e. dynamically linking against libc++).

What that means is that the control flow integrity and whole-program devirtualization features are disabled on the virtual calls on __base that are used to implement operator() on std::function. Enabling those features would be incorrect because libc++ is being linked dynamically and therefore the compiler does not have full visibility of all derived classes of __base and the calls cannot be devirtualized.

[manojgupta]

Hi Peter and Marshall,

Yunlian has moved to a different project. Can you let me know what is missing in this patch so that it can be submitted.

[ldionne]

What I don't understand is under which circumstances this changes anything, since we don't export __base from the dylib, and implicit instantiations of __base don't cause it to be exported. Can you please provide a sample program where this changes what's exported, or the LTO visibility you're mentioning?

[ldionne]

More specifically, the following program always causes the vtable for __base<int(float)> to be internal:

cat <<EOF | clang++ -xc++ - -std=c++11 && nm --demangle a.out
#include <functional>

int go(float) { return 0; }
std::function<int(float)> foo() { return go; }

int main() { foo()(3.3f); }
EOF

That's true with or without this patch, and with or without -fvisibility=hidden. We always get:

...
                 U vtable for __cxxabiv1::__si_class_type_info
0000000100003178 s vtable for std::__1::__function::__base<int (float)>
00000001000030f0 s vtable for std::__1::__function::__func<int (*)(float), std::__1::allocator<int (*)(float)>, int (float)>
0000000100003218 s vtable for std::__1::bad_function_call
...

[pcc]

Here's a standalone reproducer for the problem:

$ cat exe.cpp
#include <functional>

std::function<void()> f();

int main() {
  f()();
}
$ cat dso.cpp
#include <functional>

__attribute__((visibility("default"))) std::function<void()> f() {
  return [](){};
}
$ ~/l3/ra-cmake/bin/clang++ -fsanitize=cfi -fvisibility=hidden dso.cpp -shared -flto -fuse-ld=lld -Wl,-soname,dso.so -o dso.so -stdlib=libc++ -fno-rtti -fno-exceptions
$ ~/l3/ra-cmake/bin/clang++ -fsanitize=cfi -fvisibility=hidden exe.cpp  -flto -fuse-ld=lld dso.so -stdlib=libc++
$ LD_LIBRARY_PATH=.:$HOME/l3/ra-cmake/lib ./a.out 
Illegal instruction

With this patch:

$ LD_LIBRARY_PATH=.:$HOME/l3/ra-cmake/lib ./a.out 
[no output]

[manojgupta]

@ldionne Does Peter's example answer your questions?

[EricWF]

You have an ODR violation. You can't compile one TU with -fno-rtti -fno-exceptions and another without. You give the definitions of __base different vtables.

[pcc]

The same problem occurs whether or not exe.cpp is compiled with`-fno-exceptions -fno-rtti`.

[manojgupta]

@pcc can you please submit this patch if there are no objections?

[pcc]

I think it would be up to the libc++ maintainers to approve the patch.

[ldionne]

@pcc In your reproducer, what is ~/l3/ra-cmake/bin/clang++? Are you able to reproduce without -fuse-ld=lld? I'm trying to reproduce locally but I can't.

I know this is a small change, but I'd like to understand why it's needed since I suspect it may uncover other places where visibility is wrong -- if the problem is really on libc++'s side.

[pcc]

In D48680#1587967, @ldionne wrote:

@pcc In your reproducer, what is ~/l3/ra-cmake/bin/clang++?

That's just clang built from trunk at the time that I posted my comment.

Are you able to reproduce without -fuse-ld=lld? I'm trying to reproduce locally but I can't.

On which platform? On Linux lld (or gold) is needed in order to use LTO which is a requirement for CFI.

[manojgupta]

Friendly ping @pcc and @ldionne . We have been carrying this patch I think for too long now.
Also, we have not discovered any LTO issues elsewhere so can't tell from our side if there are other places in libc++ that need visibility annotations.

[manojgupta]

Thanks a lot!

libcxx/include/functional

public:
   static void __destroy_and_delete(__default_alloc_func* __f) {
     __f->destroy();
       __builtin_new_allocator::__deallocate_type<__default_alloc_func>(__f, 1);
   }
 };
 
 // __base provides an abstract interface for copyable functors.
 
-template<class _Fp> class __base;
+template<class _Fp> class _LIBCPP_TEMPLATE_VIS __base;
 
 template<class _Rp, class ..._ArgTypes>
 class __base<_Rp(_ArgTypes...)>
 {
     __base(const __base&);
     __base& operator=(const __base&);
 public:
     _LIBCPP_INLINE_VISIBILITY __base() {}