From c4cc9ca0603cfdf5921a82d929d680db94d1d072 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 21 Oct 2023 09:59:03 +0000 Subject: [PATCH] Do not merge fn pointer casts. --- compiler/rustc_mir_transform/src/gvn.rs | 9 ++ .../miri/tests/pass/function_pointers.rs | 5 +- .../gvn.fn_pointers.GVN.panic-abort.diff | 118 ++++++++++++++++++ .../gvn.fn_pointers.GVN.panic-unwind.diff | 118 ++++++++++++++++++ tests/mir-opt/gvn.rs | 25 ++++ 5 files changed, 272 insertions(+), 3 deletions(-) create mode 100644 tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff create mode 100644 tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index de61571fdfc..cf8a0fdb97e 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -92,6 +92,7 @@ use rustc_index::IndexVec; use rustc_macros::newtype_index; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; +use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::{self, Ty, TyCtxt, TypeAndMut}; use rustc_span::def_id::DefId; @@ -761,6 +762,14 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { Rvalue::Cast(kind, ref mut value, to) => { let from = value.ty(self.local_decls, self.tcx); let value = self.simplify_operand(value, location)?; + if let CastKind::PointerCoercion( + PointerCoercion::ReifyFnPointer | PointerCoercion::ClosureFnPointer(_), + ) = kind + { + // Each reification of a generic fn may get a different pointer. + // Do not try to merge them. + return self.new_opaque(); + } Value::Cast { kind, value, from, to } } Rvalue::BinaryOp(op, box (ref mut lhs, ref mut rhs)) => { diff --git a/src/tools/miri/tests/pass/function_pointers.rs b/src/tools/miri/tests/pass/function_pointers.rs index 1c99a96feda..8e58692a0c7 100644 --- a/src/tools/miri/tests/pass/function_pointers.rs +++ b/src/tools/miri/tests/pass/function_pointers.rs @@ -80,9 +80,8 @@ fn main() { // but Miri currently uses a fixed address for monomorphic functions. assert!(return_fn_ptr(i) == i); assert!(return_fn_ptr(i) as unsafe fn() -> i32 == i as fn() -> i32 as unsafe fn() -> i32); - // We don't check anything for `f`. Miri gives it many different addresses - // but mir-opts can turn them into the same address. - let _val = return_fn_ptr(f) != f; + // Miri gives it many different addresses to different reifications of a generic function. + assert!(return_fn_ptr(f) != f); // However, if we only turn `f` into a function pointer and use that pointer, // it is equal to itself. let f2 = f as fn() -> i32; diff --git a/tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff b/tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff new file mode 100644 index 00000000000..d8248d22d38 --- /dev/null +++ b/tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff @@ -0,0 +1,118 @@ +- // MIR for `fn_pointers` before GVN ++ // MIR for `fn_pointers` after GVN + + fn fn_pointers() -> () { + let mut _0: (); + let _1: fn(u8) -> u8; + let _2: (); + let mut _3: fn(u8) -> u8; + let _5: (); + let mut _6: fn(u8) -> u8; + let mut _9: {closure@$DIR/gvn.rs:591:19: 591:21}; + let _10: (); + let mut _11: fn(); + let mut _13: {closure@$DIR/gvn.rs:591:19: 591:21}; + let _14: (); + let mut _15: fn(); + scope 1 { + debug f => _1; + let _4: fn(u8) -> u8; + scope 2 { + debug g => _4; + let _7: {closure@$DIR/gvn.rs:591:19: 591:21}; + scope 3 { + debug closure => _7; + let _8: fn(); + scope 4 { + debug cf => _8; + let _12: fn(); + scope 5 { + debug cg => _12; + } + } + } + } + } + + bb0: { +- StorageLive(_1); ++ nop; + _1 = identity:: as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer)); + StorageLive(_2); + StorageLive(_3); + _3 = _1; +- _2 = opaque:: u8>(move _3) -> [return: bb1, unwind unreachable]; ++ _2 = opaque:: u8>(_1) -> [return: bb1, unwind unreachable]; + } + + bb1: { + StorageDead(_3); + StorageDead(_2); +- StorageLive(_4); ++ nop; + _4 = identity:: as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer)); + StorageLive(_5); + StorageLive(_6); + _6 = _4; +- _5 = opaque:: u8>(move _6) -> [return: bb2, unwind unreachable]; ++ _5 = opaque:: u8>(_4) -> [return: bb2, unwind unreachable]; + } + + bb2: { + StorageDead(_6); + StorageDead(_5); +- StorageLive(_7); +- _7 = {closure@$DIR/gvn.rs:591:19: 591:21}; +- StorageLive(_8); ++ nop; ++ _7 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21}; ++ nop; + StorageLive(_9); +- _9 = _7; +- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Normal))); ++ _9 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21}; ++ _8 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21} as fn() (PointerCoercion(ClosureFnPointer(Normal))); + StorageDead(_9); + StorageLive(_10); + StorageLive(_11); + _11 = _8; +- _10 = opaque::(move _11) -> [return: bb3, unwind unreachable]; ++ _10 = opaque::(_8) -> [return: bb3, unwind unreachable]; + } + + bb3: { + StorageDead(_11); + StorageDead(_10); +- StorageLive(_12); ++ nop; + StorageLive(_13); +- _13 = _7; +- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Normal))); ++ _13 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21}; ++ _12 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21} as fn() (PointerCoercion(ClosureFnPointer(Normal))); + StorageDead(_13); + StorageLive(_14); + StorageLive(_15); + _15 = _12; +- _14 = opaque::(move _15) -> [return: bb4, unwind unreachable]; ++ _14 = opaque::(_12) -> [return: bb4, unwind unreachable]; + } + + bb4: { + StorageDead(_15); + StorageDead(_14); + _0 = const (); +- StorageDead(_12); +- StorageDead(_8); +- StorageDead(_7); +- StorageDead(_4); +- StorageDead(_1); ++ nop; ++ nop; ++ nop; ++ nop; ++ nop; + return; + } + } + diff --git a/tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff b/tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff new file mode 100644 index 00000000000..e38a3d85209 --- /dev/null +++ b/tests/mir-opt/gvn.fn_pointers.GVN.panic-unwind.diff @@ -0,0 +1,118 @@ +- // MIR for `fn_pointers` before GVN ++ // MIR for `fn_pointers` after GVN + + fn fn_pointers() -> () { + let mut _0: (); + let _1: fn(u8) -> u8; + let _2: (); + let mut _3: fn(u8) -> u8; + let _5: (); + let mut _6: fn(u8) -> u8; + let mut _9: {closure@$DIR/gvn.rs:591:19: 591:21}; + let _10: (); + let mut _11: fn(); + let mut _13: {closure@$DIR/gvn.rs:591:19: 591:21}; + let _14: (); + let mut _15: fn(); + scope 1 { + debug f => _1; + let _4: fn(u8) -> u8; + scope 2 { + debug g => _4; + let _7: {closure@$DIR/gvn.rs:591:19: 591:21}; + scope 3 { + debug closure => _7; + let _8: fn(); + scope 4 { + debug cf => _8; + let _12: fn(); + scope 5 { + debug cg => _12; + } + } + } + } + } + + bb0: { +- StorageLive(_1); ++ nop; + _1 = identity:: as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer)); + StorageLive(_2); + StorageLive(_3); + _3 = _1; +- _2 = opaque:: u8>(move _3) -> [return: bb1, unwind continue]; ++ _2 = opaque:: u8>(_1) -> [return: bb1, unwind continue]; + } + + bb1: { + StorageDead(_3); + StorageDead(_2); +- StorageLive(_4); ++ nop; + _4 = identity:: as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer)); + StorageLive(_5); + StorageLive(_6); + _6 = _4; +- _5 = opaque:: u8>(move _6) -> [return: bb2, unwind continue]; ++ _5 = opaque:: u8>(_4) -> [return: bb2, unwind continue]; + } + + bb2: { + StorageDead(_6); + StorageDead(_5); +- StorageLive(_7); +- _7 = {closure@$DIR/gvn.rs:591:19: 591:21}; +- StorageLive(_8); ++ nop; ++ _7 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21}; ++ nop; + StorageLive(_9); +- _9 = _7; +- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Normal))); ++ _9 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21}; ++ _8 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21} as fn() (PointerCoercion(ClosureFnPointer(Normal))); + StorageDead(_9); + StorageLive(_10); + StorageLive(_11); + _11 = _8; +- _10 = opaque::(move _11) -> [return: bb3, unwind continue]; ++ _10 = opaque::(_8) -> [return: bb3, unwind continue]; + } + + bb3: { + StorageDead(_11); + StorageDead(_10); +- StorageLive(_12); ++ nop; + StorageLive(_13); +- _13 = _7; +- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Normal))); ++ _13 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21}; ++ _12 = const ZeroSized: {closure@$DIR/gvn.rs:591:19: 591:21} as fn() (PointerCoercion(ClosureFnPointer(Normal))); + StorageDead(_13); + StorageLive(_14); + StorageLive(_15); + _15 = _12; +- _14 = opaque::(move _15) -> [return: bb4, unwind continue]; ++ _14 = opaque::(_12) -> [return: bb4, unwind continue]; + } + + bb4: { + StorageDead(_15); + StorageDead(_14); + _0 = const (); +- StorageDead(_12); +- StorageDead(_8); +- StorageDead(_7); +- StorageDead(_4); +- StorageDead(_1); ++ nop; ++ nop; ++ nop; ++ nop; ++ nop; + return; + } + } + diff --git a/tests/mir-opt/gvn.rs b/tests/mir-opt/gvn.rs index 1c14f818044..7ce851905e0 100644 --- a/tests/mir-opt/gvn.rs +++ b/tests/mir-opt/gvn.rs @@ -574,6 +574,29 @@ fn repeat() { let array = [val, val, val, val, val, val, val, val, val, val]; } +/// Verify that we do not merge fn pointers created by casts. +fn fn_pointers() { + // CHECK-LABEL: fn fn_pointers( + // CHECK: [[f:_.*]] = identity:: as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer + // CHECK: opaque:: u8>([[f]]) + let f = identity as fn(u8) -> u8; + opaque(f); + // CHECK: [[g:_.*]] = identity:: as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer + // CHECK: opaque:: u8>([[g]]) + let g = identity as fn(u8) -> u8; + opaque(g); + + // CHECK: [[cf:_.*]] = const {{.*}} as fn() (PointerCoercion(ClosureFnPointer + // CHECK: opaque::([[cf]]) + let closure = || {}; + let cf = closure as fn(); + opaque(cf); + // CHECK: [[cg:_.*]] = const {{.*}} as fn() (PointerCoercion(ClosureFnPointer + // CHECK: opaque::([[cg]]) + let cg = closure as fn(); + opaque(cg); +} + fn main() { subexpression_elimination(2, 4, 5); wrap_unwrap(5); @@ -590,6 +613,7 @@ fn main() { let (direct, indirect) = duplicate_slice(); assert_eq!(direct, indirect); repeat(); + fn_pointers(); } #[inline(never)] @@ -614,3 +638,4 @@ fn identity(x: T) -> T { // EMIT_MIR gvn.slices.GVN.diff // EMIT_MIR gvn.duplicate_slice.GVN.diff // EMIT_MIR gvn.repeat.GVN.diff +// EMIT_MIR gvn.fn_pointers.GVN.diff