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

ConstGoto can produce invalid control flow in cleanup blocks #107315

Closed
saethlin opened this issue Jan 26, 2023 · 0 comments · Fixed by #107323
Closed

ConstGoto can produce invalid control flow in cleanup blocks #107315

saethlin opened this issue Jan 26, 2023 · 0 comments · Fixed by #107323
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug.

Comments

@saethlin
Copy link
Member

saethlin commented Jan 26, 2023

With #106613, the crate fallible_iterator doesn't pass MIR validation after ConstGoto (this is why CI on that PR doesn't pass):

error: internal compiler error: broken MIR in Item(WithOptConstParam { did: DefId(0:415 ~ fallible_iterator[87b6]::{impl#25}::try_fold), const_param_did: None }) (after pass ConstGoto) at bb15[0]:
                                Cleanup control flow violation: The blocks dominated by bb15 have edges to both bb14 and bb10
    --> src/lib.rs:1814:5
     |
1814 |     }
     |     ^

This is the validation check that was improved in #106612

To reproduce, build fallible_iterator 0.2.0 with that PR, and using -Zvalidate-mir and --release.

I think the bug here is that ConstGoto is capable of turning MIR that passes validation into MIR that doesn't. It is of course tempting to blame the new pass, but if I recall correctly @JakobDegen previously indicated to me that it seems more likely that a pass which changes the control flow graph is the actual problem here.

I haven't minimized the code pattern that causes the problematic transformation, but I'm including the MIR graphviz dumps of the problematic function before and after ConstGoto:

ConstGoto.before
ConstGoto.after

@rustbot label +A-mir-opt +C-bug

@rustbot rustbot added A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. labels Jan 26, 2023
@bors bors closed this as completed in 4ed8cfc Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants