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

Fri Feb 26 10:55:08 PST 2021

int3 added a comment.

Like the bundle_loader diff, this one needs more motivating detail in the commit message. It doesn't help that the Cocoa / AppKit libraries on my system don't include TBDs (perhaps because I'm not yet on Big Sur), so I'm not entirely sure what your commit message example is supposed to show. (And we should not assume that future readers will have a local system that matches yours, either.)

@jyknight's comment helps clarify things a bit, but I'm still not sure why we need a parent pointer instead of the global `currentTopLevelTapi` pointer that we currently have. Is it because we might have a TBD file with child documents that re-exports another TBD file, also with child documents?

================
Comment at: lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd:7-9
+reexported-libraries:
+  - archs:      [i386, x86_64]
+    libraries:  ['/usr/lib/system/libsystem_sim_pthread.dylib']
----------------
how is this different from the `re-exports` list on line 12 below?

(Scanning through TextStub.cpp, it seems like `reexported-libraries` is a TBD v4 thing, but this file is TBD v3, so I'm not sure it's doing anything. Are the system TBDs on your machine v4?)

================
Comment at: lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd:63
+  - archs:      [ i386, x86_64 ]
+    symbols:    [ _from_second_level_dylib ]
+
----------------
nit: I would prefer we keep libSystem.tbd looking like a subset of the real libSystem, i.e. ideally this symbol would be something that's actually defined in libsystem_sim_pthread_host

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97438