inject_panic_runtime(): Avoid double negation for 'any non rlib'

[Enselic]

This PR originally did more things .Click to expand to see.

By not trying to inject a profiler runtime when only building an rlib. This logic already exists for the panic runtime.

This makes

RUSTFLAGS="-Cinstrument-coverage" cargo build -Zbuild-std=std,profiler_builtins

work. Note that you probably also need
RUST_COMPILER_RT_FOR_PROFILER=$src/llvm-project/compiler-rt in your environment.

cc #79401

Demonstration

Before this fix you get these errors:

$ rm -rf target ; RUST_COMPILER_RT_FOR_PROFILER=/home/martin/src/llvm-project/compiler-rt RUSTFLAGS="-Cinstrument-coverage" cargo +nightly build --release -Zbuild-std=std,profiler_builtins
error: `profiler_builtins` crate (required by compiler options) is not compatible with crate attribute `#![no_core]`
error[E0152]: found duplicate lang item `manually_drop`
    = note: first definition in `core` loaded from /home/martin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-d453bab70303062c.rlib
    = note: second definition in the local crate (`core`)

With the fix the build succeeds:

$ rm -rf target ; RUST_COMPILER_RT_FOR_PROFILER=/home/martin/src/llvm-project/compiler-rt RUSTFLAGS="-Cinstrument-coverage" cargo +stage1 build --release -Zbuild-std=std,profiler_builtins
    Finished `release` profile [optimized] target(s) in 45.57s

And we can check code coverage. My example program looks like this:

fn main() {
    if std::env::args_os().nth(1) == Some("write-file".into()) {
        std::fs::write("hello.txt", "Hello, world!").unwrap();
    } else {
        println!("Hello, world!");
    }
}

when the program prints to stdout:

$ LLVM_PROFILE_FILE=stdout.profraw ./target/release/hello-world
Hello, world!

we can see that fs::write() is not being used (note the 0's):

$ /home/martin/src/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-profdata merge -sparse stdout.profraw -o stdout.profdata
$ /home/martin/src/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-cov show target/release/hello-world --sources /home/martin/src/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/src/rust/library/std/src/fs.rs --instr-profile stdout.profdata --color | grep -A 3 'pub fn write(&mut self, write: b
ool) -> &mut Self {'
 1357|      0|    pub fn write(&mut self, write: bool) -> &mut Self {
 1358|      0|        self.0.write(write);
 1359|      0|        self
 1360|      0|    }

but when we print to a file:

$ LLVM_PROFILE_FILE=file.profraw ./target/release/hello-world write-file

the code coverage shows fs::write() as being used (note the 1's):

$ /home/martin/src/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-profdata merge -sparse file.profraw -o file.profdata
$ /home/martin/src/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/bin/llvm-cov show target/release/hello-world --sources /home/martin/src/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/src/rust/library/std/src/fs.rs --instr-profile file.profdata --color | grep -A 3 'pub fn write(&mut self, write: bool) -> &mut Self {'
 1357|      1|    pub fn write(&mut self, write: bool) -> &mut Self {
 1358|      1|        self.0.write(write);
 1359|      1|        self
 1360|      1|    }

[rustbot]

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[lcnr]

r? compiler

[rustbot]

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

[Zalathar]

I'm not very familiar with the injection of profiler_builtins, but based on my limited understanding I suspect that this might be the wrong fix.

I did some playing around, and this might potentially be the more appropriate solution:

• Change profiler_builtins from #[no_std] to #[no_core], since it doesn't contain any actual Rust code.
• Remove the check that refuses to inject profiler_builtins into #[no_core] crates.

EDIT: Draft PR at #133369.

[Zalathar]

Basically, I think the underlying problem is that because profiler_builtins is currently #[no_std], it still has an implicit dependency on core. That causes problems when trying to make it a dependency of core itself.

But I'm pretty sure that:

• There is no need for profiler_builtins to actually depend on core.
• When built with -Cinstrument-coverage, core really should have a genuine dependency on profiler_builtins, to make sure the profiler runtime ends up in the final binary even if no other crate in the final binary is built with instrument-coverage.

[Zalathar]

For reference, the error message was added in #79958. It looks like the real issue (profiler doesn't need core) wasn't noticed at that time.

[Enselic]

Your solution is clearly better than mine, and I have confirmed it works for me too. Thanks!

I have now made this PR only fix the double-negation that bjorn3 pointed out.

[jieyouxu]

Thanks, you can r=me after PR CI is green (and probably update PR description).

[jieyouxu]

@bors delegate+ rollup

[bors]

✌️ @Enselic, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, please make that change, then do @bors r=@jieyouxu

[Enselic]

@bors r=@jieyouxu

[bors]

📌 Commit a0e5aea has been approved by jieyouxu

It is now in the queue for this repository.

[jieyouxu]

(I edited the magic Fixes #... comment in PR description out because github will autoclose the linked issue otherwise)

[Zalathar]

By the way, in this sort of situation I would usually suggest creating a new PR rather than changing the scope of an old one, to avoid a confusing PR history.

[Enselic]

Will do so next time 👍