[lld][COFF] Retry failed paths to take advantage of winsysroot search paths

[aeubanks]

Summary

With /winsysroot and without /machine, we don't know which paths to add to the search paths.

We do autodetect machine type and add winsysroot search paths in SymbolTable::addFile(), but that happens after all input files are opened. We could do this serially and update the search paths between input files, but then that doesn't work for the first input file, plus lld-link wants to open input files concurrently since file I/O is slow on Windows.

So instead, keep track of which files couldn't be opened and retry after adding winsysroot search paths.

Fixes #54409

[hans]

lgtm

[aeubanks]

D151815 doesn't change the order we add paths, perhaps that's better

lld/COFF/Driver.h

public:
 
   // Used by the resolver to parse .drectve section contents.
   void parseDirectives(InputFile *file);
 
   // Used by ArchiveFile to enqueue members.
   void enqueueArchiveMember(const Archive::Child &c, const Archive::Symbol &sym,
                             StringRef parentName);
 
-  void enqueuePDB(StringRef Path) { enqueuePath(Path, false, false); }
+  void enqueuePDB(StringRef Path) { enqueuePath(Path, false, false, false); }
 
   MemoryBufferRef takeBuffer(std::unique_ptr<MemoryBuffer> mb);
 
-  void enqueuePath(StringRef path, bool wholeArchive, bool lazy);
+  void enqueuePath(StringRef path, bool wholeArchive, bool lazy, bool failOk);
 
   std::unique_ptr<llvm::TarWriter> tar; // for /linkrepro
 
 private:
   // Searches a file from search paths.
   std::optional<StringRef> findFile(StringRef filename);
   std::optional<StringRef> findLib(StringRef filename);
   StringRef doFindFile(StringRef filename);
void addArchiveBuffer(MemoryBufferRef mbref, StringRef symName,
                         StringRef parentName, uint64_t offsetInArchive);
 
   void enqueueTask(std::function<void()> task);
   bool run();
 
   std::list<std::function<void()>> taskQueue;
   std::vector<StringRef> filePaths;
   std::vector<MemoryBufferRef> resources;
+  std::vector<std::tuple<std::string, bool, bool>> pathsToRetry;
 
   llvm::DenseSet<StringRef> directivesExports;
   llvm::DenseSet<StringRef> excludedSymbols;
 
   COFFLinkerContext &ctx;
 
   llvm::ToolsetLayout vsLayout = llvm::ToolsetLayout::OlderVS;
   std::string vcToolChainPath;

lld/COFF/Driver.cpp

static bool isCrtend(StringRef s) {
   if (!s.endswith(".o"))
     return false;
   s = s.drop_back(2);
   if (s.endswith("crtend"))
     return true;
   return !s.empty() && s.drop_back().endswith("crtend");
 }
 
+static bool hasPathSep(StringRef filename) {
+  return filename.find_first_of("/\\") != StringRef::npos;
+}
+
 // ErrorOr is not default constructible, so it cannot be used as the type
 // parameter of a future.
 // FIXME: We could open the file in createFutureForFile and avoid needing to
 // return an error here, but for the moment that would cost us a file descriptor
 // (a limited resource on Windows) for the duration that the future is pending.
 using MBErrPair = std::pair<std::unique_ptr<MemoryBuffer>, std::error_code>;
 
 // Create a std::future that opens and maps a file using the best strategy for
case file_magic::pecoff_executable:
     }
     [[fallthrough]];
   default:
     error(mbref.getBufferIdentifier() + ": unknown file type");
     break;
   }
 }
 
-void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy) {
+void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy,
+                               bool failOk) {
   auto future = std::make_shared<std::future<MBErrPair>>(
       createFutureForFile(std::string(path)));
   std::string pathStr = std::string(path);
   enqueueTask([=]() {
     auto mbOrErr = future->get();
     if (mbOrErr.second) {
+      if (!hasPathSep(pathStr) && failOk) {
+        ctx.driver.pathsToRetry.push_back({pathStr, wholeArchive, lazy});
+      } else {
         std::string msg =
             "could not open '" + pathStr + "': " + mbOrErr.second.message();
         // Check if the filename is a typo for an option flag. OptTable thinks
         // that all args that are not known options and that start with / are
         // filenames, but e.g. `/nodefaultlibs` is more likely a typo for
         // the option `/nodefaultlib` than a reference to a file in the root
         // directory.
         std::string nearest;
         if (ctx.optTable.findNearest(pathStr, nearest) > 1)
           error(msg);
         else
           error(msg + "; did you mean '" + nearest + "'");
+      }
     } else
       ctx.driver.addBuffer(std::move(mbOrErr.first), wholeArchive, lazy);
   });
 }
 
 void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
                                     StringRef parentName,
                                     uint64_t offsetInArchive) {
for (auto *arg : directives.args) {
     case OPT_aligncomm:
       parseAligncomm(arg->getValue());
       break;
     case OPT_alternatename:
       parseAlternateName(arg->getValue());
       break;
     case OPT_defaultlib:
       if (std::optional<StringRef> path = findLib(arg->getValue()))
-        enqueuePath(*path, false, false);
+        enqueuePath(*path, false, false, false);
       break;
     case OPT_entry:
       ctx.config.entry = addUndefined(mangle(arg->getValue()));
       break;
     case OPT_failifmismatch:
       checkFailIfMismatch(arg->getValue(), file);
       break;
     case OPT_incl:

 StringRef LinkerDriver::doFindFile(StringRef filename) {
   auto getFilename = [this](StringRef filename) -> StringRef {
     if (ctx.config.vfs)
       if (auto statOrErr = ctx.config.vfs->status(filename))
         return saver().save(statOrErr->getName());
     return filename;
   };
 
-  bool hasPathSep = (filename.find_first_of("/\\") != StringRef::npos);
-  if (hasPathSep)
+  if (hasPathSep(filename))
     return getFilename(filename);
   bool hasExt = filename.contains('.');
   for (StringRef dir : searchPaths) {
     SmallString<128> path = dir;
     sys::path::append(path, filename);
     path = SmallString<128>{getFilename(path.str())};
     if (sys::fs::exists(path.str()))
       return saver().save(path.str());
case OPT_end_lib:
       break;
     case OPT_start_lib:
       if (inLib)
         error("nested " + arg->getSpelling());
       inLib = true;
       break;
     case OPT_wholearchive_file:
       if (std::optional<StringRef> path = findFile(arg->getValue()))
-        enqueuePath(*path, true, inLib);
+        enqueuePath(*path, true, inLib, true);
       break;
     case OPT_INPUT:
       if (std::optional<StringRef> path = findFile(arg->getValue()))
-        enqueuePath(*path, isWholeArchive(*path), inLib);
+        enqueuePath(*path, isWholeArchive(*path), inLib, true);
       break;
     default:
       // Ignore other options.
       break;
     }
   }
 
   // Read all input files given via the command line.
   run();
-  if (errorCount())
-    return;
 
   // We should have inferred a machine type by now from the input files, but if
   // not we assume x64.
   if (config->machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     warn("/machine is not specified. x64 is assumed");
     config->machine = AMD64;
     addWinSysRootLibSearchPaths();
   }
+
+  // Retry again now that we've added winsysroot search paths.
+  for (auto p : pathsToRetry) {
+    auto [path, wholeArchive, lazy] = p;
+    if (std::optional<StringRef> file = findFile(path))
+      enqueuePath(*file, wholeArchive, lazy, false);
+  }
+  run();
+  if (errorCount())
+    return;
+
   config->wordsize = config->is64() ? 8 : 4;
 
   // Process files specified as /defaultlib. These must be processed after
   // addWinSysRootLibSearchPaths(), which is why they are in a separate loop.
   for (auto *arg : args.filtered(OPT_defaultlib))
     if (std::optional<StringRef> path = findLib(arg->getValue()))
-      enqueuePath(*path, false, false);
+      enqueuePath(*path, false, false, false);
   run();
   if (errorCount())
     return;
 
   // Handle /RELEASE
   if (args.hasArg(OPT_release))
     config->writeCheckSum = true;
   

lld/test/COFF/winsysroot.test

 Check that /winsysroot results in the correct machine-specific subdirectory
 being searched for the defaultlibs, for a 32-bit .obj.
 # RUN: yaml2obj %p/Inputs/hello32.yaml -o %t.obj
 # RUN: mkdir -p %t.dir/sysroot/VC/Tools/MSVC/1.1.1.1/lib/x86
 # RUN: mkdir -p %t.dir/sysroot/VC/Tools/MSVC/1.1.1.1/lib/x64
 # RUN: cp %p/Inputs/std32.lib %t.dir/sysroot/VC/Tools/MSVC/1.1.1.1/lib/x86
 # RUN: cp %p/Inputs/std64.lib %t.dir/sysroot/VC/Tools/MSVC/1.1.1.1/lib/x64
 # RUN: lld-link %t.obj /winsysroot:%t.dir/sysroot \
 # RUN:          /defaultlib:std32 /entry:main@0
 
 Check the same for a 64-bit input .obj.
 # RUN: lld-link %p/Inputs/hello64.obj /winsysroot:%t.dir/sysroot \
 # RUN:          /defaultlib:std64 /entry:main
 
+Check directly passed lib with /machine:
+# RUN: lld-link %p/Inputs/hello64.obj /winsysroot:%t.dir/sysroot /machine:x64 \
+# RUN:          std64.lib /entry:main
+
+# RUN: lld-link %t.obj /winsysroot:%t.dir/sysroot /machine:x86 \
+# RUN:          std32.lib /entry:main
+
+Check directly passed lib without /machine: (should infer from obj arch)
+# RUN: lld-link %p/Inputs/hello64.obj /winsysroot:%t.dir/sysroot \
+# RUN:          std64.lib /entry:main
+
+# RUN: lld-link %t.obj /winsysroot:%t.dir/sysroot \
+# RUN:          std32.lib /entry:main
+
+Check we don't choose the wrong arch
+# RUN: not lld-link %t.obj /winsysroot:%t.dir/sysroot \
+# RUN:          std64.lib /entry:main
+
+# RUN: not lld-link %p/Inputs/hello64.obj /winsysroot:%t.dir/sysroot \
+# RUN:          std32.lib /entry:main
+
 Check that when /winsysroot is specified, %LIB% is ignored.
 # RUN: env LIB=foo.dir/sysroot/VC/Tools/MSVC/1.1.1.1/lib/x86 not lld-link %t.obj /winsysroot:%t.dir/doesnotexist /defaultlib:std32 2>&1 | FileCheck -check-prefix=LIBIGNORED %s
 LIBIGNORED: could not open 'std32.lib'