[PATCH] D85404: [lld-macho] Handle TAPI and regular re-exports uniformly

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/test/MachO/reexport-stub.s:16
+# CHECK: Bind table:
+# CHECK-DAG: __DATA __data {{.*}} pointer 0 libreexporter ___gxx_personality_v0
+
----------------
smeenai wrote:
> int3 wrote:
> > smeenai wrote:
> > > int3 wrote:
> > > > smeenai wrote:
> > > > > I thought that for sub-libraries, the bind information reflected the actual sub-library the symbol was coming from, and that seems to be the case in a basic test I did with ld64. Can you confirm how this is supposed to behave?
> > > > > 
> > > > > (IIRC there's options to re-export specific symbols, and those bind to the re-exporting library.)
> > > > Swapping ld64 for lld in this particular test gives the same result. (sub-library.s tests something similar, and ld64 binds to the parent library there too.) Can you share the test you did that showed a binding to the sub-library?
> > > I'm using ld64 530 from Xcode 11.3.1. If I try to run this test as-is (albeit with clang instead of llvm-mc), when I try to link `test`, I get an error:
> > > 
> > >   ld: malformed mach-o, symbol table not in __LINKEDIT file './libreexporter.dylib'
> > > 
> > > I can just add a dummy symbol to work around that, and then
> > > 
> > > ```
> > > $ cat reexporter.s
> > > .globl foo
> > > foo:
> > > 	ret
> > > 
> > > $ cat test.s
> > > .globl _main
> > > _main:
> > > 	ret
> > > .data
> > > 	.quad	___gxx_personality_v0
> > > 
> > > $ clang -c reexporter.s test.s
> > > $ ld -dylib -lc++ -sub_library libc++ reexporter.o -o libreexporter.dylib
> > > $ ld -o test -lSystem -L. -lreexporter test.o
> > > $ objdump --bind test
> > > Bind table:
> > > segment  section            address    type       addend dylib            symbol
> > > __DATA   __data             0x100001000 pointer         0 libc++           ___gxx_personality_v0
> > > ```
> > hm, I must have messed up my first attempt, because I was able to see the binding to libc++ on my second try. But I figured out why sub-library.s doesn't repro the issue: bindings only get routed directly to re-exports if those re-exports have "public install names", i.e. if they live under `/usr/lib/` or if they are a top-level framework under `/System/Library/Frameworks/`. I couldn't find any code comments explaining why, but I'm guessing it's an optimization -- I suppose the libraries with "public install names" form a stable ABI, and the only reason they get re-exported by other libraries is to provide some convenience to the user (who doesn't need to pass in half a dozen linker flags). Avoiding unnecessary re-export processing at runtime is probably a performance win.
> > 
> > Addressing this seems somewhat orthogonal to this diff, so a TODO comment should suffice for now?
> Ah, interesting. And yup, a TODO sounds good.
I've added the TODO to the `DylibFile` ctor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85404/new/

https://reviews.llvm.org/D85404