protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.65k stars 15.5k forks source link

Linker assertion with LTO enabled #17006

Closed CognitiveDisson closed 1 month ago

CognitiveDisson commented 5 months ago

What version of protobuf and what language are you using? Protobuf / protoc 25.3 Language: Objective-C The issue is also reproducible on the latest main.

What operating system (Linux, Windows, ...) and version? MacOS

What runtime / compiler are you using (e.g., python version or gcc version) Xcode 15.1.0 Bazel 7.1.1

What did you do?

  1. Use genrule to generate protobuf sources using protoc.
  2. Placed these sources into objc_library and linked them to the main app.
  3. Disabled ld_prime (new default linker in Xcode 15) with: build --linkopt=-ld_classic
  4. Enabled LTO with: build --copt=-flto=thin --linkopt=-flto=thin --linkopt=-Wl,-object_path_lto,lto.o
  5. Build ios_application

I will attempt to prepare an example project that reproduces the problem, although this might be challenging.

What did you expect to see No errors.

What did you see instead?

ld: warning: -objc_stubs_small is not yet supported, use -ld_classic to enable the optimization
0  0x10449fd60  __assert_rtn + 140
1  0x1044a43dc  mach_o::relocatable::ObjC1ClassSection<arm64>::unlabeledAtomName(mach_o::relocatable::Parser<arm64>&, unsigned long long) (.cold.1) + 0
2  0x1042eff0c  mach_o::relocatable::ObjC2ClassRefsSection<arm64>::targetClassName(mach_o::relocatable::Atom<arm64> const*, ld::IndirectBindingTable const&) const + 164
3  0x1042efd68  mach_o::relocatable::ObjC2ClassRefsSection<arm64>::contentHash(mach_o::relocatable::Atom<arm64> const*, ld::IndirectBindingTable const&) const + 12
4  0x1042ecd14  mach_o::relocatable::Atom<arm64>::contentHash(ld::IndirectBindingTable const&) const + 48
5  0x1043319bc  ld::tool::SymbolTable::findSlotForReferences(ld::Atom const*, ld::Atom const**) + 268
6  0x104331f78  ld::tool::SymbolTable::add(ld::Atom const&, Options::Treatment) + 192
7  0x10433b92c  ld::tool::Resolver::doAtom(ld::Atom const&) + 1096
8  0x10433faf8  ld::tool::Resolver::linkTimeOptimize() + 692
9  0x104340a64  ld::tool::Resolver::resolve() + 436
10  0x1042a00bc  main + 592
A linker snapshot was created at:
    /tmp/Spotify_bin_unstripped-2024-05-27-150522.ld-snapshot
ld: Assertion failed: (atom->fixupCount() == 1), function targetClassName, file macho_relocatable_file.cpp, line 6725.
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Error in child process '/usr/bin/xcrun'. 1

Anything else we should know about your project / environment

After investigating, we found that the issue only arises when LTO is enabled and after we switch to a new proto compiler version. Disabling LTO or switching to ld-prime (default new linker in Xcode 15 which we can’t currently use for other reasons) solves the problem. Through a process of bisecting commits, we identified a commit that is causing the problem: [Objc] Replace reference to message's own class when calling the GPBDescriptor initializer, using a direct linker reference instead of +class. We also tried building protoc version 25.3 with the reverted changes in src/google/protobuf/compiler/objectivec/message.cc, which resolved the issue. Our assumption is that "forward declaration of the class, as a linker symbol", is causing this issue, since LTO can strip these symbols as unused.

thomasvl commented 5 months ago

@dmaclach fyi

thomasvl commented 5 months ago

Is there a reason you have to use the old linker? My guess is Apple will eventually remove it, so the new one will have to be used.

CognitiveDisson commented 5 months ago

Is there a reason you have to use the old linker? My guess is Apple will eventually remove it, so the new one will have to be used.

@thomasvl We see a significant increase in binary size with ld_prime, even with all the size optimizations. We hope Apple improves this in future versions before the old linker is removed.

github-actions[bot] commented 2 months ago

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. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] commented 1 month ago

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.