[lld-macho] Fix & fold reexport-nested-libs test into stub-link.s

[int3]

Summary

The reexport-nested-libs test added in D97438 was a bit wonky.

First, it was linking against libReexportSystem.tbd which targets the
iOS simulator, and which in turn attempted to re-export the iOS
simulator's libSystem. However, due to the way -syslibroot works, it
was actually re-exporting the macOS libSystem.

As a result, the test was not actually able to resolve the symbols in
the desired libSystem. I'm guessing that @oontvoo was confused by this
and therefore included those symbols in libReexportSystem.tbd itself.
But this means that the test wasn't actually testing the resolution of
re-exported symbols (though it did at least verify that the re-exported
libraries could be located).

After some consideration, I figured that stub-link.s could be extended
to cover what reexport-nested-libs.s was attempting to do. The test
targets macOS, so we only have one -syslibroot and no chance of
confusion.

[int3]

I've added a cautionary comment about -syslibroot in D97799: [lld-macho] Require -arch and -platform_version to always be specified. D97867: [lld-macho] Filter TAPI re-exports by target will also help to flag future such issues since we'll now check the platform of the re-exports we are attempting to link against.

lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd

14

ld64's TAPI parser is more picky than ours, so it doesn't recognize a file as valid TAPI without these dots

[oontvoo]

lld/test/MachO/Inputs/libReexportSystem.tbd

8–9

It wasn't able to find the libSystem with .dylib (because it was trying to looking for a .tbd extension.
I'm curious how your change fixed it

[oontvoo]

LGTM (with just one question)

lld/test/MachO/stub-link.s

13

Would the test that use -lReexportSystem (rather than explicitly specifying it as input) follow any different code path?

[int3]

lld/test/MachO/Inputs/libReexportSystem.tbd

8–9

hmm I'm not sure why it didn't work for you. But if resolveDylibPath doesn't find a file ending in .dylib, it will replace .dylib with .tbd and try to find that.

[int3]

lld/test/MachO/stub-link.s

13

it wouldn't cause findLibrary to be invoked for libReexportSystem, but we have other tests that use -lFoo, so that code path is covered

lld/MachO/InputFiles.cpp

for (StringRef root : config->systemLibraryRoots)
               resolveDylibPath((root + path).str()))
         return loadDylib(*dylibPath, umbrella);
 
   // TODO: Expand @loader_path, @executable_path, @rpath etc, handle -dylib_path
 
   if (currentTopLevelTapi) {
     for (InterfaceFile &child :
          make_pointee_range(currentTopLevelTapi->documents())) {
+      assert(child.documents().empty());
       if (path == child.getInstallName())
         return make<DylibFile>(child, umbrella);
-      assert(child.documents().empty());
     }
   }
 
   if (Optional<std::string> dylibPath = resolveDylibPath(path))
     return loadDylib(*dylibPath, umbrella);
 
   return {};
 }

lld/test/MachO/Inputs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation.tbd

 --- !tapi-tbd-v3
 archs:            [ x86_64 ]
 uuids:            [ 'x86_64: 00000000-0000-0000-0000-000000000000' ]
 platform:         macosx
 install-name:     '/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation'
 current-version:  0001.001.1
 compatibility-version: 150
 exports:
   - archs:         [ 'x86_64' ]
     symbols:       [ '__CFBigNumGetInt128' ]
     objc-classes:  [ NSObject ]
     objc-ivars:    [ NSConstantArray._count ]
     objc-eh-types: [ NSException ]
+...

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

 --- !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/libSystem.dylib'
 current-version:  1281
 exports:
   - archs:      [ i386, x86_64 ]
-    re-exports: [ '/usr/lib/system/libcache.dylib', ]
+    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' ]
 platform:         ios
 install-name:     '/usr/lib/system/libcache.dylib'
 current-version:  83
 parent-umbrella:  System

lld/test/MachO/Inputs/libReexportSystem.tbd

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

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

-# 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

lld/test/MachO/stub-link.s

 # REQUIRES: x86
 
 # RUN: mkdir -p %t
 #
 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
+
 # RUN: %lld -o %t/test -lSystem -lc++ -framework CoreFoundation %t/test.o
-#
 # RUN: llvm-objdump --bind --no-show-raw-insn -d -r %t/test | FileCheck %s
 
+## libReexportSystem.tbd 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: %lld -o %t/with-reexport %S/Inputs/libReexportSystem.tbd -lc++ -framework CoreFoundation %t/test.o
+# RUN: llvm-objdump --bind --no-show-raw-insn -d -r %t/with-reexport | FileCheck %s
+
 # CHECK: Disassembly of section __TEXT,__text:
 # CHECK: movq {{.*}} # [[ADDR:[0-9a-f]+]]
 
 # CHECK: Bind table:
 # CHECK-DAG: __DATA_CONST __got 0x[[ADDR]] pointer 0 libSystem ___nan
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_CLASS_$_NSObject
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_METACLASS_$_NSObject
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_IVAR_$_NSConstantArray._count
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 CoreFoundation _OBJC_EHTYPE_$_NSException
 # CHECK-DAG: __DATA __data {{.*}} pointer 0 libc++abi      ___gxx_personality_v0
 
 # RUN: llvm-objdump --macho --all-headers %t/test | \
 # RUN:     FileCheck --check-prefix=LOAD %s
 
+# RUN: llvm-objdump --macho --all-headers %t/with-reexport | \
+# RUN:     FileCheck --check-prefixes=LOAD,LOAD-REEXPORT %s
+
 # LOAD:          cmd LC_LOAD_DYLIB
 # LOAD-NEXT:               cmdsize
 # LOAD-NEXT:                  name /usr/lib/libSystem.dylib
 # LOAD-NEXT:            time stamp
 # LOAD-NEXT:       current version 1.1.1
 # LOAD-NEXT: compatibility version
 
+# LOAD-REEXPORT:          cmd LC_LOAD_DYLIB
+# LOAD-REEXPORT-NEXT:               cmdsize
+# LOAD-REEXPORT-NEXT:                  name /usr/lib/libReexportSystem.dylib
+# LOAD-REEXPORT-NEXT:            time stamp
+# LOAD-REEXPORT-NEXT:       current version 1.0.0
+# LOAD-REEXPORT-NEXT: compatibility version
+
 .section __TEXT,__text
 .global _main
 
 _main:
 ## This symbol is defined in an inner TAPI document within libSystem.tbd.
   movq ___nan@GOTPCREL(%rip), %rax
   ret