Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apple ld: "direct access in function ... to global weak symbol" #8958

Closed
daschuer opened this issue Sep 8, 2021 · 16 comments
Closed

Apple ld: "direct access in function ... to global weak symbol" #8958

daschuer opened this issue Sep 8, 2021 · 16 comments
Assignees
Labels
c++ inactive Denotes the issue/PR has not seen activity in the last 90 days.

Comments

@daschuer
Copy link

daschuer commented Sep 8, 2021

What version of protobuf and what language are you using?
Version: V3.15.8
Language: C++

What operating system (Linux, Windows, ...) and version?
macOS Catalina 10.15

What runtime / compiler are you using (e.g., python version or gcc version)
GNU 11.2.0

What did you do?
Build mixxx on the GitHub workflows using vcpck.
https://github.com/mixxxdj/mixxx/pull/4225/checks?check_run_id=3403021507
Protobuf is linked as a static library.

What did you expect to see
No warning

What did you see instead?

ld: warning: direct access in function 'std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >* google::protobuf::internal::InternalMetadata::mutable_unknown_fields_slow<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >()' from file 'libmixxx-lib.a(keys.pb.cc.o)' to global weak symbol 'void google::protobuf::internal::arena_destruct_object<google::protobuf::internal::InternalMetadata::Container<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >(void*)' from file '/Users/runner/buildenv/mixxx-deps-2.4-x64-osx-0f25dfd/installed/x64-osx/lib/libprotobuf-lite.a(message_lite.cc.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
ld: warning: direct access in function 'std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >* google::protobuf::internal::InternalMetadata::mutable_unknown_fields_slow<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >()' from file 'libmixxx-lib.a(keys.pb.cc.o)' to global weak symbol 'typeinfo for google::protobuf::internal::InternalMetadata::Container<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >' from file '/Users/runner/buildenv/mixxx-deps-2.4-x64-osx-0f25dfd/installed/x64-osx/lib/libprotobuf-lite.a(message_lite.cc.o)' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

Anything else we should know about your project / environment
The workaround is to call cmake with these flags:

    -DCMAKE_POLICY_DEFAULT_CMP0063=NEW
    -DCMAKE_C_VISIBILITY_PRESET=hidden
    -DCMAKE_CXX_VISIBILITY_PRESET=hidden
    -DCMAKE_VISIBILITY_INLINES_HIDDEN=TRUE

I have proposed a fix here microsoft/vcpkg#19908 but I was forwarded to check if the issue can be solved directly.

The flag is already used in the Ruby module, here:

$CFLAGS += " -std=gnu99 -O3 -DNDEBUG -fvisibility=hidden -Wall -Wsign-compare -Wno-declaration-after-statement"

@elharo elharo added the c++ label Sep 8, 2021
@haberman
Copy link
Member

haberman commented Sep 9, 2021

If I am understanding correctly, your proposed flags would make the C++ library compile with -fvisibility=hidden under CMake.

Have you tested with this configuration? Can you verify that the test suite and your own programs work under this flag?

I ask because -fvisibility=hidden will require that all symbols intended to be exported are properly marked as PROTOBUF_EXPORT (which will do __attribute__((visibility("default")))).

We should probably be in good shape here, since Windows depends on proper PROTOBUF_EXPORT annotations for DLL support. But it would be good to have confirmation that this is working properly.

@daschuer
Copy link
Author

daschuer commented Sep 9, 2021

I can confirm that Mixxx builds and runs with this configuration.
Unfortunately I cannot do more testing, because I have no access to a MacOs machine.
Do you expect issues at runtime or should be all issue be evealed at link time?

@Be-ing
Copy link

Be-ing commented Sep 13, 2021

I have been using local builds of Mixxx with this protobuf configuration on macOS as well as builds from GitHub Actions. I have not identified any problems from this.

@daschuer
Copy link
Author

microsoft/vcpkg#19908
Is the alternative solution in the vcpkg domain to work around the issue. Do you recommend to merge that, or will we have a solution upstream anytime soon?

@haberman
Copy link
Member

I would support adding -DCMAKE_C_VISIBILITY_PRESET=hidden, -DCMAKE_CXX_VISIBILITY_PRESET=hidden, etc. to CMake.

If/when we are supporting a build of libprotobuf.so from Bazel, I would favor adding it there too.

@BillyONeal
Copy link

I would support adding -DCMAKE_C_VISIBILITY_PRESET=hidden, -DCMAKE_CXX_VISIBILITY_PRESET=hidden, etc. to CMake.

@haberman Can you confirm that vcpkg should merge the PR indicated above then? We want to avoid merging changes in the 'exposed interface' of a thing without the nominal owners of the thing OKing wherever possible:

microsoft/vcpkg#19908 Is the alternative solution in the vcpkg domain to work around the issue. Do you recommend to merge that, or will we have a solution upstream anytime soon?

We can of course back that out if/when a change is adopted in protocol buffers' own CMakeLists.txt.

@daschuer
Copy link
Author

daschuer commented Apr 7, 2022

@haberman Please have a look at the question above. This let us make progress in the vcpkg repro. Thanks.

@haberman
Copy link
Member

haberman commented Apr 7, 2022

I would prefer to make this change in our own CMake files, so it's tested in our CI.

If we patch -DCMAKE_C_VISIBILITY_PRESET=hidden via vcpkg, then we have no visibility in our own CI of whether it is working, or if one of our changes is breaking it.

@daschuer
Copy link
Author

daschuer commented Apr 7, 2022

This bug is open since 8 Sep 2021, will you take care about it?

@haberman
Copy link
Member

haberman commented Apr 7, 2022

We rely on users to contribute fixes to system-specific problems like this.

We have many users across many different systems. We don't have the bandwidth or the background to drive fixes to a problem like this that so far has only surfaced with one package manager.

Please feel free to send a PR to add this flag to our CMakeLists.txt.

daschuer added a commit to daschuer/protobuf that referenced this issue Apr 8, 2022
…e builds

This avoids the warning:
"ld: direct access in function ... to global weak symbol"
when building with Apples clang

Fixes protocolbuffers#8958
@daschuer
Copy link
Author

daschuer commented Apr 8, 2022

Done.

@hyc
Copy link

hyc commented May 6, 2022

I have a similar issue

What version of protobuf and what language are you using?
Version: V3.6.1
Language: C++

What operating system (Linux, Windows, ...) and version?
Linux, cross compiling with MacOS SDK 11.1 to aarch64

What runtime / compiler are you using (e.g., python version or gcc version)
clang 9.0

What did you do?
Build monero github.com/monero-project/monero
Protobuf is linked as a static library.

What did you expect to see
No warning

What did you see instead?
Tons of warnings
warns.txt

Recompiling the entire library with -fvisibility=hidden removes some warnings but makes others appear instead. Selectively compiling just a few files with hidden gets me down to just a few warnings.
warn2.txt

I used this patch to selectively set hidden visibility on some files
visib_patch.txt

I couldn't find any way to eliminate those last warnings.

Is this a compiler bug, is this version of clang too old?

@daschuer
Copy link
Author

daschuer commented May 6, 2022

My attempt in #9768
fails on
cmake .. -G Ninja -DBUILD_SHARED_LIBS=1 $ ninja
Does your patch work with this?

@hyc
Copy link

hyc commented May 6, 2022

I can't test that, I'm using autoconf. Don't have ninja.

Copy link

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

@github-actions github-actions bot added the inactive Denotes the issue/PR has not seen activity in the last 90 days. label Feb 17, 2024
Copy link

github-actions bot commented Mar 2, 2024

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ inactive Denotes the issue/PR has not seen activity in the last 90 days.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants