Fix coverage ICEs (unreachable-cov-only, spanview wrong body)#85082
Closed
richkadel wants to merge 3 commits intorust-lang:masterfrom
Closed
Fix coverage ICEs (unreachable-cov-only, spanview wrong body)#85082richkadel wants to merge 3 commits intorust-lang:masterfrom
richkadel wants to merge 3 commits intorust-lang:masterfrom
Conversation
Fixes: rust-lang#85081 An `assert!()` in `FunctionCoverage` failed, because PR rust-lang#84797 replaces unreachable `Counter`s with `Unreachable` code regions to be added to the function's coverage map. To fix it, and still generate valid LLVM coverage IR, convert one unreachable to a `Counter`.
rust-highfive
added
the
S-waiting-on-review
Author
|
@wesleywiser - Since the change that causes the ICE is probably in I'm sorry I don't have a new test to include with this fix. I don't know how to create an example that leads to this situation. I only know that I've confirmed this solution works for the complicated code that triggered it. (It's clear why this happens, and it was my oversight not to have recognized the flaw in the original PR.) Thanks. |
The coverage body_span doesn't always match the function body_span.
This comment has been minimized.
This comment has been minimized.
richkadel
changed the title
Some code cleanup extracted from future (but unfinished) commit to fix coverage in attr macro functions.
richkadel
changed the title
Author
|
I moved the attr macro code to #85173 |
Author
Author
|
Closing this PR in lieu of #85215. |
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
May 15, 2021
…ks, r=tmandry coverage bug fixes and some refactoring This replaces the relevant commits (2 and 3) from PR rust-lang#85082, and also corrects an error querying for coverageinfo. 1. `coverageinfo` query needs to use the same MIR as codegen I ran into an error trying to fix dead block coverage and realized the `coverageinfo` query is getting a different MIR compared to the codegenned MIR, which can sometimes be a problem during mapgen. I changed that query to use the `InstandeDef` (which includes the generic parameter substitutions, prosibly specific to const params) instead of the `DefId` (without unknown/default const substitutions). 2. Simplified body_span and filtered span code Some code cleanup extracted from future (but unfinished) commit to fix coverage in attr macro functions. 3. Spanview needs the relevant body_span used for coverage The coverage body_span doesn't always match the function body_span. r? `@tmandry`
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
May 15, 2021
…ks, r=tmandry coverage bug fixes and some refactoring This replaces the relevant commits (2 and 3) from PR rust-lang#85082, and also corrects an error querying for coverageinfo. 1. `coverageinfo` query needs to use the same MIR as codegen I ran into an error trying to fix dead block coverage and realized the `coverageinfo` query is getting a different MIR compared to the codegenned MIR, which can sometimes be a problem during mapgen. I changed that query to use the `InstandeDef` (which includes the generic parameter substitutions, prosibly specific to const params) instead of the `DefId` (without unknown/default const substitutions). 2. Simplified body_span and filtered span code Some code cleanup extracted from future (but unfinished) commit to fix coverage in attr macro functions. 3. Spanview needs the relevant body_span used for coverage The coverage body_span doesn't always match the function body_span. r? ``@tmandry``
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
May 15, 2021
…ks, r=tmandry coverage bug fixes and some refactoring This replaces the relevant commits (2 and 3) from PR rust-lang#85082, and also corrects an error querying for coverageinfo. 1. `coverageinfo` query needs to use the same MIR as codegen I ran into an error trying to fix dead block coverage and realized the `coverageinfo` query is getting a different MIR compared to the codegenned MIR, which can sometimes be a problem during mapgen. I changed that query to use the `InstandeDef` (which includes the generic parameter substitutions, prosibly specific to const params) instead of the `DefId` (without unknown/default const substitutions). 2. Simplified body_span and filtered span code Some code cleanup extracted from future (but unfinished) commit to fix coverage in attr macro functions. 3. Spanview needs the relevant body_span used for coverage The coverage body_span doesn't always match the function body_span. r? ```@tmandry```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #85081
An
assert!()inFunctionCoveragefailed, because PR #84797replaces unreachable
Counters withUnreachablecode regions to beadded to the function's coverage map.
To fix it, and still generate valid LLVM coverage IR, convert one
unreachable to a
Counter.This PR includes two additional commits:
r? @tmandry
cc: @wesleywiser