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

[Arthur Eubanks via llvm-commits]

Author: Arthur Eubanks
Date: 2023-06-01T15:37:02-07:00
New Revision: c8ea66937bdf2f11e5c4f884e3164c98cc6520bb

URL: https://github.com/llvm/llvm-project/commit/c8ea66937bdf2f11e5c4f884e3164c98cc6520bb
DIFF: https://github.com/llvm/llvm-project/commit/c8ea66937bdf2f11e5c4f884e3164c98cc6520bb.diff

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

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. So in the loop where we read files, if we fail to open a file we can retry with the winsysroot search path potentially added by reading a previous file. This will fail if we try to open something in the winsysroot before reading a file that can give us the architecture, but shrug.

Fixes #54409

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D151815

Added: 
    

Modified: 
    lld/COFF/Driver.cpp
    lld/test/COFF/winsysroot.test

Removed: 
    


################################################################################
diff  --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 6d0b3494b6af6..cbfad98cb812e 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -247,10 +247,25 @@ void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy) {
       createFutureForFile(std::string(path)));
   std::string pathStr = std::string(path);
   enqueueTask([=]() {
-    auto mbOrErr = future->get();
-    if (mbOrErr.second) {
-      std::string msg =
-          "could not open '" + pathStr + "': " + mbOrErr.second.message();
+    auto [mb, ec] = future->get();
+    if (ec) {
+      // Retry reading the file (synchronously) now that we may have added
+      // winsysroot search paths from SymbolTable::addFile().
+      // Retrying synchronously is important for keeping the order of inputs
+      // consistent.
+      // This makes it so that if the user passes something in the winsysroot
+      // before something we can find with an architecture, we won't find the
+      // winsysroot file.
+      if (std::optional<StringRef> retryPath = findFile(pathStr)) {
+        auto retryMb = MemoryBuffer::getFile(*retryPath, /*IsText=*/false,
+                                             /*RequiresNullTerminator=*/false);
+        ec = retryMb.getError();
+        if (!ec)
+          mb = std::move(*retryMb);
+      }
+    }
+    if (ec) {
+      std::string msg = "could not open '" + pathStr + "': " + ec.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
@@ -262,7 +277,7 @@ void LinkerDriver::enqueuePath(StringRef path, bool wholeArchive, bool lazy) {
       else
         error(msg + "; did you mean '" + nearest + "'");
     } else
-      ctx.driver.addBuffer(std::move(mbOrErr.first), wholeArchive, lazy);
+      ctx.driver.addBuffer(std::move(mb), wholeArchive, lazy);
   });
 }
 

diff  --git a/lld/test/COFF/winsysroot.test b/lld/test/COFF/winsysroot.test
index f09ceb5c92805..4f5df0bd019a5 100644
--- a/lld/test/COFF/winsysroot.test
+++ b/lld/test/COFF/winsysroot.test
@@ -12,6 +12,31 @@ 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
+
+If winsysroot lib appears before we can detect arch we don't find it
+# RUN: not lld-link std64.lib %p/Inputs/hello64.obj /winsysroot:%t.dir/sysroot \
+# RUN:          /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'