-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Support dynamically-linked and/or native musl targets #40113
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eddyb (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
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.
Looks great to me! Just some minor comments but otherwise I think this is good to go.
src/bootstrap/lib.rs
Outdated
fn crt_static(&self, target: &str) -> Option<bool> { | ||
self.config.target_config.get(target) | ||
.and_then(|t| t.crt_static) | ||
} |
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.
As an idea actually, I think we can leverage this for MSVC as well. The bin/rustc.rs
script above has a hard-coded branch for MSVC but I think we could change that here. Could this code be updated with something like:
.or_else(|| {
if target.contains("msvc") {
Some(true)
} else {
None
}
})
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.
Since +crt-static was previously unconditional for msvc, I left it that way when moving the check over.
src/bootstrap/sanity.rs
Outdated
if build.musl_root(target).is_none() && build.config.build == *target { | ||
let target = build.config.target_config.entry(target.clone()) | ||
.or_insert(Default::default()); | ||
target.musl_root = Some("/usr".into()); |
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.
Could you add a comment her to what this block is doing?
While working on getting these two details sorted out, I tried to use the stage2 rustc to bootstrap from again. However, it was getting immediately SIGKILL'd even with --help or --version. readelf told me it had no .text section. Because src/librustc_back/target/linux_musl_base.rs adds -nostdlib to the linker arguments (to support building on glibc-targeting toolchains), the crt*.o startup files are not getting added, ld does not find the _start symbol, and -Wl,-O1 causes it to optimize out the entire binary. We need to conditionally pass -nostdlib to the linker, just like we conditionally provide the startup files. However, pre_link_args is used by several other targets, so I'm not sure what to change. I will rebase and take care of the other issues, though. |
b052295
to
706fc55
Compare
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.
Ouch yeah that sounds like a bad issue. That's not fixed up in this PR, right? If so, did you want to include it here or postpone for a future PR? (I'm fine with either)
src/bootstrap/compile.rs
Outdated
@@ -106,14 +106,17 @@ pub fn std_link(build: &Build, | |||
t!(fs::create_dir_all(&libdir)); | |||
add_to_sysroot(&out_dir, &libdir); | |||
|
|||
if target.contains("musl") && !target.contains("mips") { |
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.
Oh I thought we were replacing the contains("mips")
annotations with a different one? Otherwise this may end up breaking the mips-musl build :(
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.
It was my understanding that we wanted to support both +crt-static and -crt-static for user code with any build of rustc. The crt_static option in config.toml only controls if rustc itself is statically or dynamically linked.
If we want to support static linking (especially with a glibc-based toolchain) on a target, we need musl-root and the startup files for that target.
src/bootstrap/sanity.rs
Outdated
@@ -157,8 +157,15 @@ pub fn check(build: &mut Build) { | |||
panic!("the iOS target is only supported on OSX"); | |||
} | |||
|
|||
// Make sure musl-root is valid if specified | |||
if target.contains("musl") && !target.contains("mips") { |
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.
Like above, the mips check being removed here may break that build
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.
As above, mips would be dynamic-only without this change
@bors: r+ Hm ok, makes sense. Let's see if mips builds break |
📌 Commit 706fc55 has been approved by |
In regards to -nostdlib completely breaking stage2, there are three different situations I see we're trying to support here:
Cases 1 and 2 can use the toolchain-provided crt*.o, libc, and libgcc_s/libunwind, so they do not need any files copied over, and they do not need any extra linker arguments. Case 3 requires providing our own crt*.o, libc.a, and libunwind.a, and it requires -nostdlib to suppress the default toolchain versions. We can make case 2 look like case 3, since static linking is self-contained, but it is a bit of a hack (e.g. using /usr for musl-root will lock libc version to the one we copied into the sysroot when building rustc). Making case 1 look like case 3 is even more difficult and fragile. Sharing the same TargetOptions among all three is just not going to work. I see three ways to resolve this. I don't know which is most preferable.
If I completely remove -nostdlib and the target files (unconditionally), I can build a complete stage2 with crt_static=true in config.toml. Building a stage1 with crt_static=false in config.toml also works, but fails in stage2-std trying to link shared libraries against a static libunwind. I have not investigated this yet, but it seems like a separate issue. |
Support dynamically-linked and/or native musl targets These changes allow native compilation on musl-based distributions and the use of dynamic libraries on linux-musl targets. This is intended to remove limitations based on past assumptions about musl targets, while maintaining existing behavior by default. A minor related bugfix is included.
Looks like there's a failure here: https://travis-ci.org/rust-lang/rust/jobs/209064374 @bors r- |
@smaeul oh AFAIK we don't support static linking with glibc at all today (or at least if it looks like we do it wasn't intended) |
@alexcrichton IIUC, "Static linking with a glibc toolchain" means, in this context "using a glibc toolchain to statically link to musl" (which, again IIUC, is the only currently supported mode prior to this PR) |
@alexcrichton I could have been more clear there. I meant (for case 3 above) statically linking to musl with a toolchain that targets glibc, i.e. where musl-gcc or ${ARCH}-linux-musl-gcc is not available. As far as I can tell, that's the only case where musl-root or -nostdlib is necessary. |
Oh right yeah that makes sense. In reality we don't need a glibc toolchain at all, the intention is that we can produce a musl binary at any time. All we need is a linker really. But yeah I think the best solution is not passing -nostdlib if we're compiling with |
The build failures here look to be because the tests for mips are not expecting a musl-root, so the default (/usr/local) is being used. The configure script and test infrastructure will need to be updated to add an option for this and generate the appropriate libraries. |
@smaeul the configuration is in tree actually, mind updating that to pass the right configure args? |
ping @smaeul, any updates here? |
About figuring out when to pass -nostdlib and crt*, I found the easiest way to get things working both shared and static was to remove them entirely. In other words, if we just let the linker find libc and the startup files, we don't need any overriding at all (no linker flags, no musl root), as long as the linker knows where to find the files. And if the compiler is native, an appropriate cross linker exists, or musl-gcc exists, that is a valid assumption. Unfortunately, it breaks current behavior -- I don't know what's acceptable here. Otherwise, we'll need to add some sort of conditional expression to src/librustc_back/target/linux_musl_base.rs, and I don't know how to do that. I haven't worked on the CI/configure issue because I'm not sure how good of an idea it is to get this "working" but at the same time completely broken. I'd like to try to get the linking issue fixed first. |
Yeah the purpose of the musl target today is that you don't need to have musl-gcc or a similar wrapper installed, so we'll need to preserve the capability somehow. |
I’ve tried to build rustc in Alpine Linux with this patch and I hit few issues. At first, there’s a relevant part of the build script: pkgver=1.16.0
_stage0dir="$srcdir/stage0"
# Don't set wrong LD_LIBRARY_PATH, we will rather set it manually when
# invoking make.
sed -i /LD_LIBRARY_PATH/d src/bootstrap/bootstrap.py
# Precompiled rustc and rust-std from VoidLinux.
cp -flr "$srcdir"/rustc-$pkgver-*/rustc/* \
"$srcdir"/rust-std-$pkgver-*/rust-std-*/* \
"$srcdir"/cargo-*-*/cargo/* \
"$_stage0dir"/
# Note: We use llvm 3.8.1 for now.
./configure \
--build="$_ctarget" \
--host="$_ctarget" \
--prefix="/usr" \
--release-channel="stable" \
--enable-local-rust \
--local-rust-root="$_stage0dir" \
--llvm-root="/usr" \
--musl-root="/usr" \
--enable-rustbuild \
--enable-vendor \
--disable-docs \
--disable-jemalloc \
--disable-rpath
# Set LD_LIBRARY_PATH, so rustc in stage0 can find correct libs.
LD_LIBRARY_PATH="$_stage0dir/lib" \
make RUST_BACKTRACE=1 RUST_CRT_STATIC="false" VERBOSE=1
LD_LIBRARY_PATH="$_stage0dir/lib" \
make install DESTDIR="$pkgdir" I’ve removed - // Make sure that the linker/gcc really don't pull in anything, including
- // default objects, libs, etc.
- base.pre_link_args.push("-nostdlib".to_string()); The build succeeds, but rustc fails when trying to compile anything:
We can see three problems here:
I’ve fixed the second problem by removing the following snippet from - // When generating a statically linked executable there's generally some
- // small setup needed which is listed in these files. These are provided by
- // a musl toolchain and are linked by default by the `musl-gcc` script. Note
- // that `gcc` also does this by default, it just uses some different files.
- //
- // Each target directory for musl has these object files included in it so
- // they'll be included from there.
- base.pre_link_objects_exe.push("crt1.o".to_string());
- base.pre_link_objects_exe.push("crti.o".to_string());
- base.post_link_objects.push("crtn.o".to_string());
- base.pre_link_args.push("-Wl,-(".to_string());
- base.post_link_args.push("-Wl,-)".to_string());
However, statical linking is now broken:
(Yes, I’ve removed that pesky check that rejects So we probably need to make some options in @shizmob tried different approach, he added: + // To prevent some nasty linking errors, link in libgcc_s here.
+ base.pre_link_args.push("-lgcc_s".to_string()); This also works, but haven’t tested compiling with /cc @japaric |
That was my conclusion from #38075 but I'm not too fond of the idea. #40018 already makes the link_args fields conditional wrt to |
Do not assume libunwind.a is available on musl Fixes #40113, #44069, and clux/muslrust#16. libunwind.a is not copied from musl_root, so it must be integrated into the unwind crate.
I don't know if this is the right place to ask but how do I build this on Alpine? I tried cloning master and following the readme and then did this |
Use the correct crt*.o files when linking musl targets. This is supposed to support optionally using the system copy of musl libc instead of the included one if supported. This currently only affects the start files, which is enough to allow building rustc on musl targets. Most of the changes are analogous to crt-static. Excluding the start files is something musl based distributions usually patch into their copy of rustc: - https://github.com/alpinelinux/aports/blob/eb064c8/community/rust/musl-fix-linux_musl_base.patch - https://github.com/voidlinux/void-packages/blob/77400fc/srcpkgs/rust/patches/link-musl-dynamically.patch For third-party distributions that not yet carry those patches it would be nice if it was supported without the need to patch upstream sources. ## Reasons ### What breaks? Some start files were missed when originally writing the logic to swap in musl start files (gcc comes with its own start files, which are suppressed by -nostdlib, but not manually included later on). This caused #36710, which also affects rustc with the internal llvm copy or any other system libraries that need crtbegin/crtend. ### How is it fixed? The system linker already has all the logic to decide which start files to include, so we can just defer to it (except of course if it doesn't target musl). ### Why is it optional? In #40113 it was first tried to remove the start files, which broke compiling musl-targeting static binaries with a glibc-targeting compiler. This is why it eventually landed without removing the start files. Being an option side-steps the issue. ### Why are the start files still installed? This has the nice side-effect, that the produced rust-std-* binaries can still be used by on a glibc-targeting system with a rustc built against glibc. ## Does it work? With the following build script (using [musl-cross-make](https://github.com/richfelker/musl-cross-make)): https://shadowice.org/~mixi/rust-musl/build.sh, I was able to cross-compile a musl-host musl-targeting rustc on a glibc-based system. The resulting binaries are at https://shadowice.org/~mixi/rust-musl/binaries/. This also requires #50103 and #50104 (which are also applied to the branch the build script uses).
Libc dynamic library problem: It builds sucessfully,but runs failed. It looks like libc.so is dynamically linked, but the function"calloc"(which is a member of libc) is statically linked. This caused libc to fail to call calloc during loading. How to solve this problem? ps:There are both libc.a and libc.so in the compilation environment. Could rust links libc.so only, without libc.a linked ? |
@lxylbds this is a pretty ancient PR. Maybe try asking on the Rust discord? Or perhaps create a new issue if you think this is a bug? |
These changes allow native compilation on musl-based distributions and the use of dynamic libraries on linux-musl targets. This is intended to remove limitations based on past assumptions about musl targets, while maintaining existing behavior by default.
A minor related bugfix is included.