-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Statically link libstdc++ on windows-gnu #65911
Conversation
r? @kennytm (rust_highfive has picked a reviewer for you, use r? to override) |
Cc @Amanieu who found the cause and could be interested in this PR. |
@@ -279,7 +279,11 @@ fn main() { | |||
let path = PathBuf::from(s); | |||
println!("cargo:rustc-link-search=native={}", | |||
path.parent().unwrap().display()); | |||
println!("cargo:rustc-link-lib=static={}", stdcppname); | |||
if target.contains("windows") { | |||
println!("cargo:rustc-link-lib=static-nobundle={}", stdcppname); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may cause distribution issues since we ship rustc_llvm
and users like clippy/plugins will link to rustc_llvm
transitively through rustc_driver
, was there a reason though static
didn't work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think quite nasty or I'm just misunderstanding.
First let's take a look at static libstdc++:
nm /mingw64/lib/gcc/x86_64-w64-mingw32/9.2.0/libstdc++.a | grep pthread_mutex_init
U __imp_pthread_mutex_init
U pthread_mutex_init
U pthread_mutex_init
[..]
There are references to both static and dynamic symbols!
Now using static
failed undefined with 2 undefined symbols: __imp_pthread_mutex_{destroy,init}
. I honestly have no idea why picked dynamic symbols, though I haven't chased that rabbit.
It could be mitigated by dynamically linking to winpthread library (by removing static
in line 298), windows-gnu ships libwinpthread-1.dll
anyway because. But I felt it's just replacing one problem with another.
With some guidance I could try to investigate why picks dynamic pthread symbols when using static
but static-nobundle
seems to fit here (I asked for try build in OP to be sure).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm ok, I'm not really sure what's going on with static
vs static-nobundle
, but now that I think about this we're linking this into librustc_driver.so
where the "nobundle" part won't keep propagating, so this doesn't actually affect consumers, and as a result should be fine to land.
Want to revert the CI configuration back to the default? Otherwise r=me? |
I thought you might want to give it |
c814d88
to
473a498
Compare
Removed CI changing commit. |
Oh sorry I misread the ci changes, can this run the mingw test on this pr to ensure they pass before we send to bors? |
c8b551a
to
b20d8d3
Compare
All mingw tests are green: https://dev.azure.com/rust-lang/rust/_build/results?buildId=12392&view=logs&jobId=5ac1211a-cbe7-52be-dcc2-57e3991fd20c |
👍 Sounds good to me! @bors: r+ |
📌 Commit b20d8d3 has been approved by |
…lexcrichton Statically link libstdc++ on windows-gnu Fixes rust-lang#61561 by not shipping `libstdc++-6.dll` which can conflict with the GCC.
…lexcrichton Statically link libstdc++ on windows-gnu Fixes rust-lang#61561 by not shipping `libstdc++-6.dll` which can conflict with the GCC.
Statically link libstdc++ on windows-gnu Fixes #61561 by not shipping `libstdc++-6.dll` which can conflict with the GCC.
☀️ Test successful - checks-azure |
Fixes #61561 by not shipping
libstdc++-6.dll
which can conflict with the GCC.