[LLD] [COFF] Don't try to detect MSVC installations in mingw mode

[mstorsjo]

Summary

In mingw mode, all linker paths are passed explicitly to the linker
by the compiler driver. Don't try to implicitly add linker paths
from the LIB environment variable or by detecting an MSVC
installation directory.

If the /winsysroot command line parameter is explicitly passed to
lld-link while /lldmingw is specified, it could be considered reasonable
to actually include those paths. However, modifying the code to
handle only the /winsysroot case but not the other ones, when the
mingw mode has been enabled, seems like much more code complexity
for a mostly hypothetical case.

Add a test for this when case when using LIB. (The code paths for
trying to detect an MSVC installation aren't really regression tested.)

Also fix an issue in the existing test for "Check that when /winsysroot
is specified, %LIB% is ignored.", where the LIB variable pointed
to a nonexistent directory, so the test would pass even if /winsysroot
wouldn't be specified.

[hans]

lld/COFF/Driver.cpp

1515

i assume lld will warn about unused arguments if the user tries to use /winsysroot or /vctoolsdir in mingw mode?

[mstorsjo]

lld/COFF/Driver.cpp

1515

No, lld doesn't warn about unclaimed arguments.

[hans]

lld/COFF/Driver.cpp

1515

Hmm, should we make it warn in this case? Just silently ignoring /winsysroot seems like a bad experience.

[mstorsjo]

lld/COFF/Driver.cpp

1515

I guess it would be reasonable to print a warning here, if there are explicit command line arguments that we know we're not dealing with.

[mstorsjo]

Added a warning if command line arguments are ignored.

[mstorsjo]

lld/COFF/Driver.cpp

1519

Having a negated if condition, followed by an else (i.e. a double negation), is a bit non-ideal, but I feel the main codepath (first the main case is presented, followed by the exception to it) is easier to follow this way.

[hans]

lgtm

[mati865]

LGTM

[mstorsjo]

I had to revert this, as it broke one test in compiler-rt/test/profile/Windows/coverage-weak-lld.cpp, which relied on linking in an MSVC environment (finding the lib path from the environment) with -lldmingw to opt in to mingw style behaviours.

[mati865]

Should that even be considered as supported?

[mstorsjo]

In D144084#4140157, @mati865 wrote:

Should that even be considered as supported?

That's a good question... The flag in itself unlocks a whole bunch of behaviours, some which can be considered useful, and some probably just as tolerable sideeffects (like adding list markers for __CTOR_LIST__ and such) - so in general I guess it's a bit iffy. I have mentioned the flag to various people throughout the years that they can try it out to see if it gives a behaviour they need though.

For some behaviours, there are individual flags that control the behaviours (like -stdcall-fixup or -auto-import) where the default behaviour depends on whether the mingw flag is set - but there are also lots of behaviours that aren't individually controllable.

I'm not sure what would be best here - add a more specific -tolerate-duplicate-weak-sybols option or something like that? Or add a more specific flag, always passed by default by the mingw driver frontend, e.g. -detect-msvc:no?

For context - I haven't heard about a specific case where this would make any difference, I just noticed the fact that this is done unconditionally - and there's a small risk that someone ends up relying on implicitly linking in things from a WinSDK they have lying around.

[mati865]

In D144084#4140722, @mstorsjo wrote:

I'm not sure what would be best here - add a more specific -tolerate-duplicate-weak-sybols option or something like that? Or add a more specific flag, always passed by default by the mingw driver frontend, e.g. -detect-msvc:no?

Sorry if you have been waiting for me, this has slipped under my radar.
I don't know MSVC at all so I cannot provide any suggestions there but for MinGW any way that disables MSVC search by the default probably would be beneficial.

In D144084#4134321, @mstorsjo wrote:

I had to revert this, as it broke one test in compiler-rt/test/profile/Windows/coverage-weak-lld.cpp, which relied on linking in an MSVC environment (finding the lib path from the environment) with -lldmingw to opt in to mingw style behaviours.

I was under impression lld-link should follow MSVC closely (so this behaviour would be surprising) but that might just a wrong impression from a bystander.

[mstorsjo]

In D144084#4293934, @mati865 wrote:

In D144084#4140722, @mstorsjo wrote:

I'm not sure what would be best here - add a more specific -tolerate-duplicate-weak-sybols option or something like that? Or add a more specific flag, always passed by default by the mingw driver frontend, e.g. -detect-msvc:no?

Sorry if you have been waiting for me, this has slipped under my radar.

No problem - I wasn't waiting specifically for you, I just didn't have time to take much further action on this at the time, and it's not a high priority thing.

I don't know MSVC at all so I cannot provide any suggestions there but for MinGW any way that disables MSVC search by the default probably would be beneficial.

In D144084#4134321, @mstorsjo wrote:

I had to revert this, as it broke one test in compiler-rt/test/profile/Windows/coverage-weak-lld.cpp, which relied on linking in an MSVC environment (finding the lib path from the environment) with -lldmingw to opt in to mingw style behaviours.

I was under impression lld-link should follow MSVC closely (so this behaviour would be surprising) but that might just a wrong impression from a bystander.

Well, the starting point is that it should follow MSVC style behaviours, but overall I do think lld-link offers a few extensions outside of what's possible in the regular MSVC ecosystem, and that's probably fine in itself. But using -lldmingw is indeed quite loosely specified...

[mstorsjo]

Rebased on latest git main.

After https://github.com/llvm/llvm-project/pull/68077 and https://github.com/llvm/llvm-project/pull/69781 - the compiler-rt test that used -lldmingw in MSVC environments has been updated to use a more specific option; I'll go ahead and try to reland this soon.

lld/COFF/Driver.cpp

if (!args.hasArg(OPT_INPUT, OPT_wholearchive_file)) {
     else
       fatal("no input files");
   }
 
   // Construct search path list.
   searchPaths.emplace_back("");
   for (auto *arg : args.filtered(OPT_libpath))
     searchPaths.push_back(arg->getValue());
+  if (!config->mingw) {
+    // Don't automatically deduce the lib path from the environment or MSVC
+    // installations when operating in mingw mode. (This also makes LLD ignore
+    // winsysroot and vctoolsdir arguments.)
     detectWinSysRoot(args);
     if (!args.hasArg(OPT_lldignoreenv) && !args.hasArg(OPT_winsysroot))
       addLibSearchPaths();
+  }
 
   // Handle /ignore
   for (auto *arg : args.filtered(OPT_ignore)) {
     SmallVector<StringRef, 8> vec;
     StringRef(arg->getValue()).split(vec, ',');
     for (StringRef s : vec) {
       if (s == "4037")
         config->warnMissingOrderSymbol = false;

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 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
+# RUN: env LIB=%t.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'
+
+Check that when -lldmingw is specified, %LIB% is ignored.
+# RUN: env LIB=%t.dir/sysroot/VC/Tools/MSVC/1.1.1.1/lib/x86 not lld-link -lldmingw %t.obj /defaultlib:std32 -libpath:%t.dir/sysroot/VC/Tools/MSVC/1.1.1./lib/x86 2>&1 | FileCheck -check-prefix=LIBIGNORED_MINGW %s
+LIBIGNORED_MINGW: could not open 'libstd32.a'