Page MenuHomePhabricator

[libc++] Always build with -fvisibility=hidden
ClosedPublic

Authored by ldionne on Jun 4 2019, 10:37 AM.

Details

Summary

This avoids symbols being accidentally exported from the dylib when they
shouldn't. The next step is to use a pragma to apply hidden visibility
to all declarations (unless otherwise specified), which will allow us
to drop the per-declaration hidden visibility attributes we currently
have.

This also has the nice side effect of making sure the dylib exports the
same symbols regardless of the optimization level.

PR38138

Event Timeline

ldionne created this revision.Jun 4 2019, 10:37 AM
ldionne added a subscriber: phosek.Jun 4 2019, 10:40 AM

@phosek You may want to take a look too.

This is in response to https://bugs.llvm.org/show_bug.cgi?id=38138, correct?

Yup. I have a note in the commit message.

mclow.lists accepted this revision.Jun 5 2019, 8:55 AM

I'm fine with this.

This revision is now accepted and ready to land.Jun 5 2019, 8:55 AM

I'll ship this next week once I've tested it on a Linux Docker image (I don't have access to that right now). I'm not sure, but I think I remember trying to make this change a while back and breaking the bots.

I finally did the test on a docker image mirroring the build bot configuration, and it doesn't appear to break anything. I'll submit this now.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 8:05 AM

Apologies for the late notice, we've just moved one our builders http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux-noexceptions to use gcc rather than clang and we've just picked up some test failures due to this change. In summary when compiling libc++ with GCC the shared object defines a symbol as hidden that clang does not which is causing test failures due to linker errors. I've put the details in https://bugs.llvm.org/show_bug.cgi?id=43140 . It looks like the failure was first seen on http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11/builds/345 (not run by us).

Apologies for the late notice, we've just moved one our builders http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux-noexceptions to use gcc rather than clang and we've just picked up some test failures due to this change. In summary when compiling libc++ with GCC the shared object defines a symbol as hidden that clang does not which is causing test failures due to linker errors. I've put the details in https://bugs.llvm.org/show_bug.cgi?id=43140 . It looks like the failure was first seen on http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11/builds/345 (not run by us).

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Ping. What's the progress? Unless I'm mistaken, all libc++ failures on our buildbot are related to this: http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22010

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Ping. What's the progress? Unless I'm mistaken, all libc++ failures on our buildbot are related to this: http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22010

https://reviews.llvm.org/D66970 would fix the issue, but I'm waiting on a review. I'll check-in a minimal fix to the problem in the meantime.

Quick update for watchers:

The bug mentioned by Peter has been fixed now, but I'm looking into other link errors when using GCC (because there's more).

Ping. What's the progress? Unless I'm mistaken, all libc++ failures on our buildbot are related to this: http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22010

https://reviews.llvm.org/D66970 would fix the issue, but I'm waiting on a review. I'll check-in a minimal fix to the problem in the meantime.

https://reviews.llvm.org/rCXX370926 should fix the problem

Thanks. It seems to fix the test I used to bisect this. It will take some time before buildbot confirms this but I'm going to presume it's good now. If it fails more, I'll ping you.