rust-lang/rust · #160564

volatile: allow accesses to non-AM memory to trap

RalfJung · merged Sep 3, 20267 files · 71 + / 50
compiler/rustc_codegen_llvm/src/intrinsic.rs50 + / 32
@@ -138,6 +138,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,@@ -327,9 +363,12 @@ 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 {@@ -344,6 +383,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 {@@ -608,39 +654,11 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {             }              sym::black_box => {+                // This `unwrap` is justified by `intrinsic_call_expects_place_always` declaring+                // this intrinsic as always needing a return place.                 let result = PlaceRef { 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;
library/core/src/ptr/mod.rs12 + / 14
@@ -2082,10 +2082,10 @@ pub const unsafe fn write_unaligned<T>(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. ///@@ -2120,9 +2120,8 @@ pub const unsafe fn write_unaligned<T>(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. ///@@ -2189,10 +2188,10 @@ pub const unsafe fn read_volatile<T>(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_mut`]. 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_mut`]. 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 on zero-sized types (e.g., if a zero-sized type is passed /// to `write_volatile`) are noops and may be ignored.@@ -2228,9 +2227,8 @@ pub const unsafe fn read_volatile<T>(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. ///
library/core/src/sync/atomic_load_volatile.md3 + / 2
@@ -17,8 +17,9 @@ are two cases of usage that need to be distinguished:   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`][crate::ptr::without_provenance]. The access must not trap. It-  can cause side-effects, but those must not affect Rust-allocated memory in any way.+  created with [`without_provenance`][crate::ptr::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.  In both cases, the access is also considered atomic with the given `order`. This allows synchronization with other threads or devices that share memory with this program.
library/core/src/sync/atomic_store_volatile.md3 + / 2
@@ -16,8 +16,9 @@ usage that need to be distinguished:   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_mut`][crate::ptr::without_provenance_mut]. The access must not-  trap. It can cause side-effects, but those must not affect Rust-allocated memory in any way.+  created with [`without_provenance_mut`][crate::ptr::without_provenance_mut]. 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.  In both cases, the access is also considered atomic with the given `order`. This allows synchronization with other threads or devices that share memory with this program.
tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-2.rs1 + / 0
@@ -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)]
tests/codegen-llvm/i128-x86-align.rs1 + / 0
@@ -1,5 +1,6 @@ //@ only-x86_64 //@ compile-flags: -Copt-level=3 -C no-prepopulate-passes --crate-type=lib+//@ min-llvm-version: 23  // On LLVM 17 and earlier LLVM's own data layout specifies that i128 has 8 byte alignment, // while rustc wants it to have 16 byte alignment. This test checks that we handle this
tests/codegen-llvm/intrinsics/volatile.rs1 + / 0
@@ -1,4 +1,5 @@ //@ compile-flags: -C no-prepopulate-passes+//@ min-llvm-version: 23  #![crate_type = "lib"] #![feature(core_intrinsics)]