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

coverage: Skip instrumenting a function if no spans were extracted from MIR #118852

Merged
merged 3 commits into from
Dec 18, 2023

Conversation

Zalathar
Copy link
Contributor

The immediate symptoms of #118643 were fixed by #118666, but some users reported that their builds now encounter another coverage-related ICE:

error: internal compiler error: compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs:98:17: A used function should have had coverage mapping data but did not: (...)

I was able to reproduce at least one cause of this error: if no relevant spans could be extracted from a function, but the function contains CoverageKind::SpanMarker statements, then codegen still thinks the function is instrumented and complains about the fact that it has no coverage spans.

This PR prevents that from happening in two ways:

  • If we didn't extract any relevant spans from MIR, skip instrumenting the entire function and don't create a FunctionCoverateInfo for it.
  • If coverage codegen sees a CoverageKind::SpanMarker statement, skip it early and avoid creating func_coverage.

Fixes #118850.

@rustbot
Copy link
Collaborator

rustbot commented Dec 12, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 12, 2023
@Zalathar
Copy link
Contributor Author

@rustbot label +A-code-coverage

@rustbot
Copy link
Collaborator

rustbot commented Dec 12, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Dec 12, 2023
@Zalathar
Copy link
Contributor Author

Another thing I tried was having the InstrumentCoverage MIR pass take all those CoverageKind::SpanMarker statements and replace them with StatementKind::Nop.

However, that doesn't work because Nop isn't allowed during the phase that InstrumentCoverage runs in.

@Zalathar
Copy link
Contributor Author

Either of the patches should fix the issue on their own, but I think they're both worthwhile.

(If we skip instrumenting a function with no spans, then the function won't have FunctionCoverageInfo, so func_coverage never gets created either way. But I had already been meaning to make marker statements not materialize func_coverage anyway.)

Coverage marker statements should have no effect on codegen, but in some cases
they could have the side-effect of creating a `func_coverage` entry for their
enclosing function. That can lead to an ICE for functions that don't actually
have any coverage spans.
@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 17, 2023

📌 Commit e0de143 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 17, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#118852 (coverage: Skip instrumenting a function if no spans were extracted from MIR)
 - rust-lang#118905 ([AIX] Fix XCOFF metadata)
 - rust-lang#118967 (Add better ICE messages for some undescriptive panics)
 - rust-lang#119051 (Replace `FileAllocationInfo` with `FileEndOfFileInfo`)
 - rust-lang#119059 (Deny `~const` trait bounds in inherent impl headers)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 418ae3e into rust-lang:master Dec 18, 2023
@rustbot rustbot added this to the 1.76.0 milestone Dec 18, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2023
Rollup merge of rust-lang#118852 - Zalathar:no-spans, r=cjgillot

coverage: Skip instrumenting a function if no spans were extracted from MIR

The immediate symptoms of rust-lang#118643 were fixed by rust-lang#118666, but some users reported that their builds now encounter another coverage-related ICE:

```
error: internal compiler error: compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs:98:17: A used function should have had coverage mapping data but did not: (...)
```

I was able to reproduce at least one cause of this error: if no relevant spans could be extracted from a function, but the function contains `CoverageKind::SpanMarker` statements, then codegen still thinks the function is instrumented and complains about the fact that it has no coverage spans.

This PR prevents that from happening in two ways:
- If we didn't extract any relevant spans from MIR, skip instrumenting the entire function and don't create a `FunctionCoverateInfo` for it.
- If coverage codegen sees a `CoverageKind::SpanMarker` statement, skip it early and avoid creating `func_coverage`.

---

Fixes rust-lang#118850.
@Zalathar Zalathar deleted the no-spans branch December 18, 2023 10:13
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) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A used function should have had coverage mapping data but did not
6 participants