[clang] Make the driver not diagnose errors on nonexistent linker inputs
authorNico Weber <thakis@chromium.org>
Mon, 13 Sep 2021 12:57:38 +0000 (08:57 -0400)
committerNico Weber <thakis@chromium.org>
Mon, 13 Sep 2021 12:57:38 +0000 (08:57 -0400)
When nonexistent linker inputs are passed to the driver, the linker
now errors out, instead of the compiler. If the linker does not run,
clang now emits a "warning: linker input unused" instead of an error
for nonexistent files.

The motivation for this change is that I noticed that
`clang-cl /winsysroot sysroot main.cc ole32.lib` emitted a
"ole32.lib not found" error, even though the linker finds it just fine when
I run `clang-cl /winsysroot sysroot main.cc /link ole32.lib`.

The same problem occurs if running `clang-cl main.cc ole32.lib` in a
non-MSVC shell.

The problem is that DiagnoseInputExistence() only looked for libs in %LIB%,
but MSVCToolChain uses much more involved techniques.

For this particular problem, we could make DiagnoseInputExistence() ask
the toolchain to see if it can find a .lib file, but in general the
driver can't know what the linker will do to find files, so it shouldn't
try. For example, if we implement PR24616, lld-link will look in the
registry to determine a good default for %LIB% if it isn't set.

This is less or a problem for the gcc driver, since .a paths there are
either passed via -l flags (which honor -L), or via a qualified path
(that doesn't honor -L) -- but for example ld.lld's --chroot flag
can also trigger this problem. Without this patch,
`clang -fuse-ld=lld -Wl,--chroot,some/dir /file.o` will complain that
`/file.o` doesn't exist, even though
`clang -fuse-ld=lld -Wl,--chroot,some/dir -Wl,/file.o` succeeds just fine.

This implements rnk's suggestion on the old bug PR27234.

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

clang/lib/Driver/Driver.cpp
clang/test/Driver/cl-inputs.c
clang/test/Driver/cl-link.c
clang/test/Driver/unknown-arg.c
flang/test/Driver/missing-input.f90

index 7dea484..50791ec 100644 (file)
@@ -2130,19 +2130,6 @@ bool Driver::DiagnoseInputExistence(const DerivedArgList &Args, StringRef Value,
   if (getVFS().exists(Value))
     return true;
 
-  if (IsCLMode()) {
-    if (!llvm::sys::path::is_absolute(Twine(Value)) &&
-        llvm::sys::Process::FindInEnvPath("LIB", Value, ';'))
-      return true;
-
-    if (Args.hasArg(options::OPT__SLASH_link) && Ty == types::TY_Object) {
-      // Arguments to the /link flag might cause the linker to search for object
-      // and library files in paths we don't know about. Don't error in such
-      // cases.
-      return true;
-    }
-  }
-
   if (TypoCorrect) {
     // 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
@@ -2162,6 +2149,34 @@ bool Driver::DiagnoseInputExistence(const DerivedArgList &Args, StringRef Value,
     }
   }
 
+  // Don't error on apparently non-existent linker inputs, because they
+  // can be influenced by linker flags the clang driver might not understand.
+  // Examples:
+  // - `clang -fuse-ld=lld -Wl,--chroot,some/dir /file.o` will make lld look
+  //   for some/dir/file.o
+  // - `clang-cl main.cc ole32.lib` in a a non-MSVC shell will make the driver
+  //   module look for an MSVC installation in the registry. (We could ask
+  //   the MSVCToolChain object if it can find `ole32.lib`, but the logic to
+  //   look in the registry might move into lld-link in the future so that
+  //   lld-link invocations in non-MSVC shells just work too.)
+  // - `clang-cl ... /link ...` can pass arbitrary flags to the linker,
+  //   including /libpath:, which is used to find .lib and .obj files.
+  // So do not diagnose this on the driver level. Rely on the linker diagnosing
+  // it. (If we don't end up invoking the linker, this means we'll emit a
+  // "'linker' input unused [-Wunused-command-line-argument]" warning instead
+  // of an error.)
+  //
+  // Only do this skip after the typo correction step above. `/Brepo` is treated
+  // as TY_Object, but it's clearly a typo for `/Brepro`. It seems fine to emit
+  // an error if we have a flag that's within an edit distance of 1 from a
+  // flag. (Users can use `-Wl,` or `/linker` to launder the flag past the
+  // driver in the unlikely case they run into this.)
+  //
+  // Don't do this skip in clang-cl mode for inputs that start with a '/' --
+  // else we'd pass options like /libpath: through to the linker silently.
+  if (Ty == types::TY_Object && !(IsCLMode() && Value.startswith("/")))
+    return true;
+
   Diag(clang::diag::err_drv_no_such_file) << Value;
   return false;
 }
index 35e4fbf..2b9d559 100644 (file)
@@ -55,9 +55,9 @@
 // LIBINPUT: "cl-test.lib"
 
 // RUN: env LIB=%S/Inputs/cl-libs %clang_cl -fuse-ld=link -### -- %s cl-test2.lib 2>&1 | FileCheck -check-prefix=LIBINPUT2 %s
-// LIBINPUT2: error: no such file or directory: 'cl-test2.lib'
+// LIBINPUT2-NOT: error: no such file or directory: 'cl-test2.lib'
 // LIBINPUT2: link.exe"
-// LIBINPUT2-NOT: "cl-test2.lib"
+// LIBINPUT2: "cl-test2.lib"
 
 // RUN: %clang_cl -fuse-ld=link -### -- %s /nonexisting.lib 2>&1 | FileCheck -check-prefix=LIBINPUT3 %s
 // LIBINPUT3: error: no such file or directory: '/nonexisting.lib'
index e2f5397..dcb7801 100644 (file)
 // DEBUG: link.exe
 // DEBUG: "-debug"
 
+// Don't pass through /libpath: if it's not after a /link flag:
+// RUN: %clang_cl /Tc%s /libpath:foo -fuse-ld=link -### /link /libpath:bar 2>&1 | FileCheck --check-prefix=LIBPATH %s
+// LIBPATH: error: no such file or directory: '/libpath:foo'
+// LIBPATH: libpath:bar
+
 // PR27234
 // RUN: %clang_cl /Tc%s nonexistent.obj -fuse-ld=link -### /link /libpath:somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
 // RUN: %clang_cl /Tc%s nonexistent.lib -fuse-ld=link -### /link /libpath:somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.obj -fuse-ld=link -### /winsysroot somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.lib -fuse-ld=link -### /winsysroot somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.obj -fuse-ld=link -### 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.lib -fuse-ld=link -### 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
 // NONEXISTENT-NOT: no such file
 // NONEXISTENT: link.exe
-// NONEXISTENT: "/libpath:somepath"
 // NONEXISTENT: nonexistent
 
 // RUN: %clang_cl /Tc%s -fuse-ld=lld -### 2>&1 | FileCheck --check-prefix=USE_LLD %s
index 0e3e5bf..75ae18f 100644 (file)
@@ -10,6 +10,8 @@
 // RUN:     FileCheck %s --check-prefix=CL-DID-YOU-MEAN
 // RUN: %clang_cl /Brepo -### -- %s 2>&1 | \
 // RUN:     FileCheck %s --check-prefix=CL-DID-YOU-MEAN-SLASH
+// RUN: %clang_cl /Brepo -### /Tc%s /link 2>&1 | \
+// RUN:     FileCheck %s --check-prefix=CL-DID-YOU-MEAN-SLASH
 // RUN: not %clang_cl -cake-is-lie -%0 -%d -HHHH -munknown-to-clang-option -print-stats -funknown-to-clang-option -c -Werror=unknown-argument -### -- %s 2>&1 | \
 // RUN:     FileCheck %s --check-prefix=CL-ERROR
 // RUN: not %clang_cl -helo -Werror=unknown-argument -### -- %s 2>&1 | \
index d9d444e..44ee6d8 100644 (file)
@@ -9,12 +9,12 @@
 ! FLANG DRIVER (flang-new)
 !--------------------------
 ! RUN: not %flang  2>&1 | FileCheck %s --check-prefix=FLANG-NO-FILE
-! RUN: not %flang %t 2>&1 | FileCheck %s --check-prefix=FLANG-NONEXISTENT-FILE
+! RUN: not %flang %t.f90 2>&1 | FileCheck %s --check-prefix=FLANG-NONEXISTENT-FILE
 
 !-----------------------------------------
 ! FLANG FRONTEND DRIVER (flang-new -fc1)
 !-----------------------------------------
-! RUN: not %flang_fc1 %t 2>&1  | FileCheck %s --check-prefix=FLANG-FC1-NONEXISTENT-FILE
+! RUN: not %flang_fc1 %t.f90 2>&1  | FileCheck %s --check-prefix=FLANG-FC1-NONEXISTENT-FILE
 ! RUN: not %flang_fc1 %S 2>&1  | FileCheck %s --check-prefix=FLANG-FC1-DIR
 
 !-----------------------