search

Kogia-sima / rust-covfix

Fix Rust coverage data based on source code
MIT License
14 stars 3 forks source link

How to work around Rust inlining of functions? #2

Closed Robbepop closed 4 years ago

Robbepop commented 4 years ago

Hi, at ink! we use rust-covfix in order to polish the results of grcov for our project. All in all it really improves the reports, however, when it comes to smaller functions they seem to be inlined away aggressively so that rust-covfix won't recognize them as being covered and flag them as uncovered.

I made an experiment where I manually added

#[cfg_attr(debug_assertions, inline(never))]

to some of those small functions and the reports improved significantly since the functions then were recognized again. Now, it is completely natural that I am not going to manually put these annotations everywhere in my codebase just to improve the coverage reports.

We are using the following RUSTFLAGS in order to compile the codebase for coverage reports:

-Zprofile
-Zmir-opt-level=0
-Zpanic_abort_tests
-Ccodegen-units=1
-Cinline-threshold=0
-Copt-level=0
-Clink-dead-code
-Coverflow-checks=off

My hope was that those above flags should actually prevent aggressive inlining of small functions. However, as with the experiments above this doesn't seem to be the case.

The reported branch coverage for the experimented file was

Hope we can find a solution in order to improve this situation.

Links to the coverage report for the file in question:

Considering this statement the functions should actually never be inlined with the given set of compiler flags: https://github.com/rust-lang/rust/blob/1.29.0/src/librustc_codegen_llvm/back/write.rs#L2097

Kogia-sima commented 4 years ago

It is a bug in rustc (https://github.com/rust-lang/rust/issues/70426).

Maybe those functions is inlined due to -Cinline-threshold=0 flag. Remove it and add -C panic=abort.

Kogia-sima commented 4 years ago

You can find that example::add function is inlined with -Cinline-threshold=0 flag in https://godbolt.org/z/5a75uj

Robbepop commented 4 years ago

It is a bug in rustc (rust-lang/rust#70426).

Maybe those functions is inlined due to -Cinline-threshold=0 flag. Remove it and add -C panic=abort.

Thank you for the quick answer! What exactly does the -C panic=abort bring to the table for coverage reports?

You can find that example::add function is inlined with -Cinline-threshold=0 flag in https://godbolt.org/z/5a75uj

Neat idea. :)

Kogia-sima commented 4 years ago

According to documentation -Z panic_abort_tests flag is to support compiling tests with -C panic=abort. I thought you've forgatten the -Cpanic=abort flag because you specify -Zpanic_abort_tests only. That makes no sense. You should specify both flags, or remove -Zpanic_abort_tests if you don't specify -Cpanic=abort flag.

Robbepop commented 4 years ago

We have implemented your suggestion to remove Cinline-threshold=0 flag and also removed the -Z panic_abort_tests flag. Now coverage reports are fixed for those functions. Thanks!

This can be closed now.