[Driver][MinGW] Always put libc argument last, even if non-standard #149434
+13
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver Author: Keno Fischer (Keno) ChangesI was attempting to build openblas with clang in msys2's These symbols come from the To preserve the intent of letting the user override which crt is chosen, duplicate the (first) user chosen crt Full diff: https://github.com/llvm/llvm-project/pull/149434.diff 1 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp
index 7d093d20b3dd9..c849b3ce0cfcc 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -88,8 +88,13 @@ void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
for (auto Lib : Args.getAllArgValues(options::OPT_l))
if (StringRef(Lib).starts_with("msvcr") ||
StringRef(Lib).starts_with("ucrt") ||
- StringRef(Lib).starts_with("crtdll"))
+ StringRef(Lib).starts_with("crtdll")) {
+ // Respect the user's chosen crt variant, but still provide it
+ // again as the last linker argument, because some of the libraries
+ // we added above may depend on it.
+ CmdArgs.push_back(Args.MakeArgStringRef(StringRef(Lib)));
return;
+ }
CmdArgs.push_back("-lmsvcrt");
}
|
|
@llvm/pr-subscribers-clang Author: Keno Fischer (Keno) ChangesI was attempting to build openblas with clang in msys2's These symbols come from the To preserve the intent of letting the user override which crt is chosen, duplicate the (first) user chosen crt Full diff: https://github.com/llvm/llvm-project/pull/149434.diff 1 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp
index 7d093d20b3dd9..c849b3ce0cfcc 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -88,8 +88,13 @@ void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
for (auto Lib : Args.getAllArgValues(options::OPT_l))
if (StringRef(Lib).starts_with("msvcr") ||
StringRef(Lib).starts_with("ucrt") ||
- StringRef(Lib).starts_with("crtdll"))
+ StringRef(Lib).starts_with("crtdll")) {
+ // Respect the user's chosen crt variant, but still provide it
+ // again as the last linker argument, because some of the libraries
+ // we added above may depend on it.
+ CmdArgs.push_back(Args.MakeArgStringRef(StringRef(Lib)));
return;
+ }
CmdArgs.push_back("-lmsvcrt");
}
|
10 tasks
This comment was marked as outdated.
This comment was marked as outdated.
Keno
force-pushed
the
kf/driverlastcrt
branch
2 times, most recently
from
39fc8e6 to
f8b59ee
Compare
|
why not use lld? |
It's how the toolchain happens to be configured, and we can't tell users of clang that they need to use lld if they want their builds to work, especially if they work fine with gcc. |
Keno
added a commit
to JuliaLang/llvm-project
that referenced
this pull request
Jul 18, 2025
This implements the mingw `-mcrtdll` option recently added to gcc. This option is useful for having the compiler be in charge of crt version selection while only shipping a single copy of mingw for a multi-ABI toolchain. That said, there are various ABI dependent compiler libraries (e.g. libstdc++), so a certain degree of ABI awareness is nevertheless required in order to use this option correctly. See also llvm#149434
mstorsjo
reviewed
Jul 28, 2025
| if (StringRef(Lib).starts_with("msvcr") || | ||
| StringRef(Lib).starts_with("ucrt") || | ||
| StringRef(Lib).starts_with("crtdll")) | ||
| StringRef(Lib).starts_with("crtdll")) { | ||
| Lib = (llvm::Twine("-l") + Lib).str(); |
There was a problem hiding this comment.
I'd kinda prefer to declare a new variable instead of reusing Lib here. Especially with Lib being declared auto, its type is not entirely obvious here, so it would feel clearer if we'd declare the new one e.g. explicitly std::string here.
I was attempting to build openblas with clang in msys2's `ucrt64` environment (I'm aware of the `clang64` environment, but I wanted libstdc++). The openblas link failed with the following: ``` clang -march=native -mtune=native -m64 -O2 -fno-asynchronous-unwind-tables -O2 -DSMALL_MATRIX_OPT -DMS_ABI -DMAX_STACK_ALLOC=2048 -Wall -m64 -DF_INTERFACE_GFORT -DDYNAMIC_ARCH -DSMP_SERVER -DNO_WARMUP -DMAX_CPU_NUMBER=512 -DMAX_PARALLEL_NUMBER=1 -DBUILD_SINGLE=1 -DBUILD_DOUBLE=1 -DBUILD_COMPLEX=1 -DBUILD_COMPLEX16=1 -DVERSION=\"0.3.29\" -UASMNAME -UASMFNAME -UNAME -UCNAME -UCHAR_NAME -UCHAR_CNAME -DASMNAME= -DASMFNAME=_ -DNAME=_ -DCNAME= -DCHAR_NAME=\"_\" -DCHAR_CNAME=\"\" -DNO_AFFINITY -I.. libopenblas64_.def dllinit.obj \ -shared -o ../libopenblas64_.dll -Wl,--out-implib,../libopenblas64_.dll.a \ -Wl,--whole-archive ../libopenblas64_p-r0.3.29.a -Wl,--no-whole-archive -LC:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0 -LC:/msys64/ucrt64/bin/../lib/gcc -LC:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../../../x86_64-w64-mingw32/lib/../lib -LC:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../../../lib -LC:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../../../x86_64-w64-mingw32/lib -LC:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../.. -lgfortran -lmingwex -lmsvcrt -lquadmath -lm -lpthread -lmingwex -lmsvcrt -defaultlib:advapi32 -lgfortran -defaultlib:advapi32 -lgfortran C:/msys64/ucrt64/bin/ld: C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../../../lib/libmingw32.a(lib64_libmingw32_a-pseudo-reloc.o): in function `__report_error': D:/W/B/src/mingw-w64/mingw-w64-crt/crt/pseudo-reloc.c:157:(.text+0x59): undefined reference to `abort' C:/msys64/ucrt64/bin/ld: C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../../../lib/libmingw32.a(lib64_libmingw32_a-tlsthrd.o): in function `___w64_mingwthr_add_key_dtor': D:/W/B/src/mingw-w64/mingw-w64-crt/crt/tlsthrd.c:48:(.text+0xa5): undefined reference to `calloc' C:/msys64/ucrt64/bin/ld: C:/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/15.1.0/../../../../lib/libmingw32.a(lib64_libmingw32_a-pesect.o): in function `_FindPESectionByName': D:/W/B/src/mingw-w64/mingw-w64-crt/crt/pesect.c:79:(.text+0xfd): undefined reference to `strncmp' ``` These symbols come from the `-lmingw32` dep that the driver added and are ordinarily found in `-lmsvcrt`, which got skipped here, because openblas passed `-lmsvcrt` explicitly. Since we always add these libraries at the end here, I think that clang is "at fault" (as opposed to a user or packaging mistake) and should have added some crt here. To preserve the intent of letting the user override which crt is chosen, duplicate the (first) user chosen crt `-l` into this position, although we should perhaps consider an explicit `-mcrtdll` like gcc has as well.
|
Thanks! FWIW I think this change qualifies to be backported to the 21.x release branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was attempting to build openblas with clang in msys2's
ucrt64environment (I'm aware of theclang64environment, but I wanted libstdc++). The openblas link failed with the following:These symbols come from the
-lmingw32dep that the driver added and are ordinarily found in-lmsvcrt, which got skipped here, because openblas passed-lmsvcrtexplicitly earlier in the link line. Since we always add these libraries at the end here, I think that clang is "at fault" (as opposed to a user or packaging mistake) and should have added some crt here.To preserve the intent of letting the user override which crt is chosen, duplicate the (first) user chosen crt
-linto this position, although we should perhaps consider an explicit-mcrtdlllike gcc has as well.