From 6a7c730210aa32eb3c95d83f1a1780c470827912 Mon Sep 17 00:00:00 2001 From: Strophox Date: Mon, 30 Sep 2024 20:29:05 +0200 Subject: [PATCH 1/8] start implementing alloc_mark_init_rec, write_zero_if_uninit, write ptr_write_access.rs tests (modifying, initializing memory, provenance) --- .../rustc_const_eval/src/interpret/memory.rs | 20 ++++++ .../src/mir/interpret/allocation.rs | 6 ++ src/tools/miri/src/shims/native_lib.rs | 27 ++++---- .../tests/native-lib/pass/ptr_write_access.rs | 69 +++++++++++++++++++ .../miri/tests/native-lib/ptr_write_access.c | 32 +++++++++ src/tools/miri/tests/ui.rs | 1 + 6 files changed, 142 insertions(+), 13 deletions(-) create mode 100644 src/tools/miri/tests/native-lib/pass/ptr_write_access.rs create mode 100644 src/tools/miri/tests/native-lib/ptr_write_access.c diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 07566e9fda274..2ba36fd8dd89b 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -944,6 +944,26 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { interp_ok(()) } + pub fn alloc_mark_init_rec(&mut self, id: AllocId) -> InterpResult<'tcx> { + let mut reachable = rustc_data_structures::fx::FxHashSet::default(); + let mut todo = vec![id]; + while let Some(id) = todo.pop() { + if reachable.insert(id) { + // This is a new allocation, add the allocation it points to `todo`. + if let Ok((alloc, _)) = self.get_alloc_raw_mut(id) { + let range = AllocRange { start: Size::ZERO, size: Size::from_bytes(alloc.len()) }; + alloc + .write_zero_if_uninit(range) + .map_err(|e| e.to_interp_error(id))?; + todo.extend( + alloc.provenance().provenances().filter_map(|prov| prov.get_alloc_id()), + ); + } + } + } + Ok(()) + } + /// Create a lazy debug printer that prints the given allocation and all allocations it points /// to, recursively. #[must_use] diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 509f2667b35b2..6653398079199 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -643,6 +643,12 @@ impl Allocation Ok(()) } + /// Write zeroes to the unitialized memory within the given memory range. + pub fn write_zero_if_uninit(&mut self, range: AllocRange) -> AllocResult { + self.mark_init(range, true); // TODO: Write zeroes? + Ok(()) + } + /// Remove all provenance in the given memory range. pub fn clear_provenance(&mut self, cx: &impl HasDataLayout, range: AllocRange) -> AllocResult { self.provenance.clear(range, cx)?; diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index e7a4251242e2a..b73479b8f4deb 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -152,9 +152,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if !matches!(arg.layout.backend_repr, BackendRepr::Scalar(_)) { throw_unsup_format!("only scalar argument types are support for native calls") } - libffi_args.push(imm_to_carg(this.read_immediate(arg)?, this)?); + let imm = this.read_immediate(arg)?; + if matches!(arg.layout.ty.kind(), ty::RawPtr(_, rustc_ast::Mutability::Mut)) { + let ptr = this.read_pointer(&imm)?; + let Ok((alloc_id, _size, _prov_extra)) = this.ptr_try_get_alloc_id(ptr, 0) else { + todo!(); // TODO: Handle absolute-address-returned case. + }; + this.alloc_mark_init_rec(alloc_id)?; + } + libffi_args.push(imm_to_carg(imm, this)?); } + // TODO: Directly collect into correct Vec? -- lifetime issues. // Convert them to `libffi::high::Arg` type. let libffi_args = libffi_args .iter() @@ -238,18 +247,10 @@ fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'t ty::Uint(UintTy::U64) => CArg::UInt64(v.to_scalar().to_u64()?), ty::Uint(UintTy::Usize) => CArg::USize(v.to_scalar().to_target_usize(cx)?.try_into().unwrap()), - ty::RawPtr(_, mutability) => { - // Arbitrary mutable pointer accesses are not currently supported in Miri. - if mutability.is_mut() { - throw_unsup_format!( - "unsupported mutable pointer type for native call: {}", - v.layout.ty - ); - } else { - let s = v.to_scalar().to_pointer(cx)?.addr(); - // This relies on the `expose_provenance` in `addr_from_alloc_id`. - CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize())) - } + ty::RawPtr(..) => { + let s = v.to_scalar().to_pointer(cx)?.addr(); + // This relies on the `expose_provenance` in `addr_from_alloc_id`. + CArg::RawPtr(std::ptr::with_exposed_provenance_mut(s.bytes_usize())) } _ => throw_unsup_format!("unsupported argument type for native call: {}", v.layout.ty), }) diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs new file mode 100644 index 0000000000000..b55b7b18b4c4d --- /dev/null +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -0,0 +1,69 @@ +// Only works on Unix targets +//@ignore-target: windows wasm +//@only-on-host + +use std::mem::MaybeUninit; + +fn main() { + test_modify_int(); + + test_init_int(); + + test_init_array(); + + test_swap_ptr(); +} + +fn test_modify_int() { + extern "C" { + fn modify_int(ptr: *mut i32); + } + + let mut x = 1; + unsafe { modify_int(&mut x) }; + + assert_eq!(x, 3); +} + +fn test_init_int() { + extern "C" { + fn init_int(ptr: *mut i32); + } + + let mut x = MaybeUninit::::uninit(); + let x = unsafe { + init_int(x.as_mut_ptr()); + x.assume_init() + }; + + assert_eq!(x, 29); +} + +fn test_init_array() { + extern "C" { + fn init_array(ptr: *mut i32, len: usize, value: i32); + } + + const LEN: usize = 4; + let init_value = 5; + + let mut array = MaybeUninit::<[i32; LEN]>::uninit(); + let array = unsafe { + init_array((*array.as_mut_ptr()).as_mut_ptr(), LEN, init_value); + array.assume_init() + }; + + assert_eq!(array, [init_value; LEN]); +} + +fn test_swap_ptr() { + extern "C" { + fn swap_ptr(x: *mut *const i32, y: *mut *const i32); + } + + let x = 6; + let [mut ptr0, mut ptr1] = [&x, std::ptr::null()]; + unsafe { swap_ptr(&mut ptr0, &mut ptr1); }; + + assert_eq!(unsafe { *ptr1 }, x); +} \ No newline at end of file diff --git a/src/tools/miri/tests/native-lib/ptr_write_access.c b/src/tools/miri/tests/native-lib/ptr_write_access.c new file mode 100644 index 0000000000000..a72b9e0f05c22 --- /dev/null +++ b/src/tools/miri/tests/native-lib/ptr_write_access.c @@ -0,0 +1,32 @@ +#include + +// See comments in build_native_lib() +#define EXPORT __attribute__((visibility("default"))) + +/* Test: test_modify_int */ + +EXPORT void modify_int(int *ptr) { + *ptr += 2; +} + +/* Test: test_init_int */ + +EXPORT void init_int(int *ptr) { + *ptr = 29; +} + +/* Test: test_init_array */ + +EXPORT void init_array(int *array, size_t len, int value) { + for (size_t i = 0; i < len; i++) { + array[i] = value; + } +} + +/* Test: test_swap_ptr */ + +EXPORT void swap_ptr(const int **x, const int **y) { + const int *tmp = *x; + *x = *y; + *y = tmp; +} \ No newline at end of file diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 9553a37c9a82f..9b9542b88a962 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -64,6 +64,7 @@ fn build_native_lib() -> PathBuf { // FIXME: Automate gathering of all relevant C source files in the directory. "tests/native-lib/scalar_arguments.c", "tests/native-lib/ptr_read_access.c", + "tests/native-lib/ptr_write_access.c", // Ensure we notice serious problems in the C code. "-Wall", "-Wextra", From bf5d369ba548fba7ba806478bae087db0d1b97f0 Mon Sep 17 00:00:00 2001 From: Strophox Date: Tue, 19 Nov 2024 06:44:50 +0100 Subject: [PATCH 2/8] intermediate commit (no compile) --- .../src/const_eval/dummy_machine.rs | 4 +- .../src/const_eval/machine.rs | 2 +- .../rustc_const_eval/src/interpret/cast.rs | 2 +- .../rustc_const_eval/src/interpret/machine.rs | 4 +- .../rustc_const_eval/src/interpret/memory.rs | 36 ++++++++++----- .../src/mir/interpret/allocation.rs | 20 +++++++-- .../interpret/allocation/provenance_map.rs | 9 ++++ .../rustc_middle/src/mir/interpret/pointer.rs | 9 ++++ src/tools/miri/src/machine.rs | 7 ++- src/tools/miri/src/shims/native_lib.rs | 18 ++++---- .../tests/native-lib/pass/ptr_write_access.rs | 45 +++++++++++++++---- .../miri/tests/native-lib/ptr_write_access.c | 24 +++++++--- 12 files changed, 134 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs index e49d702127de5..f587865c72994 100644 --- a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs @@ -168,9 +168,9 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine { }) } - fn expose_ptr( + fn expose_provenance( _ecx: &mut InterpCx<'tcx, Self>, - _ptr: interpret::Pointer, + _provenance: Self::Provenance, ) -> interpret::InterpResult<'tcx> { unimplemented!() } diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 19c3195aaa4a3..1c8fab306a9a6 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -586,7 +586,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { } #[inline(always)] - fn expose_ptr(_ecx: &mut InterpCx<'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> { + fn expose_provenance(_ecx: &mut InterpCx<'tcx, Self>, _provenance: Self::Provenance) -> InterpResult<'tcx> { // This is only reachable with -Zunleash-the-miri-inside-of-you. throw_unsup_format!("exposing pointers is not possible at compile-time") } diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index 2d1bb5c955172..1ad3283383bb6 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -238,7 +238,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { let scalar = src.to_scalar(); let ptr = scalar.to_pointer(self)?; match ptr.into_pointer_or_addr() { - Ok(ptr) => M::expose_ptr(self, ptr)?, + Ok(ptr) => M::expose_provenance(self, ptr.provenance)?, Err(_) => {} // Do nothing, exposing an invalid pointer (`None` provenance) is a NOP. }; interp_ok(ImmTy::from_scalar( diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index dbe09d55b2d3c..45b309398db21 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -329,9 +329,9 @@ pub trait Machine<'tcx>: Sized { /// Marks a pointer as exposed, allowing it's provenance /// to be recovered. "Pointer-to-int cast" - fn expose_ptr( + fn expose_provenance( ecx: &mut InterpCx<'tcx, Self>, - ptr: Pointer, + provenance: Self::Provenance, ) -> InterpResult<'tcx>; /// Convert a pointer with provenance into an allocation-offset pair and extra provenance info. diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 2ba36fd8dd89b..f9d68cb93d1d1 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -943,25 +943,37 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { self.get_alloc_raw_mut(id)?.0.mutability = Mutability::Not; interp_ok(()) } - - pub fn alloc_mark_init_rec(&mut self, id: AllocId) -> InterpResult<'tcx> { - let mut reachable = rustc_data_structures::fx::FxHashSet::default(); + + pub fn prepare_for_native_call(&mut self, id: AllocId, initial_prov: M) -> InterpResult<'tcx> { + let mut done = rustc_data_structures::fx::FxHashSet::default(); let mut todo = vec![id]; while let Some(id) = todo.pop() { - if reachable.insert(id) { + if done.insert(id) { // This is a new allocation, add the allocation it points to `todo`. - if let Ok((alloc, _)) = self.get_alloc_raw_mut(id) { - let range = AllocRange { start: Size::ZERO, size: Size::from_bytes(alloc.len()) }; - alloc - .write_zero_if_uninit(range) + let (_size, _align, kind, mutability) = self.get_alloc_info(id); // TODO: Rebasing will give me the mutaility. + + // If there is no data behind this pointer, skip this. + if !matches!(kind, AllocKind::LiveData) { + continue; + } + + let alloc = self.get_alloc_raw(id)?; + for prov in alloc.provenance().provenances() { + M::expose_provenance(self, prov)?; // TODO: Is this right? + if let Some(id) = prov.get_alloc_id() { + todo.push(id); + } + } + + // Prepare for possible write from native code if mutable. + if mutability.is_mut() { + self.get_alloc_raw_mut(id)? + .prepare_for_native_call() .map_err(|e| e.to_interp_error(id))?; - todo.extend( - alloc.provenance().provenances().filter_map(|prov| prov.get_alloc_id()), - ); } } } - Ok(()) + interp_ok(()) } /// Create a lazy debug printer that prints the given allocation and all allocations it points diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 6653398079199..be900ff5d5651 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -643,9 +643,23 @@ impl Allocation Ok(()) } - /// Write zeroes to the unitialized memory within the given memory range. - pub fn write_zero_if_uninit(&mut self, range: AllocRange) -> AllocResult { - self.mark_init(range, true); // TODO: Write zeroes? + /// Initialize all previously uninitialized bytes in the entire allocation, and set + /// provenance of everything to `Wildcard` + pub fn prepare_for_native_call(&mut self) -> AllocResult { + let full_range = AllocRange { start: Size::ZERO, size: Size::from_bytes(self.len()) }; + // Overwrite uninitialized bytes. + for chunk in self.init_mask.range_as_init_chunks(full_range) { + if !chunk.is_init() { + let uninit_bytes = &mut self.bytes[chunk.range().start.bytes_usize()..chunk.range().end.bytes_usize()]; + uninit_bytes.fill(0); + } + } + // Mark everything as initialized now. + self.mark_init(full_range, true); + + // Set provenance of all bytes to wildcard. + self.provenance.write_wildcards(self.len()); + Ok(()) } diff --git a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs index 5c47fc6a399e4..e650ed7de7d7e 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs @@ -195,6 +195,15 @@ impl ProvenanceMap { Ok(()) } + + pub fn write_wildcards(&mut self, _alloc_size: usize) { + // We can only write wildcards in Miri. + assert!(Prov::OFFSET_IS_ADDR, "writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false"); + let _wildcard = Prov::WILDCARD.unwrap(); + + // TODO: This. + + } } /// A partial, owned list of provenance to transfer into another allocation. diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index 1d5afe22573e0..25c7c26ddd974 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -66,6 +66,9 @@ pub trait Provenance: Copy + fmt::Debug + 'static { /// pointer, and implement ptr-to-int transmutation by stripping provenance. const OFFSET_IS_ADDR: bool; + /// If wildcard provenance is implemented, contains the unique, general wildcard provenance variant. + const WILDCARD: Option; + /// Determines how a pointer should be printed. fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result; @@ -168,6 +171,9 @@ impl Provenance for CtfeProvenance { // so ptr-to-int casts are not possible (since we do not know the global physical offset). const OFFSET_IS_ADDR: bool = false; + // `CtfeProvenance` does not implement wildcard provenance. + const WILDCARD: Option = None; + fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Print AllocId. fmt::Debug::fmt(&ptr.provenance.alloc_id(), f)?; // propagates `alternate` flag @@ -197,6 +203,9 @@ impl Provenance for AllocId { // so ptr-to-int casts are not possible (since we do not know the global physical offset). const OFFSET_IS_ADDR: bool = false; + // `AllocId` does not implement wildcard provenance. + const WILDCARD: Option = None; + fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Forward `alternate` flag to `alloc_id` printing. if f.alternate() { diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 9c1951ec87a71..8ad9df0742de1 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -268,6 +268,9 @@ impl interpret::Provenance for Provenance { /// We use absolute addresses in the `offset` of a `StrictPointer`. const OFFSET_IS_ADDR: bool = true; + /// Miri implements wildcard provenance. + const WILDCARD: Option = Some(Provenance::Wildcard); + fn get_alloc_id(self) -> Option { match self { Provenance::Concrete { alloc_id, .. } => Some(alloc_id), @@ -1241,8 +1244,8 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { /// Called on `ptr as usize` casts. /// (Actually computing the resulting `usize` doesn't need machine help, /// that's just `Scalar::try_to_int`.) - fn expose_ptr(ecx: &mut InterpCx<'tcx, Self>, ptr: StrictPointer) -> InterpResult<'tcx> { - match ptr.provenance { + fn expose_provenance(ecx: &mut InterpCx<'tcx, Self>, provenance: Self::Provenance) -> InterpResult<'tcx> { + match provenance { Provenance::Concrete { alloc_id, tag } => ecx.expose_ptr(alloc_id, tag), Provenance::Wildcard => { // No need to do anything for wildcard pointers as diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index b73479b8f4deb..4cd8735bd0976 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -153,17 +153,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("only scalar argument types are support for native calls") } let imm = this.read_immediate(arg)?; - if matches!(arg.layout.ty.kind(), ty::RawPtr(_, rustc_ast::Mutability::Mut)) { - let ptr = this.read_pointer(&imm)?; - let Ok((alloc_id, _size, _prov_extra)) = this.ptr_try_get_alloc_id(ptr, 0) else { - todo!(); // TODO: Handle absolute-address-returned case. + libffi_args.push(imm_to_carg(&imm, this)?); + if matches!(arg.layout.ty.kind(), ty::RawPtr(..)) { + let ptr = imm.to_scalar().to_pointer(self)?; + // We use `get_alloc_id` for its best-effort behaviour with Wildcard provenance. + let Ok(alloc_id) = ptr.provenance.and_then(|prov| prov.get_alloc_id()) else { + // Pointer without provenance may access any memory. + continue; }; - this.alloc_mark_init_rec(alloc_id)?; + this.prepare_for_native_call(alloc_id, ptr.provenance)?; + // TODO: Write tests for (forgetting to) expose: -initial allocation -recursively all allocations -unexposed pointers. } - libffi_args.push(imm_to_carg(imm, this)?); } - // TODO: Directly collect into correct Vec? -- lifetime issues. // Convert them to `libffi::high::Arg` type. let libffi_args = libffi_args .iter() @@ -229,7 +231,7 @@ impl<'a> CArg { /// Extract the scalar value from the result of reading a scalar from the machine, /// and convert it to a `CArg`. -fn imm_to_carg<'tcx>(v: ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> { +fn imm_to_carg<'tcx>(v: &ImmTy<'tcx>, cx: &impl HasDataLayout) -> InterpResult<'tcx, CArg> { interp_ok(match v.layout.ty.kind() { // If the primitive provided can be converted to a type matching the type pattern // then create a `CArg` of this primitive value with the corresponding `CArg` constructor. diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index b55b7b18b4c4d..a5db8e97b42fe 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -12,6 +12,8 @@ fn main() { test_init_array(); test_swap_ptr(); + + test_dangling(); } fn test_modify_int() { @@ -19,10 +21,10 @@ fn test_modify_int() { fn modify_int(ptr: *mut i32); } - let mut x = 1; + let mut x = 11; unsafe { modify_int(&mut x) }; - assert_eq!(x, 3); + assert_eq!(x, 12); } fn test_init_int() { @@ -36,7 +38,7 @@ fn test_init_int() { x.assume_init() }; - assert_eq!(x, 29); + assert_eq!(x, 21); } fn test_init_array() { @@ -45,11 +47,11 @@ fn test_init_array() { } const LEN: usize = 4; - let init_value = 5; + let init_value = 41; let mut array = MaybeUninit::<[i32; LEN]>::uninit(); let array = unsafe { - init_array((*array.as_mut_ptr()).as_mut_ptr(), LEN, init_value); + init_array(array.as_mut_ptr().cast::(), LEN, init_value); array.assume_init() }; @@ -58,12 +60,37 @@ fn test_init_array() { fn test_swap_ptr() { extern "C" { - fn swap_ptr(x: *mut *const i32, y: *mut *const i32); + fn swap_ptr(pptr0: *mut *const i32, pptr1: *mut *const i32); } - let x = 6; - let [mut ptr0, mut ptr1] = [&x, std::ptr::null()]; - unsafe { swap_ptr(&mut ptr0, &mut ptr1); }; + let x = 51; + let mut ptr0 = &x; + let mut ptr1 = std::ptr::null(); + unsafe { swap_ptr(&mut ptr0, &mut ptr1) }; assert_eq!(unsafe { *ptr1 }, x); +} + +fn test_init_static_inner() { + extern "C" { + fn init_static_inner(pptr: *const *mut MaybeUninit); + } + + static mut INNER: MaybeUninit = MaybeUninit::uninit(); + static STATIC: *mut MaybeUninit = &raw mut INNER; + unsafe { init_static_inner(&STATIC) } + + assert_eq!(unsafe { INNER.assume_init() }, 61); +} + +fn test_dangling() { + extern "C" { + fn write_nullptr(pptr: *mut *const i32); + } + + let x = 71; + let mut ptr = &raw const x; + drop(x); + unsafe { write_nullptr(&mut ptr) }; + assert_eq!(ptr) } \ No newline at end of file diff --git a/src/tools/miri/tests/native-lib/ptr_write_access.c b/src/tools/miri/tests/native-lib/ptr_write_access.c index a72b9e0f05c22..9bf2f02cc1881 100644 --- a/src/tools/miri/tests/native-lib/ptr_write_access.c +++ b/src/tools/miri/tests/native-lib/ptr_write_access.c @@ -6,13 +6,13 @@ /* Test: test_modify_int */ EXPORT void modify_int(int *ptr) { - *ptr += 2; + *ptr += 1; } /* Test: test_init_int */ EXPORT void init_int(int *ptr) { - *ptr = 29; + *ptr = 12; } /* Test: test_init_array */ @@ -25,8 +25,20 @@ EXPORT void init_array(int *array, size_t len, int value) { /* Test: test_swap_ptr */ -EXPORT void swap_ptr(const int **x, const int **y) { - const int *tmp = *x; - *x = *y; - *y = tmp; +EXPORT void swap_ptr(const int **pptr0, const int **pptr1) { + const int *tmp = *pptr0; + *pptr0 = *pptr1; + *pptr1 = tmp; +} + +/* Test: test_init_static_inner */ + +EXPORT void init_static_inner(int **const pptr) { + **pptr = 1234; +} + +/* Test: test_dangling */ + +EXPORT void write_nullptr(const int **pptr) { + *pptr = NULL; } \ No newline at end of file From 8b90deea99669947c7fb4cb8a53451601a7414f0 Mon Sep 17 00:00:00 2001 From: Strophox Date: Tue, 19 Nov 2024 15:11:37 +0100 Subject: [PATCH 3/8] fix current compile errors, fmt --- .../src/const_eval/machine.rs | 10 ++++---- .../rustc_const_eval/src/interpret/memory.rs | 23 +++++++++++++------ .../src/mir/interpret/allocation.rs | 9 ++++---- .../interpret/allocation/provenance_map.rs | 10 ++++---- src/tools/miri/src/shims/native_lib.rs | 11 +++++---- .../tests/native-lib/pass/ptr_write_access.rs | 6 +++-- 6 files changed, 44 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 1c8fab306a9a6..bc6cacfdafc0a 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -21,9 +21,8 @@ use crate::errors::{LongRunning, LongRunningWarn}; use crate::fluent_generated as fluent; use crate::interpret::{ self, AllocId, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame, GlobalAlloc, ImmTy, - InterpCx, InterpResult, MPlaceTy, OpTy, Pointer, RangeSet, Scalar, compile_time_machine, - interp_ok, throw_exhaust, throw_inval, throw_ub, throw_ub_custom, throw_unsup, - throw_unsup_format, + InterpCx, InterpResult, MPlaceTy, OpTy, RangeSet, Scalar, compile_time_machine, interp_ok, + throw_exhaust, throw_inval, throw_ub, throw_ub_custom, throw_unsup, throw_unsup_format, }; /// When hitting this many interpreted terminators we emit a deny by default lint @@ -586,7 +585,10 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { } #[inline(always)] - fn expose_provenance(_ecx: &mut InterpCx<'tcx, Self>, _provenance: Self::Provenance) -> InterpResult<'tcx> { + fn expose_provenance( + _ecx: &mut InterpCx<'tcx, Self>, + _provenance: Self::Provenance, + ) -> InterpResult<'tcx> { // This is only reachable with -Zunleash-the-miri-inside-of-you. throw_unsup_format!("exposing pointers is not possible at compile-time") } diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index f9d68cb93d1d1..48e53c584b774 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -943,32 +943,41 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { self.get_alloc_raw_mut(id)?.0.mutability = Mutability::Not; interp_ok(()) } - - pub fn prepare_for_native_call(&mut self, id: AllocId, initial_prov: M) -> InterpResult<'tcx> { + + pub fn prepare_for_native_call( + &mut self, + id: AllocId, + initial_prov: M::Provenance, + ) -> InterpResult<'tcx> { + // Expose provenance of the root allocation. + M::expose_provenance(self, initial_prov)?; // TODO: Is this the right way to expose provenance? + let mut done = rustc_data_structures::fx::FxHashSet::default(); let mut todo = vec![id]; while let Some(id) = todo.pop() { if done.insert(id) { // This is a new allocation, add the allocation it points to `todo`. - let (_size, _align, kind, mutability) = self.get_alloc_info(id); // TODO: Rebasing will give me the mutaility. + let info = self.get_alloc_info(id); // If there is no data behind this pointer, skip this. - if !matches!(kind, AllocKind::LiveData) { + if !matches!(info.kind, AllocKind::LiveData) { continue; } let alloc = self.get_alloc_raw(id)?; for prov in alloc.provenance().provenances() { - M::expose_provenance(self, prov)?; // TODO: Is this right? + //M::expose_provenance(self, prov)?; // TODO: Is this the right way to expose provenance? + mutable borrow here gives issues due to provenances iterator lifetime... if let Some(id) = prov.get_alloc_id() { todo.push(id); } } // Prepare for possible write from native code if mutable. - if mutability.is_mut() { + if info.mutbl.is_mut() { + let tcx = self.tcx; self.get_alloc_raw_mut(id)? - .prepare_for_native_call() + .0 + .prepare_for_native_call(&tcx) .map_err(|e| e.to_interp_error(id))?; } } diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index be900ff5d5651..b9d6ba09244fe 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -645,12 +645,13 @@ impl Allocation /// Initialize all previously uninitialized bytes in the entire allocation, and set /// provenance of everything to `Wildcard` - pub fn prepare_for_native_call(&mut self) -> AllocResult { + pub fn prepare_for_native_call(&mut self, cx: &impl HasDataLayout) -> AllocResult { let full_range = AllocRange { start: Size::ZERO, size: Size::from_bytes(self.len()) }; // Overwrite uninitialized bytes. for chunk in self.init_mask.range_as_init_chunks(full_range) { if !chunk.is_init() { - let uninit_bytes = &mut self.bytes[chunk.range().start.bytes_usize()..chunk.range().end.bytes_usize()]; + let uninit_bytes = &mut self.bytes + [chunk.range().start.bytes_usize()..chunk.range().end.bytes_usize()]; uninit_bytes.fill(0); } } @@ -658,8 +659,8 @@ impl Allocation self.mark_init(full_range, true); // Set provenance of all bytes to wildcard. - self.provenance.write_wildcards(self.len()); - + self.provenance.write_wildcards(self.len(), cx); + Ok(()) } diff --git a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs index e650ed7de7d7e..9d0863b3a739b 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs @@ -196,13 +196,15 @@ impl ProvenanceMap { Ok(()) } - pub fn write_wildcards(&mut self, _alloc_size: usize) { + pub fn write_wildcards(&mut self, _alloc_size: usize, _cx: &impl HasDataLayout) { // We can only write wildcards in Miri. - assert!(Prov::OFFSET_IS_ADDR, "writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false"); + assert!( + Prov::OFFSET_IS_ADDR, + "writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false" + ); let _wildcard = Prov::WILDCARD.unwrap(); - - // TODO: This. + // TODO: Write wildcards into `self` in intervals of `cx.data_layout().pointer_size` + remaining bytes? } } diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index 4cd8735bd0976..1209d106e00f8 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -155,14 +155,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let imm = this.read_immediate(arg)?; libffi_args.push(imm_to_carg(&imm, this)?); if matches!(arg.layout.ty.kind(), ty::RawPtr(..)) { - let ptr = imm.to_scalar().to_pointer(self)?; + let ptr = imm.to_scalar().to_pointer(this)?; + let Some(prov) = ptr.provenance else { + // Pointer without provenance may access any memory. + continue; + }; // We use `get_alloc_id` for its best-effort behaviour with Wildcard provenance. - let Ok(alloc_id) = ptr.provenance.and_then(|prov| prov.get_alloc_id()) else { + let Some(alloc_id) = prov.get_alloc_id() else { // Pointer without provenance may access any memory. continue; }; - this.prepare_for_native_call(alloc_id, ptr.provenance)?; - // TODO: Write tests for (forgetting to) expose: -initial allocation -recursively all allocations -unexposed pointers. + this.prepare_for_native_call(alloc_id, prov)?; } } diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index a5db8e97b42fe..92852dcff7cda 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -92,5 +92,7 @@ fn test_dangling() { let mut ptr = &raw const x; drop(x); unsafe { write_nullptr(&mut ptr) }; - assert_eq!(ptr) -} \ No newline at end of file + assert_eq!(ptr, std::ptr::null()); +} + +// TODO: Write tests for (forgetting to) expose: -initial allocation -recursively all allocations -unexposed pointers. \ No newline at end of file From e383a47b415a567f7a51ee9ab4e2c0d42501e031 Mon Sep 17 00:00:00 2001 From: Lucas W <47983521+Strophox@users.noreply.github.com> Date: Fri, 22 Nov 2024 13:28:17 +0100 Subject: [PATCH 4/8] Add FIXME in src/tools/miri/src/shims/native_lib.rs Co-authored-by: Ralf Jung --- src/tools/miri/src/shims/native_lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index 1209d106e00f8..007b7e0af53bf 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -168,6 +168,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.prepare_for_native_call(alloc_id, prov)?; } } + + // FIXME: In the future, we should also call `prepare_for_native_call` on all previously + // exposed allocations, since C may access any of them. // Convert them to `libffi::high::Arg` type. let libffi_args = libffi_args From 2826ff4a35572128c91f69ffdad1a087fb79e15a Mon Sep 17 00:00:00 2001 From: Strophox Date: Fri, 22 Nov 2024 13:33:00 +0100 Subject: [PATCH 5/8] first code review fixes --- .../rustc_const_eval/src/interpret/memory.rs | 45 ++++++++++--------- .../src/mir/interpret/allocation.rs | 3 +- src/tools/miri/src/shims/native_lib.rs | 5 ++- .../tests/native-lib/pass/ptr_write_access.rs | 4 +- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 48e53c584b774..87c6a9ebc6d2a 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -950,36 +950,37 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { initial_prov: M::Provenance, ) -> InterpResult<'tcx> { // Expose provenance of the root allocation. - M::expose_provenance(self, initial_prov)?; // TODO: Is this the right way to expose provenance? + M::expose_provenance(self, initial_prov)?; let mut done = rustc_data_structures::fx::FxHashSet::default(); let mut todo = vec![id]; while let Some(id) = todo.pop() { - if done.insert(id) { - // This is a new allocation, add the allocation it points to `todo`. - let info = self.get_alloc_info(id); + if !done.insert(id) { + continue; + } + // This is a new allocation, add the allocations it points to to `todo`. + let info = self.get_alloc_info(id); - // If there is no data behind this pointer, skip this. - if !matches!(info.kind, AllocKind::LiveData) { - continue; - } + // If there is no data behind this pointer, skip this. + if !matches!(info.kind, AllocKind::LiveData) { + continue; + } - let alloc = self.get_alloc_raw(id)?; - for prov in alloc.provenance().provenances() { - //M::expose_provenance(self, prov)?; // TODO: Is this the right way to expose provenance? + mutable borrow here gives issues due to provenances iterator lifetime... - if let Some(id) = prov.get_alloc_id() { - todo.push(id); - } + let alloc = self.get_alloc_raw(id)?; + for prov in alloc.provenance().provenances() { + //M::expose_provenance(self, prov)?; // TODO: mutable borrow here gives issues due to provenances iterator lifetime... + if let Some(id) = prov.get_alloc_id() { + todo.push(id); } + } - // Prepare for possible write from native code if mutable. - if info.mutbl.is_mut() { - let tcx = self.tcx; - self.get_alloc_raw_mut(id)? - .0 - .prepare_for_native_call(&tcx) - .map_err(|e| e.to_interp_error(id))?; - } + // Prepare for possible write from native code if mutable. + if info.mutbl.is_mut() { + let tcx = self.tcx; + self.get_alloc_raw_mut(id)? + .0 + .prepare_for_native_call(&tcx) + .map_err(|e| e.to_interp_error(id))?; } } interp_ok(()) diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index b9d6ba09244fe..475aac368cf39 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -644,7 +644,8 @@ impl Allocation } /// Initialize all previously uninitialized bytes in the entire allocation, and set - /// provenance of everything to `Wildcard` + /// provenance of everything to `Wildcard`. Before calling this, make sure all + /// provenance in this allocation is exposed! pub fn prepare_for_native_call(&mut self, cx: &impl HasDataLayout) -> AllocResult { let full_range = AllocRange { start: Size::ZERO, size: Size::from_bytes(self.len()) }; // Overwrite uninitialized bytes. diff --git a/src/tools/miri/src/shims/native_lib.rs b/src/tools/miri/src/shims/native_lib.rs index 007b7e0af53bf..da4ae77f2352d 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -154,15 +154,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } let imm = this.read_immediate(arg)?; libffi_args.push(imm_to_carg(&imm, this)?); + // If we are passing a pointer, prepare the memory it points to. if matches!(arg.layout.ty.kind(), ty::RawPtr(..)) { let ptr = imm.to_scalar().to_pointer(this)?; let Some(prov) = ptr.provenance else { - // Pointer without provenance may access any memory. + // Pointer without provenance may not access any memory. continue; }; // We use `get_alloc_id` for its best-effort behaviour with Wildcard provenance. let Some(alloc_id) = prov.get_alloc_id() else { - // Pointer without provenance may access any memory. + // Wildcard pointer, whatever it points to must be already exposed. continue; }; this.prepare_for_native_call(alloc_id, prov)?; diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index 92852dcff7cda..2edf524a80eaf 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -88,8 +88,8 @@ fn test_dangling() { fn write_nullptr(pptr: *mut *const i32); } - let x = 71; - let mut ptr = &raw const x; + let x = Box::new(71); + let mut ptr = x.into_raw(); drop(x); unsafe { write_nullptr(&mut ptr) }; assert_eq!(ptr, std::ptr::null()); From 26602ebadf24e930dcc7b36b2a34b6a73ee93c5a Mon Sep 17 00:00:00 2001 From: Strophox Date: Fri, 22 Nov 2024 16:28:35 +0100 Subject: [PATCH 6/8] attempt write_wildcards in provenance_map, attempt to fix test_dangling --- .../mir/interpret/allocation/provenance_map.rs | 17 ++++++++++++++--- .../tests/native-lib/pass/ptr_write_access.rs | 4 ++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs index 9d0863b3a739b..cb9596e2bb5b8 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs @@ -196,15 +196,26 @@ impl ProvenanceMap { Ok(()) } - pub fn write_wildcards(&mut self, _alloc_size: usize, _cx: &impl HasDataLayout) { + pub fn write_wildcards(&mut self, alloc_size: usize, cx: &impl HasDataLayout) { // We can only write wildcards in Miri. assert!( Prov::OFFSET_IS_ADDR, "writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false" ); - let _wildcard = Prov::WILDCARD.unwrap(); + let wildcard = Prov::WILDCARD.unwrap(); + let ptr_size = cx.data_layout().pointer_size; - // TODO: Write wildcards into `self` in intervals of `cx.data_layout().pointer_size` + remaining bytes? + // Write wildcards in intervals of pointer size. + let end = Size::from_bytes(alloc_size / ptr_size.bytes_usize()); + for offset in Size::ZERO..end { + self.ptrs.insert(offset, wildcard); + } + // Write wildcards into the remaining bytes. + let last = Size::from_bytes(alloc_size); + let bytes = self.bytes.get_or_insert_with(Box::default); + for offset in end..last { + bytes.insert(offset, wildcard); + } // TODO: Is the above even remotely correct? } } diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index 2edf524a80eaf..11516b4592f04 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -88,8 +88,8 @@ fn test_dangling() { fn write_nullptr(pptr: *mut *const i32); } - let x = Box::new(71); - let mut ptr = x.into_raw(); + let x = vec![71]; + let mut ptr = x.as_ptr(); drop(x); unsafe { write_nullptr(&mut ptr) }; assert_eq!(ptr, std::ptr::null()); From 5c27b83425bbb51fed23323c7433fdc0aaecf6ea Mon Sep 17 00:00:00 2001 From: Strophox Date: Sat, 23 Nov 2024 16:14:15 +0100 Subject: [PATCH 7/8] fix/tweak tests --- .../tests/native-lib/pass/ptr_write_access.rs | 41 +++++++++++-------- .../miri/tests/native-lib/ptr_write_access.c | 20 +++++---- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index 11516b4592f04..f46a3930328ed 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -13,6 +13,8 @@ fn main() { test_swap_ptr(); + test_init_interior_mutable(); + test_dangling(); } @@ -43,19 +45,18 @@ fn test_init_int() { fn test_init_array() { extern "C" { - fn init_array(ptr: *mut i32, len: usize, value: i32); + fn init_array(ptr: *mut i32, len: usize); } - const LEN: usize = 4; - let init_value = 41; + const LEN: usize = 3; let mut array = MaybeUninit::<[i32; LEN]>::uninit(); let array = unsafe { - init_array(array.as_mut_ptr().cast::(), LEN, init_value); + init_array(array.as_mut_ptr().cast::(), LEN); array.assume_init() }; - assert_eq!(array, [init_value; LEN]); + assert_eq!(array, [31; LEN]); } fn test_swap_ptr() { @@ -63,36 +64,42 @@ fn test_swap_ptr() { fn swap_ptr(pptr0: *mut *const i32, pptr1: *mut *const i32); } - let x = 51; - let mut ptr0 = &x; + let x = 41; + let mut ptr0 = &raw const x; let mut ptr1 = std::ptr::null(); unsafe { swap_ptr(&mut ptr0, &mut ptr1) }; assert_eq!(unsafe { *ptr1 }, x); } -fn test_init_static_inner() { +fn test_init_interior_mutable() { extern "C" { - fn init_static_inner(pptr: *const *mut MaybeUninit); + fn init_interior_mutable(pptr: *const UnsafeInterior); } - static mut INNER: MaybeUninit = MaybeUninit::uninit(); - static STATIC: *mut MaybeUninit = &raw mut INNER; - unsafe { init_static_inner(&STATIC) } + #[repr(C)] + struct UnsafeInterior { + mut_ptr: *mut i32 + } + unsafe impl Sync for UnsafeInterior {} + + let mut x = MaybeUninit::::uninit(); + let unsafe_interior = UnsafeInterior { mut_ptr: x.as_mut_ptr() }; + unsafe { init_interior_mutable(&unsafe_interior) }; - assert_eq!(unsafe { INNER.assume_init() }, 61); + assert_eq!(unsafe { x.assume_init() }, 51); } fn test_dangling() { extern "C" { - fn write_nullptr(pptr: *mut *const i32); + fn overwrite_ptr(pptr: *mut *const i32); } - let x = vec![71]; + let x = vec![61]; let mut ptr = x.as_ptr(); drop(x); - unsafe { write_nullptr(&mut ptr) }; + unsafe { overwrite_ptr(&mut ptr) }; assert_eq!(ptr, std::ptr::null()); } -// TODO: Write tests for (forgetting to) expose: -initial allocation -recursively all allocations -unexposed pointers. \ No newline at end of file +// TODO: Write tests for (forgetting to) expose: -initial allocation -recursively all allocations -unexposed pointers. diff --git a/src/tools/miri/tests/native-lib/ptr_write_access.c b/src/tools/miri/tests/native-lib/ptr_write_access.c index 9bf2f02cc1881..d424e7b6059ed 100644 --- a/src/tools/miri/tests/native-lib/ptr_write_access.c +++ b/src/tools/miri/tests/native-lib/ptr_write_access.c @@ -12,14 +12,14 @@ EXPORT void modify_int(int *ptr) { /* Test: test_init_int */ EXPORT void init_int(int *ptr) { - *ptr = 12; + *ptr = 21; } /* Test: test_init_array */ -EXPORT void init_array(int *array, size_t len, int value) { +EXPORT void init_array(int *array, size_t len) { for (size_t i = 0; i < len; i++) { - array[i] = value; + array[i] = 31; } } @@ -31,14 +31,18 @@ EXPORT void swap_ptr(const int **pptr0, const int **pptr1) { *pptr1 = tmp; } -/* Test: test_init_static_inner */ +/* Test: test_init_interior_mutable */ -EXPORT void init_static_inner(int **const pptr) { - **pptr = 1234; +typedef struct UnsafeInterior { + int *mut_ptr; +} UnsafeInterior; + +EXPORT void init_interior_mutable(const UnsafeInterior *u_ptr) { + *(u_ptr->mut_ptr) = 51; } /* Test: test_dangling */ -EXPORT void write_nullptr(const int **pptr) { +EXPORT void overwrite_ptr(const int **pptr) { *pptr = NULL; -} \ No newline at end of file +} From f86ba13900468e12ccb4d69a9cf300d8e08648da Mon Sep 17 00:00:00 2001 From: Strophox Date: Sat, 23 Nov 2024 16:41:44 +0100 Subject: [PATCH 8/8] adapt signatures to take &self and use interior mutability, expose provenances --- compiler/rustc_const_eval/src/const_eval/dummy_machine.rs | 2 +- compiler/rustc_const_eval/src/const_eval/machine.rs | 2 +- compiler/rustc_const_eval/src/interpret/machine.rs | 2 +- compiler/rustc_const_eval/src/interpret/memory.rs | 2 +- src/tools/miri/src/alloc_addresses/mod.rs | 7 +++---- src/tools/miri/src/borrow_tracker/mod.rs | 4 ++-- src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs | 4 ++-- src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs | 4 ++-- src/tools/miri/src/machine.rs | 2 +- 9 files changed, 14 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs index f587865c72994..817acfcca74bc 100644 --- a/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs @@ -169,7 +169,7 @@ impl<'tcx> interpret::Machine<'tcx> for DummyMachine { } fn expose_provenance( - _ecx: &mut InterpCx<'tcx, Self>, + _ecx: &InterpCx<'tcx, Self>, _provenance: Self::Provenance, ) -> interpret::InterpResult<'tcx> { unimplemented!() diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index bc6cacfdafc0a..e5534da8fea9e 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -586,7 +586,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { #[inline(always)] fn expose_provenance( - _ecx: &mut InterpCx<'tcx, Self>, + _ecx: &InterpCx<'tcx, Self>, _provenance: Self::Provenance, ) -> InterpResult<'tcx> { // This is only reachable with -Zunleash-the-miri-inside-of-you. diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 45b309398db21..6c480cbcf38a6 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -330,7 +330,7 @@ pub trait Machine<'tcx>: Sized { /// Marks a pointer as exposed, allowing it's provenance /// to be recovered. "Pointer-to-int cast" fn expose_provenance( - ecx: &mut InterpCx<'tcx, Self>, + ecx: &InterpCx<'tcx, Self>, provenance: Self::Provenance, ) -> InterpResult<'tcx>; diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 87c6a9ebc6d2a..67717d635212b 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -968,7 +968,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { let alloc = self.get_alloc_raw(id)?; for prov in alloc.provenance().provenances() { - //M::expose_provenance(self, prov)?; // TODO: mutable borrow here gives issues due to provenances iterator lifetime... + M::expose_provenance(self, prov)?; if let Some(id) = prov.get_alloc_id() { todo.push(id); } diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index fe7d8db245b94..196324bb79d44 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -286,9 +286,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { - fn expose_ptr(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); - let global_state = this.machine.alloc_addresses.get_mut(); + fn expose_ptr(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); + let mut global_state = this.machine.alloc_addresses.borrow_mut(); // In strict mode, we don't need this, so we can save some cycles by not tracking it. if global_state.provenance_mode == ProvenanceMode::Strict { return interp_ok(()); @@ -299,7 +299,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return interp_ok(()); } trace!("Exposing allocation id {alloc_id:?}"); - let global_state = this.machine.alloc_addresses.get_mut(); global_state.exposed.insert(alloc_id); if this.machine.borrow_tracker.is_some() { this.expose_tag(alloc_id, tag)?; diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 4883613dea504..9808102f4ba66 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -302,8 +302,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } - fn expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); + fn expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.sb_expose_tag(alloc_id, tag), diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 745316913d992..355ed1e0f31b7 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -1011,8 +1011,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Mark the given tag as exposed. It was found on a pointer with the given AllocId. - fn sb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); + fn sb_expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); // Function pointers and dead objects don't have an alloc_extra so we ignore them. // This is okay because accessing them is UB anyway, no need for any Stacked Borrows checks. diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 255a3578aaec3..17329e7b4b5be 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -532,8 +532,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Mark the given tag as exposed. It was found on a pointer with the given AllocId. - fn tb_expose_tag(&mut self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { - let this = self.eval_context_mut(); + fn tb_expose_tag(&self, alloc_id: AllocId, tag: BorTag) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); // Function pointers and dead objects don't have an alloc_extra so we ignore them. // This is okay because accessing them is UB anyway, no need for any Tree Borrows checks. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 8ad9df0742de1..b6f07446be7d7 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1244,7 +1244,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { /// Called on `ptr as usize` casts. /// (Actually computing the resulting `usize` doesn't need machine help, /// that's just `Scalar::try_to_int`.) - fn expose_provenance(ecx: &mut InterpCx<'tcx, Self>, provenance: Self::Provenance) -> InterpResult<'tcx> { + fn expose_provenance(ecx: &InterpCx<'tcx, Self>, provenance: Self::Provenance) -> InterpResult<'tcx> { match provenance { Provenance::Concrete { alloc_id, tag } => ecx.expose_ptr(alloc_id, tag), Provenance::Wildcard => {