Binaries generate malformed coverage profile when built with `-Z instrument-coverage -C link-dead-code -O`

[Mrmaxmeier]

llvm-cov refuses to load coverage data for this code when running rustc with optimizations enabled:

fn bar() {
    loop {}
}

pub trait Trait {
    fn foo(&self) {
        bar();
    }
}

impl Trait for u8 {}

fn main() {
    println!("hi")
}

Here's how I'm generating the coverage data:

#!/bin/sh
LLVM_TOOLCHAIN=$HOME/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/

rm -rf main target default.profdata default.profraw

# RUSTFLAGS="-Z instrument-coverage" cargo run --verbose --release

rustc main.rs -O -o main -Z instrument-coverage
./main

$LLVM_TOOLCHAIN/llvm-profdata merge -sparse default.profraw -o default.profdata
$LLVM_TOOLCHAIN/llvm-cov show --instr-profile=default.profdata main

And what I'm observing:

+ rustc main.rs -O -o main -Z instrument-coverage
+ ./main
hi
+ /home/mrmaxmeier/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin//llvm-profdata merge -sparse default.profraw -o default.profdata
+ /home/mrmaxmeier/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin//llvm-cov show --instr-profile=default.profdata main
error: main: Failed to load coverage: Malformed instrumentation profile data

rustc --version --verbose:

rustc 1.50.0-nightly (c919f490b 2020-11-17)
binary: rustc
commit-hash: c919f490bbcd2b29b74016101f7ec71aaa24bdbb
commit-date: 2020-11-17
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly

cc @richkadel #79121

[richkadel]

Great bug report. I'll take a look.

Thanks!

[richkadel]

Partial update here...

I did some experimentation, making small changes to the code, and turning on and off the -O flag.

I was able to create a modified MCVE, that also appears to make minimal changes to the LLVM IR.

Here is your sample code with a couple of additional lines, commented out. If I uncomment those lines, the example works as expected:

fn bar() {
    loop {}
}

pub trait Trait {
    fn foo(&self) {
     // if true {
            bar();
     // }
    }
}

impl Trait for u8 {}

fn main() {
    println!("hi")
}

If I uncomment these lines (by replacing slashes with spaces, just to minimize the changes to code locations propagated to lower-level representations), there are some small but notable changes:

1. The symbol definitions for two symbols used by LLVM profiling are gone. Note, the removed variables would have been used by the LLVM instrprof.increment intrinsic to increment counters for code regions in the function Trait::foo():

@__profc__RNvYhNtCs4fqI2P2rA04_11issue_791755Trait3fooB5_ = private global [3 x i64] zeroinitializer, section "__llvm_prf_cnts", align 8
@__profd__RNvYhNtCs4fqI2P2rA04_11issue_791755Trait3fooB5_ = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5234861019717404860, i64 -7566766773726293094, i64* getelementptr inbounds ([3 x i64], [3 x i64]* @__profc__RNvYhNtCs4fqI2P2rA04_11issue_791755Trait3fooB5_, i32 0, i32 0), i8* bitcast (void (i8*)* @_RNvYhNtCs4fqI2P2rA04_11issue_791755Trait3fooB5_ to i8*), i8* null, i32 3, [2 x i16] zeroinitializer }, section "__llvm_prf_data", align 8

2. The function definition for Trait::foo() is still defined, and nearly unchanged, but 3 lines that increment the counter for Trait::foo() (that used the variables that were removed) are also removed.

  %pgocount = load i64, i64* getelementptr inbounds ([3 x i64], [3 x i64]* @__profc__RNvYhNtCs4fqI2P2rA04_11issue_791755Trait3fooB5_, i64 0, i64 1), align 8
  %1 = add i64 %pgocount1, 1
  store i64 %0, i64* getelementptr inbounds ([3 x i64], [3 x i64]* @__profc__RNvYhNtCs4fqI2P2rA04_11issue_791755Trait3fooB5_, i64 0, i64 1), align 8

Those are the only significant changes.

In both the working and non-working examples, everything else is the same (with the exception of a couple of locally-scoped variable symbols, like %pgocount vs. %pgocount1, which can be ignored); including the fact that Trait::foo() is incrementing counters for bla(), in both cases.

Note, bla() has an LLVM IR function definition in both cases as well, but it's never called. I assume Rust inlined the body of bla() into Trait::foo(), which would explain how the counters ended up there.

The @__profc__... and @__profd__... variables for bla() include function pointers to bla(), which may explain why the definition for function bla() cannot be removed.

Intermediate conclusion

I don't have a definitive answer yet, but I'm still investigating. I did reproduce the llvm-cov error message (granted, the message is very vague), without -O, after making a code change in an attempt to add uncalled closures to the coverage map. (Unused closures are normally not passed to codegen.)

I suspect (still to be proven) that the coverage map in the LLVM IR for your original example still includes references to counters from Trait::foo(), but the __prof*__ variables are no longer defined. If I'm right, both failing test cases have the same underlying cause.

cc: @tmandry

[richkadel]

Quick note: PR #79109 includes several improvements, and one important improvement is, we no longer automatically set -Clink-dead-code. (It's no longer recommended or needed for -Z instrument-coverage.)

I tried your sample with the modifications in PR #79109, and no longer get the coverage failures, with -O, unless I add -Clink-dead-code as well.

I think we can mark this Issue resolved, once that PR lands.

[tmandry]

I think we should still track this somewhere, but I can rename the issue to include -C link-dead-code once #79109 lands which would make it much lower priority.

[richkadel]

Note, I suspect there are other combinations of compiler options and optimizations that could break coverage instrumentation, and they may not be easy to predict, but the documented examples should work, and I'll try to make note of other options we expect to work as well (including -O if confirmed for larger samples). I know cargo build --release also works.

[richkadel]

I can rename the issue to include -C link-dead-code once #79109 lands which would make it much lower priority.

@tmandry - Heads up, since #79109 has landed, can you add -C link-dead-code to the title, as you suggested?

Thanks!

[briansmith]

It's no longer recommended or needed for -Z instrument-coverage.)

This is good to know, especially for people migrating from other coverage mechanisms. I didn't find it in the documentation in https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/source-based-code-coverage.html. We should add such a statement to that doc.

[briansmith]

Also #64685 (comment):

I implemented coverage for unused functions in a different way, which is actually more complete than anything I was getting from -C link-dead-code. The -C link-dead-code didn't help much before, and now it's redundant.

There is more discussion about this topic in that issue.

I'm guessing that a lot of people can work around this issue by simply removing -C link-dead-code from their code coverage scripts.

[richkadel]

We should add such a statement to that doc.

The document does include the following statement:

Note that some compiler options, combined with -Zinstrument-coverage, can produce LLVM IR and/or linked binaries that are incompatible with LLVM coverage maps. For example, coverage requires references to actual functions in LLVM IR. If any covered function is optimized out, the coverage tools may not be able to process the coverage results. If you need to pass additional options, with coverage enabled, test them early, to confirm you will get the coverage results you expect.

I don't know if there's a strong reason to add a statement about this specific option. I wouldn't object if someone wants to make that change though.