interpret: simplify pointer arithmetic logic

Author: RalfJung

src/alloc_addresses/mod.rs

@@ -11,7 +11,7 @@ use rand::Rng;
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_span::Span;
-use rustc_target::abi::{Align, HasDataLayout, Size};
+use rustc_target::abi::{Align, Size};
 
 use crate::{concurrency::VClock, *};
 
@@ -307,15 +307,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         let (prov, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
         let alloc_id = prov.alloc_id();
-        let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?;
 
-        // Add offset with the right kind of pointer-overflowing arithmetic.
-        let dl = ecx.data_layout();
-        let absolute_addr = dl.overflowing_offset(base_addr, offset.bytes()).0;
-        Ok(interpret::Pointer::new(
+        // Get a pointer to the beginning of this allocation.
+        let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?;
+        let base_ptr = interpret::Pointer::new(
             Provenance::Concrete { alloc_id, tag },
-            Size::from_bytes(absolute_addr),
-        ))
+            Size::from_bytes(base_addr),
+        );
+        // Add offset with the right kind of pointer-overflowing arithmetic.
+        Ok(base_ptr.wrapping_offset(offset, ecx))
     }
 
     /// When a pointer is used for a memory access, this computes where in which allocation the
@@ -341,12 +341,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let base_addr = *ecx.machine.alloc_addresses.borrow().base_addr.get(&alloc_id).unwrap();
 
         // Wrapping "addr - base_addr"
-        #[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(ecx.overflowing_signed_offset(addr.bytes(), neg_base_addr).0),
-        ))
+        let rel_offset = ecx.truncate_to_target_usize(addr.bytes().wrapping_sub(base_addr));
+        Some((alloc_id, Size::from_bytes(rel_offset)))
     }
 }
 

src/helpers.rs

@@ -606,7 +606,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         }
         // The part between the end_ptr and the end of the place is also frozen.
         // So pretend there is a 0-sized `UnsafeCell` at the end.
-        unsafe_cell_action(&place.ptr().offset(size, this)?, Size::ZERO)?;
+        unsafe_cell_action(&place.ptr().wrapping_offset(size, this), Size::ZERO)?;
         // Done!
         return Ok(());
 
@@ -975,7 +975,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         loop {
             // FIXME: We are re-getting the allocation each time around the loop.
             // Would be nice if we could somehow "extend" an existing AllocRange.
-            let alloc = this.get_ptr_alloc(ptr.offset(len, this)?, size1)?.unwrap(); // not a ZST, so we will get a result
+            let alloc = this.get_ptr_alloc(ptr.wrapping_offset(len, this), size1)?.unwrap(); // not a ZST, so we will get a result
             let byte = alloc.read_integer(alloc_range(Size::ZERO, size1))?.to_u8()?;
             if byte == 0 {
                 break;
@@ -1039,7 +1039,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 break;
             } else {
                 wchars.push(wchar_int.try_into().unwrap());
-                ptr = ptr.offset(size, this)?;
+                ptr = ptr.wrapping_offset(size, this);
             }
         }
 

src/shims/foreign_items.rs

@@ -622,7 +622,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 {
                     let idx = u64::try_from(idx).unwrap();
                     #[allow(clippy::arithmetic_side_effects)] // idx < num, so this never wraps
-                    let new_ptr = ptr.offset(Size::from_bytes(num - idx - 1), this)?;
+                    let new_ptr = ptr.wrapping_offset(Size::from_bytes(num - idx - 1), this);
                     this.write_pointer(new_ptr, dest)?;
                 } else {
                     this.write_null(dest)?;
@@ -646,7 +646,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     .iter()
                     .position(|&c| c == val);
                 if let Some(idx) = idx {
-                    let new_ptr = ptr.offset(Size::from_bytes(idx as u64), this)?;
+                    let new_ptr = ptr.wrapping_offset(Size::from_bytes(idx as u64), this);
                     this.write_pointer(new_ptr, dest)?;
                 } else {
                     this.write_null(dest)?;

src/shims/unix/env.rs

@@ -82,7 +82,7 @@ impl<'tcx> UnixEnvVars<'tcx> {
         };
         // The offset is used to strip the "{name}=" part of the string.
         let var_ptr = var_ptr
-            .offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx)?;
+            .wrapping_offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx);
         Ok(Some(var_ptr))
     }
 

src/shims/unix/fs.rs

@@ -996,7 +996,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     &this.ptr_to_mplace(entry, dirent64_layout),
                 )?;
 
-                let name_ptr = entry.offset(Size::from_bytes(d_name_offset), this)?;
+                let name_ptr = entry.wrapping_offset(Size::from_bytes(d_name_offset), this);
                 this.write_bytes_ptr(name_ptr, name_bytes.iter().copied())?;
 
                 Some(entry)

src/shims/unix/linux/mem.rs

@@ -53,7 +53,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // We just allocated this, the access is definitely in-bounds and fits into our address space.
             // mmap guarantees new mappings are zero-init.
             this.write_bytes_ptr(
-                ptr.offset(Size::from_bytes(old_size), this).unwrap().into(),
+                ptr.wrapping_offset(Size::from_bytes(old_size), this).into(),
                 std::iter::repeat(0u8).take(usize::try_from(increase).unwrap()),
             )
             .unwrap();

tests/fail/intrinsics/out_of_bounds_ptr_2.rs

@@ -1,5 +1,5 @@
 //@compile-flags: -Zmiri-permissive-provenance
-//@normalize-stderr-test: "to \d+ bytes of memory" -> "to $$BYTES bytes of memory"
+//@normalize-stderr-test: "\d+ bytes" -> "$$BYTES bytes"
 
 fn main() {
     let ptr = Box::into_raw(Box::new(0u32));

tests/fail/intrinsics/out_of_bounds_ptr_2.stderr

@@ -1,7 +1,6 @@
 fn main() {
     let v = [0i8; 4];
     let x = &v as *const i8;
-    // The error is inside another function, so we cannot match it by line
     let x = unsafe { x.offset(5) }; //~ERROR: expected a pointer to 5 bytes of memory
     panic!("this should never print: {:?}", x);
 }