-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Regression in 2024-11-28: llvm-cov export errors with "truncated coverage data" #133606
Comments
Hmm, my guess is that 619a272 is making it possible for all of a CGU's functions to be discarded at a late stage, so parts of the metadata end up being empty in a way that the Finding and testing a fix is probably going to be non-trivial (especially without a small repro), so for now it's probably wisest to revert. |
Regrettably, I don't have a reproducer smaller than "our entire large project with a bunch of deps" :-( |
This reverts commit adf9b5f, reversing changes made to af1ca15. Reverting due to <rust-lang#133606>.
Yeah, minimizing this sort of coverage regression is annoying, so I generally don't expect issue reporters to do it themselves. |
Revert rust-lang#133418 (Store coverage source regions as `Span`) due to regression rust-lang#133606 This reverts commit adf9b5f, reversing changes made to af1ca15. Reverting rust-lang#133418 due to regressions reported at rust-lang#133606. r? jieyouxu
Rollup of 7 pull requests Successful merges: - rust-lang#131323 (Support `clobber_abi` in AVR inline assembly) - rust-lang#133092 (Always set the deployment target when building std) - rust-lang#133134 (Don't use a SyntheticProvider for literally every type) - rust-lang#133538 (Better diagnostic for fn items in variadic functions) - rust-lang#133590 (Rename `-Zparse-only`) - rust-lang#133592 (Misc: better instructions for envrc, ignore `/build` instead of `build/`) - rust-lang#133608 (Revert rust-lang#133418 (Store coverage source regions as `Span`) due to regression rust-lang#133606) r? `@ghost` `@rustbot` modify labels: rollup
Revert rust-lang#133418 (Store coverage source regions as `Span`) due to regression rust-lang#133606 This reverts commit adf9b5f, reversing changes made to af1ca15. Reverting rust-lang#133418 due to regressions reported at rust-lang#133606. r? jieyouxu
Revert PR #133608 landed. |
Hi! It's not minimized but https://github.com/arnodb/quirky_binder/actions/runs/12090701392/job/33718146447 faces exactly this problem. It occurs with a build script. Unfortunately it's one of the last modules of the workspace to compile. Commit Hope this helps. |
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-medium |
The revert has landed and seems to have fixed the regression, so closing as fixed. (I'll need to be mindful of this failure when working on a new version of #133418.) |
coverage: Store coverage source regions as `Span` until codegen (take 2) This is an attempt to re-land rust-lang#133418: > Historically, coverage spans were converted into line/column coordinates during the MIR instrumentation pass. > This PR moves that conversion step into codegen, so that coverage spans spend most of their time stored as Span instead. > In addition to being conceptually nicer, this also reduces the size of coverage mappings in MIR, because Span is smaller than 4x u32. That PR was reverted by rust-lang#133608, because in some circumstances not covered by our test suite we were emitting coverage metadata that was causing `llvm-cov` to exit with an error (rust-lang#133606). --- The implementation here is *mostly* the same, but adapted for subsequent changes in the relevant code (e.g. rust-lang#134163). I believe that the changes in rust-lang#134163 should be sufficient to prevent the problem that required the original PR to be reverted. But I haven't been able to reproduce the original breakage in a regression test, and the `llvm-cov` error message is extremely unhelpful, so I can't completely rule out the possibility of this breaking again. r? jieyouxu (reviewer of the original PR)
Rollup merge of rust-lang#134497 - Zalathar:spans, r=jieyouxu coverage: Store coverage source regions as `Span` until codegen (take 2) This is an attempt to re-land rust-lang#133418: > Historically, coverage spans were converted into line/column coordinates during the MIR instrumentation pass. > This PR moves that conversion step into codegen, so that coverage spans spend most of their time stored as Span instead. > In addition to being conceptually nicer, this also reduces the size of coverage mappings in MIR, because Span is smaller than 4x u32. That PR was reverted by rust-lang#133608, because in some circumstances not covered by our test suite we were emitting coverage metadata that was causing `llvm-cov` to exit with an error (rust-lang#133606). --- The implementation here is *mostly* the same, but adapted for subsequent changes in the relevant code (e.g. rust-lang#134163). I believe that the changes in rust-lang#134163 should be sufficient to prevent the problem that required the original PR to be reverted. But I haven't been able to reproduce the original breakage in a regression test, and the `llvm-cov` error message is extremely unhelpful, so I can't completely rule out the possibility of this breaking again. r? jieyouxu (reviewer of the original PR)
A new version of the original PR has been merged in #134497, and will be included in the next nightly. Based on my testing, I believe that this issue should not reoccur, but let me know if something does go wrong. |
Code
Beginning with today's nightly, processing coverage produced by rustc binaries fails. Here is an example CI run: https://github.com/pyca/cryptography/actions/runs/12077661037/job/33681030705?pr=12066
The relevant snippet:
#133418 is the only change in the regression range with a commit message mentioning coverage. cc: @Zalathar
Happy Thanksgiving!
Version it worked on
rustc 1.85.0-nightly (6b6a867ae 2024-11-27)
Version with regression
rustc 1.85.0-nightly (a2545fd6f 2024-11-28)
The text was updated successfully, but these errors were encountered: