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

LTO builds on recent nightlies fail "to get bitcode from object file" #292

Open
phil-opp opened this issue May 12, 2020 · 12 comments
Open

Comments

@phil-opp
Copy link

phil-opp commented May 12, 2020

We recently got a bug report for the cargo-xbuild project that builds with LTO fail on recent nightlies: rust-osdev#69. The error message is "error: failed to get bitcode from object file for LTO (Bitcode section not found in object file)". This also seems to affect xargo.

@toku-sa-n created a minimal example to reproduce this issue at https://github.com/toku-sa-n/cargo_bug. Just clone the project and run xargo build --target cargo_settings with a recent nightly to see the error.

We are pretty sure that this is caused by https://github.com/rust-lang/cargo/pull/8192/files (bisect shows rustc commit rust-lang/rust#71925), but don't know what's the best way to fix this. The only thing that worked so far is to set RUSTFLAGS to -Clinker-plugin-lto when compiling the sysroot, but maybe there's a better way.

@RalfJung
Copy link
Collaborator

Thanks for the heads-up! I see the cargo-xbuild issue is closed by adding -Clinker-plugin-lto. Do you know if that's also what rustc bootstrap / -Zbuild-std are doing?

@phil-opp
Copy link
Author

I couldn't find any related changes for -Zbuild-std, maybe I looked in the wrong places. Looking at rustc bootstrap is a good idea though. It seems like std is build with manually passing -Cembed-bitcode=yes via RUSTFLAGS, which apparently overrides the -Cembed-bitcode=no created by cargo:

https://github.com/rust-lang/rust/blob/a2e0b48e6eef854521d3199ee9e327aab298f071/src/bootstrap/compile.rs#L235-L245

I'll try ot change cargo-xbuild to try that approach instead of passing -Clinker-plugin-lto.

@RalfJung
Copy link
Collaborator

I couldn't find any related changes for -Zbuild-std, maybe I looked in the wrong places.

Maybe it doesn't need anything as it builds a special-purpose libstd just for this project -- so either they'll both be LTO or neither of them are.

@phil-opp
Copy link
Author

phil-opp commented May 13, 2020

Passing -Cembed-bitcode=yes via RUSTFLAGS seems to work. Both -Cembed-bitcode=no and -Cembed-bitcode=yes are passed now to rustc, but it seems like the latter wins. It seems a bit hacky to me, but I guess if rustc bootstrap does it this way it should be fine.

Interestingly, -Zbuild-std results in rustc commands without any -Cembed-bitcode arguments at all. So your guess might be right.

@johnthagen
Copy link
Contributor

We see build breakages over at the min-sized-rust #16 repository as well. For a very minimal example, see:

For those trying to minimize the size of Rust binaries, using Xargo + LTO together is extremely important, so I'm hoping that this is able to be fixed in a way that is transparent to the end user.

johnthagen added a commit to johnthagen/min-sized-rust that referenced this issue Jun 14, 2020
johnthagen added a commit to johnthagen/min-sized-rust that referenced this issue Jun 14, 2020
@johnthagen
Copy link
Contributor

For those looking to work around this issue, the easiest method I found was to add .cargo/config file in the root of the project with the contents:

[build]
# This is needed to work around: https://github.com/japaric/xargo/issues/292
rustflags = ["-Cembed-bitcode=yes"]

Though this work-around will make the projects not build on stable:

error: unknown codegen option: `embed-bitcode`

@RalfJung
Copy link
Collaborator

so I'm hoping that this is able to be fixed in a way that is transparent to the end user.

I don't think I'll have time to work on this (and AFAIK nobody else is on it, either). I'll happily review a PR though. :)

@RalfJung
Copy link
Collaborator

Though this work-around will make the projects not build on stable:

I mean xargo can't really be used with stable anyway, building libstd is a nightly-only feature that can only possibly be done on stable by using undocumented internal env vars that should not be used without blessing from the compiler team.

@johnthagen
Copy link
Contributor

I mean xargo can't really be used with stable anyway, building libstd is a nightly-only feature that can only possibly be done on stable by using undocumented internal env vars that should not be used without blessing from the compiler team.

Agreed. It's a super minor point that probably doesn't matter much. Previously some of the examples in min-sized-rust could be built without change on both stable and Xargo with LTO (such as only using no_main or no_std). On stable you wouldn't get as much size reduction as with Xargo, but if you wanted to have the flexibility to choose you could before I added the .cargo/config files to those subprojects.

But yeah, I don't think it's a huge deal as people can read the docs and remove -Cembed-bitcode=yes to build on stable, or use RUSTFLAGS to dynamically pick. I was just trying to keep the examples as simple and flexible for people as possible.

@lberrymage
Copy link

lberrymage commented Jul 17, 2020

For those looking to work around this issue, the easiest method I found was to add .cargo/config file in the root of the project with the contents:

[build]
# This is needed to work around: https://github.com/japaric/xargo/issues/292
rustflags = ["-Cembed-bitcode=yes"]

Though this work-around will make the projects not build on stable:

error: unknown codegen option: `embed-bitcode`

Update: -Cembed-bitcode is now stable. It's also recommended to use .cargo/config.toml instead of .cargo/config.

This information about -Cbitcode-in-rlib=no & -Clto incompatibility appears noteworthy as well.

Just want to keep this information up-to-date for others who stumble across this solution (thank you by the way for that).

@RalfJung
Copy link
Collaborator

Update: -Cembed-bitcode was renamed to -Cbitcode-in-rlib in nightly

No, the other way around 🤣

@lberrymage
Copy link

Whoops! I got a little confused between these two issues. Was I just looking at the older, outdated commit?

I'll update my comment to prevent confusion; thank you.

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

No branches or pull requests

4 participants