From 712ceaba35967f4ebc272634f417b5f5400601f8 Mon Sep 17 00:00:00 2001 From: Strophox Date: Mon, 30 Sep 2024 20:29:05 +0200 Subject: [PATCH] extend Miri to correctly pass mutable pointers through FFI Co-authored-by: Ralf Jung --- .../src/const_eval/dummy_machine.rs | 6 +- .../src/const_eval/machine.rs | 10 +- .../rustc_const_eval/src/interpret/cast.rs | 2 +- .../rustc_const_eval/src/interpret/machine.rs | 8 +- .../rustc_const_eval/src/interpret/memory.rs | 46 ++++ .../src/mir/interpret/allocation.rs | 22 ++ .../interpret/allocation/provenance_map.rs | 19 ++ .../rustc_middle/src/mir/interpret/pointer.rs | 9 + src/tools/miri/src/alloc_addresses/mod.rs | 10 +- src/tools/miri/src/borrow_tracker/mod.rs | 4 +- .../src/borrow_tracker/stacked_borrows/mod.rs | 4 +- .../src/borrow_tracker/tree_borrows/mod.rs | 4 +- src/tools/miri/src/machine.rs | 7 +- src/tools/miri/src/shims/native_lib.rs | 50 +++-- .../tests/native-lib/pass/ptr_read_access.rs | 27 +-- .../tests/native-lib/pass/ptr_write_access.rs | 208 ++++++++++++++++++ .../miri/tests/native-lib/ptr_read_access.c | 8 +- .../miri/tests/native-lib/ptr_write_access.c | 90 ++++++++ src/tools/miri/tests/ui.rs | 1 + 19 files changed, 476 insertions(+), 59 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/const_eval/dummy_machine.rs b/compiler/rustc_const_eval/src/const_eval/dummy_machine.rs index e49d702127d..817acfcca74 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( - _ecx: &mut InterpCx<'tcx, Self>, - _ptr: interpret::Pointer, + fn expose_provenance( + _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 19c3195aaa4..e5534da8fea 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_ptr(_ecx: &mut InterpCx<'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx> { + fn expose_provenance( + _ecx: &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 2d1bb5c9551..1ad3283383b 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 dbe09d55b2d..a180d5da941 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -327,11 +327,11 @@ pub trait Machine<'tcx>: Sized { addr: u64, ) -> InterpResult<'tcx, Pointer>>; - /// Marks a pointer as exposed, allowing it's provenance + /// Marks a pointer as exposed, allowing its provenance /// to be recovered. "Pointer-to-int cast" - fn expose_ptr( - ecx: &mut InterpCx<'tcx, Self>, - ptr: Pointer, + fn expose_provenance( + ecx: &InterpCx<'tcx, Self>, + 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 07566e9fda2..a53c3762656 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -944,6 +944,52 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { interp_ok(()) } + /// Handle the effect an FFI call might have on the state of allocations. + /// This overapproximates the modifications which external code might make to memory: + /// We set all reachable allocations as initialized, mark all provenances as exposed + /// and overwrite them with `Provenance::WILDCARD`. + 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)?; + + let mut done = FxHashSet::default(); + let mut todo = vec![id]; + while let Some(id) = todo.pop() { + if !done.insert(id) { + // We already saw this allocation before, don't process it again. + continue; + } + let info = self.get_alloc_info(id); + + // If there is no data behind this pointer, skip this. + if !matches!(info.kind, AllocKind::LiveData) { + continue; + } + + // Expose all provenances in this allocation, and add them to `todo`. + let alloc = self.get_alloc_raw(id)?; + for prov in alloc.provenance().provenances() { + M::expose_provenance(self, prov)?; + 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() { + self.get_alloc_raw_mut(id)? + .0 + .prepare_for_native_write() + .map_err(|e| e.to_interp_error(id))?; + } + } + interp_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 509f2667b35..d6f8fed755f 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -643,6 +643,28 @@ impl Allocation Ok(()) } + /// Initialize all previously uninitialized bytes in the entire allocation, and set + /// provenance of everything to `Wildcard`. Before calling this, make sure all + /// provenance in this allocation is exposed! + pub fn prepare_for_native_write(&mut self) -> AllocResult { + let full_range = AllocRange { start: Size::ZERO, size: Size::from_bytes(self.len()) }; + // Overwrite uninitialized bytes with 0, to ensure we don't leak whatever their value happens to be. + 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(()) + } + /// 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/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs b/compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs index 5c47fc6a399..3a83b184d83 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,25 @@ impl ProvenanceMap { Ok(()) } + + /// Overwrites all provenance in the allocation with wildcard provenance. + /// + /// Provided for usage in Miri and panics otherwise. + pub fn write_wildcards(&mut self, alloc_size: usize) { + assert!( + Prov::OFFSET_IS_ADDR, + "writing wildcard provenance is not supported when `OFFSET_IS_ADDR` is false" + ); + let wildcard = Prov::WILDCARD.unwrap(); + + // Remove all pointer provenances, then write wildcards into the whole byte range. + self.ptrs.clear(); + let last = Size::from_bytes(alloc_size); + let bytes = self.bytes.get_or_insert_with(Box::default); + for offset in Size::ZERO..last { + bytes.insert(offset, wildcard); + } + } } /// 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 1d5afe22573..25c7c26ddd9 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/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index fe7d8db245b..f7295fd7d8a 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,8 +299,10 @@ 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); + // Release the global state before we call `expose_tag`, which may call `get_alloc_info_extra`, + // which may need access to the global state. + drop(global_state); 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 4883613dea5..9808102f4ba 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 745316913d9..355ed1e0f31 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 255a3578aae..17329e7b4b5 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 9c1951ec87a..b6f07446be7 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: &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 e7a4251242e..4082b8eed45 100644 --- a/src/tools/miri/src/shims/native_lib.rs +++ b/src/tools/miri/src/shims/native_lib.rs @@ -3,8 +3,11 @@ use std::ops::Deref; use libffi::high::call as ffi; use libffi::low::CodePtr; -use rustc_abi::{BackendRepr, HasDataLayout}; -use rustc_middle::ty::{self as ty, IntTy, UintTy}; +use rustc_abi::{BackendRepr, HasDataLayout, Size}; +use rustc_middle::{ + mir::interpret::Pointer, + ty::{self as ty, IntTy, UintTy}, +}; use rustc_span::Symbol; use crate::*; @@ -75,6 +78,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { unsafe { ffi::call::<()>(ptr, libffi_args.as_slice()) }; return interp_ok(ImmTy::uninit(dest.layout)); } + ty::RawPtr(..) => { + let x = unsafe { ffi::call::<*const ()>(ptr, libffi_args.as_slice()) }; + let ptr = Pointer::new(Provenance::Wildcard, Size::from_bytes(x.addr())); + Scalar::from_pointer(ptr, this) + } _ => throw_unsup_format!("unsupported return type for native call: {:?}", link_name), }; interp_ok(ImmTy::from_scalar(scalar, dest.layout)) @@ -152,8 +160,26 @@ 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)?; + 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 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 { + // Wildcard pointer, whatever it points to must be already exposed. + continue; + }; + 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 @@ -220,7 +246,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. @@ -238,18 +264,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_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs index 46eb5778b32..3ccfecc6fb3 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs @@ -3,17 +3,14 @@ //@only-on-host fn main() { - test_pointer(); - - test_simple(); - - test_nested(); - - test_static(); + test_access_pointer(); + test_access_simple(); + test_access_nested(); + test_access_static(); } -// Test void function that dereferences a pointer and prints its contents from C. -fn test_pointer() { +/// Test function that dereferences an int pointer and prints its contents from C. +fn test_access_pointer() { extern "C" { fn print_pointer(ptr: *const i32); } @@ -23,8 +20,8 @@ fn test_pointer() { unsafe { print_pointer(&x) }; } -// Test function that dereferences a simple struct pointer and accesses a field. -fn test_simple() { +/// Test function that dereferences a simple struct pointer and accesses a field. +fn test_access_simple() { #[repr(C)] struct Simple { field: i32, @@ -39,8 +36,8 @@ fn test_simple() { assert_eq!(unsafe { access_simple(&simple) }, -42); } -// Test function that dereferences nested struct pointers and accesses fields. -fn test_nested() { +/// Test function that dereferences nested struct pointers and accesses fields. +fn test_access_nested() { use std::ptr::NonNull; #[derive(Debug, PartialEq, Eq)] @@ -61,8 +58,8 @@ fn test_nested() { assert_eq!(unsafe { access_nested(&nested_2) }, 97); } -// Test function that dereferences static struct pointers and accesses fields. -fn test_static() { +/// Test function that dereferences a static struct pointer and accesses fields. +fn test_access_static() { #[repr(C)] struct Static { value: i32, 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 00000000000..4045ef3cee5 --- /dev/null +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -0,0 +1,208 @@ +// Only works on Unix targets +//@ignore-target: windows wasm +//@only-on-host +//@compile-flags: -Zmiri-permissive-provenance + + +#![feature(box_as_ptr)] + +use std::mem::MaybeUninit; +use std::ptr::null; + +fn main() { + test_increment_int(); + test_init_int(); + test_init_array(); + test_init_static_inner(); + test_exposed(); + test_swap_ptr(); + test_swap_ptr_tuple(); + test_overwrite_dangling(); + test_pass_dangling(); + test_swap_ptr_triple_dangling(); + test_return_ptr(); +} + +/// Test function that modifies an int. +fn test_increment_int() { + extern "C" { + fn increment_int(ptr: *mut i32); + } + + let mut x = 11; + + unsafe { increment_int(&mut x) }; + assert_eq!(x, 12); +} + +/// Test function that initializes an int. +fn test_init_int() { + extern "C" { + fn init_int(ptr: *mut i32, val: i32); + } + + let mut x = MaybeUninit::::uninit(); + let val = 21; + + let x = unsafe { + init_int(x.as_mut_ptr(), val); + x.assume_init() + }; + assert_eq!(x, val); +} + +/// Test function that initializes an array. +fn test_init_array() { + extern "C" { + fn init_array(ptr: *mut i32, len: usize, val: i32); + } + + const LEN: usize = 3; + let mut array = MaybeUninit::<[i32; LEN]>::uninit(); + let val = 31; + + let array = unsafe { + init_array(array.as_mut_ptr().cast::(), LEN, val); + array.assume_init() + }; + assert_eq!(array, [val; LEN]); +} + +/// Test function that initializes an int pointed to by an immutable static. +fn test_init_static_inner() { + #[repr(C)] + struct SyncPtr { + ptr: *mut i32 + } + unsafe impl Sync for SyncPtr {} + + extern "C" { + fn init_static_inner(s_ptr: *const SyncPtr, val: i32); + } + + static mut INNER: MaybeUninit = MaybeUninit::uninit(); + #[allow(static_mut_refs)] + static STATIC: SyncPtr = SyncPtr { ptr: unsafe { INNER.as_mut_ptr() } }; + let val = 41; + + let inner = unsafe { + init_static_inner(&STATIC, val); + INNER.assume_init() + }; + assert_eq!(inner, val); +} + +// Test function that marks an allocation as exposed. +fn test_exposed() { + extern "C" { + fn ignore_ptr(ptr: *const i32); + } + + let x = 51; + let ptr = &raw const x; + let p = ptr.addr(); + + unsafe { ignore_ptr(ptr) }; + assert_eq!(unsafe { *(p as *const i32) }, x); +} + +/// Test function that swaps two pointers and exposes the alloc of an int. +fn test_swap_ptr() { + extern "C" { + fn swap_ptr(pptr0: *mut *const i32, pptr1: *mut *const i32); + } + + let x = 61; + let (mut ptr0, mut ptr1) = (&raw const x, null()); + + unsafe { swap_ptr(&mut ptr0, &mut ptr1) }; + assert_eq!(unsafe { *ptr1 }, x); +} + +/// Test function that swaps two pointers in a struct and exposes the alloc of an int. +fn test_swap_ptr_tuple() { + #[repr(C)] + struct Tuple { + ptr0: *const i32, + ptr1: *const i32, + } + + extern "C" { + fn swap_ptr_tuple(t_ptr: *mut Tuple); + } + + let x = 71; + let mut tuple = Tuple { ptr0: &raw const x, ptr1: null() }; + + unsafe { swap_ptr_tuple(&mut tuple) } + assert_eq!(unsafe { *tuple.ptr1 }, x); +} + +/// Test function that interacts with a dangling pointer. +fn test_overwrite_dangling() { + extern "C" { + fn overwrite_ptr(pptr: *mut *const i32); + } + + let b = Box::new(81); + let mut ptr = Box::as_ptr(&b); + drop(b); + + unsafe { overwrite_ptr(&mut ptr) }; + assert_eq!(ptr, null()); +} + +/// Test function that passes a dangling pointer. +fn test_pass_dangling() { + extern "C" { + fn ignore_ptr(ptr: *const i32); + } + + let b = Box::new(91); + let ptr = Box::as_ptr(&b); + drop(b); + + unsafe { ignore_ptr(ptr) }; +} + +/// Test function that interacts with a struct storing a dangling pointer. +fn test_swap_ptr_triple_dangling() { + #[repr(C)] + struct Triple { + ptr0: *const i32, + ptr1: *const i32, + ptr2: *const i32, + } + + extern "C" { + fn swap_ptr_triple_dangling(t_ptr: *const Triple); + } + + let x = 101; + let b = Box::new(111); + let ptr = Box::as_ptr(&b); + drop(b); + let z = 121; + let triple = Triple { + ptr0: &raw const x, + ptr1: ptr, + ptr2: &raw const z + }; + + unsafe { swap_ptr_triple_dangling(&triple) } + assert_eq!(unsafe { *triple.ptr2 }, x); +} + + +/// Test function that directly returns its pointer argument. +fn test_return_ptr() { + extern "C" { + fn return_ptr(ptr: *const i32) -> *const i32; + } + + let x = 131; + let ptr = &raw const x; + + let ptr = unsafe { return_ptr(ptr) }; + assert_eq!(unsafe { *ptr }, x); +} diff --git a/src/tools/miri/tests/native-lib/ptr_read_access.c b/src/tools/miri/tests/native-lib/ptr_read_access.c index 540845d53a7..3b427d6033e 100644 --- a/src/tools/miri/tests/native-lib/ptr_read_access.c +++ b/src/tools/miri/tests/native-lib/ptr_read_access.c @@ -3,13 +3,13 @@ // See comments in build_native_lib() #define EXPORT __attribute__((visibility("default"))) -/* Test: test_pointer */ +/* Test: test_access_pointer */ EXPORT void print_pointer(const int *ptr) { printf("printing pointer dereference from C: %d\n", *ptr); } -/* Test: test_simple */ +/* Test: test_access_simple */ typedef struct Simple { int field; @@ -19,7 +19,7 @@ EXPORT int access_simple(const Simple *s_ptr) { return s_ptr->field; } -/* Test: test_nested */ +/* Test: test_access_nested */ typedef struct Nested { int value; @@ -38,7 +38,7 @@ EXPORT int access_nested(const Nested *n_ptr) { return n_ptr->value; } -/* Test: test_static */ +/* Test: test_access_static */ typedef struct Static { int value; 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 00000000000..b54c5d86b21 --- /dev/null +++ b/src/tools/miri/tests/native-lib/ptr_write_access.c @@ -0,0 +1,90 @@ +#include + +// See comments in build_native_lib() +#define EXPORT __attribute__((visibility("default"))) + +/* Test: test_increment_int */ + +EXPORT void increment_int(int *ptr) { + *ptr += 1; +} + +/* Test: test_init_int */ + +EXPORT void init_int(int *ptr, int val) { + *ptr = val; +} + +/* Test: test_init_array */ + +EXPORT void init_array(int *array, size_t len, int val) { + for (size_t i = 0; i < len; i++) { + array[i] = val; + } +} + +/* Test: test_init_static_inner */ + +typedef struct SyncPtr { + int *ptr; +} SyncPtr; + +EXPORT void init_static_inner(const SyncPtr *s_ptr, int val) { + *(s_ptr->ptr) = val; +} + +/* Tests: test_exposed, test_pass_dangling */ + +EXPORT void ignore_ptr(__attribute__((unused)) const int *ptr) { + return; +} + +/* Test: test_expose_int */ +EXPORT void expose_int(const int *int_ptr, const int **pptr) { + *pptr = int_ptr; +} + +/* Test: test_swap_ptr */ + +EXPORT void swap_ptr(const int **pptr0, const int **pptr1) { + const int *tmp = *pptr0; + *pptr0 = *pptr1; + *pptr1 = tmp; +} + +/* Test: test_swap_ptr_tuple */ + +typedef struct Tuple { + int *ptr0; + int *ptr1; +} Tuple; + +EXPORT void swap_ptr_tuple(Tuple *t_ptr) { + int *tmp = t_ptr->ptr0; + t_ptr->ptr0 = t_ptr->ptr1; + t_ptr->ptr1 = tmp; +} + +/* Test: test_overwrite_dangling */ + +EXPORT void overwrite_ptr(const int **pptr) { + *pptr = NULL; +} + +/* Test: test_swap_ptr_triple_dangling */ + +typedef struct Triple { + int *ptr0; + int *ptr1; + int *ptr2; +} Triple; + +EXPORT void swap_ptr_triple_dangling(Triple *t_ptr) { + int *tmp = t_ptr->ptr0; + t_ptr->ptr0 = t_ptr->ptr2; + t_ptr->ptr2 = tmp; +} + +EXPORT const int *return_ptr(const int *ptr) { + return ptr; +} diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 9553a37c9a8..9b9542b88a9 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",