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

SimplifyBranchSame assumes discriminant is the same as variant index. #89485

Closed
tmiasko opened this issue Oct 3, 2021 · 4 comments · Fixed by #89489
Closed

SimplifyBranchSame assumes discriminant is the same as variant index. #89485

tmiasko opened this issue Oct 3, 2021 · 4 comments · Fixed by #89489
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Oct 3, 2021

$ cat a.rs 
#[derive(Debug, Eq, PartialEq)]
pub enum Type {
    A = 1,
    B = 2,
}
pub fn encode(v: Type) -> Type {
    match v {
        Type::A => Type::B,
        _ => v,
    }
}
fn main() {
  assert_eq!(Type::B, encode(Type::A));
}
$ rustc -Zmir-opt-level=0 a.rs && ./a
$ rustc a.rs && ./a
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `B`,
 right: `A`', a.rs:13:3
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Originally found by @bonega when experimenting with discriminants starting from 1 in #88984 (comment).

@rustbot modify labels: +regression-from-stable-to-stable +I-unsound

@tmiasko tmiasko added the C-bug Category: This is a bug. label Oct 3, 2021
@rustbot rustbot added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 3, 2021
@tmiasko tmiasko changed the title SimplifyBranchSame assumes discriminants are the same as variant index. SimplifyBranchSame assumes discriminant is the same as variant index. Oct 3, 2021
@Urgau
Copy link
Member

Urgau commented Oct 3, 2021

After a quick godbolt it appears that the regression appear with Rust 1.48.0

@apiraino
Copy link
Contributor

apiraino commented Oct 3, 2021

A bisection seems to lead to 043f6d7, nightly 2020-09-26

from e599b53 to 043f6d7

found 11 bors merge commits in the specified range
  commit[0] 2020-09-24UTC: Auto merge of #76918 - ishitatsuyuki:match-fastpath, r=oli-obk
  commit[1] 2020-09-24UTC: Auto merge of #77014 - tmiasko:arena, r=Mark-Simulacrum
  commit[2] 2020-09-25UTC: Auto merge of #77172 - jonas-schievink:rollup-a041rou, r=jonas-schievink
  commit[3] 2020-09-25UTC: Auto merge of #76844 - simonvandel:fix-76803, r=wesleywiser
  commit[4] 2020-09-25UTC: Auto merge of #77144 - flip1995:clippyup, r=Manishearth
  commit[5] 2020-09-25UTC: Auto merge of #77041 - lcnr:const-eval-perf, r=ecstatic-morse
  commit[6] 2020-09-25UTC: Auto merge of #77152 - vandenheuvel:update_chalk_further, r=jackh726
  commit[7] 2020-09-25UTC: Auto merge of #73453 - erikdesjardins:tuplayout, r=eddyb
  commit[8] 2020-09-25UTC: Auto merge of #77157 - tmiasko:simplify-cfg-dup, r=jonas-schievink
  commit[9] 2020-09-25UTC: Auto merge of #77198 - jonas-schievink:rollup-i59i41h, r=jonas-schievink
  commit[10] 2020-09-25UTC: Auto merge of #77201 - matthewjasper:rename-get-unchecked, r=spastorino

@apiraino
Copy link
Contributor

apiraino commented Oct 3, 2021

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 3, 2021
@nbdd0121
Copy link
Contributor

nbdd0121 commented Oct 3, 2021

Looks like it is mostly likely caused by #76844 which is supposed to fix #76803. cc @simonvandel @wesleywiser

@camelid camelid added the A-mir-opt Area: MIR optimizations label Oct 3, 2021
@bors bors closed this as completed in a479766 Oct 4, 2021
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. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants