From 1ee055f0ecbadd9e6c51182a7179d8f8acfd4720 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 12 Oct 2023 08:25:22 +0200 Subject: [PATCH] add some comments and some cleanup around Miri intptrcast --- .../rustc_const_eval/src/interpret/memory.rs | 10 +++++ src/tools/miri/src/intptrcast.rs | 37 +++++++++---------- src/tools/miri/src/machine.rs | 18 ++++++--- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index ce666e6af3b..a646f09dee0 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1249,6 +1249,11 @@ pub fn scalar_may_be_null(&self, scalar: Scalar) -> InterpResult< /// Turning a "maybe pointer" into a proper pointer (and some information /// about where it points), or an absolute address. + /// + /// The result must be used immediately; it is not allowed to convert + /// the returned data back into a `Pointer` and store that in machine state. + /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and + /// we don't have an operation to turn it back into `M::Provenance`.) pub fn ptr_try_get_alloc_id( &self, ptr: Pointer>, @@ -1267,6 +1272,11 @@ pub fn ptr_try_get_alloc_id( } /// Turning a "maybe pointer" into a proper pointer (and some information about where it points). + /// + /// The result must be used immediately; it is not allowed to convert + /// the returned data back into a `Pointer` and store that in machine state. + /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and + /// we don't have an operation to turn it back into `M::Provenance`.) #[inline(always)] pub fn ptr_get_alloc_id( &self, diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs index 4fd0af35304..154d86375ca 100644 --- a/src/tools/miri/src/intptrcast.rs +++ b/src/tools/miri/src/intptrcast.rs @@ -119,24 +119,14 @@ pub fn expose_ptr( Ok(()) } - pub fn ptr_from_addr_transmute( - _ecx: &MiriInterpCx<'mir, 'tcx>, - addr: u64, - ) -> Pointer> { - trace!("Transmuting {:#x} to a pointer", addr); - - // We consider transmuted pointers to be "invalid" (`None` provenance). - Pointer::new(None, Size::from_bytes(addr)) - } - pub fn ptr_from_addr_cast( ecx: &MiriInterpCx<'mir, 'tcx>, addr: u64, ) -> InterpResult<'tcx, Pointer>> { trace!("Casting {:#x} to a pointer", addr); + // Potentially emit a warning. let global_state = ecx.machine.intptrcast.borrow(); - match global_state.provenance_mode { ProvenanceMode::Default => { // The first time this happens at a particular location, print a warning. @@ -158,7 +148,12 @@ pub fn ptr_from_addr_cast( ProvenanceMode::Permissive => {} } - // This is how wildcard pointers are born. + // We do *not* look up the `AllocId` here! This is a `ptr as usize` cast, and it is + // completely legal to do a cast and then `wrapping_offset` to another allocation and only + // *then* do a memory access. So the allocation that the pointer happens to point to on a + // cast is fairly irrelevant. Instead we generate this as a "wildcard" pointer, such that + // *every time the pointer is used*, we do an `AllocId` lookup to find the (exposed) + // allocation it might be referencing. Ok(Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr))) } @@ -219,22 +214,27 @@ fn alloc_base_addr( }) } - /// Convert a relative (tcx) pointer to an absolute address. - pub fn rel_ptr_to_addr( + /// Convert a relative (tcx) pointer to a Miri pointer. + pub fn ptr_from_rel_ptr( ecx: &MiriInterpCx<'mir, 'tcx>, ptr: Pointer, - ) -> InterpResult<'tcx, u64> { + tag: BorTag, + ) -> InterpResult<'tcx, Pointer> { let (alloc_id, offset) = ptr.into_parts(); // offset is relative (AllocId provenance) let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id)?; // Add offset with the right kind of pointer-overflowing arithmetic. let dl = ecx.data_layout(); - Ok(dl.overflowing_offset(base_addr, offset.bytes()).0) + let absolute_addr = dl.overflowing_offset(base_addr, offset.bytes()).0; + Ok(Pointer::new( + Provenance::Concrete { alloc_id, tag }, + Size::from_bytes(absolute_addr), + )) } /// When a pointer is used for a memory access, this computes where in which allocation the /// access is going. - pub fn abs_ptr_to_rel( + pub fn ptr_get_alloc( ecx: &MiriInterpCx<'mir, 'tcx>, ptr: Pointer, ) -> Option<(AllocId, Size)> { @@ -252,12 +252,11 @@ pub fn abs_ptr_to_rel( let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id).unwrap(); // Wrapping "addr - base_addr" - let dl = ecx.data_layout(); #[allow(clippy::cast_possible_wrap)] // we want to wrap here let neg_base_addr = (base_addr as i64).wrapping_neg(); Some(( alloc_id, - Size::from_bytes(dl.overflowing_signed_offset(addr.bytes(), neg_base_addr).0), + Size::from_bytes(ecx.overflowing_signed_offset(addr.bytes(), neg_base_addr).0), )) } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 3de27460860..96c734c5500 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1136,19 +1136,16 @@ fn adjust_alloc_base_pointer( _ => {} } } - let absolute_addr = intptrcast::GlobalStateInner::rel_ptr_to_addr(ecx, ptr)?; let tag = if let Some(borrow_tracker) = &ecx.machine.borrow_tracker { borrow_tracker.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine) } else { // Value does not matter, SB is disabled BorTag::default() }; - Ok(Pointer::new( - Provenance::Concrete { alloc_id: ptr.provenance, tag }, - Size::from_bytes(absolute_addr), - )) + intptrcast::GlobalStateInner::ptr_from_rel_ptr(ecx, ptr, tag) } + /// Called on `usize as ptr` casts. #[inline(always)] fn ptr_from_addr_cast( ecx: &MiriInterpCx<'mir, 'tcx>, @@ -1157,6 +1154,9 @@ fn ptr_from_addr_cast( intptrcast::GlobalStateInner::ptr_from_addr_cast(ecx, addr) } + /// 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<'mir, 'tcx, Self>, ptr: Pointer, @@ -1174,11 +1174,17 @@ fn expose_ptr( /// Convert a pointer with provenance into an allocation-offset pair, /// or a `None` with an absolute address if that conversion is not possible. + /// + /// This is called when a pointer is about to be used for memory access, + /// an in-bounds check, or anything else that requires knowing which allocation it points to. + /// The resulting `AllocId` will just be used for that one step and the forgotten again + /// (i.e., we'll never turn the data returned here back into a `Pointer` that might be + /// stored in machine state). fn ptr_get_alloc( ecx: &MiriInterpCx<'mir, 'tcx>, ptr: Pointer, ) -> Option<(AllocId, Size, Self::ProvenanceExtra)> { - let rel = intptrcast::GlobalStateInner::abs_ptr_to_rel(ecx, ptr); + let rel = intptrcast::GlobalStateInner::ptr_get_alloc(ecx, ptr); rel.map(|(alloc_id, size)| { let tag = match ptr.provenance {