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

Regression: lto = true in build.rs causes cargo test --release --doc to fail to link #8654

Closed
briansmith opened this issue Aug 26, 2020 · 4 comments · Fixed by #8657
Closed
Assignees
Labels
A-lto Area: link-time optimization C-bug Category: bug regression-from-stable-to-stable Regression in stable that worked in a previous stable release.

Comments

@briansmith
Copy link

This regression occurs in the beta:

Problem

cargo test --doc --release fails with linker error "fatal error LNK1107: invalid or corrupt file: cannot read at 0xD2A" on Windows using MSVC 2019 when lto = true is in the release build profile, in the beta and nightly channels. cargo test --doc (non-release builds) succeeds. cargo test --doc --release succeeds in the current stable release, cargo 1.45.1 (f242df6ed 2020-07-22).

Cargo.toml:

[package]
name = "lto-rustdoc"
version = "0.1.0"
edition = "2018"

[profile.release]
opt-level = 3
lto = true

src/lib.rs:

/// ```
/// lto_rustdoc::foo();
/// ```
pub fn foo() {}

The failures do not occur when the current stable cargo is used.

Possible Solution(s)

I suspect that PR #8192 or one of the follow-ups such as PR #8349 caused this regression.

Notes

$ cargo +beta version
cargo 1.47.0-beta (51b66125b 2020-08-19)

This also occurs in Nightly. It has been occurring in nightly for several weeks, and in the last few weeks it has occurred in Beta. Thus, the regression most likely happened before the last promotion of Nightly to Beta.

I used Process Monitor to capture the command lines for Cargo 1.45.1:

"C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.27.29110\bin\HostX64\x64\link.exe"
/NOLOGO
/NXCOMPAT
/LIBPATH:C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib
C:\Users\b\AppData\Local\Temp\rustdoctestiHYAmZ\rust_out.rust_out.7rcbfp3g-cgu.0.rcgu.o
/OUT:C:\Users\b\AppData\Local\Temp\rustdoctestiHYAmZ\rust_out
C:\Users\b\AppData\Local\Temp\rustdoctestiHYAmZ\rust_out.33dyzt1ekirinwy8.rcgu.o
/OPT:REF,NOICF
/DEBUG
/NATVIS:C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\etc\intrinsic.natvis
/NATVIS:C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\etc\liballoc.natvis
/NATVIS:C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\etc\libcore.natvis
/NATVIS:C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\etc\libstd.natvis
/LIBPATH:P:\testcases\lto-rustdoc\target\release\deps
/LIBPATH:P:\testcases\lto-rustdoc\target\release\deps
/LIBPATH:C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib
P:\testcases\lto-rustdoc\target\release\deps\liblto_rustdoc-5744db2bc717fc58.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libstd-098718ea823008f4.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libpanic_unwind-6a7fb52d1138932f.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libhashbrown-6a663e98da9bc9d7.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc_std_workspace_alloc-35acfeb1bd722cd7.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libbacktrace-5789469546c93c74.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc_demangle-6b40c36b7a5ce598.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libunwind-ce5287493729af9a.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libcfg_if-3d66b8897882cc1e.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\liblibc-2f1bb63f49853f4b.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\liballoc-e769bcc0ed78b3c4.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc_std_workspace_core-2182f71ae05a8995.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libcore-77514af2604d16ea.rlib
C:\Users\b\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libcompiler_builtins-ebafcb87dc3d8679.rlib
advapi32.lib
ws2_32.lib
userenv.lib
msvcrt.lib

and the current beta:

"C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.27.29110\bin\HostX64\x64\link.exe"
/NOLOGO
/NXCOMPAT
/LIBPATH:C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib
C:\Users\b\AppData\Local\Temp\rustdoctestNeUiEv\rust_out.rust_out.7rcbfp3g-cgu.0.rcgu.o
/OUT:C:\Users\b\AppData\Local\Temp\rustdoctestNeUiEv\rust_out
C:\Users\b\AppData\Local\Temp\rustdoctestNeUiEv\rust_out.33dyzt1ekirinwy8.rcgu.o
/OPT:REF,NOICF
/DEBUG
/NATVIS:C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\etc\intrinsic.natvis
/NATVIS:C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\etc\liballoc.natvis
/NATVIS:C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\etc\libcore.natvis
/NATVIS:C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\etc\libstd.natvis
/LIBPATH:P:\testcases\lto-rustdoc\target\release\deps
/LIBPATH:P:\testcases\lto-rustdoc\target\release\deps
/LIBPATH:C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib
P:\testcases\lto-rustdoc\target\release\deps\liblto_rustdoc-c5f7efb3b772d87e.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libstd-f5b401742159082b.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libpanic_unwind-e179a42e3654f4e1.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc_demangle-6606b5facb12279a.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libhashbrown-1a0e7b07a7e9bb53.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc_std_workspace_alloc-9aeb694399775ad0.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libunwind-4fc2d77659a37b5c.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libcfg_if-8e0f5ece44f1eaea.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\liblibc-fad582f67483dcdf.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\liballoc-0f8a186f598b0388.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc_std_workspace_core-ec163dda744bc8b6.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libcore-6d2170587abafa35.rlib
C:\Users\b\.rustup\toolchains\beta-x86_64-pc-windows-msvc\lib\rustlib\x86_64-pc-windows-msvc\lib\libcompiler_builtins-e07801ab81cac207.rlib
advapi32.lib
ws2_32.lib
userenv.lib
msvcrt.lib

The command lines seem to be the same.

@briansmith briansmith added the C-bug Category: bug label Aug 26, 2020
@briansmith briansmith changed the title Regression: lto = true in build.rs cause cargo test --release --doc to fail to link on Windows Regression: lto = true in build.rs causes cargo test --release --doc to fail to link on Windows Aug 26, 2020
@briansmith briansmith changed the title Regression: lto = true in build.rs causes cargo test --release --doc to fail to link on Windows Regression: lto = true in build.rs causes cargo test --release --doc to fail to link Aug 27, 2020
@briansmith
Copy link
Author

The same problem occurs on Linux too:

 note: /home/travis/build/briansmith/ring/target/x86_64-unknown-linux-gnu/release/deps/libring-a6aeb6a391324468.rlib: error adding symbols: File format not recognized
          collect2: error: ld returned 1 exit status

@ehuss
Copy link
Contributor

ehuss commented Aug 27, 2020

Thanks for the report! It looks like Cargo needs to pass the -Clto flag to rustdoc. In the past, rustdoc didn't process codegen flags, but it looks like it works now.

cc rust-lang/rust#60575 #6570

I'll try to fix this soonish.

@ehuss ehuss self-assigned this Aug 27, 2020
@ehuss ehuss added A-lto Area: link-time optimization regression-from-stable-to-stable Regression in stable that worked in a previous stable release. labels Aug 27, 2020
@Mark-Simulacrum
Copy link
Member

This seems like a potential regression in rustdoc, regardless -- generally speaking since presumably rustdoc isn't actually using these codegen arguments we probably shouldn't behave differently in the presence of them? At the very least we need to include this in release notes.

@ehuss
Copy link
Contributor

ehuss commented Aug 27, 2020

The issue is that Cargo is now building LTO dependencies with -Clinker-plugin-lto, which requires the final step to use LTO. The doc tests are unable to link without the -Clto flag. The ability to pass codegen options to rustdoc was added in rust-lang/rust#63827 (I think), but Cargo was never updated to pass them. I'll probably just start with LTO, but we should consider passing other codegen flags in the future.

@bors bors closed this as completed in 55e59bd Aug 28, 2020
ehuss pushed a commit to ehuss/cargo that referenced this issue Aug 28, 2020
Fix LTO with doctests.

This fixes an issue where `cargo test --release` would fail to run doctests if LTO is set in `profile.release` or `profile.bench`.

The issue is that dependencies were built with `-Clinker-plugin-lto`, but the final rustdoc invocation did not issue `-C lto`. This causes a link failure (or crash!) because the necessary object code was missing.  This is because rustdoc historically did not support codegen flags, so Cargo has never passed them in.  Rustdoc now supports codegen flags (via rust-lang/rust#63827), so it should be safe to start passing them in.  For now, I am only adding LTO, but more should be added in the future.

There are two bugs here. One is that LTO flags aren't passed to rustdoc. The other is that the "doctest" unit was using the wrong profile (it was using dev/release when it should be using test/bench).

There are two distinct scenarios here.  One where just `release` has `lto` set.  And one where both `release` and `bench` have `lto` set.  This is relevant because LTO mostly cares about what the final artifact wants, and in the case where `bench` does not have `lto` set, then LTO will not be used for tests. This will hopefully be a little cleaner in the future when rust-lang#6988 is stabilized, which causes the test/bench profiles to *inherit* from the dev/bench profiles, which means you won't need to manually synchronize the test/bench profiles with dev/release.

Fixes rust-lang#8654
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lto Area: link-time optimization C-bug Category: bug regression-from-stable-to-stable Regression in stable that worked in a previous stable release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants