[lld-macho] handle options -search_paths_first, -search_dylibs_first

[gkm]

• s/auto/opt::Arg/

[gkm]

• check args once and save result in Config::searchDylibsFirst
• split some long command lines in lld/test/MachO/link-search-order.s

[int3]

lld/MachO/Driver.cpp

107–111

at some point we should create a LinkerDriver class like in lld-ELF, then we can move driver-only options there

108

this should probably be an if instead of a return

lld/test/MachO/link-search-order.s

53–55

nit: align the right column

[gkm]

• Fix braino in findLibrary() (consecutive returns: first always shadows second)
• Refine tests to catch the braino

[gkm]

lld/test/MachO/link-search-order.s

53–55

I already submitted a major rework of the tests before seeing your comments. This open-source phabricator does not keep a history of diff revisions, so I can't see the original version on which you commented. Have a look at the latest and tell me if the right column still needs attention.

[int3]

This open-source phabricator does not keep a history of diff revisions

You can see previous diffs by clicking on the link in e.g. gkm updated this revision to Diff 293468.

lld/test/MachO/link-search-order.s

49–51

I was talking about aligning these

[thakis]

This seems to break tests everywhere, eg http://45.33.8.238/linux/28628/step_11.txt

Ptal, and revert if it takes a while to fix.

[gkm]

@thakis, the failure only affects incremental build & test. Everything is good on a clean tree. What do you advise now?

[thakis]

Sounds like that puts it in the "takes a while to figure out" category, so revert for now?

What exactly is the problem?

[int3]

@thakis I think @gkm's question is around whether incremental builds need to be supported, since the public LLVM buildbots all seem to test clean builds only.

[thakis]

All devs do incremental builds. Of course incremental builds should work.

I went ahead and fixed this in https://github.com/llvm/llvm-project/commit/0389eff4047a74bb1ba6c0603c9002b5c73b203e

Going forward, please don't leave stuff broken overnight.

[gkm]

In D88054#2292668, @thakis wrote:

All devs do incremental builds. Of course incremental builds should work.

I went ahead and fixed this in https://github.com/llvm/llvm-project/commit/0389eff4047a74bb1ba6c0603c9002b5c73b203e

Going forward, please don't leave stuff broken overnight.

Thanks very much for rescuing a n00b. I'll do better next time. 🤔

lld/MachO/Config.h

 };
 
 struct Configuration {
   Symbol *entry;
   bool hasReexports = false;
   bool allLoad = false;
   bool forceLoadObjC = false;
   bool staticLink = false;
+  bool searchDylibsFirst = false;
   uint32_t headerPad;
   llvm::StringRef installName;
   llvm::StringRef outputFile;
   llvm::MachO::Architecture arch;
   PlatformInfo platform;
   llvm::MachO::HeaderFileType outputType;
   std::vector<llvm::StringRef> systemLibraryRoots;
   std::vector<llvm::StringRef> librarySearchPaths;

lld/MachO/Driver.cpp

 }
 
 void MachOOptTable::printHelp(const char *argv0, bool showHidden) const {
   PrintHelp(lld::outs(), (std::string(argv0) + " [options] file...").c_str(),
             "LLVM Linker", showHidden);
   lld::outs() << "\n";
 }
 
-static Optional<std::string> findWithExtension(StringRef base,
-                                               ArrayRef<StringRef> extensions) {
+static Optional<std::string>
+findAlongPathsWithExtensions(StringRef name, ArrayRef<StringRef> extensions) {
+  llvm::SmallString<261> base;
+  for (StringRef dir : config->librarySearchPaths) {
+    base = dir;
+    path::append(base, Twine("lib") + name);
     for (StringRef ext : extensions) {
       Twine location = base + ext;
       if (fs::exists(location))
         return location.str();
     }
+  }
   return {};
 }
 
 static Optional<std::string> findLibrary(StringRef name) {
-  llvm::SmallString<261> location;
-  for (StringRef dir : config->librarySearchPaths) {
-      location = dir;
-      path::append(location, Twine("lib") + name);
-      if (Optional<std::string> path =
-              findWithExtension(location, {".tbd", ".dylib", ".a"}))
-        return path;
+  if (config->searchDylibsFirst) {
+            findAlongPathsWithExtensions(name, {".tbd", ".dylib"}))
+    return findAlongPathsWithExtensions(name, {".a"});
   }
-  return {};
+  return findAlongPathsWithExtensions(name, {".tbd", ".dylib", ".a"});
 }
 
 static Optional<std::string> findFramework(StringRef name) {
   llvm::SmallString<260> symlink;
   StringRef suffix;
   std::tie(name, suffix) = name.split(",");
   for (StringRef dir : config->frameworkSearchPaths) {
     symlink = dir;
if (roots.size() && roots.back() == "/")
     roots.clear();
   // NOTE: roots can never be empty - add an empty root to simplify the library
   // and framework search path computation.
   if (roots.empty())
     roots.emplace_back("");
 
   getLibrarySearchPaths(args, roots, config->librarySearchPaths);
   getFrameworkSearchPaths(args, roots, config->frameworkSearchPaths);
+  if (const opt::Arg *arg =
+          args.getLastArg(OPT_search_paths_first, OPT_search_dylibs_first))
+    config->searchDylibsFirst =
+        (arg && arg->getOption().getID() == OPT_search_dylibs_first);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
 
   if (args.hasArg(OPT_v)) {
     message(getLLDVersion());
     message(StringRef("Library search paths:") +
             (config->librarySearchPaths.size()
                  ? "\n\t" + llvm::join(config->librarySearchPaths, "\n\t")
                  : ""));
for (const auto &arg : args) {
     case OPT_install_name:
     case OPT_rpath:
     case OPT_sub_library:
     case OPT_Z:
     case OPT_arch:
     case OPT_syslibroot:
     case OPT_sectcreate:
     case OPT_dynamic:
+    case OPT_search_paths_first:
+    case OPT_search_dylibs_first:
       // handled elsewhere
       break;
     default:
       warnIfUnimplementedOption(opt);
       break;
     }
   }
 

lld/MachO/Options.td

def Z : Flag<["-"], "Z">,
      Group<grp_libs>;
 def syslibroot : Separate<["-"], "syslibroot">,
      MetaVarName<"<rootdir>">,
      HelpText<"Prepend <rootdir> to all library and framework search paths">,
      Flags<[HelpHidden]>,
      Group<grp_libs>;
 def search_paths_first : Flag<["-"], "search_paths_first">,
      HelpText<"Search for lib<name>.dylib and lib<name>.a at each step in traversing search path (default for Xcode 4 and later)">,
-     Flags<[HelpHidden]>,
      Group<grp_libs>;
 def search_dylibs_first : Flag<["-"], "search_dylibs_first">,
      HelpText<"Search for lib<name>.dylib on first pass, then for lib<name>.a on second pass through search path (default for Xcode 3 and earlier)">,
-     Flags<[HelpHidden]>,
      Group<grp_libs>;
 def framework : Separate<["-"], "framework">,
      MetaVarName<"<name>">,
      HelpText<"Search for <name>.framework/<name> on the framework search path">,
      Flags<[HelpHidden]>,
      Group<grp_libs>;
 def weak_framework : Separate<["-"], "weak_framework">,
      MetaVarName<"<name>">,

lld/test/MachO/link-search-order.s

 # REQUIRES: x86
 
-# RUN: mkdir -p %t
+# RUN: mkdir -p %t %tA %tD
 #
 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %p/Inputs/libhello.s -o %t/hello.o
 # RUN: lld -flavor darwinnew -dylib -install_name @executable_path/libhello.dylib %t/hello.o -o %t/libhello.dylib
 #
 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %p/Inputs/libgoodbye.s -o %t/goodbye.o
-# RUN: lld -flavor darwinnew -dylib -install_name @executable_path/libgoodbye.dylib %t/goodbye.o -o %t/libgoodbye.dylib
-# RUN: llvm-ar --format=darwin crs %t/libgoodbye.a %t/goodbye.o
+# RUN: lld -flavor darwinnew -dylib -install_name @executable_path/libgoodbye.dylib %t/goodbye.o -o %tD/libgoodbye.dylib
+#
+# RUN: llvm-ar --format=darwin crs %tA/libgoodbye.a %t/goodbye.o
 #
 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
-# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z -L%t -lhello -lgoodbye -lSystem %t/test.o
 #
-# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck %s
-
-# CHECK: @executable_path/libhello.dylib
-# CHECK: @executable_path/libgoodbye.dylib
-# CHECK: /usr/lib/libSystem.B.dylib
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tA -L%tD -L%t -lhello -lgoodbye -lSystem %t/test.o
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=ARCHIVE %s
+#
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tA -L%tD -L%t -lhello -lgoodbye -lSystem %t/test.o -search_paths_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=ARCHIVE %s
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tD -L%tA -L%t -lhello -lgoodbye -lSystem %t/test.o -search_paths_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=DYLIB %s
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tA       -L%t -lhello -lgoodbye -lSystem %t/test.o -search_paths_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=ARCHIVE %s
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tD      -L%t -lhello -lgoodbye -lSystem %t/test.o -search_paths_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=DYLIB %s
+#
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tA -L%tD -L%t -lhello -lgoodbye -lSystem %t/test.o -search_dylibs_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=DYLIB %s
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tD -L%tA -L%t -lhello -lgoodbye -lSystem %t/test.o -search_dylibs_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=DYLIB %s
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tA       -L%t -lhello -lgoodbye -lSystem %t/test.o -search_dylibs_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=ARCHIVE %s
+# RUN: lld -flavor darwinnew -L%S/Inputs/MacOSX.sdk/usr/lib -o %t/test -Z \
+# RUN:     -L%tD       -L%t -lhello -lgoodbye -lSystem %t/test.o -search_dylibs_first
+# RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=DYLIB %s
+
+# DYLIB: @executable_path/libhello.dylib
+# DYLIB: @executable_path/libgoodbye.dylib
+# DYLIB: /usr/lib/libSystem.B.dylib
+
+# ARCHIVE: @executable_path/libhello.dylib
+# ARCHIVE-NOT: @executable_path/libgoodbye.dylib
+# ARCHIVE: /usr/lib/libSystem.B.dylib
 
 .section __TEXT,__text
 .global _main
 
 _main:
   movl $0x2000004, %eax                         # write()
   mov $1, %rdi                                  # stdout
   movq _hello_world@GOTPCREL(%rip), %rsi
   mov $13, %rdx                                 # length
   syscall
 
   movl $0x2000004, %eax                         # write()