rust-lang / rust

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

Enabling `+crt-static` in a blanket way breaks dynamic libraries including proc macros #71651

Open petrochenkov opened 4 years ago

petrochenkov commented 4 years ago

This is pretty much https://github.com/rust-lang/cargo/issues/7563 generalized, which was fixed in https://github.com/rust-lang/rust/pull/69519, which I don't consider a correct or satisfactory solution. https://github.com/rust-lang/rust/pull/71586 may be a good preliminary reading.


If you are building something with Cargo and set -C target-feature=+crt-static through RUSTFLAGS, or crt-static is enabled by default through a target specification, it will be set for all crates during the build, including proc macros and cdylibs (and build scripts?).

In most cases this is not an intention when we are enabling crt-static. In most cases the intention is to enable it for executables only.

So if enabling crt-static for executables (via env var or a target spec) also enables it for libraries, it usually results only in "collateral damage". If the target doesn't support +crt-static for libaries, we just get an error like https://github.com/rust-lang/cargo/issues/7563 reported. If the target supports +crt-static for libraries, we get a very weird library which is unlikely to work when dynamically loaded as a proc macro crate (I didn't verify that though).

As a result, we need a way to enable crt-static for executables without collaterally damaging libaries. Moreover, this way should be the default.


https://github.com/rust-lang/rust/pull/71586#discussion_r416054543 lists some possible alternatives of doing this.

petrochenkov commented 4 years ago

I think a new option -Ctarget-feature=+crt-static-dylib (and corresponding target spec defaults) is the way to go here.


The only thing we need to assess is compatibility for windows-msvc and wasm32-wasi targets which currently supports linking .dlls and .wasm libraries with +crt-static.

In case of WASM we can enable +crt-static-dylib by default though the target spec and keep full compatibility.

In case of Windows-MSVC I need @retep998's expertise here. How common it is to use +crt-static to build DLLs on that target? Would it be a noticeable breakage if we required using the new +crt-static-dylib for that instead? (In any case we can always enable a special compatibility case "+crt-static implies +crt-static-dylib" for the windows-msvc target, if that turns out necessary.)

retep998 commented 4 years ago

+crt-static for the cdylib crate type on Windows is completely valid and I imagine should be used just as often as +crt-static is for regular executables. Switching to a different flag for cdylibs would be a noticeable breakage, and I'd much prefer we don't break Windows users for an issue that doesn't affect them.

The big issue you have to care about on Windows with regards to the CRT is that you never share CRT objects across a dll boundary unless both sides are using the same dynamic CRT. Because using CRT types is very rare in the Rust ecosystem, and the proc macro interface does not have any CRT types in it, I see no reason why +crt-static can't be used for proc macros on Windows (although there's also no point as the proc macro binaries are never distributed).

petrochenkov commented 4 years ago

@alexcrichton Could you answer a couple of questions regarding the meaning of +crt-static on wasm32-wasi?

Is it a common case to bundle libc (I'm not even sure what it does on the target) into wasm 1) executables and 2) libraries? (I suspect the answer is yes, since that's the default in the target spec.)

Is it desirable to support the case of bundling libc, but linking dynamically to some other libraries?

alexcrichton commented 4 years ago

For wasm32-wasi the meaning of crt-static is sort of unrelated to dynamic/static linkage since the dynamic linkage story with wasm isn't quite as specified as other platforms. The meaning of crt-static is, as you've found, whether the bundled libc is used or whether an external libc is provided.

The purpose here is somewhat similar to MinGW where, by default, if you don't compile any C code in your application cargo build "just works". If, however, you'd like to link C/Rust code into the same wasm binary then you'll want the libc used to be the same, so whatever compiler is used to compile the C code should provide the libc for Rust as well. Using -crt-static indicates "go use some other compiler's libc implementation".

It's currently most common to not pass anything, meaning +crt-static is very common. AFAIK there's almost no major users of -crt-static for wasi. I am not personally wed at all to using crt-static for this option, and I think it'd be fine to tweak wasm to use something else entirely or a new definition.

smaeul commented 4 years ago

Part of the confusion here is that +crt-static means something totally different on MSVC and Linux. On MSVC, the flag is meaningful on any crate type (except rlib/staticlib, which don't get linked), and affects only the local crate. On Linux (musl), the flag is only meaningful on executables, but it affects everything linked into that executable. And on WASM it means something else entirely.

As far as I'm aware, crt-static works fine on MSVC, and there's no need to change anything there.

  • Ignore +crt-static for libraries if the target doesn't support it. This is fragile, musl actually supports it despite the current value of the flag of the musl target spec. If the flag is enabled, proc macros will break.

I think this is the correct answer. Can you please expand on what makes it fragile? And what you're referring to that "musl actually supports"?

Linking a dylib/cdylib against musl's libc.a is not and will never be supported -- it puts a second, conflicting copy of libc global data (e.g. malloc state and locks) in your executable at runtime, which is more than likely to crash. The situation would be the same for glibc if rustc supported +crt-static there.

Ignoring +crt-static when compiling a dylib/cdylib, as opposed to producing an error like we do now, shouldn't cause any problems, because ignoring the flag doesn't "leak" into the executable. Since DSOs can't be linked from static executables anyway, the presence or absence of the dylib/cdylib crate cannot affect the resulting binary.

I would also expect it to be common for projects to provide both a dylib crate and executables (CLI, tests, etc.). I only see two ways to compile both from the same cargo command with crt_static_default == true and crt_static_allows_dylibs == false: either rustc ignores +crt-static for crate types other than Executable, or cargo specifies the right option on a per-crate basis.

petrochenkov commented 4 years ago

@smaeul Thanks for all the references, I'll return to this in a few days and read them.

Linking a dylib/cdylib against musl's libc.a is not and will never be supported -- it puts a second, conflicting copy of libc global data (e.g. malloc state and locks) in your executable at runtime, which is more than likely to crash.

Sometimes having a separate copy of libc with all the global data in a .so is exactly the desired outcome. My main use case for this setup is a plugin to a dynamic binary instrumentation system, which must not share libc with the instrumented program. I'm doing it with bionic libc though, not with musl, and it works for a non-toy project.

And what you're referring to that "musl actually supports"?

With musl I tried it with "hello world"-like examples and it worked too :)

UPD: Clarification - I did all of this with C++ and gcc, not with Rust (it doesn't matter much in this case though).

petrochenkov commented 4 years ago

Some history in links: https://github.com/rust-lang/rust/pull/71586#issuecomment-626328134

da-x commented 3 years ago

If anyone's interested, here's the workaround I use for the simple case where the top crate has a program and you just want that crate to build static programs:

1) Intercept rustc:

RUSTC=/path/to/rustc.wrap cargo build

2) Sample rustc.wrap script:

#!/bin/bash

crate_name=0

for i in "$@" ; do
    if [[ $i == "--crate-name" ]] ; then
    crate_name=1
    continue
    elif [[ "$crate_name" == "1" ]] ; then
    if [[ "$i" == "crate-name-of-your-program" ]] ; then
        set -- "$@" -C target-feature=+crt-static
        break
    fi
    fi
    crate_name=0
done

exec rustc "$@"
petrochenkov commented 2 years ago

Copypasting from https://github.com/rust-lang/rust/pull/90020#issuecomment-985215220:

I went through major targets supporting dynamic linking and they all seem to expose relatively similar behavior with regards to linking CRT (libc or analogues) dynamically or statically. Here's some summary.

An executable and all its dynamic dependencies must share a common CRT for things like cross-binary exceptions and cross-binary allocations/deallocations on platform like Windows (which don't do most of OS interactions through CRT), and maybe for other shared resource management as well on platforms like Linux (which do most of OS interactions through CRT). Next I'll consider different kinds of end binaries and their relationships with dynamic and static CRT.

Whether a dynamic library may require managing shared resources is ultimately a user's choice. We can easily take "yes, it requires managing shared resources, and thus requires shared CRT" as a default for both Rust dylibs and native dylibs, and add a linking modifier for opting out, -needs-shareed-crt or something.

Side note: for targets doing dynamic linking in userspace via ELF interpreter field (Linux), we can remove the interpreter field and speedup their launch if there's no dynamic dependencies at all (CRT or others).

So the question is what "with(out) an explicit request" above means. In general, I'd say -Ctarget-feature=-crt-static is explicit enough because dynamic linking is never exotic, but -Ctarget-feature=+crt-static is not explicit enough.

Considering the above, I think my suggested solution is:

oxalica commented 1 year ago

This issue is still not resolved after a year. It blocks me from static linking (using glibc) projects has any proc-macro crates in the dependency closure. I think it's not something like "how to deal with crt-static on libs". It's the crt-static should applied to any build-time dependencies in the first place.

Maybe we should make cargo aware of this option in [profile.*], so we could treat build-time dependencies differently through [profile.*.build-override].