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

Currently, it was delibrately impleneted to not handle this case, but as it has turnt out, we need this feature.
The concrete use case is
       `System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa` reexports
               /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit , which then rexports
                    /System/Library/PrivateFrameworks/UIFoundation.framework/Versions/A/UIFoundation

The current implemention uses a global currentTopLevelTapi, which is not reset until it finishes loading the whole tree.
This is a problem because if the top-level is set to Cocoa, then when we get to UIFoundation, it will try to find UIFoundation in the current top level, which is Cocoa and will not find it.

The right thing should be:
 - When loading a library from a TBD file, re-exports need to be looked up in the auxiliary documents within the same TBD.
 - When loading from an actual dylib, no additional TBD documents need to be examined.
 - In no case does a re-export mentioned in one TBD file need to be looked up in a document in an auxiliary document from a different TBD file

Differential Revision: https://reviews.llvm.org/D97438

Author: oontvoo

lld/MachO/Driver.h

@@ -15,6 +15,12 @@
 #include "llvm/Option/OptTable.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+namespace llvm {
+namespace MachO {
+class InterfaceFile;
+} // namespace MachO
+} // namespace llvm
+
 namespace lld {
 namespace macho {
 

lld/MachO/InputFiles.cpp

@@ -575,19 +575,16 @@ static Optional<DylibFile *> loadDylib(StringRef path, DylibFile *umbrella) {
 
 // TBD files are parsed into a series of TAPI documents (InterfaceFiles), with
 // the first document storing child pointers to the rest of them. When we are
-// processing a given TBD file, we store that top-level document here. When
-// processing re-exports, we search its children for potentially matching
-// documents in the same TBD file. Note that the children themselves don't
-// point to further documents, i.e. this is a two-level tree.
+// processing a given TBD file, we store that top-level document in
+// currentTopLevelTapi. When processing re-exports, we search its children for
+// potentially matching documents in the same TBD file. Note that the children
+// themselves don't point to further documents, i.e. this is a two-level tree.
 //
-// ld64 allows a TAPI re-export to reference documents nested within other TBD
-// files, but that seems like a strange design, so this is an intentional
-// deviation.
-const InterfaceFile *currentTopLevelTapi = nullptr;
-
 // Re-exports can either refer to on-disk files, or to documents within .tbd
 // files.
-static Optional<DylibFile *> findDylib(StringRef path, DylibFile *umbrella) {
+static Optional<DylibFile *>
+findDylib(StringRef path, DylibFile *umbrella,
+          const InterfaceFile *currentTopLevelTapi) {
   if (path::is_absolute(path, path::Style::posix))
     for (StringRef root : config->systemLibraryRoots)
       if (Optional<std::string> dylibPath =
@@ -631,8 +628,10 @@ static bool isImplicitlyLinked(StringRef path) {
   return false;
 }
 
-void loadReexport(StringRef path, DylibFile *umbrella) {
-  Optional<DylibFile *> reexport = findDylib(path, umbrella);
+void loadReexport(StringRef path, DylibFile *umbrella,
+                  const InterfaceFile *currentTopLevelTapi) {
+  Optional<DylibFile *> reexport =
+      findDylib(path, umbrella, currentTopLevelTapi);
   if (!reexport)
     error("unable to locate re-export with install name " + path);
   else if (isImplicitlyLinked(path))
@@ -690,7 +689,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
       const auto *c = reinterpret_cast<const dylib_command *>(cmd);
       StringRef reexportPath =
           reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
-      loadReexport(reexportPath, umbrella);
+      loadReexport(reexportPath, umbrella, nullptr);
     }
 
     // FIXME: What about LC_LOAD_UPWARD_DYLIB, LC_LAZY_LOAD_DYLIB,
@@ -701,7 +700,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
       const auto *c = reinterpret_cast<const dylib_command *>(cmd);
       StringRef dylibPath =
           reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
-      Optional<DylibFile *> dylib = findDylib(dylibPath, umbrella);
+      Optional<DylibFile *> dylib = findDylib(dylibPath, umbrella, nullptr);
       if (!dylib)
         error(Twine("unable to locate library '") + dylibPath +
               "' loaded from '" + toString(this) + "' for -flat_namespace");
@@ -758,17 +757,11 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
     }
   }
 
-  bool isTopLevelTapi = false;
-  if (currentTopLevelTapi == nullptr) {
-    currentTopLevelTapi = &interface;
-    isTopLevelTapi = true;
-  }
+  const InterfaceFile *topLevel =
+      interface.getParent() == nullptr ? &interface : interface.getParent();
 
   for (InterfaceFileRef intfRef : interface.reexportedLibraries())
-    loadReexport(intfRef.getInstallName(), umbrella);
-
-  if (isTopLevelTapi)
-    currentTopLevelTapi = nullptr;
+    loadReexport(intfRef.getInstallName(), umbrella, topLevel);
 }
 
 ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f)

lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd

@@ -0,0 +1,10 @@
+--- !tapi-tbd-v3
+archs:            [ i386, x86_64 ]
+uuids:            [ 'i386: 00000000-0000-0000-0000-000000000000', 'x86_64: 00000000-0000-0000-0000-000000000001' ]
+platform:         ios
+install-name:     '/usr/lib/libReexportSystem.dylib'
+exports:
+  - archs:      [ i386, x86_64 ]
+    re-exports: [ '/usr/lib/libSystem' ]
+    symbols:    [ __crashreporter_info__, _cache_create ]
+

lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd

@@ -6,8 +6,8 @@ install-name:     '/usr/lib/libSystem.dylib'
 current-version:  1281
 exports:
   - archs:      [ i386, x86_64 ]
-    re-exports: [ '/usr/lib/system/libcache.dylib' ]
-    symbols:    [ __crashreporter_info__ ]
+    re-exports: [ '/usr/lib/system/libcache.dylib', ]
+    symbols:    [ __crashreporter_info__, _cache_create ]
 --- !tapi-tbd-v3
 archs:            [ i386, x86_64 ]
 uuids:            [ 'i386: 00000000-0000-0000-0000-000000000002', 'x86_64: 00000000-0000-0000-0000-000000000003' ]

lld/test/MachO/reexport-nested-lib.s

@@ -0,0 +1,28 @@
+# REQUIRES: x86
+#
+# This tests that we can reference symbols from a dylib,
+# re-exported by a top-level tapi document, which itself is
+# re-exported by another top-level tapi document.
+#
+# RUN: rm -rf %t; mkdir -p %t
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
+# RUN: %lld -o %t/test -syslibroot %S/Inputs/iPhoneSimulator.sdk -lReexportSystem %t/test.o
+# RUN: llvm-objdump %t/test --macho --bind %t/test | FileCheck %s
+
+# CHECK: segment  section   address            type        addend  dylib               symbol
+# CHECK: __DATA   __data    0x{{[0-9a-f]*}}    pointer     0       libReexportSystem __crashreporter_info__
+# CHECK: __DATA   __data    0x{{[0-9a-f]*}}    pointer     0       libReexportSystem _cache_create
+
+.text
+.globl _main
+
+_main:
+  ret
+
+.data
+// This symbol is from libSystem, which is re-exported by libReexportSystem.
+// Reference it here to verify that it is visible.
+.quad __crashreporter_info__
+
+// This symbol is from /usr/lib/system/libcache.dylib, which is re-exported in libSystem.
+.quad _cache_create

llvm/include/llvm/TextAPI/MachO/InterfaceFile.h

@@ -338,6 +338,9 @@ class InterfaceFile {
   ///\param Document The library to inline with top level library.
   void addDocument(std::shared_ptr<InterfaceFile> &&Document);
 
+  /// Returns the pointer to parent document if exists or nullptr otherwise.
+  InterfaceFile *getParent() const { return Parent; }
+
   /// Get the list of inlined libraries.
   ///
   /// \return Returns a list of the inlined frameworks.
@@ -431,6 +434,7 @@ class InterfaceFile {
   std::vector<std::shared_ptr<InterfaceFile>> Documents;
   std::vector<std::pair<Target, std::string>> UUIDs;
   SymbolMapType Symbols;
+  InterfaceFile *Parent = nullptr;
 };
 
 template <typename DerivedT, typename KeyInfoT, typename BucketT>

llvm/lib/TextAPI/MachO/InterfaceFile.cpp

@@ -124,6 +124,7 @@ void InterfaceFile::addDocument(std::shared_ptr<InterfaceFile> &&Document) {
                                   const std::shared_ptr<InterfaceFile> &RHS) {
                                  return LHS->InstallName < RHS->InstallName;
                                });
+  Document->Parent = this;
   Documents.insert(Pos, Document);
 }