Skip to content
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

Don't bundle static libc. #1327

Closed
wants to merge 1 commit into from
Closed

Conversation

hvenev
Copy link

@hvenev hvenev commented Apr 28, 2019

No description provided.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (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.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 28, 2019

Ping me when CI is green.

@hvenev
Copy link
Author

hvenev commented Apr 28, 2019

Ping.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 29, 2019

cc @alexcrichton this LGTM, but it makes me wonder why don't we always use static-nobundle instead of static for all targets linking statically (e.g. redox, wasi, etc.).

@alexcrichton
Copy link
Member

This PR would unfortunately break both these targets as distributed with rust-lang/rust. The usage of static here is the precise use case for where static was designed for, which is updating the artifacts so C library doesn't have to be distributed separately but rather it's bundled with the Rust distribution.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 29, 2019

@alexcrichton I'm not sure I understand. So we don't ship the C library with rustup, so that it is just linked like a normal library when linking the binary, but we statically link it and "bundle it" into the libc rlib distributed with rustc ? If so, does static linking here bundle the whole C library, or only what libc exposes? Also, static-nobundle wouldn't do this, but instead would require a C library to be shipped with rust, and when linking the standard library, this C library would need to be available?

@alexcrichton
Copy link
Member

Yes, linking with static will find libc.a and then take all the object files from there and put them in liblibc.rlib. With static-nobundle that doesn't happen and simply passes -lc, which would break cross compilation use cases for musl and wasi support which doesn't require installing the sysroot.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 29, 2019

So one more thing. How does cross compiling for a #![no_std] binary that uses the libc crate from crates.io without the use_std feature work ? No link(name = "c") attributes are added in that case, so this is just a bare extern {} block.

@alexcrichton
Copy link
Member

I think it just basically doesn't work, building a fully linked binary with #[no_std] is pretty unstable and never something we've really optimized for

@hvenev
Copy link
Author

hvenev commented Apr 29, 2019

I don't see how musl is that different from glibc, uclibc, or dietlibc. Shouldn't something like target_vendor = "rust" be used to decide whether to bundle libc in liblibc.rlib versus using the one provided by the C toolchain?

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 29, 2019

I don't see how musl is that different from glibc

IIUC glibc is dynamically linked, while musl appears to be always statically linked. I suppose that if we were to statically link glibc, then we'd have a similar issue.

@hvenev
Copy link
Author

hvenev commented Apr 29, 2019

Not necessarily. My platform does not support static linking and uses musl.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 30, 2019

And when compiling with rustc musl gets dynamically linked? Which target triple is that? (I thought rustc always statically linked musl for the musl targets).

@hvenev
Copy link
Author

hvenev commented Apr 30, 2019

A slightly modified mips-openwrt-linux-musl. With this patch applied and a similar change to libunwind in rustc, code compiles and runs just fine. Even unwinding seems to be there (not tested) and backtrace on panic works.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 30, 2019

@alexcrichton to support that it might make sense to move the logic of whether musl is statically or dynamically linked to the target specification file.

@hvenev
Copy link
Author

hvenev commented Apr 30, 2019

Isn't the crt-static target feature supposed to be doing that?

@alexcrichton
Copy link
Member

Dealing with crt linkage on musl is basically an endless world of pain unfortunately in what we have set up in rustc. It's all workable one way or another but it's very meticulously crafted right now and I would discourage large changes without having a fuller picture of what's happening.

@hvenev
Copy link
Author

hvenev commented Apr 30, 2019

I agree with you. I've opened a new issue at rust-lang/rfcs#2695. I'd like to hear your suggestions about how to deal with this.

@alexcrichton
Copy link
Member

I can be around to answer questions from time to time, but I do not have the time to help design a new system and or help architect changes.

@gnzlbg
Copy link
Contributor

gnzlbg commented May 2, 2019

I think that the fix here would be to just set target_env to dynmusl, such that the target triplet becomes mips-openwrt-linux-dynmusl, as proposed here: rust-lang/rust#59302 , at least until a solution for rust-lang/rfcs#2695 would be RFCed and implemented.

@mati865
Copy link
Contributor

mati865 commented May 3, 2019

Such target_env has to be implemented first.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 6, 2019

@hvenev I'm closing this, I think that the next step here is to design a solution in rust-lang/rust for how to support both statically and dynamically-linked musl targets. And then propose a combined set of patches that implements support for this in rustc and libc, without breaking the statically linked musl targets that we already support.

@hvenev it seems that rust-lang/rust#59302 is a good place to discuss and organize work around this.

@petrochenkov
Copy link
Contributor

Posted a plan for the fixing this in rustc repo rust-lang/rust#72274.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants