diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 4ae85af897527..28bd9b0126b56 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -171,6 +171,42 @@ fn call_simple_intrinsic<'ll, 'tcx>( )) } +impl<'ll, 'tcx> Builder<'_, 'll, 'tcx> { + fn black_box(&mut self, result: PlaceRef<'tcx, &'ll Value>, span: Span) { + let result_val_span = [result.val.llval]; + // We need to "use" the argument in some way LLVM can't introspect, and on + // targets that support it we can typically leverage inline assembly to do + // this. LLVM's interpretation of inline assembly is that it's, well, a black + // box. This isn't the greatest implementation since it probably deoptimizes + // more than we want, but it's so far good enough. + // + // For zero-sized types, the location pointed to by the result may be + // uninitialized. Do not "use" the result in this case; instead just clobber + // the memory. + let (constraint, inputs): (&str, &[_]) = if result.layout.is_zst() { + ("~{memory}", &[]) + } else { + ("r,~{memory}", &result_val_span) + }; + crate::asm::inline_asm_call( + self, + "", + constraint, + inputs, + self.type_void(), + &[], + true, + false, + llvm::AsmDialect::Att, + &[span], + false, + None, + None, + ) + .unwrap_or_else(|| bug!("failed to generate inline asm call for `black_box`")); + } +} + impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { fn codegen_intrinsic_call( &mut self, @@ -356,9 +392,10 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { let ptr = args[0].immediate(); let abi_align = result_layout.align.abi; let ptr_align = if name == sym::volatile_load { abi_align } else { Align::ONE }; + let need_black_box = llvm_version < (23, 0, 0); if result_layout.is_zst() { return IntrinsicResult::Operand(OperandValue::ZeroSized); - } else if let BackendRepr::Scalar(scalar) = result_layout.backend_repr { + } else if let BackendRepr::Scalar(scalar) = result_layout.backend_repr && !need_black_box { let load = self.volatile_load(self.type_from_scalar(scalar), ptr, ptr_align); self.to_immediate_scalar(load, scalar) } else { @@ -376,6 +413,13 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { }; let llval = self.volatile_load(llty, ptr, ptr_align); self.store(llval, temp.val.llval, abi_align); + if need_black_box { + // LLVM up until v22 considers volatile reads `willreturn` and hence can + // move UB from further down up across this read. To prevent that, insert an + // inline asm block that, as far as LLVM is concerned, might not terminate, + // and hence should prevent such reordering. + self.black_box(temp, span); + } return if result_place.is_none() { IntrinsicResult::Operand(self.load_operand(temp).val) } else { @@ -605,41 +649,13 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> { sym::black_box => { let result = PlaceRef { + // This `unwrap` is justified by `intrinsic_call_expects_place_always` declaring + // this intrinsic as always needing a return place. val: result_place.unwrap(), layout: result_layout, }; args[0].val.store(self, result); - let result_val_span = [result.val.llval]; - // We need to "use" the argument in some way LLVM can't introspect, and on - // targets that support it we can typically leverage inline assembly to do - // this. LLVM's interpretation of inline assembly is that it's, well, a black - // box. This isn't the greatest implementation since it probably deoptimizes - // more than we want, but it's so far good enough. - // - // For zero-sized types, the location pointed to by the result may be - // uninitialized. Do not "use" the result in this case; instead just clobber - // the memory. - let (constraint, inputs): (&str, &[_]) = if result.layout.is_zst() { - ("~{memory}", &[]) - } else { - ("r,~{memory}", &result_val_span) - }; - crate::asm::inline_asm_call( - self, - "", - constraint, - inputs, - self.type_void(), - &[], - true, - false, - llvm::AsmDialect::Att, - &[span], - false, - None, - None, - ) - .unwrap_or_else(|| bug!("failed to generate inline asm call for `black_box`")); + self.black_box(result, span); // We have copied the value to `result` already. return IntrinsicResult::WroteIntoPlace; diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 41f4b837efaea..35b7485de7b19 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -2078,10 +2078,10 @@ pub const unsafe fn write_unaligned(dst: *mut T, src: T) { /// semantics associated to their manipulation, and cannot be used as general purpose memory. /// Here, any address value is possible, including 0 and [`usize::MAX`], so long as the semantics /// of such a read are well-defined by the target hardware. The provenance of the pointer is -/// irrelevant, and it can be created with [`without_provenance`]. The access must not trap. It -/// can cause side-effects, but those must not affect Rust-allocated memory in any way. This -/// access is still not considered [atomic], and as such it cannot be used for inter-thread -/// synchronization. +/// irrelevant, and it can be created with [`without_provenance`]. The access is allowed to trap, +/// which must immediately abort the process. It can also cause other side-effects, but those +/// must not affect Rust-allocated memory in any way. This access is still not considered +/// [atomic], and as such it cannot be used for inter-thread synchronization. /// /// Note that volatile memory operations where T is a zero-sized type are noops and may be ignored. /// @@ -2116,9 +2116,8 @@ pub const unsafe fn write_unaligned(dst: *mut T, src: T) { /// Behavior is undefined if any of the following conditions are violated: /// /// * `src` must be either [valid] for reads, or `T` must be a ZST, or `src` must point to memory -/// outside of all Rust allocations and reading from that memory must: -/// - not trap, and -/// - not cause any memory inside a Rust allocation to be modified. +/// outside of all Rust allocations and reading from that memory must not cause any memory inside +/// a Rust allocation to be modified. /// /// * `src` must be properly aligned. /// @@ -2184,10 +2183,10 @@ pub const unsafe fn read_volatile(src: *const T) -> T { /// semantics associated to their manipulation, and cannot be used as general purpose memory. /// Here, any address value is possible, including 0 and [`usize::MAX`], so long as the semantics /// of such a write are well-defined by the target hardware. The provenance of the pointer is -/// irrelevant, and it can be created with [`without_provenance`]. The access must not trap. It -/// can cause side-effects, but those must not affect Rust-allocated memory in any way. This -/// access is still not considered [atomic], and as such it cannot be used for inter-thread -/// synchronization. +/// irrelevant, and it can be created with [`without_provenance`]. The access is allowed to trap, +/// which must immediately abort the process. It can also cause side-effects, but those must not +/// affect Rust-allocated memory in any way. This access is still not considered [atomic], and as +/// such it cannot be used for inter-thread synchronization. /// /// Note that volatile memory operations on zero-sized types (e.g., if a zero-sized type is passed /// to `write_volatile`) are noops and may be ignored. @@ -2223,9 +2222,8 @@ pub const unsafe fn read_volatile(src: *const T) -> T { /// Behavior is undefined if any of the following conditions are violated: /// /// * `dst` must be either [valid] for writes, or `T` must be a ZST, or `dst` must point to memory -/// outside of all Rust allocations and writing to that memory must: -/// - not trap, and -/// - not cause any memory inside a Rust allocation to be modified. +/// outside of all Rust allocations and writing to that memory must not cause any memory inside a +/// Rust allocation to be modified. /// /// * `dst` must be properly aligned. /// diff --git a/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs b/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs index e355602f2c3f1..07e92fbfde6dc 100644 --- a/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs +++ b/tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs @@ -8,6 +8,7 @@ //@ [strong] compile-flags: -Z stack-protector=strong //@ [none] compile-flags: -Z stack-protector=none //@ compile-flags: -C opt-level=2 -Z merge-functions=disabled +//@ min-llvm-version: 23 #![crate_type = "lib"] #![allow(internal_features)] diff --git a/tests/codegen-llvm/intrinsics/volatile.rs b/tests/codegen-llvm/intrinsics/volatile.rs index 42ffd196f3d86..a93348703cee9 100644 --- a/tests/codegen-llvm/intrinsics/volatile.rs +++ b/tests/codegen-llvm/intrinsics/volatile.rs @@ -1,4 +1,5 @@ //@ compile-flags: -C no-prepopulate-passes +//@ min-llvm-version: 23 #![crate_type = "lib"] #![feature(core_intrinsics)]