rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.34k stars 12.72k forks source link

rustc always links against non-debug Windows runtime #39016

Open jdm opened 7 years ago

jdm commented 7 years ago

When building rust-mozjs with debug symbols enabled, the underlying C++ library links against the debug Windows runtime (msvcrtd.lib) rather than the regular one (msvcrt.lib). Unfortunately, rustc always links against msvcrt.lib unconditionally: https://github.com/rust-lang/rust/blob/da2ce2276873242a101f205537e7ce297d68f8dd/src/vendor/libc/src/windows.rs#L150 . This makes it impossible to build a debug-enabled version of rust-mozjs on Windows.

jdm commented 7 years ago

Would there be any objection to doing something like:

#[cfg(all(target_env = "msvc", debug))] // " if " -- appease style checker
#[link(name = "msvcrtd")]
extern {}

#[cfg(all(target_env = "msvc", not(debug_assertions)))] // " if " -- appease style checker
#[link(name = "msvcrt")]
extern {}

cc @retep998

retep998 commented 7 years ago

We already have a feature for choosing whether to link the static or dynamic CRT. It would be a fairly trivial matter to add another feature to choose whether to link the debug or release CRT (which I did suggest but nobody seemed to notice). I'd rather not tie this sort of thing to whether optimizations are enabled.

jdm commented 7 years ago

@retep998 says that the way to fix this is to find the relevant bits in https://github.com/rust-lang/rust/pull/37545 and duplicate it for crt-debug.

nox commented 7 years ago

37545 landed, what is needed to make some progress on this?

retep998 commented 7 years ago

@nox As I said before, someone has to find the relevant bits in #37545 and duplicate it for crt-debug.

asajeffrey commented 6 years ago

This cost me a couple of days debugging why debugmozjs builds of mozjs_sys were failing with link errors under msvc.

Neurrone commented 6 years ago

Would love to be able to configure this as well for c++ code.

jdm commented 5 years ago

It would make sense to transfer this issue to https://github.com/rust-lang/libc now that the responsible code is in that repository, but I don't have the permissions to do so.

gnzlbg commented 5 years ago

cc @alexcrichton @retep998 @newpavlov - please review the libc C PR at: https://github.com/rust-lang/libc/pull/1433 - I'm not sure how this will interact with linking libc both via libstd and crates.io, what happens if the features are different in each, what happens if the binary is #[no_std] and no libstd is linked, etc.

retep998 commented 5 years ago

Solving this issue requires changes both in rust and libc.

gnzlbg commented 5 years ago

Since libstd depends on libc, and libc would need to be compiled differently for debug and release on windows, wouldn't we need to ship two libstds one for debug and one for release ?

retep998 commented 5 years ago

Only if there are actual changes to the bindings in libc depending on debug vs release. Simply linking to a different CRT is already handled in std's libc using deferred linking decisions.

upsuper commented 4 years ago

This looks like a pretty big footgun, and it can cause mysterious runtime error taking people lots of time to figure out. Until this gets fixed, there should probably be some document / compiler message warning about this, so that people know that they need to have their C/C++ program always link against the non-debug crt.

nox commented 4 years ago

I'm curious how this can be done cleanly, when the base windows target itself adds -lmsvcrt directly into the link args.

retep998 commented 4 years ago

@nox That is specifically for the gcc linker flavor, so -gnu targets only, where the debug CRT isn't a thing anyway. For -msvc where this matters we already have dynamic CRT selection through the libc crate. It just has to be expanded to also handle the option of debug vs non-debug CRT and ditto for the CRT target feature in rustc for it.

petrochenkov commented 4 years ago

I don't think this needs any support from rustc?

A new cargo feature needs to be added to libc crate. Then libc crate will link debug crt using #[link(..., cfg(the_debug_crt_feature))] (the link-time configuration in this case will be done during the user build, not when we build std on CI).

cc crate will probably need a debug_crt method for building C/C++ code with debug crt (similar to already existing static_crt).

So this issue can be closed in favor of two new issues in the repos mentioned above.

retep998 commented 4 years ago

@petrochenkov How is the user supposed to specify cargo features of the libc that std depends on? And why should choosing the debug CRT be different from choosing the static CRT?

petrochenkov commented 4 years ago

@retep998

How is the user supposed to specify cargo features of the libc that std depends on?

By specifying the feature in Cargo.toml, apparently. The only thing that is necessary is for the the_debug_crt_feature or feature = the_debug_crt_feature predicate to be defined during the user build. Then link-time configuration will work, you don't need to build libc or std with those features enabled. (Someone needs to make a proof of concept to show that it indeed works, though.)

And why should choosing the debug CRT be different from choosing the static CRT?

Because +crt-static does many different things on different platforms (cc https://github.com/rust-lang/rust/pull/71586), and the compiler has to be aware of it and change its behavior (e.g. pass different linker options or startup objects). Debug CRT simply changes name of the linked library, libc crate can do that without rustc's involvement.

The only reason to do this through rustc that I can see is to officially bless the feature name, if libc seems to not be official enough.

uazu commented 3 years ago

For anyone else coming here with MSVCRTD issues, I found that setting CFLAGS and CXXFLAGS in the CMD environment to -MDd before doing cargo build solved our problem, and the modification to libc in the 1433 patch was not necessary.

Here's the full scenario: We are building a Rust library using cxx crate to aid with interfacing to C++. The Rust code does network I/O with mio crate so is using libc. The resulting code is compiled as a "staticlib" in order to get a LIB to link with the rest of a (much larger) C++ application which we are attempting to contribute to. The C++ team use debug builds for testing on Windows, so we have to fit in with that. Looking at the LIB generated normally by cargo build using dumpbin /all, I see various mentions of MSVCRT. After observing the processes started during the build using Process Monitor, and checking cc crate documentation, I tried setting CFLAGS=-MDd and CXXFLAGS=-MDd. This was observed to insert the -MDd into the cl.exe command-line in a place suitable to override the default of -MD. After a complete rebuild, checking the dumpbin /all output I see only mentions of MSVCRTD. Now when I do a debug (/MDd) build of the C++ code with the generated Rust LIB, it succeeds.

If this is suitable as a solution for others, it would be good to document it somewhere. I do not fully understand Windows building and linking, so I can't be sure myself.

ahicks92 commented 2 years ago

Late to this discussion (and still hoping this gets improved) but that's an interesting workaround. The problem is that when you e.g. try to use cmake or whatever you still need the debug vs. release knowledge to propagate downward. I would guess that setting CFLAGS and CXXFLAGS at the top level only works by chance, and not reliably.

ChrisDenton commented 1 year ago

I really want to find a way forward here as it's a common issue when trying to integrate Rust with existing C/C++ builds.

I do find it slightly odd that linking the C runtime is the responsibility of the libc crate on Windows. That seems unusual compared to other platforms? But ok, I can live with that.

@petrochenkov suggested adding a feature in people's Cargo.toml, which seems a bit odd for this? Like I'm not entirely clear how this will work in practice without special Cargo support, otherwise it puts a lot of the effort of supporting this onto crate authors. A global --cfg (i.e. set in rustflags) might work better. It is super important that everything agrees on which CRT to use. Though I'm still not clear on the benefits of --cfg over a compiler codegen option.

So to sum up I think to do this outside of rustc will require changes to:

Is that right?

Btw, there should also be an option to not link a CRT. This is the default for no_std on Windows but is not yet supported for std builds. Having this option is important for more exotic CRTs.

danakj commented 1 year ago

Hitting this issue in Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1434719

ChrisDenton commented 1 month ago

Update on this: You can now use a build script (or other build method) to disable the default CRT for a binary. Then you can add back the one you want. However, you need to use a linker argument:

For the dynamic CRT use: /nodefaultlib:msvcrt For the static CRT use: /nodefaultlib:libcmt

This can be done in a build script, for example:

fn main() {
    // Don't link the default CRT
    println!("cargo::rustc-link-arg-bins=/nodefaultlib:msvcrt")`
    // Link the debug CRT instead
    println!("cargo::rustc-link-arg-bins=/defaultlib:msvcrtd")`
}

Which isn't as nice as having a proper feature but it's at least serviceable.

sagudev commented 1 month ago

Update on this: You can now use a build script (or other build method) to disable the default CRT for a binary. Then you can add back the one you want. However, you need to use a linker argument:

For the dynamic CRT use: /nodefaultlib:msvcrt For the static CRT use: /nodefaultlib:libcmt

This can be done in a build script, for example:

fn main() { // Don't link the default CRT println!("cargo::rustc-link-arg-bins=/nodefaultlib:msvcrt") // Link the debug CRT instead println!("cargo::rustc-link-arg-bins=/defaultlib:msvcrtd") }

Which isn't as nice as having a proper feature but it's at least serviceable.

What is MSRV for this?

ChrisDenton commented 1 month ago

1.79

retep998 commented 1 month ago

Also note that if you do that, you need to make sure every C/C++ library you link against was built using the correct version of the CRT as well. If you end up mixing them then you're going to have a bad time with a lot of linker errors.