[lld/mac] Print dylib search details with --print-dylib-search or RC_TRACE_DYLIB_SEARCHING

[thakis]

Summary

For debugging dylib loading, it's useful to have some insight into what
the linker is doing.

ld64 has the undocumented RC_TRACE_DYLIB_SEARCHING env var
for this printing dylib search candidates.

This adds a flag --print-dylib-search to make lld print the seame information.
It's useful for users, but also for writing tests. The output is formatted
slightly differently than ld64, but we still support RC_TRACE_DYLIB_SEARCHING
to offer at least a compatible way to trigger this.

ld64 has both -print_statistics and -trace_symbol_output to enable
diagnostics output. I went with "print" since that seems like a more
straightforward name.

[thakis]

clang-format

[alexander-shaposhnikov]

lld/MachO/DriverUtils.cpp

179–182

perhaps, logDylibSearchResult would be a better name ?
also - maybe use StringRef instead of Twine ?

192

dylibPath.str() ?

[thakis]

lld/MachO/DriverUtils.cpp

179–182

The last caller in this file passes a Twine, so StringRef doesn't work.

I like the shorter name.

192

This is just moving code around that was here previously. std::string(...) is how you convert a string view to a string in c++17, so I suppose this is consistent with that.

[alexander-shaposhnikov]

lld/MachO/DriverUtils.cpp

179–182

I think readability of the code matters, not sure if "searchedDylib" is a sufficiently descriptive name

192

quick grepping shows some minor inconsistency even inside lld/MachO. It doesn't change the "functional" side of the problem, but having a consistent code would definitely be a plus. I'm not against any of these methods, but wanted to point this out / so that at least the new code is written in a more consistent way.

[int3]

lld/MachO/DriverUtils.cpp

179–182

"logSearchedDylib"? I'm fine with anything, but I'd prefer not to have Java-length verbosity

[thakis]

Thanks!

lld/MachO/DriverUtils.cpp

179–182

Maybe didSearchDylib?

[int3]

lld/MachO/DriverUtils.cpp

179–182

eh, that has the same information as searchedDylib but is one character longer :p "log" describes what it's doing at least

[thakis]

try to make test pass on windows

lld/MachO/DriverUtils.cpp

179–182

Ah, naming :)

I don't love "logSearchedDylib". "log" is obviously a verb here, and it logs a "searched dylib", but what's a "searched dylib"? "searched" acts as an adjective here but isn't one and it sounds weird to me. (Then again, I'm not a native speaker – on the third hand, so are others who might read this.) "logDylibSearch" is a bit better but then it's not clear if we log the start or the end of the search. "searchedDylib" is ok since the verb is "search", it's in past tense so it's over, and "dylib" is an ok object. The drawback is that you can read "searched" as an adjective if you squint enough (cf "logSearchedDylib"), and. "didSearchDylib" addresses that.

logDylibSearchResult() feels a bit long compared to our other names.

I think the usual takeaway when you have a list of options where none is obviously the best is that the decision doesn't really matter all that much – especially if it's easy to change later on :) So I think I'll just leave this as is for now, and if anyone feels motivated enough to change it later, go for it.

lld/MachO/Config.h

struct Configuration {
   bool forceLoadObjC = false;
   bool forceLoadSwift = false;
   bool staticLink = false;
   bool implicitDylibs = false;
   bool isPic = false;
   bool headerPadMaxInstallNames = false;
   bool ltoNewPassManager = LLVM_ENABLE_NEW_PASS_MANAGER;
   bool markDeadStrippableDylib = false;
+  bool printDylibSearch = false;
   bool printEachFile = false;
   bool printWhyLoad = false;
   bool searchDylibsFirst = false;
   bool saveTemps = false;
   bool adhocCodesign = false;
   bool emitFunctionStarts = false;
   bool emitBitcodeBundle = false;
   bool emitEncryptionInfo = false;

lld/MachO/Driver.h

 void printArchiveMemberLoad(StringRef reason, const InputFile *);
 
 // Helper class to export dependency info.
 class DependencyTracker {
 public:
   explicit DependencyTracker(llvm::StringRef path);
 
   // Adds the given path to the set of not-found files.
-  inline void logFileNotFound(std::string path) {
-    if (active)
-      notFounds.insert(std::move(path));
-  }
-
   inline void logFileNotFound(const Twine &path) {
     if (active)
       notFounds.insert(path.str());
   }
 
   // Writes the dependencies to specified path.
   // The content is sorted by its Op Code, then within each section,
   // alphabetical order.

lld/MachO/Driver.cpp

for (const Arg *arg : args.filtered(OPT_U))
     symtab->addDynamicLookup(arg->getValue());
 
   config->mapFile = args.getLastArgValue(OPT_map);
   config->outputFile = args.getLastArgValue(OPT_o, "a.out");
   config->astPaths = args.getAllArgValues(OPT_add_ast_path);
   config->headerPad = args::getHex(args, OPT_headerpad, /*Default=*/32);
   config->headerPadMaxInstallNames =
       args.hasArg(OPT_headerpad_max_install_names);
+  config->printDylibSearch =
+      args.hasArg(OPT_print_dylib_search) || getenv("RC_TRACE_DYLIB_SEARCHING");
   config->printEachFile = args.hasArg(OPT_t);
   config->printWhyLoad = args.hasArg(OPT_why_load);
   config->outputType = getOutputType(args);
   if (const Arg *arg = args.getLastArg(OPT_bundle_loader)) {
     if (config->outputType != MH_BUNDLE)
       error("-bundle_loader can only be used with MachO bundle output");
     addFile(arg->getValue(), /*forceLoadArchive=*/false, /*isExplicit=*/false,
             /*isBundleLoader=*/true);

lld/MachO/DriverUtils.cpp

case OPT_sectcreate:
       break;
     default:
       os << toString(*arg) << "\n";
     }
   }
   return std::string(data.str());
 }
 
-Optional<std::string> macho::resolveDylibPath(StringRef path) {
-  // TODO: if a tbd and dylib are both present, we should check to make sure
-  // they are consistent.
-  if (fs::exists(path))
-    return std::string(path);
-  else
+static void searchedDylib(const Twine &path, bool found) {
+  if (config->printDylibSearch)
+    message("searched " + path + (found ? ", found " : ", not found"));
+  if (!found)
     depTracker->logFileNotFound(path);
+}
 
-  SmallString<261> location = path;
-  path::replace_extension(location, ".tbd");
-  if (fs::exists(location))
-    return std::string(location);
-  else
-    depTracker->logFileNotFound(location);
+Optional<std::string> macho::resolveDylibPath(StringRef dylibPath) {
+  // TODO: if a tbd and dylib are both present, we should check to make sure
+  // they are consistent.
+  bool dylibExists = fs::exists(dylibPath);
+  searchedDylib(dylibPath, dylibExists);
+  if (dylibExists)
+    return std::string(dylibPath);
+
+  SmallString<261> tbdPath = dylibPath;
+  path::replace_extension(tbdPath, ".tbd");
+  bool tbdExists = fs::exists(tbdPath);
+  searchedDylib(tbdPath, tbdExists);
+  if (tbdExists)
+    return std::string(tbdPath);
   return {};
 }
 
 // It's not uncommon to have multiple attempts to load a single dylib,
 // especially if it's a commonly re-exported core library.
 static DenseMap<CachedHashStringRef, DylibFile *> loadedDylibs;
 
 DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
macho::findPathCombination(const Twine &name,
                            const std::vector<StringRef> &roots,
                            ArrayRef<StringRef> extensions) {
   SmallString<261> base;
   for (StringRef dir : roots) {
     base = dir;
     path::append(base, name);
     for (StringRef ext : extensions) {
       Twine location = base + ext;
-      if (fs::exists(location))
+      bool exists = fs::exists(location);
+      searchedDylib(location, exists);
+      if (exists)
         return saver.save(location.str());
-      else
-        depTracker->logFileNotFound(location);
     }
   }
   return {};
 }
 
 StringRef macho::rerootPath(StringRef path) {
   if (!path::is_absolute(path, path::Style::posix) || path.endswith(".o"))
     return path;

lld/MachO/Options.td

def lto_legacy_pass_manager: Flag<["--"], "lto-legacy-pass-manager">,
     HelpText<"Use the legacy pass manager in LLVM">,
     Group<grp_lld>;
 def no_lto_legacy_pass_manager : Flag<["--"], "no-lto-legacy-pass-manager">,
     HelpText<"Use the new pass manager in LLVM">,
     Group<grp_lld>;
 def time_trace: Flag<["--"], "time-trace">, HelpText<"Record time trace">;
 def time_trace_granularity_eq: Joined<["--"], "time-trace-granularity=">,
     HelpText<"Minimum time granularity (in microseconds) traced by time profiler">;
-def time_trace_file_eq: Joined<["--"], "time-trace-file=">, HelpText<"Specify time trace output file">;
-def deduplicate_literals: Flag<["--"], "deduplicate-literals">, HelpText<"Enable literal deduplication">;
+def time_trace_file_eq: Joined<["--"], "time-trace-file=">,
+    HelpText<"Specify time trace output file">;
+def deduplicate_literals: Flag<["--"], "deduplicate-literals">,
+    HelpText<"Enable literal deduplication">;
+def print_dylib_search: Flag<["--"], "print-dylib-search">,
+    HelpText<"Print which paths lld searched when trying to find dylibs">;
 
 // This is a complete Options.td compiled from Apple's ld(1) manpage
 // dated 2018-03-07 and cross checked with ld64 source code in repo
 // https://github.com/apple-opensource/ld64 at git tag "512.4" dated
 // 2018-03-18.
 
 // Flags<[HelpHidden]> marks options that are not yet ported to lld,
 // and serve as a scoreboard for annotating our progress toward

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

 # RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %p/Inputs/libgoodbye.s -o %t/goodbye.o
 # RUN: %lld -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
 
 ################ default, which is the same as -search_paths_first
 # RUN: %lld -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:     -L%tA -L%tD -L%t -lhello -lgoodbye -lSystem %t/test.o \
+# RUN:     --print-dylib-search | FileCheck --check-prefix=ARCHIVESEARCH -DPATH=%t %s
 # RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=ARCHIVE %s
 
 ################ Test all permutations of -L%t{A,D} with -search_paths_first
 # RUN: %lld -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 -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 -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 -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
 
 ################ Test all permutations of -L%t{A,D} with -search_dylibs_first
-# RUN: %lld -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: env RC_TRACE_DYLIB_SEARCHING=1 %lld -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:     | FileCheck --check-prefix=DYLIBSEARCH -DPATH=%t %s
 # RUN: llvm-objdump --macho --dylibs-used %t/test | FileCheck --check-prefix=DYLIB %s
 # RUN: %lld -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 -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 -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.dylib
 
+# DYLIBSEARCH:      searched {{.*}}/MacOSX.sdk/usr/lib{{[/\\]}}libhello.tbd, not found
+# DYLIBSEARCH-NEXT: searched {{.*}}/MacOSX.sdk/usr/lib{{[/\\]}}libhello.dylib, not found
+# DYLIBSEARCH-NEXT: searched [[PATH]]A{{[/\\]}}libhello.tbd, not found
+# DYLIBSEARCH-NEXT: searched [[PATH]]A{{[/\\]}}libhello.dylib, not found
+# DYLIBSEARCH:      searched [[PATH]]{{[/\\]}}libhello.dylib, found
+# DYLIBSEARCH:      searched [[PATH]]D{{[/\\]}}libgoodbye.dylib, found
+
 # ARCHIVE:     @executable_path/libhello.dylib
 # ARCHIVE-NOT: @executable_path/libgoodbye.dylib
 # ARCHIVE:     /usr/lib/libSystem.dylib
 
+# ARCHIVESEARCH:      searched {{.*}}/MacOSX.sdk/usr/lib{{[/\\]}}libhello.tbd, not found
+# ARCHIVESEARCH-NEXT: searched {{.*}}/MacOSX.sdk/usr/lib{{[/\\]}}libhello.dylib, not found
+# ARCHIVESEARCH-NEXT: searched {{.*}}/MacOSX.sdk/usr/lib{{[/\\]}}libhello.a, not found
+# ARCHIVESEARCH-NEXT: searched [[PATH]]A{{[/\\]}}libhello.tbd, not found
+# ARCHIVESEARCH-NEXT: searched [[PATH]]A{{[/\\]}}libhello.dylib, not found
+# ARCHIVESEARCH-NEXT: searched [[PATH]]A{{[/\\]}}libhello.a, not found
+# ARCHIVESEARCH:      searched [[PATH]]{{[/\\]}}libhello.dylib, found
+# ARCHIVESEARCH:      searched [[PATH]]A{{[/\\]}}libgoodbye.a, found
+
 .section __TEXT,__text
 .global _main
 
 _main:
   movl $0x2000004, %eax                         # write()
   mov $1, %rdi                                  # stdout
   movq _hello_world@GOTPCREL(%rip), %rsi
   mov $13, %rdx                                 # length