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

Fix invalid codegen during debuginfo lowering #105482

Merged
merged 4 commits into from
Dec 10, 2022

Conversation

wesleywiser
Copy link
Member

In order for LLVM to correctly generate debuginfo for msvc, we sometimes need to spill arguments to the stack and perform some direct & indirect offsets into the value. Previously, this code always performed those actions, even when not required as LLVM would clean it up during optimization.

However, when MIR inlining is enabled, this can cause problems as the operations occur prior to the spilled value being initialized. To solve this, we first calculate the necessary offsets using just the type which is side-effect free and does not alter the LLVM IR. Then, if we are in a situation which requires us to generate the LLVM IR (and this situation only occurs for arguments, not local variables) then we perform the same calculation again, this time generating the appropriate LLVM IR as we go.

r? @tmiasko but feel free to reassign if you want 🙂

Fixes #105386

@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 9, 2022
This will allow us to separate the act of calculating the offsets from
creating LLVM IR that performs the actions.
In order for LLVM to correctly generate debuginfo for msvc, we sometimes
need to spill arguments to the stack and perform some direct & indirect
offsets into the value. Previously, this code always performed those
actions, even when not required as LLVM would clean it up during
optimization.

However, when MIR inlining is enabled, this can cause problems as the
operations occur prior to the spilled value being initialized. To solve
this, we first calculate the necessary offsets using just the type which
is side-effect free and does not alter the LLVM IR. Then, if we are in a
situation which requires us to generate the LLVM IR (and this situation
only occurs for arguments, not local variables) then we perform the same
calculation again, this time generating the appropriate LLVM IR as we
go.
@tmiasko
Copy link
Contributor

tmiasko commented Dec 9, 2022

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 9, 2022

📌 Commit 7253057 has been approved by tmiasko

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 9, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2022
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#104460 (Migrate parts of `rustc_expand` to session diagnostics)
 - rust-lang#105192 (Point at LHS on binop type err if relevant)
 - rust-lang#105234 (Remove unneeded field from `SwitchTargets`)
 - rust-lang#105239 (Avoid heap allocation when truncating thread names)
 - rust-lang#105410 (Consider `parent_count` for const param defaults)
 - rust-lang#105482 (Fix invalid codegen during debuginfo lowering)

Failed merges:

 - rust-lang#105411 (Introduce `with_forced_trimmed_paths`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ab50529 into rust-lang:master Dec 10, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Variable debuginfo introduces undefined behaviour
4 participants