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.

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

Inconsistent coverage reports with files with 0% coverage #85718

Closed
Adam-Gleave opened this issue May 26, 2021 · 1 comment · Fixed by #92142
Closed

Inconsistent coverage reports with files with 0% coverage #85718

Adam-Gleave opened this issue May 26, 2021 · 1 comment · Fixed by #92142
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.

Comments

@Adam-Gleave
Copy link
Contributor

When generating test coverage for a module that is placed behind a feature gate, the source file is omitted from the coverage report unless at least one function has been used in a test. This seems strange, since files that are not behind features are included with 0% test coverage.

Coverage is generated using the steps in the source-based coverage guide, including passing --all-features to cargo test and cargo cov (although I'm unsure if the latter is necessary).

For example, we have an io feature in our codebase. This feature enables the use of the io module:

#[cfg(feature = "io")]
mod io;

If we try and generate a coverage report without making use of anything in this module, but with --all-features enabled, the source file is not included:

genhtml coverage/coverage.info --legend --title="Coverage" --output-directory=coverage
Processing file src/lib.rs
Processing file ...

# `io.rs` is not processed

When we add a test function that uses some element of the io module, it gets included:

genhtml coverage/coverage.info --legend --title="Coverage" --output-directory=coverage
Processing file src/lib.rs
Processing file ...
Processing file src/io.rs

But this is now counted as 100% coverage, although only a couple of lines of code are utilised in the test.

This seems very strange, since we have another file (error.rs) that is not behind any features, and always shows up in coverage results, even though coverage of this file is 0%.

Also, if the #[cfg(feature = "io")] directive before the module include is removed, the problem still persists. It seems very inconsistent, and I'm not quite sure why this is all happening!

Using rustc v1.52.1, on the nightly channel.

@Adam-Gleave Adam-Gleave added the C-bug Category: This is a bug. label May 26, 2021
@wesleywiser wesleywiser added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label May 26, 2021
@richkadel
Copy link
Contributor

I don't have enough information in this report to understand the problem. Can you provide an MCVE?

-Z instrument-coverage is generally not aware of features. Whatever code actually gets parsed and compiled is passed through the coverage instrumentation. By that time, most features have already been handled.

When you talk about "file coverage", without really understanding the issue, my guesses may be way off base.

But (guessing) one possible factor is "unused function coverage". You could try -Z instrument-coverage=except-unused-functions (or except-unused-generics).

If adding one of those options makes your results more consistent, that's a good clue as to what's going on.

If the file is only processed under certain circumstances, the default is to generate coverage of all known unused functions (parsed but not called).

If the compiler is able to decide a file is not used at all, it may be able to skip the file, in which case there would be no coverage, because nothing in the file would be known to coverage during the code generation step.

wesleywiser added a commit to wesleywiser/rust that referenced this issue Dec 20, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 5, 2022
…ioning, r=tmandry

[code coverage] Fix missing dead code in modules that are never called

The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).

The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.

This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.

Fixes rust-lang#91661
Fixes rust-lang#86177
Fixes rust-lang#85718
Fixes rust-lang#79622

r? `@tmandry`
cc `@richkadel`

This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 12, 2022
…ioning, r=tmandry

[code coverage] Fix missing dead code in modules that are never called

The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).

The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.

This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.

Fixes rust-lang#91661
Fixes rust-lang#86177
Fixes rust-lang#85718
Fixes rust-lang#79622

r? `@tmandry`
cc `@richkadel`

This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 13, 2022
…ioning, r=tmandry

[code coverage] Fix missing dead code in modules that are never called

The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).

The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.

This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.

Fixes rust-lang#91661
Fixes rust-lang#86177
Fixes rust-lang#85718
Fixes rust-lang#79622

r? ``@tmandry``
cc ``@richkadel``

This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 13, 2022
…ioning, r=tmandry

[code coverage] Fix missing dead code in modules that are never called

The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).

The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.

This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.

Fixes rust-lang#91661
Fixes rust-lang#86177
Fixes rust-lang#85718
Fixes rust-lang#79622

r? ```@tmandry```
cc ```@richkadel```

This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
@bors bors closed this as completed in ef57f24 Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants