Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upSupport dynamically-linked and/or native musl targets #40113
Conversation
rust-highfive
assigned
eddyb
Feb 26, 2017
This comment has been minimized.
This comment has been minimized.
|
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. |
This comment has been minimized.
This comment has been minimized.
rust-highfive
assigned
alexcrichton
and unassigned
eddyb
Feb 26, 2017
alexcrichton
reviewed
Feb 27, 2017
|
Looks great to me! Just some minor comments but otherwise I think this is good to go. |
| fn crt_static(&self, target: &str) -> Option<bool> { | ||
| self.config.target_config.get(target) | ||
| .and_then(|t| t.crt_static) | ||
| } |
This comment has been minimized.
This comment has been minimized.
alexcrichton
Feb 27, 2017
Member
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
}
})
This comment has been minimized.
This comment has been minimized.
smaeul
Mar 6, 2017
Author
Contributor
Since +crt-static was previously unconditional for msvc, I left it that way when moving the check over.
| 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()); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
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. |
smaeul
force-pushed the
smaeul:native-musl
branch
3 times, most recently
from
b052295
to
706fc55
Mar 6, 2017
alexcrichton
reviewed
Mar 6, 2017
|
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) |
| @@ -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") { | |||
This comment has been minimized.
This comment has been minimized.
alexcrichton
Mar 6, 2017
Member
Oh I thought we were replacing the contains("mips") annotations with a different one? Otherwise this may end up breaking the mips-musl build :(
This comment has been minimized.
This comment has been minimized.
smaeul
Mar 6, 2017
Author
Contributor
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.
| @@ -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") { | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors: r+ Hm ok, makes sense. Let's see if mips builds break |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
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. |
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this pull request
Mar 8, 2017
bors
added a commit
that referenced
this pull request
Mar 8, 2017
This comment has been minimized.
This comment has been minimized.
|
Looks like there's a failure here: https://travis-ci.org/rust-lang/rust/jobs/209064374 @bors r- |
This comment has been minimized.
This comment has been minimized.
|
@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) |
This comment has been minimized.
This comment has been minimized.
|
@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) |
This comment has been minimized.
This comment has been minimized.
|
@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. |
This comment has been minimized.
This comment has been minimized.
|
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 |
This comment has been minimized.
This comment has been minimized.
|
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. |
This comment has been minimized.
This comment has been minimized.
|
@smaeul the configuration is in tree actually, mind updating that to pass the right configure args? |
This comment has been minimized.
This comment has been minimized.
|
ping @smaeul, any updates here? |
This comment has been minimized.
This comment has been minimized.
|
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. |
This comment has been minimized.
This comment has been minimized.
|
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. |
This comment has been minimized.
This comment has been minimized.
jirutka
commented
Apr 7, 2017
•
|
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 |
This comment has been minimized.
This comment has been minimized.
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 |
smaeul
added some commits
Aug 22, 2017
smaeul
force-pushed the
smaeul:native-musl
branch
from
749dc84
to
e6cd941
Aug 22, 2017
This comment has been minimized.
This comment has been minimized.
|
@alexcrichton I was able to replace the offending commit and get tests to pass, so everything should be good to go. |
This comment has been minimized.
This comment has been minimized.
|
@bors: r+ |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Aug 23, 2017
This comment has been minimized.
This comment has been minimized.
|
|
bors
merged commit e6cd941
into
rust-lang:master
Aug 23, 2017
bors
referenced this pull request
Aug 23, 2017
Merged
Run the miri test suite on the aux builder and travis #43628
golddranks
referenced this pull request
Aug 24, 2017
Closed
The newest nightly breaks static musl builds. #44069
This comment has been minimized.
This comment has been minimized.
|
I think this PR broke static musl builds. Posted a new issue and a testcase: #44069 |
smaeul
referenced this pull request
Aug 24, 2017
Merged
Do not assume libunwind.a is available on musl #44070
bors
added a commit
that referenced
this pull request
Aug 24, 2017
bors
added a commit
that referenced
this pull request
Aug 24, 2017
bors
added a commit
that referenced
this pull request
Aug 25, 2017
This comment has been minimized.
This comment has been minimized.
corbinu
commented
Aug 29, 2017
|
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 |
smaeul commentedFeb 26, 2017
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.