Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix MIR inlining of asm_unwind #102778

Merged
merged 2 commits into from
Oct 8, 2022
Merged

Fix MIR inlining of asm_unwind #102778

merged 2 commits into from
Oct 8, 2022

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Oct 7, 2022

The MIR inlining currently doesn't handle inline asm's unwind edge correctly.

This code will cause ICE:

struct D;

impl Drop for D {
    fn drop(&mut self) {}
}

#[inline(always)]
fn foo() {
    let _d = D;
    unsafe { std::arch::asm!("", options(may_unwind)) };
}

pub fn main() {
    foo();
}

This PR fixes this issue. I also take the opportunity to extract common code into a method.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 7, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 7, 2022

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 7, 2022
@tmiasko
Copy link
Contributor

tmiasko commented Oct 7, 2022

Thanks!

@tmiasko
Copy link
Contributor

tmiasko commented Oct 7, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Oct 7, 2022

📌 Commit 2423483 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 Oct 7, 2022
@tmiasko
Copy link
Contributor

tmiasko commented Oct 7, 2022

@bors delegate=nbdd0121

@bors
Copy link
Contributor

bors commented Oct 7, 2022

✌️ @nbdd0121 can now approve this pull request

@nbdd0121
Copy link
Contributor Author

nbdd0121 commented Oct 7, 2022

@bors r=tmiasko

@bors
Copy link
Contributor

bors commented Oct 7, 2022

📌 Commit 18a1de0836cb5ceae21b0578bfba029404617c97 has been approved by tmiasko

It is now in the queue for this repository.

@rust-log-analyzer

This comment has been minimized.

@nbdd0121
Copy link
Contributor Author

nbdd0121 commented Oct 7, 2022

@bors r=tmiasko

@bors
Copy link
Contributor

bors commented Oct 7, 2022

📌 Commit fc83427 has been approved by tmiasko

It is now in the queue for this repository.

@wesleywiser
Copy link
Member

I believe options(may_unwind) is unstable and therefore this fix does not need to be backported to beta. Is that correct?

@nbdd0121
Copy link
Contributor Author

nbdd0121 commented Oct 7, 2022

That's correct.

@wesleywiser
Copy link
Member

Great, thank you! 🙂

@petrochenkov petrochenkov assigned tmiasko and unassigned petrochenkov Oct 8, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 8, 2022
Fix MIR inlining of asm_unwind

The MIR inlining currently doesn't handle inline asm's unwind edge correctly.

This code will cause ICE:
```rust
struct D;

impl Drop for D {
    fn drop(&mut self) {}
}

#[inline(always)]
fn foo() {
    let _d = D;
    unsafe { std::arch::asm!("", options(may_unwind)) };
}

pub fn main() {
    foo();
}
```

This PR fixes this issue. I also take the opportunity to extract common code into a method.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 8, 2022
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#101520 (Allow transmutes between the same types after erasing lifetimes)
 - rust-lang#102675 (Remove `mir::CastKind::Misc`)
 - rust-lang#102778 (Fix MIR inlining of asm_unwind)
 - rust-lang#102785 (Remove `DefId` from some `SelectionCandidate` variants)
 - rust-lang#102788 (Update rustc-dev-guide)
 - rust-lang#102789 (Update browser UI test version)
 - rust-lang#102797 (rustdoc: remove no-op CSS `.rightside { position: initial }`)
 - rust-lang#102798 (rustdoc: add main-heading and example-wrap link CSS to big selector)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2f26649 into rust-lang:master Oct 8, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 8, 2022
# for free to join this conversation on GitHub. Already have an account? # 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.

8 participants