Miscompilation due to MatchBranchSimplification MIR pass mixing up discriminants

[RalfJung]

#120614 introduced a miscompilation; turns out this question poked at incorrect logic in the MatchBranchSimplification MIR pass.

#124122 is a WIP patch to fix this (and also contains a testcase that currently gets miscompiled), but as a critical soundness issue this deserves tracking nonetheless.

[RalfJung]

Cc @rust-lang/wg-mir-opt

[Noratrieb]

does it make sense to quickly throw up a PR disabling it to leave more time for fixing it properly and thoroughly?

[dianqk]

does it make sense to quickly throw up a PR disabling it to leave more time for fixing it properly and thoroughly?

I like this: #124122 (comment).
I think that disabling some transforms where miscompilation might occur is feasible in #124122.

[Noratrieb]

yeah, could you put up a quick PR to disable it, gating it behind unsound MIR opts?

[dianqk]

yeah, could you put up a quick PR to disable it, gating it behind unsound MIR opts?

I will do it today. :)

[dianqk]

For me, but it's more appropriate to disable unsigned and signed conversions in #124122. But might this require a longer review? I'm not sure, but at least the conversions that are retained should be clear.
@oli-obk Do you have any suggestions?

[dianqk]

For me, but it's more appropriate to disable unsigned and signed conversions in #124122.

This is just the usual operation on LLVM: disabling certain conversions, rather than disabling the entire Pass. Of course, compared to LLVM's passes, the passes in MIR-opt are much smaller.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical