From 7d44887374ee667bd5aeb5e861032b8ce4093b29 Mon Sep 17 00:00:00 2001 From: dianqk Date: Tue, 25 Mar 2025 22:21:18 +0800 Subject: [PATCH] Invalid dereferences for all non-local mutations --- compiler/rustc_mir_transform/src/gvn.rs | 30 +++++++++-------- ...mute.undef_union_as_integer.GVN.32bit.diff | 3 +- ...mute.undef_union_as_integer.GVN.64bit.diff | 3 +- ...in.DestinationPropagation.panic-abort.diff | 10 ++---- ...n.DestinationPropagation.panic-unwind.diff | 10 ++---- ..._aggregate_to_copy_miscompile.foo.GVN.diff | 2 +- .../simplify_aggregate_to_copy_miscompile.rs | 33 ++++++++++++++++--- ..._copy_miscompile.set_discriminant.GVN.diff | 20 +++++++++++ 8 files changed, 75 insertions(+), 36 deletions(-) create mode 100644 tests/mir-opt/simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 046f36bdedd..68bc0ffce6b 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -1728,12 +1728,18 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { self.tcx } - fn visit_place(&mut self, place: &mut Place<'tcx>, _: PlaceContext, location: Location) { + fn visit_place(&mut self, place: &mut Place<'tcx>, context: PlaceContext, location: Location) { self.simplify_place_projection(place, location); + if context.is_mutating_use() && !place.projection.is_empty() { + // Non-local mutation maybe invalidate deref. + self.invalidate_derefs(); + } + self.super_place(place, context, location); } fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) { self.simplify_operand(operand, location); + self.super_operand(operand, location); } fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, location: Location) { @@ -1751,22 +1757,18 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> { self.assign(local, value); Some(value) } else { - // Non-local assignments maybe invalidate deref. - self.invalidate_derefs(); value }; - let Some(value) = value else { return }; - - if let Some(const_) = self.try_as_constant(value) { - *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_))); - } else if let Some(local) = self.try_as_local(value, location) - && *rvalue != Rvalue::Use(Operand::Move(local.into())) - { - *rvalue = Rvalue::Use(Operand::Copy(local.into())); - self.reused_locals.insert(local); + if let Some(value) = value { + if let Some(const_) = self.try_as_constant(value) { + *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_))); + } else if let Some(local) = self.try_as_local(value, location) + && *rvalue != Rvalue::Use(Operand::Move(local.into())) + { + *rvalue = Rvalue::Use(Operand::Copy(local.into())); + self.reused_locals.insert(local); + } } - - return; } self.super_statement(stmt, location); } diff --git a/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff b/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff index febb6bfb0a4..2ac9769a0e7 100644 --- a/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff +++ b/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff @@ -10,8 +10,9 @@ StorageLive(_1); StorageLive(_2); - _2 = (); +- _1 = Union32 { value: move _2 }; + _2 = const (); - _1 = Union32 { value: move _2 }; ++ _1 = Union32 { value: const () }; StorageDead(_2); _0 = move _1 as u32 (Transmute); StorageDead(_1); diff --git a/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.64bit.diff b/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.64bit.diff index febb6bfb0a4..2ac9769a0e7 100644 --- a/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.64bit.diff +++ b/tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.64bit.diff @@ -10,8 +10,9 @@ StorageLive(_1); StorageLive(_2); - _2 = (); +- _1 = Union32 { value: move _2 }; + _2 = const (); - _1 = Union32 { value: move _2 }; ++ _1 = Union32 { value: const () }; StorageDead(_2); _0 = move _1 as u32 (Transmute); StorageDead(_1); diff --git a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff index 557320f0179..ef418798faa 100644 --- a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff +++ b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff @@ -5,11 +5,10 @@ let mut _0: (); let _1: main::Un; let mut _2: u32; - let mut _3: u32; scope 1 { debug un => _1; scope 3 (inlined std::mem::drop::) { - debug _x => _3; + debug _x => _2; } } scope 2 (inlined val) { @@ -17,13 +16,10 @@ bb0: { StorageLive(_1); - StorageLive(_2); - nop; _1 = Un { us: const 1_u32 }; + StorageLive(_2); + _2 = copy (_1.0: u32); StorageDead(_2); - StorageLive(_3); - _3 = copy (_1.0: u32); - StorageDead(_3); StorageDead(_1); return; } diff --git a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff index 557320f0179..ef418798faa 100644 --- a/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff +++ b/tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff @@ -5,11 +5,10 @@ let mut _0: (); let _1: main::Un; let mut _2: u32; - let mut _3: u32; scope 1 { debug un => _1; scope 3 (inlined std::mem::drop::) { - debug _x => _3; + debug _x => _2; } } scope 2 (inlined val) { @@ -17,13 +16,10 @@ bb0: { StorageLive(_1); - StorageLive(_2); - nop; _1 = Un { us: const 1_u32 }; + StorageLive(_2); + _2 = copy (_1.0: u32); StorageDead(_2); - StorageLive(_3); - _3 = copy (_1.0: u32); - StorageDead(_3); StorageDead(_1); return; } diff --git a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff index 19a00f977eb..54c11679f0c 100644 --- a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff +++ b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff @@ -20,7 +20,7 @@ StorageLive(_2); StorageLive(_3); _3 = &(*_1); - _2 = get(move _3) -> [return: bb1, unwind unreachable]; + _2 = get::>(move _3) -> [return: bb1, unwind unreachable]; } bb1: { diff --git a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs index 87b52ae4808..33e31b56977 100644 --- a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs +++ b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs @@ -9,15 +9,16 @@ #![crate_type = "lib"] //@ test-mir-pass: GVN #![allow(internal_features)] -#![feature(rustc_attrs, core_intrinsics)] +#![feature(core_intrinsics, custom_mir, rustc_attrs)] + +use std::intrinsics::mir::*; // EMIT_MIR simplify_aggregate_to_copy_miscompile.foo.GVN.diff -#[no_mangle] fn foo(v: &mut Option) -> Option { // CHECK-LABEL: fn foo( // CHECK-SAME: [[v:_.*]]: &mut Option // CHECK: [[v_alias_1:_.*]] = &(*_1) - // CHECK-NEXT: [[v_alias_2:_.*]] = get(move [[v_alias_1]]) + // CHECK-NEXT: [[v_alias_2:_.*]] = get::>(move [[v_alias_1]]) // CHECK: (*[[v]]) = const Option::::None; // CHECK-NOT: _0 = copy (*[[v_alias_2]]) // CHECK: _0 = Option::::Some @@ -30,9 +31,31 @@ fn foo(v: &mut Option) -> Option { } } -#[no_mangle] +pub enum Value { + V0(i32), + V1(i32), +} + +// EMIT_MIR simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff +#[custom_mir(dialect = "runtime", phase = "initial")] +fn set_discriminant(v: &mut Value) -> Value { + // CHECK-LABEL: fn set_discriminant( + mir! { + let v_: &Value; + { + Call(v_ = get(v), ReturnTo(ret), UnwindUnreachable()) + } + ret = { + let col: i32 = Field(Variant(*v_, 0), 0); + SetDiscriminant(*v, 1); + RET = Value::V0(col); + Return() + } + } +} + #[inline(never)] #[rustc_nounwind] -fn get(v: &Option) -> &Option { +fn get(v: &T) -> &T { v } diff --git a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff new file mode 100644 index 00000000000..41f9763d024 --- /dev/null +++ b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff @@ -0,0 +1,20 @@ +- // MIR for `set_discriminant` before GVN ++ // MIR for `set_discriminant` after GVN + + fn set_discriminant(_1: &mut Value) -> Value { + let mut _0: Value; + let mut _2: &Value; + let mut _3: i32; + + bb0: { + _2 = get::(copy _1) -> [return: bb1, unwind unreachable]; + } + + bb1: { + _3 = copy (((*_2) as variant#0).0: i32); + discriminant((*_1)) = 1; + _0 = Value::V0(copy _3); + return; + } + } +