Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a479766

Browse files
committedOct 4, 2021
Auto merge of rust-lang#89489 - FabianWolff:issue-89485, r=oli-obk
Fix unsound optimization with explicit variant discriminants Fixes rust-lang#89485.
2 parents 44593ae + dd9b476 commit a479766

File tree

2 files changed

+52
-8
lines changed

2 files changed

+52
-8
lines changed
 

‎compiler/rustc_mir_transform/src/simplify_try.rs

+34-8
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,12 @@ pub struct SimplifyBranchSame;
544544

545545
impl<'tcx> MirPass<'tcx> for SimplifyBranchSame {
546546
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
547+
// This optimization is disabled by default for now due to
548+
// soundness concerns; see issue #89485 and PR #89489.
549+
if !tcx.sess.opts.debugging_opts.unsound_mir_opts {
550+
return;
551+
}
552+
547553
trace!("Running SimplifyBranchSame on {:?}", body.source);
548554
let finder = SimplifyBranchSameOptimizationFinder { body, tcx };
549555
let opts = finder.find();
@@ -706,12 +712,24 @@ impl<'a, 'tcx> SimplifyBranchSameOptimizationFinder<'a, 'tcx> {
706712
let helper = |rhs: &Rvalue<'tcx>,
707713
place: &Place<'tcx>,
708714
variant_index: &VariantIdx,
715+
switch_value: u128,
709716
side_to_choose| {
710717
let place_type = place.ty(self.body, self.tcx).ty;
711718
let adt = match *place_type.kind() {
712719
ty::Adt(adt, _) if adt.is_enum() => adt,
713720
_ => return StatementEquality::NotEqual,
714721
};
722+
// We need to make sure that the switch value that targets the bb with
723+
// SetDiscriminant is the same as the variant discriminant.
724+
let variant_discr = adt.discriminant_for_variant(self.tcx, *variant_index).val;
725+
if variant_discr != switch_value {
726+
trace!(
727+
"NO: variant discriminant {} does not equal switch value {}",
728+
variant_discr,
729+
switch_value
730+
);
731+
return StatementEquality::NotEqual;
732+
}
715733
let variant_is_fieldless = adt.variants[*variant_index].fields.is_empty();
716734
if !variant_is_fieldless {
717735
trace!("NO: variant {:?} was not fieldless", variant_index);
@@ -740,20 +758,28 @@ impl<'a, 'tcx> SimplifyBranchSameOptimizationFinder<'a, 'tcx> {
740758
(
741759
StatementKind::Assign(box (_, rhs)),
742760
StatementKind::SetDiscriminant { place, variant_index },
743-
)
744-
// we need to make sure that the switch value that targets the bb with SetDiscriminant (y), is the same as the variant index
745-
if Some(variant_index.index() as u128) == y_target_and_value.value => {
761+
) if y_target_and_value.value.is_some() => {
746762
// choose basic block of x, as that has the assign
747-
helper(rhs, place, variant_index, x_target_and_value.target)
763+
helper(
764+
rhs,
765+
place,
766+
variant_index,
767+
y_target_and_value.value.unwrap(),
768+
x_target_and_value.target,
769+
)
748770
}
749771
(
750772
StatementKind::SetDiscriminant { place, variant_index },
751773
StatementKind::Assign(box (_, rhs)),
752-
)
753-
// we need to make sure that the switch value that targets the bb with SetDiscriminant (x), is the same as the variant index
754-
if Some(variant_index.index() as u128) == x_target_and_value.value => {
774+
) if x_target_and_value.value.is_some() => {
755775
// choose basic block of y, as that has the assign
756-
helper(rhs, place, variant_index, y_target_and_value.target)
776+
helper(
777+
rhs,
778+
place,
779+
variant_index,
780+
x_target_and_value.value.unwrap(),
781+
y_target_and_value.target,
782+
)
757783
}
758784
_ => {
759785
trace!("NO: statements `{:?}` and `{:?}` not considered equal", x, y);

‎src/test/ui/mir/issue-89485.rs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Regression test for issue #89485.
2+
3+
// run-pass
4+
5+
#[derive(Debug, Eq, PartialEq)]
6+
pub enum Type {
7+
A = 1,
8+
B = 2,
9+
}
10+
pub fn encode(v: Type) -> Type {
11+
match v {
12+
Type::A => Type::B,
13+
_ => v,
14+
}
15+
}
16+
fn main() {
17+
assert_eq!(Type::B, encode(Type::A));
18+
}

0 commit comments

Comments
 (0)
Please sign in to comment.