Invalid LLVM coverage data produced when compiled with `-C opt-level=1`

[catenacyber]

I tried this code:

use num_integer::Roots;
 
fn main() {
    let a:u32 = 54321; 
    let c = a.cbrt();
    println!("Hello, world {} {}!", a, c);
}

compiled with RUSTFLAGS="-Zinstrument-coverage -C opt-level=1"
and Cargo.toml having

[dependencies]
num-integer = "0.1"

I expected to see this happen:
Running llvm-profdata merge -j=1 -sparse, then llvm-cov should give me a coverage report

Instead, this happened:
llvm-cov fails with `Failed to load coverage: Malformed instrumentation profile data

When building with -C opt-level=0, the buggy behavior does not show up with this reproducer

Meta

rustc --version --verbose:

rustc 1.52.0-nightly (07194ffcd 2021-02-10)
binary: rustc
commit-hash: 07194ffcd25b0871ce560b9f702e52db27ac9f77
commit-date: 2021-02-10
host: x86_64-unknown-linux-gnu
release: 1.52.0-nightly
LLVM version: 11.0.1

Backtrace

There is no backtrace here as the failure happens with llvm-cov
I used the following clang

clang --version
clang version 12.0.0 (https://github.com/llvm/llvm-project.git f086e85eea94a51eb42115496ac5d24f07bc8791)

Using this patch in clang

diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index b75738bc360c..1ad5d930eafd 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -525,7 +525,8 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
       if (Error Err = CFR->template getFuncName<Endian>(ProfileNames, FuncName))
         return Err;
       if (FuncName.empty())
-        return make_error<InstrProfError>(instrprof_error::malformed);
+        FuncName = "lola";
+        //return make_error<InstrProfError>(instrprof_error::malformed);
       ++CovMapNumUsedRecords;
       Records.emplace_back(Version, FuncName, FuncHash, Mapping,
                            FileRange.StartingIndex, FileRange.Length);

I get more information with
./bin/llvm-cov show -name-regex="lol" -instr-profile=dump.profdata -object=/path/to/rustbinary
It shows the pretendedly unnamed function code which can be found here https://github.com/rust-num/num-integer/blob/master/src/roots.rs#L174

  174|      0|fn fixpoint<T, F>(mut x: T, f: F) -> T
  175|      0|where
  176|      0|    T: Integer + Copy,
  177|      0|    F: Fn(T) -> T,
  178|      0|{
...

cc @richkadel

Original discussion comes from google/oss-fuzz#4697

[catenacyber]

Another thing I just noticed is that the problematic functions (ie fixpoint in the reproducer case), in addition to be hinted inline, are also called by hinted inline functions themselves, and these caller functions are in traits implementations...

[richkadel]

@catenacyber - Thanks for filing this issue. I was able to build and test your example, and I can reproduce the problem.

Just to be clear, this is not exactly a bug. Due to certain requirements of LLVM's InstrProf coverage, some compiler (and/or linker) optimizations are not compatible. But coverage instrumentation is not expected to be used in production software, so limiting coverage instrumentation to non-optimized code is acceptable.

I may be able to issue a compiler warning when a -C opt-level greater than zero (0) is combined with -Z instrument-coverage. (As we have found with certain MIR optimization levels, making this an error could be problematic, so I hesitate to go beyond a warning.) It may also be possible to disable opt-level optimizations if coverage enabled, but again, from the MIR optimization experience, it may be hard to be sure what optimizations to disable, and where. Nevertheless, I'll look into it.

But I'm concerned that this issue does not really get to the real problem.

As I understand it from your last couple of postings on google/oss-fuzz#4697, you still have not figured out why your program is failing when coverage is enabled and you have set -Copt-level=0.

Can you provide a minimal example and steps to reproduce the problem where you have set -Copt-level=0, but still get corrupted coverage data?

[catenacyber]

Thanks @richkadel

I also think that -C opt-level is not the real problem, only a convenient way to get a reproducer where changing one byte shows the buggy behavior.
But I am not sure (there may be multiple problems)

I will work on minimizing the example and steps to reproduce the problem with -Copt-level=0 (and double check that I have no optimizations)

[richkadel]

@wesleywiser @tmandry - I've been thinking about how to reduce friction for cases like the one that prompted this issue (google/oss-fuzz#4697). Looking at that pull request, and seeing the challenges that @catenacyber has had, trying to disable optimizations, I think this must be a case where another tool is wrapping invocations of the Rust compiler, and forcing certain flags on it. It seems (in this case) possible to add -Z instrument-coverage to at least parts of the build process, but it's not clear how or even if it's possible to disable the optimization flags that conflict with -Z instrument-coverage.

We know that -O1 (aka -C opt-level=1 or higher does not work with coverage, per the trivial example shown in this issue. Sure, the program runs, but it would also run without -Z instrument-coverage; but it fails to generate coverage reports, which is the only reason for including the flag.

I think we should flag this combination as a compiler error (not a warning), both in command line option processing, and at any point downstream that an incompatible option might be enabled (for example, I believe that when LTO is enabled, the compiler also enables -O1 late in the compiler stages). (We could also issue an error when LTO options are included with -Z instrument-coverage, but I'm just trying to cover all of the bases. Maybe something else enables LTO, late in the compile process.)

This way, the problem @catenacyber is having would manifest itself at the point of actually trying to compile the program, which should help @catenacyber debug the root cause.

I don't think the error would ever be a "bad" thing, because the developer can avoid the error by not adding -Z instrument-coverage, if the optimization flags are important or unavoidable. Otherwise, if they want coverage, they will need to find a way to remove the optimization flags.

In both cases, this approach ensures the developer is in control.

The other option would be to automatically disable optimizations when -Z instrument-coverage is enabled, but I feel that this approach could hide potential side effects, for instance, if they've enabled an inlining optimization, but we magically disable it under the hood, they could be surprised by the result; and it probably would not be clear that the problem was related to including the -Z instrument-coverage flag.

WDYT?

[catenacyber]

I now think I have indeed 2 problems.
The first one is indeed that -Z instrument-coverage is incompatible with optimisations such as shipped in by cargo build --release

The second one, I can reproduce with suricata
The important settings are

export CC=clang
export CXX=clang++
export CFLAGS="-O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION  -fprofile-instr-generate -fcoverage-mapping -pthread -Wl,--no-as-needed -Wl,-ldl -Wl,-lm -Wno-unused-command-line-argument"
export CXXFLAGS="-O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION  -fprofile-instr-generate -fcoverage-mapping -pthread -Wl,--no-as-needed -Wl,-ldl -Wl,-lm -Wno-unused-command-line-argument -stdlib=libc++"
export RUSTFLAGS="--cfg fuzzing -Cdebuginfo=1 -Cforce-frame-pointers -Zinstrument-coverage -C link-arg=-lc++ -C debug-assertions=n"
./configure --disable-shared --enable-debug --enable-fuzztargets

Then running llvm-cov, I get Failed to load coverage: Malformed instrumentation profile data error: Could not load coverage information

So, I will investigate further the second problem (I did not manage to have a reproducer with Rust only yet) and create another issue about it
And for this issue, I think the combination of -Zinstrument-coverage with some optimisation deserves a warning or an error

[richkadel]

cargo build --release should not be a problem.

Are you trying to generate coverage from both C++ and Rust in the same binary? This has not been tried. I think it should work if you are using LLVM 11. I'm not sure about LLVM 12. Clang MUST generate Coverage Mapping Format version 4.

Note that there are multiple new version in the works, so generating both Rust and C/C++ coverage in the same binary is going to be difficult to keep working as versions progress. You may want to consider instrumenting a binary with only Rust coverage, and a separate binary with only C/C++.

This recommendation is new, based on conversations I just had yesterday (Feb 22), but it is probably good advice.

You are breaking new ground by attempting to generate coverage from the same binary from two different languages.

Note, your C/C++ flags also have coverage options and optimization options (like -O1). I don't know what optimizations are supported, but from what we've seen with LLVM optimization issues with Rust coverage, I suspect many optimizations in Clang/LLVM are also incompatible with Clang coverage.

[catenacyber]

cargo build --release should not be a problem.

It sets opt-level to 3 for some crates... So is it really not a problem ?

Are you trying to generate coverage from both C++ and Rust in the same binary?

And it works for my ecc-diff-fuzzer (where there is only a little part of Rust)
Yes, and this worked at some point for Suricata (maybe LLVM 11 indeed)

I think it should work if you are using LLVM 11

I will try that again

Clang MUST generate Coverage Mapping Format version 4.

Version 5 seems only an extension, so it should not be incompatible, or am I missing something ?

You may want to consider instrumenting a binary with only Rust coverage, and a separate binary with only C/C++.

It looks like when I try to instrument only rust for coverage (ie setting RUSTFLAGS, but not CFLAGS), running my compiled binary does not produce any default.profraw file
I get one when I set LDFLAGS, but then I still get Malformed instrumentation profile data with llvm-cov

TL;DR
I will try again LLVM 11, current rustc nightly and disabling optimizations

[catenacyber]

I will try again LLVM 11, current rustc nightly and disabling optimizations

Still getting Malformed instrumentation profile data with llvm-cov-11 clang-11 compilation...

[richkadel]

Note, your C/C++ flags also have coverage options and optimization options (like -O1). I don't know what optimizations are supported, but from what we've seen with LLVM optimization issues with Rust coverage, I suspect many optimizations in Clang/LLVM are also incompatible with Clang coverage.

I need to correct at least one thing I said here. I spoke to a colleague that is more familiar with Clang and LLVM, and was told that Clang's implementation of LLVM InstrProf coverage is compatible with at least some of Clang's optimization levels. (Clang -O1 and -O2 at least, maybe higher.)

It's not clear to me if Clang's -O1 is exactly the same as Rust's -O1. (I doubt they are required to be identical, but I'm not sure how similar they are, particularly when it comes to correlating optimization levels to LLVM passes and options.)

But it's reasonable to believe that it's possible for the Rust Compiler to support some optimization levels with coverage instrumentation at the same time.

The sample in this issue shows that is not the case, or at least, not always the case.

The current workaround is to disable optimizations when instrumenting for coverage. In most cases, coverage testing is not used in production, so some performance degradation is not going to be a major sacrifice (generally).

We can leave this issue in place, in case someone has ideas on how to expand support for coverage to work with some optimization levels. (Just setting expectations though: this is not currently a priority feature request, and AFAIK, no one is actively working on it.)

Here is something I did discover though. I compiled the sample, with and without -O1, and added --emit=llvm-ir,link to dump LLVM IR *.ll files. Then I counted the occurrences of loading the counters:

$ grep 'pgocount = load' $(find target -name '*.ll') | wc -l
199
$ grep 'pgocount = load' $(find target_with_O1 -name '*.ll') | wc -l
169

30 counters are no longer present in LLVM IR.

This lines up with what @catenacyber saw as well.

Something, either in Rust or due to Rust-specific LLVM passes or options, is removing counters and are probably corrupting the coverage counts expected in the coverage map.

[wesleywiser]

@richkadel

Looking at that pull request, and seeing the challenges that @catenacyber has had, trying to disable optimizations, I think this must be a case where another tool is wrapping invocations of the Rust compiler, and forcing certain flags on it. It seems (in this case) possible to add -Z instrument-coverage to at least parts of the build process, but it's not clear how or even if it's possible to disable the optimization flags that conflict with -Z instrument-coverage.

I think having an external tool that invokes rustc for you, providing the correct flags and then potentially runs tests and collects the coverage data for you would be great! cargo-flamegraph does something similar and packages an entire workflow into an easy to use command.

[tmandry]

FYI, linking instrumented Rust and C/C++ in the same binary is likely to fail (or it could work now but break in the future.) The reason is that the separate compilers each expect their own version of the runtime instrumentation library, but only one can be loaded in the binary. There are no compatibility guarantees between versions.

[richkadel]

@wesleywiser @tmandry - Before I put together a PR, I'd like to make sure you agree with it and will approve it.

tl;dr - I believe issuing an error when -O1 and -Z instrument-coverage are combined is by far the best mitigation for rustc. (And offering an additional flag that allows the combination, without error, is not a bad idea either.)

As I see it, we have three options:

1. Do nothing. There is already a warning in the Rust Unstable Book that some optimization flags are not compatible with -Z instrument-coverage. Users beware.
2. Issue a warning. This is nearly the same as doing nothing, especially when rustc is called via some wrapper (as I think is the case that triggered this issue:
   • The developer may never see the warning.
   • Warnings are easily ignored, and often don't mean the program will fail.
   • Since a program compiled with -O and -Z instrument-coverage may not fail in some cases, the warning will just be an annoyance.
3. Issue an error. This is my preference:
   • It forces the user to do something about it.
   • In the case of tools that wrap rustc, the developer's workflow will fail early, giving them a fighting chance to find and fix the rustc flag combination.
   • If anyone really cares about optimizing binaries with coverage enabled, this will ensure the fix is prioritized.
   • If no one really cares, then there is no reason to provide support for it, and an error is fine.

We could also add one more flag/option to override the error, so if someone really wants to try combining them, they still can, but they must proactively enable it.

Either way, the error means there are no surprises, and it won't be possible to build a program that might "just happen to work" with the combination one day, but fail to compile after some small change in their program that triggers the coverage failure.

[catenacyber]

FYI, linking instrumented Rust and C/C++ in the same binary is likely to fail (or it could work now but break in the future.) The reason is that the separate compilers each expect their own version of the runtime instrumentation library, but only one can be loaded in the binary. There are no compatibility guarantees between versions.

Thanks @tmandry
If I get it correctly, that is just adding the constraint that I use compatible LLVM/clang/rustc when instrumenting my Rust+C program, right ?