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

-Zinstrument-coverage ignores file (or module) #86177

Closed
taiki-e opened this issue Jun 9, 2021 · 1 comment · Fixed by #92142
Closed

-Zinstrument-coverage ignores file (or module) #86177

taiki-e opened this issue Jun 9, 2021 · 1 comment · Fixed by #92142
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@taiki-e
Copy link
Member

taiki-e commented Jun 9, 2021

-Z instrument-coverage seems to ignore files or modules where all items are dead code.

Originally found in taiki-e/cargo-llvm-cov#26.

How to reproduce

mkdir repro
cd repro
mkdir src

echo 'mod module;
pub use module::*;
#[test]
fn f() {}' >src/lib.rs

echo 'pub fn func(x: u32) {
    match x {
        0 => {}
        1 => {}
        _ => {}
    }
}' >src/module.rs

echo '[package]
name = "repro"
version = "0.0.0"' >Cargo.toml

RUSTFLAGS="-Z instrument-coverage" \
    LLVM_PROFILE_FILE="repro-%m.profraw" \
    cargo test --tests

cargo profdata -- merge \
    -sparse repro-*.profraw -o repro.profdata

cargo cov -- report \
    $( \
      for file in \
        $( \
          RUSTFLAGS="-Z instrument-coverage" \
            cargo test --tests --no-run --message-format=json \
              | jq -r "select(.profile.test == true) | .filenames[]" \
              | grep -v dSYM - \
        ); \
      do \
        printf "%s %s " -object $file; \
      done \
    ) \
  --instr-profile=repro.profdata --summary-only

Result (src/module.rs is ignored):

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
src/lib.rs                          3                 0   100.00%           3                 0   100.00%           3                 0   100.00%           0                 0         -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               3                 0   100.00%           3                 0   100.00%           3                 0   100.00%           0                 0         -

Workaround

The known workaround is using -C link-dead-code, but it seems not the correct workaround and is likely to cause another problem. See also taiki-e/cargo-llvm-cov#26 (comment)

Meta

rustc --version --verbose:

rustc 1.54.0-nightly (ed597e7e1 2021-06-08)
binary: rustc
commit-hash: ed597e7e19d0fe716d9f81b1e840a5abbfd7c28d
commit-date: 2021-06-08
host: x86_64-apple-darwin
release: 1.54.0-nightly
LLVM version: 12.0.1

cc @richkadel
@rustbot label A-code-coverage

@taiki-e taiki-e added the C-bug Category: This is a bug. label Jun 9, 2021
@rustbot rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Jun 9, 2021
@inquisitivecrystal
Copy link
Contributor

@rustbot label +T-compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 9, 2021
taiki-e added a commit to taiki-e/cargo-llvm-cov that referenced this issue Jun 9, 2021
bors bot added a commit to taiki-e/cargo-llvm-cov that referenced this issue Jun 9, 2021
28: Revert "Use -Clink-dead-code"  r=taiki-e a=taiki-e

This reverts commit 40fdbd8.

> Due to a bug of `-Zinstrument-coverage`, some files may be ignored. There is a known workaround for this issue, but note that the workaround is likely to cause another problem. See rust-lang/rust#86177 and #26 for more.

Co-authored-by: Taiki Endo <[email protected]>
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
bors bot added a commit to taiki-e/cargo-llvm-cov that referenced this issue Jan 15, 2022
125: Update coverage output in test to nightly-2022-01-15 r=taiki-e a=taiki-e

rust-lang/rust#86177 (#21, #26) has been fixed 🎉 

Co-authored-by: Taiki Endo <[email protected]>
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants