[PATCH] D97438: [lld-macho] Change loadReexport to handle the case where a TAPI re-exports to reference documents nested within other TBD.

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/test/MachO/reexport-nested-lib.s:19-21
+// This symbol is from libSystem, which is re-exported by libReexportSystem.
+// Reference it here to verify that it is visible.
+.quad __crashreporter_info__
----------------
I don't think this is testing the right thing. (The test passes for me even without your code changes applied.)

I believe the case we are trying to fix is as follows:

* libFooBar.tbd contains libFoo.dylib and libBar.dylib. libFoo re-exports libBar. libBar contains some symbol `Bar`.
* libReexportsFoo.tbd contains libReexportsFoo.dylib which re-exports libFoo. It doesn't matter whether it re-exports anything else.

Prior to this fix, we would have failed to look up libBar because we would be looking inside libReexportsFoo.tbd, instead of libFoo.tbd.

Right now we are testing that a symbol in libFoo can be resolved by the linker, but we actually need to test for a symbol that's in a library that libFoo (libSystem in your case) re-exports. And FWIW, the fake macOS libSystem has `__nan` which should fit the bill :)

P.S. it would be good to have a comment that explains the motivation behind the test. Right now the comment talks about re-exports but doesn't touch on the TBD files that contain them, which I think is the key bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97438