From 58d7c2d5a760c1adfa4c3984eeb12787f5ad5b1d Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 9 Jul 2025 22:30:15 -0700 Subject: [PATCH 1/2] Make UB transmutes really UB in LLVM Ralf suggested in that UB transmutes shouldn't be trapping, which happened for the one path that PR was changing, but there's another path as well, so this PR changes that other path to match. --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 12 +++++------ tests/codegen/intrinsics/transmute.rs | 22 ++++++++++++-------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index cbbb0196890fd..1e3b76e5f933d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -236,14 +236,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { || operand.layout.is_uninhabited() || cast.is_uninhabited() { - if !operand.layout.is_uninhabited() { - // Since this is known statically and the input could have existed - // without already having hit UB, might as well trap for it. - bx.abort(); - } + // We can't use unreachable because that's a terminator, and we + // need something that can be in the middle of a basic block. + bx.assume(bx.cx().const_bool(false)); - // Because this transmute is UB, return something easy to generate, - // since it's fine that later uses of the value are probably UB. + // We still need to return a value of the appropriate type, but + // it's already UB so do the easiest thing available. return OperandValue::poison(bx, cast); } diff --git a/tests/codegen/intrinsics/transmute.rs b/tests/codegen/intrinsics/transmute.rs index e375724bc1bbd..36d6a2f722f23 100644 --- a/tests/codegen/intrinsics/transmute.rs +++ b/tests/codegen/intrinsics/transmute.rs @@ -58,9 +58,9 @@ pub unsafe fn check_bigger_array(x: [u32; 3]) -> [u32; 7] { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_to_empty_array(x: [u32; 5]) -> [u32; 0] { - // CHECK-NOT: trap - // CHECK: call void @llvm.trap - // CHECK-NOT: trap + // CHECK-NOT: call + // CHECK: call void @llvm.assume(i1 false) + // CHECK-NOT: call mir! { { RET = CastTransmute(x); @@ -88,9 +88,9 @@ pub unsafe fn check_from_empty_array(x: [u32; 0]) -> [u32; 5] { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_to_uninhabited(x: u16) { - // CHECK-NOT: trap - // CHECK: call void @llvm.trap - // CHECK-NOT: trap + // CHECK-NOT: call + // CHECK: call void @llvm.assume(i1 false) + // CHECK-NOT: call mir! { let temp: BigNever; { @@ -104,6 +104,9 @@ pub unsafe fn check_to_uninhabited(x: u16) { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_from_uninhabited(x: BigNever) -> u16 { + // CHECK-NOT: call + // CHECK: call void @llvm.assume(i1 false) + // CHECK-NOT: call // CHECK: ret i16 poison mir! { { @@ -401,9 +404,9 @@ pub unsafe fn check_issue_109992(x: ()) -> [(); 1] { pub unsafe fn check_unit_to_never(x: ()) { // This uses custom MIR to avoid MIR optimizations having removed ZST ops. - // CHECK-NOT: trap - // CHECK: call void @llvm.trap - // CHECK-NOT: trap + // CHECK-NOT: call + // CHECK: call void @llvm.assume(i1 false) + // CHECK-NOT: call mir! { let temp: ZstNever; { @@ -420,6 +423,7 @@ pub unsafe fn check_unit_from_never(x: ZstNever) -> () { // This uses custom MIR to avoid MIR optimizations having removed ZST ops. // CHECK: start + // CHECK-NEXT: call void @llvm.assume(i1 false) // CHECK-NEXT: ret void mir! { { From f5fc8727dbbf8c9e93bb0822b2e5bfa77dbd0208 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 10 Jul 2025 09:17:28 -0700 Subject: [PATCH 2/2] Add `BuilderMethods::unreachable_nonterminator` So places that need `unreachable` but in the middle of a basic block can call that instead of figuring out the best way to do it. --- compiler/rustc_codegen_ssa/src/mir/rvalue.rs | 8 +--- .../rustc_codegen_ssa/src/traits/builder.rs | 10 +++++ tests/codegen/intrinsics/transmute.rs | 41 +++++++++---------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs index 1e3b76e5f933d..bf3b1e73b94c8 100644 --- a/compiler/rustc_codegen_ssa/src/mir/rvalue.rs +++ b/compiler/rustc_codegen_ssa/src/mir/rvalue.rs @@ -207,9 +207,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { { // These cases are all UB to actually hit, so don't emit code for them. // (The size mismatches are reachable via `transmute_unchecked`.) - // We can't use unreachable because that's a terminator, and we - // need something that can be in the middle of a basic block. - bx.assume(bx.cx().const_bool(false)) + bx.unreachable_nonterminator(); } else { // Since in this path we have a place anyway, we can store or copy to it, // making sure we use the destination place's alignment even if the @@ -236,9 +234,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { || operand.layout.is_uninhabited() || cast.is_uninhabited() { - // We can't use unreachable because that's a terminator, and we - // need something that can be in the middle of a basic block. - bx.assume(bx.cx().const_bool(false)); + bx.unreachable_nonterminator(); // We still need to return a value of the appropriate type, but // it's already UB so do the easiest thing available. diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 9d367748c2a8a..0f1358ee50850 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -136,6 +136,16 @@ pub trait BuilderMethods<'a, 'tcx>: ) -> Self::Value; fn unreachable(&mut self); + /// Like [`Self::unreachable`], but for use in the middle of a basic block. + fn unreachable_nonterminator(&mut self) { + // This is the preferred LLVM incantation for this per + // https://llvm.org/docs/Frontend/PerformanceTips.html#other-things-to-consider + // Other backends may override if they have a better way. + let const_true = self.cx().const_bool(true); + let poison_ptr = self.const_poison(self.cx().type_ptr()); + self.store(const_true, poison_ptr, Align::ONE); + } + fn add(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value; fn fadd(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value; fn fadd_fast(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value; diff --git a/tests/codegen/intrinsics/transmute.rs b/tests/codegen/intrinsics/transmute.rs index 36d6a2f722f23..c9a1cd58af338 100644 --- a/tests/codegen/intrinsics/transmute.rs +++ b/tests/codegen/intrinsics/transmute.rs @@ -29,28 +29,28 @@ pub struct Aggregate8(u8); // CHECK-LABEL: @check_bigger_size( #[no_mangle] pub unsafe fn check_bigger_size(x: u16) -> u32 { - // CHECK: call void @llvm.assume(i1 false) + // CHECK: store i1 true, ptr poison, align 1 transmute_unchecked(x) } // CHECK-LABEL: @check_smaller_size( #[no_mangle] pub unsafe fn check_smaller_size(x: u32) -> u16 { - // CHECK: call void @llvm.assume(i1 false) + // CHECK: store i1 true, ptr poison, align 1 transmute_unchecked(x) } // CHECK-LABEL: @check_smaller_array( #[no_mangle] pub unsafe fn check_smaller_array(x: [u32; 7]) -> [u32; 3] { - // CHECK: call void @llvm.assume(i1 false) + // CHECK: store i1 true, ptr poison, align 1 transmute_unchecked(x) } // CHECK-LABEL: @check_bigger_array( #[no_mangle] pub unsafe fn check_bigger_array(x: [u32; 3]) -> [u32; 7] { - // CHECK: call void @llvm.assume(i1 false) + // CHECK: store i1 true, ptr poison, align 1 transmute_unchecked(x) } @@ -58,9 +58,9 @@ pub unsafe fn check_bigger_array(x: [u32; 3]) -> [u32; 7] { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_to_empty_array(x: [u32; 5]) -> [u32; 0] { - // CHECK-NOT: call - // CHECK: call void @llvm.assume(i1 false) - // CHECK-NOT: call + // CHECK: start + // CHECK-NEXT: store i1 true, ptr poison, align 1 + // CHECK-NEXT: ret void mir! { { RET = CastTransmute(x); @@ -73,9 +73,9 @@ pub unsafe fn check_to_empty_array(x: [u32; 5]) -> [u32; 0] { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_from_empty_array(x: [u32; 0]) -> [u32; 5] { - // CHECK-NOT: call - // CHECK: call void @llvm.assume(i1 false) - // CHECK-NOT: call + // CHECK: start + // CHECK-NEXT: store i1 true, ptr poison, align 1 + // CHECK-NEXT: ret void mir! { { RET = CastTransmute(x); @@ -88,9 +88,9 @@ pub unsafe fn check_from_empty_array(x: [u32; 0]) -> [u32; 5] { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_to_uninhabited(x: u16) { - // CHECK-NOT: call - // CHECK: call void @llvm.assume(i1 false) - // CHECK-NOT: call + // CHECK: start + // CHECK-NEXT: store i1 true, ptr poison, align 1 + // CHECK-NEXT: ret void mir! { let temp: BigNever; { @@ -104,10 +104,9 @@ pub unsafe fn check_to_uninhabited(x: u16) { #[no_mangle] #[custom_mir(dialect = "runtime", phase = "optimized")] pub unsafe fn check_from_uninhabited(x: BigNever) -> u16 { - // CHECK-NOT: call - // CHECK: call void @llvm.assume(i1 false) - // CHECK-NOT: call - // CHECK: ret i16 poison + // CHECK: start + // CHECK-NEXT: store i1 true, ptr poison, align 1 + // CHECK-NEXT: ret i16 poison mir! { { RET = CastTransmute(x); @@ -404,9 +403,9 @@ pub unsafe fn check_issue_109992(x: ()) -> [(); 1] { pub unsafe fn check_unit_to_never(x: ()) { // This uses custom MIR to avoid MIR optimizations having removed ZST ops. - // CHECK-NOT: call - // CHECK: call void @llvm.assume(i1 false) - // CHECK-NOT: call + // CHECK: start + // CHECK-NEXT: store i1 true, ptr poison, align 1 + // CHECK-NEXT: ret void mir! { let temp: ZstNever; { @@ -423,7 +422,7 @@ pub unsafe fn check_unit_from_never(x: ZstNever) -> () { // This uses custom MIR to avoid MIR optimizations having removed ZST ops. // CHECK: start - // CHECK-NEXT: call void @llvm.assume(i1 false) + // CHECK-NEXT: store i1 true, ptr poison, align 1 // CHECK-NEXT: ret void mir! { {