[lld-macho] Have --reproduce account for path rerooting

[int3]

Summary

We need to account for path rerooting when generating the response
file. We could either reroot the paths before generating the file, or pass
through the original filenames and change just the syslibroot. I've opted for
the latter, in order that the reproduction run more closely mirrors the
original.

We must also be careful *not* to make an absolute path relative if it is
shadowed by a rerooted path. See repro6.tar in reroot-path.s for
details.

I've moved the call to createResponseFile() after the initialization of
config->systemLibraryRoots, since it now needs to know what those roots are.

[oontvoo]

LGTM

lld/test/MachO/reroot-path.s

28

should this be %lld (or some other pre-set param) to ensure we're testing the "right" ld64.ldd?
(In theory, the machine could already have ld64.lld)

[int3]

lld/test/MachO/reroot-path.s

28

I think llvm-lit will ensure these paths map to the right things. If you run llvm-lit -vva you can see that this line invokes ld64.lld via an absolute path. (Same with other tools like llvm-objdump`)

lld/MachO/Driver.h

 
 // Check for both libfoo.dylib and libfoo.tbd (in that order).
 llvm::Optional<std::string> resolveDylibPath(llvm::StringRef path);
 
 llvm::Optional<DylibFile *> loadDylib(llvm::MemoryBufferRef mbref,
                                       DylibFile *umbrella = nullptr,
                                       bool isBundleLoader = false);
 
+// Search for all possible combinations of `{root}/{name}.{extension}`.
+// If \p extensions are not specified, then just search for `{root}/{name}`.
+llvm::Optional<llvm::StringRef>
+findPathCombination(const llvm::Twine &name,
+                    const std::vector<llvm::StringRef> &roots,
+                    ArrayRef<llvm::StringRef> extensions = {""});
+
+// If -syslibroot is specified, absolute paths to non-object files may be
+// rerooted.
+llvm::StringRef rerootPath(llvm::StringRef path);
+
 llvm::Optional<InputFile *> loadArchiveMember(MemoryBufferRef, uint32_t modTime,
                                               StringRef archiveName,
                                               bool objCOnly);
 
 uint32_t getModTime(llvm::StringRef path);
 
 void printArchiveMemberLoad(StringRef reason, const InputFile *);
 

lld/MachO/Driver.cpp

case OPT_dylib:
     return MH_DYLIB;
   case OPT_execute:
     return MH_EXECUTE;
   default:
     llvm_unreachable("internal error");
   }
 }
 
-// Search for all possible combinations of `{root}/{name}.{extension}`.
-// If \p extensions are not specified, then just search for `{root}/{name}`.
-static Optional<StringRef>
-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))
-        return saver.save(location.str());
-      else
-        depTracker->logFileNotFound(location);
-    }
-  }
-  return {};
-}
-
 static Optional<StringRef> findLibrary(StringRef name) {
   if (config->searchDylibsFirst) {
     if (Optional<StringRef> path = findPathCombination(
             "lib" + name, config->librarySearchPaths, {".tbd", ".dylib"}))
       return path;
     return findPathCombination("lib" + name, config->librarySearchPaths,
                                {".a"});
   }
   return findPathCombination("lib" + name, config->librarySearchPaths,
                              {".tbd", ".dylib", ".a"});
 }
 
-// If -syslibroot is specified, absolute paths to non-object files may be
-// rerooted.
-static StringRef rerootPath(StringRef path) {
-  if (!path::is_absolute(path, path::Style::posix) || path.endswith(".o"))
-    return path;
-
-  if (Optional<StringRef> rerootedPath =
-          findPathCombination(path, config->systemLibraryRoots))
-    return *rerootedPath;
-
-  return path;
-}
-
 static Optional<std::string> findFramework(StringRef name) {
   SmallString<260> symlink;
   StringRef suffix;
   std::tie(name, suffix) = name.split(",");
   for (StringRef dir : config->frameworkSearchPaths) {
     symlink = dir;
     path::append(symlink, name + ".framework", name);
 
std::unique_ptr<Archive> file =
       CHECK(Archive::create(mb),
             mb.getBufferIdentifier() + ": failed to parse archive");
   Archive *archive = file.get();
   make<std::unique_ptr<Archive>>(std::move(file)); // take ownership
 
   std::vector<ArchiveMember> v;
   Error err = Error::success();
 
-  // Thin archives refer to .o files, so --reproduces needs the .o files too.
+  // Thin archives refer to .o files, so --reproduce needs the .o files too.
   bool addToTar = archive->isThin() && tar;
 
   for (const Archive::Child &c : archive->children(err)) {
     MemoryBufferRef mbref =
         CHECK(c.getMemoryBufferRef(),
               mb.getBufferIdentifier() +
                   ": could not get the buffer for a child of the archive");
     if (addToTar)
if (args.hasArg(OPT_help)) {
     parser.printHelp(argsArr[0], /*showHidden=*/false);
     return true;
   }
   if (args.hasArg(OPT_version)) {
     message(getLLDVersion());
     return true;
   }
 
+  config = make<Configuration>();
+  symtab = make<SymbolTable>();
+  target = createTargetInfo(args);
+  depTracker =
+      make<DependencyTracker>(args.getLastArgValue(OPT_dependency_info));
+
+  config->systemLibraryRoots = getSystemLibraryRoots(args);
   if (const char *path = getReproduceOption(args)) {
     // Note that --reproduce is a debug option so you can ignore it
     // if you are trying to understand the whole picture of the code.
     Expected<std::unique_ptr<TarWriter>> errOrWriter =
         TarWriter::create(path, path::stem(path));
     if (errOrWriter) {
       tar = std::move(*errOrWriter);
       tar->append("response.txt", createResponseFile(args));
       tar->append("version.txt", getLLDVersion() + "\n");
     } else {
       error("--reproduce: " + toString(errOrWriter.takeError()));
     }
   }
 
-  config = make<Configuration>();
-  symtab = make<SymbolTable>();
-  target = createTargetInfo(args);
-
-  depTracker =
-      make<DependencyTracker>(args.getLastArgValue(OPT_dependency_info, ""));
-
   if (auto *arg = args.getLastArg(OPT_threads_eq)) {
     StringRef v(arg->getValue());
     unsigned threads = 0;
     if (!llvm::to_integer(v, threads, 0) || threads == 0)
       error(arg->getSpelling() + ": expected a positive integer, but got '" +
             arg->getValue() + "'");
     parallel::strategy = hardware_concurrency(threads);
     config->thinLTOJobs = v;
#endif
   if (const Arg *arg =
           args.getLastArg(OPT_flat_namespace, OPT_twolevel_namespace))
     config->namespaceKind = arg->getOption().getID() == OPT_twolevel_namespace
                                 ? NamespaceKind::twolevel
                                 : NamespaceKind::flat;
 
   config->undefinedSymbolTreatment = getUndefinedSymbolTreatment(args);
 
-  config->systemLibraryRoots = getSystemLibraryRoots(args);
   config->librarySearchPaths =
       getLibrarySearchPaths(args, config->systemLibraryRoots);
   config->frameworkSearchPaths =
       getFrameworkSearchPaths(args, config->systemLibraryRoots);
   if (const Arg *arg =
           args.getLastArg(OPT_search_paths_first, OPT_search_dylibs_first))
     config->searchDylibsFirst =
         arg->getOption().getID() == OPT_search_dylibs_first;

lld/MachO/DriverUtils.cpp

 }
 
 static std::string rewritePath(StringRef s) {
   if (fs::exists(s))
     return relativeToRoot(s);
   return std::string(s);
 }
 
+static std::string rewriteInputPath(StringRef s) {
+  // Don't bother rewriting "absolute" paths that are actually under the
+  // syslibroot; simply rewriting the syslibroot is sufficient.
+  if (rerootPath(s) == s && fs::exists(s))
+    return relativeToRoot(s);
+  return std::string(s);
+}
+
 // Reconstructs command line arguments so that so that you can re-run
 // the same command with the same inputs. This is for --reproduce.
 std::string macho::createResponseFile(const InputArgList &args) {
   SmallString<0> data;
   raw_svector_ostream os(data);
 
   // Copy the command line to the output while rewriting paths.
   for (const Arg *arg : args) {
     switch (arg->getOption().getID()) {
     case OPT_reproduce:
       break;
     case OPT_INPUT:
-      os << quote(rewritePath(arg->getValue())) << "\n";
+      os << quote(rewriteInputPath(arg->getValue())) << "\n";
       break;
     case OPT_o:
       os << "-o " << quote(path::filename(arg->getValue())) << "\n";
       break;
     case OPT_filelist:
       if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
         for (StringRef path : args::getLines(*buffer))
-          os << quote(rewritePath(path)) << "\n";
+          os << quote(rewriteInputPath(path)) << "\n";
+      break;
+    case OPT_force_load:
+    case OPT_weak_library:
+      os << arg->getSpelling() << " "
+         << quote(rewriteInputPath(arg->getValue())) << "\n";
       break;
     case OPT_F:
     case OPT_L:
     case OPT_bundle_loader:
     case OPT_exported_symbols_list:
-    case OPT_force_load:
     case OPT_order_file:
     case OPT_rpath:
     case OPT_syslibroot:
     case OPT_unexported_symbols_list:
       os << arg->getSpelling() << " " << quote(rewritePath(arg->getValue()))
          << "\n";
       break;
     case OPT_sectcreate:
Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
   // Note that DylibFile's ctor may recursively invoke loadDylib(), which can
   // cause loadedDylibs to get resized and its iterators invalidated. As such,
   // we redo the key lookup here instead of caching an iterator from our earlier
   // lookup at the start of the function.
   loadedDylibs[path] = file;
   return file;
 }
 
+Optional<StringRef>
+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))
+        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;
+
+  if (Optional<StringRef> rerootedPath =
+          findPathCombination(path, config->systemLibraryRoots))
+    return *rerootedPath;
+
+  return path;
+}
+
 Optional<InputFile *> macho::loadArchiveMember(MemoryBufferRef mb,
                                                uint32_t modTime,
                                                StringRef archiveName,
                                                bool objCOnly) {
   switch (identify_magic(mb.getBuffer())) {
   case file_magic::macho_object:
     if (!objCOnly || hasObjCSection(mb))
       return make<ObjFile>(mb, modTime, archiveName);

lld/test/MachO/reroot-path.s

 ## and therefore verifies that we still fall back to the original path if no
 ## file exists at the rerooted path.
 # RUN: llvm-ar rcs %t/foo.a %t/foo.o
 # RUN: %lld -dylib %t/foo.o -o %t/libfoo.dylib
 # RUN: llvm-ar rcs %t/%:t/bar.a %t/bar.o
 # RUN: %lld -dylib %t/bar.o -o %t/%:t/libbar.dylib
 
 ## Test our various file-loading flags to make sure all bases are covered.
-# RUN: %lld -lSystem -syslibroot %t %t/foo.a %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: %lld -lSystem -syslibroot %t -force_load %t/foo.a -force_load %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: %lld -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# RUN: %lld -lSystem -syslibroot %t -weak_library %t/libfoo.dylib -weak_library %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: %lld --reproduce %t/repro1.tar -lSystem -syslibroot %t %t/foo.a %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro1.tar -C %t
+# RUN: cd %t/repro1; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+# RUN: %lld --reproduce %t/repro2.tar -lSystem -syslibroot %t -force_load %t/foo.a -force_load %t/bar.a %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro2.tar -C %t
+# RUN: cd %t/repro2; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+# RUN: %lld --reproduce %t/repro3.tar -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro3.tar -C %t
+# RUN: cd %t/repro3; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+# RUN: %lld --reproduce %t/repro4.tar -lSystem -syslibroot %t -weak_library %t/libfoo.dylib -weak_library %t/libbar.dylib %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro4.tar -C %t
+# RUN: cd %t/repro4; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
 # RUN: echo "%t/libfoo.dylib" > %t/filelist
 # RUN: echo "%t/libbar.dylib" >> %t/filelist
-# RUN: %lld -lSystem -syslibroot %t -filelist %t/filelist %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
-# CHECK: [[DIR]]/{{(lib)?}}bar
+# RUN: %lld --reproduce %t/repro5.tar -lSystem -syslibroot %t -filelist %t/filelist %t/test.o -o /dev/null -t | FileCheck %s -DDIR="%t/%:t"
+# RUN: tar xf %t/repro5.tar -C %t
+# RUN: cd %t/repro5; ld64.lld @response.txt | FileCheck %s -DDIR="%:t/%:t"
+
+## The {{^}} ensures that we only match relative paths if DIR is relative.
+# CHECK: {{^}}[[DIR]]/{{(lib)?}}bar
 
 ## Paths to object files don't get rerooted.
 # RUN: mv %t/bar.o %t/%:t/bar.o
 # RUN: not %lld -lSystem -syslibroot %t %t/foo.o %t/bar.o %t/test.o -o \
 # RUN:   /dev/null 2>&1 | FileCheck %s --check-prefix=OBJ
 # OBJ: error: cannot open {{.*[\\/]}}bar.o: {{[Nn]}}o such file or directory
 
 ## Now create a "decoy" libfoo.dylib under %t/%:t to demonstrate that the
 ## rerooted path takes precedence over the original path. We will get an
 ## undefined symbol error since we aren't loading %t/libfoo.dylib.
 # RUN: cp %t/%:t/libbar.dylib %t/%:t/libfoo.dylib
-# RUN: not %lld -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o \
-# RUN:   -o /dev/null 2>&1 | FileCheck %s --check-prefix=UNDEF
+# RUN: not %lld --reproduce %t/repro6.tar -lSystem -syslibroot %t %t/libfoo.dylib %t/libbar.dylib %t/test.o \
+# RUN:   -o /dev/null -t 2> %t/error | FileCheck %s -DDIR="%t/%:t" --check-prefix=FOO
+# RUN: FileCheck %s --check-prefix=UNDEF < %t/error
+
+# RUN: tar xf %t/repro6.tar -C %t
+# RUN: cd %t/repro6; not ld64.lld @response.txt 2> %t/error | FileCheck %s -DDIR="%:t/%:t" --check-prefix=FOO
+# RUN: FileCheck %s --check-prefix=UNDEF < %t/error
+
+# FOO: [[DIR]]/libfoo.dylib
 # UNDEF: error: undefined symbol: _foo
 
 #--- foo.s
 .globl _foo
 _foo:
 
 #--- bar.s
 .globl _bar