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.

Sign up for GitHub

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

Jump to bottom

Incompatibility using both -Zinstrument-coverage and -Z build-std #79401

Open
catenacyber opened this issue Nov 25, 2020 · 8 comments · May be fixed by #133300
Open

Incompatibility using both -Zinstrument-coverage and -Z build-std #79401

catenacyber opened this issue Nov 25, 2020 · 8 comments · May be fixed by #133300
Assignees
@Enselic
Labels
-Zbuild-std Unstable Cargo option: Compile the standard library yourself. A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@catenacyber
Copy link

catenacyber commented Nov 25, 2020 •
edited by rustbot
Loading

I tried this code:

git clone https://github.com/OISF/suricata.git
cd suricata
export RUSTFLAGS="-Zinstrument-coverage"
sh autogen.sh
./configure --disable-shared --enable-fuzztargets --disable-gccmarch-native --enable-debug-validation
make

This ends up running cargo build --release -Z build-std

I expected to see this happen:
Compilation succeeds

Instead, this happened:
Compilation failed with

Updating crates.io index
   Compiling core v0.0.0 (/Users/catena/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core)
error[E0152]: found duplicate lang item `f32`
   --> /Users/catena/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/num/f32.rs:319:1
    |
319 | / impl f32 {
320 | |     /// The radix or base of the internal representation of `f32`.
321 | |     #[stable(feature = "assoc_int_consts", since = "1.43.0")]
322 | |     pub const RADIX: u32 = 2;
...   |
937 | |     }
938 | | }
    | |_^
    |
    = note: the lang item is first defined in crate `core`.
    = note: first definition in `core` loaded from /Users/catena/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/libcore-e3cd79f6ad1173fd.rlib
    = note: second definition in the local crate (`core`)

and many more errors about other duplicate items

Meta

This is part of #34701 cc @richkadel

rustc --version --verbose:

rustc 1.50.0-nightly (603ab5bd6 2020-11-15)
binary: rustc
commit-hash: 603ab5bd6e0ffefafa7411cd8bd23a6ca82bcff0
commit-date: 2020-11-15
host: x86_64-apple-darwin
release: 1.50.0-nightly
Backtrace 

    Updating crates.io index
   Compiling core v0.0.0 (/Users/catena/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core)
error[E0152]: found duplicate lang item `f32`
   --> /Users/catena/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/num/f32.rs:319:1
    |
319 | / impl f32 {
320 | |     /// The radix or base of the internal representation of `f32`.
321 | |     #[stable(feature = "assoc_int_consts", since = "1.43.0")]
322 | |     pub const RADIX: u32 = 2;
...   |
937 | |     }
938 | | }
    | |_^
    |
    = note: the lang item is first defined in crate `core`.
    = note: first definition in `core` loaded from /Users/catena/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/x86_64-apple-darwin/lib/libcore-e3cd79f6ad1173fd.rlib
    = note: second definition in the local crate (`core`)

@catenacyber catenacyber added the C-bug Category: This is a bug. label Nov 25, 2020
@catenacyber
Copy link
Author

By the way, there is a current PR OISF/suricata#5582 with a fix : not using -Z build-std when -Zinstrument-coverage is in RUSTFLAGS

@richkadel
Copy link
Contributor

@catenacyber - Thanks for the bug report!

I did some experimenting (briefly), and I can repro this issue, for instance, with:

RUSTFLAGS="-Zinstrument-coverage" cargo build --release -Z build-std --target x86_64-unknown-linux-gnu

I don't fully understand how -Z build-std works, but from the error messages, it looks like the compiler may have built a new version of the std library (as intended), and then the compiler uses that new version but also (?) loads another version of std (or two versions of core).

One copy loads the standard LangItems, and the second copy also tries to load them but fails because they were already loaded.

(This is a minimually-educated guess, from the error messages.)

Anyhow, I don't think -Zinstrument-coverage would be doing anything specifically to cause this problem, so I thought, maybe it's not specific to -Zinstrument-coverage.

So I tried:

RUSTFLAGS="-Zprofile" cargo build --release -Z build-std --target x86_64-unknown-linux-gnu

Sure enough, I get the same errors.

Since cargo -Z build-std is also an "experimental" feature, I would redirect this issue to the build-std feature experts, to see if they can figure out why some compiler options (like -Zinstrument-coverage and -Zprofile) conflict in this way.

Note, both -Z instrument-coverage and -Z profile do have a dependency on the profiler_builtins library in rust/library, so one place they might look is where the profiler_builtins library gets loaded. Maybe it's forcing the default std library to load, as a direct dependency of profiler_builtins, rather than using the std that was just built.

@catenacyber
Copy link
Author

Indeed, I do not know which experimental feature should fix this.
So, please redirect this report how you see fit 👍

@richkadel
Copy link
Contributor

richkadel commented Dec 3, 2020 •
edited
Loading

@alexcrichton - Is cargo -Z build-std something you would be familiar with? Or if not, do you know who might be? (Thanks!)

@alexcrichton
Copy link
Member

I believe the profiler_builtins crate is the key here. Using -Zbuild-std builds a fixed set of crates and profiler_builtins is not included in that set. This would probably be an issue for https://github.com/rust-lang/wg-cargo-std-aware/issues since I don't think there's much rust-lang/rust can do here for this.

@richkadel
Copy link
Contributor

@catenacyber - Would you mind copy/pasting your issue into that list (perhaps just linking back to here for context if helpful)?

Thanks!

@catenacyber catenacyber mentioned this issue Dec 3, 2020
Open
@ldm0 ldm0 mentioned this issue Aug 25, 2022
Closed
@ldm0 ldm0 mentioned this issue Sep 3, 2022
Closed
@workingjubilee workingjubilee added the -Zbuild-std Unstable Cargo option: Compile the standard library yourself. label Mar 13, 2023
@taiki-e
Copy link
Member

taiki-e commented Jul 12, 2023

@rustbot label +A-code-coverage

@rustbot rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Jul 12, 2023
@taiki-e taiki-e mentioned this issue Sep 27, 2023
Closed
@Enselic Enselic linked a pull request Nov 21, 2024 that will close this issue
Open
1 task
@Enselic
Copy link
Member

Enselic commented Nov 21, 2024 •
edited
Loading

@rustbot claim

For me the fix in #133300 makes this work. See the PR for detailed instructions.

@Enselic Enselic added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Enselic Enselic

Labels
-Zbuild-std Unstable Cargo option: Compile the standard library yourself. A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
7 participants
@alexcrichton @Enselic @richkadel @catenacyber @taiki-e @workingjubilee @rustbot