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

[stable] Backport fix for malformed coverage #117857

Closed
wants to merge 2 commits into from

Conversation

Zalathar
Copy link
Contributor

This method is trying to detect macro invocations, so that it can split a span
into two parts just after the `!` of the invocation.

Under some circumstances (probably involving nested macros), it gets confused
and produces a span that is larger than the original span, and possibly extends
outside its enclosing function and even into an adjacent file.

In extreme cases, that can result in malformed coverage mappings that cause
`llvm-cov` to fail. For now, we at least want to detect these egregious cases
and avoid them, so that coverage reports can still be produced.
@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2023

r? @wesleywiser

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

@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2023

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against stable. Please double check that you specified the right target!

@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 Nov 13, 2023
@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

Currently this backport hasn't been approved; I'm just filing this PR so that everything's ready if it does get approved, since the original PR won't apply cleanly on this branch due to minor cleanups to the affected code and test directories.

Without the workaround applied, this test will produce malformed mappings that
cause `llvm-cov` to fail.

(And if it does emit well-formed mappings, they should be obviously incorrect.)
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v4' (SHA:b4ffde65f46336ab88eb53be808477a3936bae11)
Download action repository 'actions/upload-artifact@v3' (SHA:a8a3f3ad30e3422c9c7b888a15615d19a852ae32)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
  TOOLSTATE_REPO: https://github.com/rust-lang-nursery/rust-toolstate
  CACHE_DOMAIN: ci-caches.rust-lang.org
  IMAGE: mingw-check-tidy
##[endgroup]
error: PRs changing the `beta` channel should be sent to the `beta` branch!
##[error]Process completed with exit code 1.

@Zalathar
Copy link
Contributor Author

I'm not sure what the proper procedure is for merging a last-minute beta backport to pre-release stable, but at least this PR has the adjusted versions of the patches for reference.

@Mark-Simulacrum
Copy link
Member

Cherry picked into the stable release PR (#117843).

@Zalathar Zalathar deleted the coverage-backport branch November 18, 2023 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants