Skip to content

Conversation

@Zalathar
Copy link
Member

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
Member 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
Member 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
Member 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
Collaborator

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