Skip to content

Early otherwise branch MIR opt is unsound #95162

Closed
@JakobDegen

Description

@JakobDegen

I re-wrote this pass in #91840 ; however, I don't think the new version is sound either (#95161). I'm currently working on a MCVE miscompilation, but quoting from the PR:

This is unsound. Someone could write code like this:

let Q = val;
if discriminant(P) == otherwise {
    let ptr = &mut Q as *mut _ as *mut u8;
    unsafe { *ptr = 10; } // Any invalid value for the type
}

match P {
    A => match Q {
        A => {
            // code
        }
        _ => {
            // don't use Q
        }
    }
    _ => {
        // don't use Q
    }
};

Hoisting the discriminant(Q) out of the A arm causes us to compute the discriminant of an
invalid value, which is UB.

In order to fix this, we would either need to show that the discriminant computation of
place is computed in all branches, including the otherwise branch, or we would need
another analysis pass to determine that the place is fully initialized. It might even be best
to have the hoisting be performed in a different pass and just do the CFG changing in this
pass.

For an example of a test that almost miscompiles, see here. The opt doesn't fire on that particular version, but it would if there was a SimplifyCFG immediately before.

@rustbot label +A-mir +A-mir-opt +T-compiler

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-MIRArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlA-mir-optArea: MIR optimizationsC-bugCategory: This is a bug.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions