[lld/mac] When loading reexports, look for basename in -F / -L first

[thakis]

Summary

Matches ld64 (cf Options::findIndirectDylib()), and fixes PR51218.

[thakis]

tweak test comments

[thakis]

actually pipe to FileCheck and fix issues this identified :|

[thakis]

I patched this in on my linux box and the test passes fine there. Maybe the presubmit bot built the wrong version of the patch? The output for the 2nd build looks like the one for the 1st build.

[int3]

lld/test/MachO/sub-library.s

97

should we test -reexport_framework as well?

[int3]

lld/test/MachO/sub-library.s

97

nvm, I realize that we are testing the lookup for LC_REEXPORT_DYLIB, not the option... ignore that

[thakis]

Thanks!

lld/MachO/InputFiles.cpp

 // 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.
 //
 // Re-exports can either refer to on-disk files, or to documents within .tbd
 // files.
 static DylibFile *findDylib(StringRef path, DylibFile *umbrella,
                             const InterfaceFile *currentTopLevelTapi) {
+  // Search order:
+  // 1. Install name basename in -F / -L directories.
+  {
+    StringRef basename = path::filename(path);
+    SmallString<128> frameworkName;
+    path::append(frameworkName, basename + ".framework", basename);
+    bool isFramework = path.endswith(frameworkName);
+    if (isFramework) {
+      for (StringRef dir : config->frameworkSearchPaths) {
+        SmallString<128> candidate = dir;
+        path::append(candidate, frameworkName);
+        if (Optional<std::string> dylibPath = resolveDylibPath(candidate))
+          return loadDylib(*dylibPath, umbrella);
+      }
+    } else if (Optional<StringRef> dylibPath = findPathCombination(
+                   basename, config->librarySearchPaths, {".tbd", ".dylib"}))
+      return loadDylib(*dylibPath, umbrella);
+  }
+
+  // 2. As absolute path.
   if (path::is_absolute(path, path::Style::posix))
     for (StringRef root : config->systemLibraryRoots)
       if (Optional<std::string> dylibPath =
               resolveDylibPath((root + path).str()))
         return loadDylib(*dylibPath, umbrella);
 
+  // 3. As relative path.
+
   // TODO: Handle -dylib_file
 
+  // Replace @executable_path, @loader_path, @rpath prefixes in install name.
   SmallString<128> newPath;
   if (config->outputType == MH_EXECUTE &&
       path.consume_front("@executable_path/")) {
     // ld64 allows overriding this with the undocumented flag -executable_path.
     // lld doesn't currently implement that flag.
     // FIXME: Consider using finalOutput instead of outputFile.
     path::append(newPath, path::parent_path(config->outputFile), path);
     path = newPath;
for (StringRef rpath : umbrella->rpaths) {
         path::remove_filename(newPath);
       }
       path::append(newPath, rpath, path.drop_front(strlen("@rpath/")));
       if (Optional<std::string> dylibPath = resolveDylibPath(newPath))
         return loadDylib(*dylibPath, umbrella);
     }
   }
 
+  // FIXME: Should this be further up?
   if (currentTopLevelTapi) {
     for (InterfaceFile &child :
          make_pointee_range(currentTopLevelTapi->documents())) {
       assert(child.documents().empty());
       if (path == child.getInstallName()) {
         auto file = make<DylibFile>(child, umbrella);
         file->parseReexports(child);
         return file;

lld/test/MachO/sub-library.s

 # RUN: llvm-otool -l %t/libgoodbye3.dylib | FileCheck %s \
 # RUN:   --check-prefix=REEXPORT-HEADERS -DPATH=%t/Hello.framework/Hello
 
 ## But it doesn't match .dylib extensions:
 # RUN: not %lld -dylib -L%t -sub_umbrella libhello -lhello %t/libgoodbye.o \
 # RUN:   -o %t/libgoodbye.dylib 2>&1 | FileCheck %s --check-prefix=MISSING-FRAMEWORK
 # MISSING-FRAMEWORK: error: -sub_umbrella libhello does not match a supplied dylib
 
+
+## Check that -F (but not -L) can override the search path in install_name for
+## frameworks.
+# RUN: mkdir -p %t/Hello2.framework
+# RUN: %lld -dylib %t/libhello.o \
+# RUN:   -install_name /path/to/Hello2.framework/Hello2 \
+# RUN:   -o %t/Hello2.framework/Hello2
+# RUN: %lld -dylib -o %t/libgoodbye4.dylib %t/libgoodbye.o \
+# RUN:   -reexport_library %t/Hello2.framework/Hello2
+# RUN: not %lld -lSystem -o %t/hello %t/libgoodbye4.dylib %t/sub-library.o 2>&1 \
+# RUN:   FileCheck %s --check-prefix=NOTFOUND
+# RUN: not %lld -lSystem -o %t/hello -L%t %t/libgoodbye4.dylib %t/sub-library.o 2>&1 \
+# RUN:   FileCheck %s --check-prefix=NOTFOUND
+# NOTFOUND: unable to locate re-export with install name /path/to/Hello2.framework/Hello2
+# RUN: %lld -lSystem -o %t/hello -F%t %t/libgoodbye4.dylib %t/sub-library.o
+
+## Check that -L (but not -F) can override the search path in install_name for
+## libraries.
+# RUN: %lld -dylib %t/libhello.o \
+# RUN:   -install_name /Users/thakis/to/libhello2.dylib \
+# RUN:   -o %t/libhello2.dylib
+# RUN: %lld -dylib -o %t/libgoodbye5.dylib %t/libgoodbye.o \
+# RUN:   -reexport_library %t/libhello2.dylib
+# RUN: not %lld -lSystem -o %t/hello %t/libgoodbye5.dylib %t/sub-library.o 2>&1 \
+# RUN:   FileCheck %s --check-prefix=NOTFOUND2
+# RUN: not %lld -lSystem -o %t/hello -F%t %t/libgoodbye5.dylib %t/sub-library.o 2>&1 \
+# RUN:   FileCheck %s --check-prefix=NOTFOUND2
+# NOTFOUND2: unable to locate re-export with install name /path/to/libhello2.dylib
+# RUN: %lld -lSystem -o %t/hello -L%t %t/libgoodbye5.dylib %t/sub-library.o
+
 .text
 .globl _main
 
 _main:
   movl $0x2000004, %eax # write() syscall
   mov $1, %rdi # stdout
   movq _hello_world@GOTPCREL(%rip), %rsi
   mov $13, %rdx # length of str