Rollup merge of #98811 - RalfJung:interpret-alloc-range, r=oli-obk

Interpret: AllocRange Debug impl, and use it more consistently

The two commits are pretty independent but it did not seem worth having two PRs for them.
r? ``@oli-obk``

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -276,7 +276,7 @@ pub fn deallocate_ptr(
         kind: MemoryKind<M::MemoryKind>,
     ) -> InterpResult<'tcx> {
         let (alloc_id, offset, tag) = self.ptr_get_alloc_id(ptr)?;
-        trace!("deallocating: {}", alloc_id);
+        trace!("deallocating: {alloc_id:?}");
 
         if offset.bytes() != 0 {
             throw_ub_format!(
@@ -289,10 +289,10 @@ pub fn deallocate_ptr(
             // Deallocating global memory -- always an error
             return Err(match self.tcx.get_global_alloc(alloc_id) {
                 Some(GlobalAlloc::Function(..)) => {
-                    err_ub_format!("deallocating {}, which is a function", alloc_id)
+                    err_ub_format!("deallocating {alloc_id:?}, which is a function")
                 }
                 Some(GlobalAlloc::Static(..) | GlobalAlloc::Memory(..)) => {
-                    err_ub_format!("deallocating {}, which is static memory", alloc_id)
+                    err_ub_format!("deallocating {alloc_id:?}, which is static memory")
                 }
                 None => err_ub!(PointerUseAfterFree(alloc_id)),
             }
@@ -302,21 +302,17 @@ pub fn deallocate_ptr(
         debug!(?alloc);
 
         if alloc.mutability == Mutability::Not {
-            throw_ub_format!("deallocating immutable allocation {}", alloc_id);
+            throw_ub_format!("deallocating immutable allocation {alloc_id:?}");
         }
         if alloc_kind != kind {
             throw_ub_format!(
-                "deallocating {}, which is {} memory, using {} deallocation operation",
-                alloc_id,
-                alloc_kind,
-                kind
+                "deallocating {alloc_id:?}, which is {alloc_kind} memory, using {kind} deallocation operation"
             );
         }
         if let Some((size, align)) = old_size_and_align {
             if size != alloc.size() || align != alloc.align {
                 throw_ub_format!(
-                    "incorrect layout on deallocation: {} has size {} and alignment {}, but gave size {} and alignment {}",
-                    alloc_id,
+                    "incorrect layout on deallocation: {alloc_id:?} has size {} and alignment {}, but gave size {} and alignment {}",
                     alloc.size().bytes(),
                     alloc.align.bytes(),
                     size.bytes(),
@@ -815,7 +811,7 @@ fn write_allocation_track_relocs<'tcx, Tag: Provenance, Extra>(
                 continue;
             }
 
-            write!(fmt, "{}", id)?;
+            write!(fmt, "{id:?}")?;
             match self.ecx.memory.alloc_map.get(id) {
                 Some(&(kind, ref alloc)) => {
                     // normal alloc
@@ -859,25 +855,21 @@ fn write_allocation_track_relocs<'tcx, Tag: Provenance, Extra>(
 
 /// Reading and writing.
 impl<'tcx, 'a, Tag: Provenance, Extra> AllocRefMut<'a, 'tcx, Tag, Extra> {
+    /// `range` is relative to this allocation reference, not the base of the allocation.
     pub fn write_scalar(
         &mut self,
         range: AllocRange,
         val: ScalarMaybeUninit<Tag>,
     ) -> InterpResult<'tcx> {
         let range = self.range.subrange(range);
-        debug!(
-            "write_scalar in {} at {:#x}, size {}: {:?}",
-            self.alloc_id,
-            range.start.bytes(),
-            range.size.bytes(),
-            val
-        );
+        debug!("write_scalar at {:?}{range:?}: {val:?}", self.alloc_id);
         Ok(self
             .alloc
             .write_scalar(&self.tcx, range, val)
             .map_err(|e| e.to_interp_error(self.alloc_id))?)
     }
 
+    /// `offset` is relative to this allocation reference, not the base of the allocation.
     pub fn write_ptr_sized(
         &mut self,
         offset: Size,
@@ -896,6 +888,7 @@ pub fn write_uninit(&mut self) -> InterpResult<'tcx> {
 }
 
 impl<'tcx, 'a, Tag: Provenance, Extra> AllocRef<'a, 'tcx, Tag, Extra> {
+    /// `range` is relative to this allocation reference, not the base of the allocation.
     pub fn read_scalar(
         &self,
         range: AllocRange,
@@ -906,24 +899,16 @@ pub fn read_scalar(
             .alloc
             .read_scalar(&self.tcx, range, read_provenance)
             .map_err(|e| e.to_interp_error(self.alloc_id))?;
-        debug!(
-            "read_scalar in {} at {:#x}, size {}: {:?}",
-            self.alloc_id,
-            range.start.bytes(),
-            range.size.bytes(),
-            res
-        );
+        debug!("read_scalar at {:?}{range:?}: {res:?}", self.alloc_id);
         Ok(res)
     }
 
-    pub fn read_integer(
-        &self,
-        offset: Size,
-        size: Size,
-    ) -> InterpResult<'tcx, ScalarMaybeUninit<Tag>> {
-        self.read_scalar(alloc_range(offset, size), /*read_provenance*/ false)
+    /// `range` is relative to this allocation reference, not the base of the allocation.
+    pub fn read_integer(&self, range: AllocRange) -> InterpResult<'tcx, ScalarMaybeUninit<Tag>> {
+        self.read_scalar(range, /*read_provenance*/ false)
     }
 
+    /// `offset` is relative to this allocation reference, not the base of the allocation.
     pub fn read_pointer(&self, offset: Size) -> InterpResult<'tcx, ScalarMaybeUninit<Tag>> {
         self.read_scalar(
             alloc_range(offset, self.tcx.data_layout().pointer_size),
@@ -931,6 +916,7 @@ pub fn read_pointer(&self, offset: Size) -> InterpResult<'tcx, ScalarMaybeUninit
         )
     }
 
+    /// `range` is relative to this allocation reference, not the base of the allocation.
     pub fn check_bytes(
         &self,
         range: AllocRange,

compiler/rustc_const_eval/src/interpret/traits.rs

 use std::convert::TryFrom;
 
-use rustc_middle::mir::interpret::{InterpResult, Pointer, PointerArithmetic};
+use rustc_middle::mir::interpret::{alloc_range, InterpResult, Pointer, PointerArithmetic};
 use rustc_middle::ty::{
     self, Ty, TyCtxt, COMMON_VTABLE_ENTRIES_ALIGN, COMMON_VTABLE_ENTRIES_DROPINPLACE,
     COMMON_VTABLE_ENTRIES_SIZE,
@@ -102,18 +102,18 @@ pub fn read_size_and_align_from_vtable(
             )?
             .expect("cannot be a ZST");
         let size = vtable
-            .read_integer(
+            .read_integer(alloc_range(
                 pointer_size * u64::try_from(COMMON_VTABLE_ENTRIES_SIZE).unwrap(),
                 pointer_size,
-            )?
+            ))?
             .check_init()?;
         let size = size.to_machine_usize(self)?;
         let size = Size::from_bytes(size);
         let align = vtable
-            .read_integer(
+            .read_integer(alloc_range(
                 pointer_size * u64::try_from(COMMON_VTABLE_ENTRIES_ALIGN).unwrap(),
                 pointer_size,
-            )?
+            ))?
             .check_init()?;
         let align = align.to_machine_usize(self)?;
         let align = Align::from_bytes(align).map_err(|e| err_ub!(InvalidVtableAlignment(e)))?;

compiler/rustc_middle/src/mir/interpret/allocation.rs

@@ -160,12 +160,18 @@ pub fn to_interp_error<'tcx>(self, alloc_id: AllocId) -> InterpError<'tcx> {
 }
 
 /// The information that makes up a memory access: offset and size.
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone)]
 pub struct AllocRange {
     pub start: Size,
     pub size: Size,
 }
 
+impl fmt::Debug for AllocRange {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "[{:#x}..{:#x}]", self.start.bytes(), self.end().bytes())
+    }
+}
+
 /// Free-starting constructor for less syntactic overhead.
 #[inline(always)]
 pub fn alloc_range(start: Size, size: Size) -> AllocRange {

compiler/rustc_middle/src/mir/interpret/error.rs

@@ -334,36 +334,30 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                 p,
             ),
             PointerUseAfterFree(a) => {
-                write!(f, "pointer to {} was dereferenced after this allocation got freed", a)
+                write!(f, "pointer to {a:?} was dereferenced after this allocation got freed")
             }
             PointerOutOfBounds { alloc_id, alloc_size, ptr_offset, ptr_size: Size::ZERO, msg } => {
                 write!(
                     f,
-                    "{}{alloc_id} has size {alloc_size}, so pointer at offset {ptr_offset} is out-of-bounds",
-                    msg,
-                    alloc_id = alloc_id,
+                    "{msg}{alloc_id:?} has size {alloc_size}, so pointer at offset {ptr_offset} is out-of-bounds",
                     alloc_size = alloc_size.bytes(),
-                    ptr_offset = ptr_offset,
                 )
             }
             PointerOutOfBounds { alloc_id, alloc_size, ptr_offset, ptr_size, msg } => write!(
                 f,
-                "{}{alloc_id} has size {alloc_size}, so pointer to {ptr_size} byte{ptr_size_p} starting at offset {ptr_offset} is out-of-bounds",
-                msg,
-                alloc_id = alloc_id,
+                "{msg}{alloc_id:?} has size {alloc_size}, so pointer to {ptr_size} byte{ptr_size_p} starting at offset {ptr_offset} is out-of-bounds",
                 alloc_size = alloc_size.bytes(),
                 ptr_size = ptr_size.bytes(),
                 ptr_size_p = pluralize!(ptr_size.bytes()),
-                ptr_offset = ptr_offset,
             ),
             DanglingIntPointer(0, CheckInAllocMsg::InboundsTest) => {
                 write!(f, "null pointer is not a valid pointer for this operation")
             }
             DanglingIntPointer(0, msg) => {
-                write!(f, "{}null pointer is not a valid pointer", msg)
+                write!(f, "{msg}null pointer is not a valid pointer")
             }
             DanglingIntPointer(i, msg) => {
-                write!(f, "{}0x{:x} is not a valid pointer", msg, i)
+                write!(f, "{msg}{i:#x} is not a valid pointer")
             }
             AlignmentCheckFailed { required, has } => write!(
                 f,
@@ -371,8 +365,8 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                 has.bytes(),
                 required.bytes()
             ),
-            WriteToReadOnly(a) => write!(f, "writing to {} which is read-only", a),
-            DerefFunctionPointer(a) => write!(f, "accessing {} which contains a function", a),
+            WriteToReadOnly(a) => write!(f, "writing to {a:?} which is read-only"),
+            DerefFunctionPointer(a) => write!(f, "accessing {a:?} which contains a function"),
             ValidationFailure { path: None, msg } => {
                 write!(f, "constructing invalid value: {}", msg)
             }

compiler/rustc_middle/src/mir/interpret/mod.rs

@@ -190,11 +190,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     }
 }
 
-impl fmt::Display for AllocId {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt::Debug::fmt(self, f)
-    }
-}
+// No "Display" since AllocIds are not usually user-visible.
 
 #[derive(TyDecodable, TyEncodable)]
 enum AllocDiscriminant {
@@ -470,7 +466,7 @@ fn reserve_and_set_dedup(self, alloc: GlobalAlloc<'tcx>) -> AllocId {
             return alloc_id;
         }
         let id = alloc_map.reserve();
-        debug!("creating alloc {:?} with id {}", alloc, id);
+        debug!("creating alloc {alloc:?} with id {id:?}");
         alloc_map.alloc_map.insert(id, alloc.clone());
         alloc_map.dedup.insert(alloc, id);
         id
@@ -538,7 +534,7 @@ pub fn get_global_alloc(self, id: AllocId) -> Option<GlobalAlloc<'tcx>> {
     pub fn global_alloc(self, id: AllocId) -> GlobalAlloc<'tcx> {
         match self.get_global_alloc(id) {
             Some(alloc) => alloc,
-            None => bug!("could not find allocation for {}", id),
+            None => bug!("could not find allocation for {id:?}"),
         }
     }
 
@@ -546,7 +542,7 @@ pub fn global_alloc(self, id: AllocId) -> GlobalAlloc<'tcx> {
     /// call this function twice, even with the same `Allocation` will ICE the compiler.
     pub fn set_alloc_id_memory(self, id: AllocId, mem: ConstAllocation<'tcx>) {
         if let Some(old) = self.alloc_map.lock().alloc_map.insert(id, GlobalAlloc::Memory(mem)) {
-            bug!("tried to set allocation ID {}, but it was already existing as {:#?}", id, old);
+            bug!("tried to set allocation ID {id:?}, but it was already existing as {old:#?}");
         }
     }
 

compiler/rustc_middle/src/mir/interpret/pointer.rs

@@ -144,7 +144,7 @@ fn fmt(ptr: &Pointer<Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         }
         // Print offset only if it is non-zero.
         if ptr.offset.bytes() > 0 {
-            write!(f, "+0x{:x}", ptr.offset.bytes())?;
+            write!(f, "+{:#x}", ptr.offset.bytes())?;
         }
         Ok(())
     }
@@ -181,7 +181,7 @@ impl<Tag: Provenance> fmt::Debug for Pointer<Option<Tag>> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.provenance {
             Some(tag) => Provenance::fmt(&Pointer::new(tag, self.offset), f),
-            None => write!(f, "0x{:x}", self.offset.bytes()),
+            None => write!(f, "{:#x}", self.offset.bytes()),
         }
     }
 }

compiler/rustc_middle/src/mir/interpret/value.rs

@@ -167,7 +167,7 @@ impl<Tag: Provenance> fmt::LowerHex for Scalar<Tag> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Scalar::Ptr(ptr, _size) => write!(f, "pointer to {:?}", ptr),
-            Scalar::Int(int) => write!(f, "0x{:x}", int),
+            Scalar::Int(int) => write!(f, "{:#x}", int),
         }
     }
 }

compiler/rustc_middle/src/mir/pretty.rs

@@ -716,12 +716,12 @@ fn visit_constant(&mut self, c: &Constant<'tcx>, loc: Location) {
                 }
                 write!(w, "{}", display_allocation(tcx, alloc.inner()))
             };
-        write!(w, "\n{}", id)?;
+        write!(w, "\n{id:?}")?;
         match tcx.get_global_alloc(id) {
             // This can't really happen unless there are bugs, but it doesn't cost us anything to
             // gracefully handle it and allow buggy rustc to be debugged via allocation printing.
             None => write!(w, " (deallocated)")?,
-            Some(GlobalAlloc::Function(inst)) => write!(w, " (fn: {})", inst)?,
+            Some(GlobalAlloc::Function(inst)) => write!(w, " (fn: {inst})")?,
             Some(GlobalAlloc::Static(did)) if !tcx.is_foreign_item(did) => {
                 match tcx.eval_static_initializer(did) {
                     Ok(alloc) => {

compiler/rustc_middle/src/ty/consts/int.rs

@@ -452,6 +452,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 impl fmt::LowerHex for ScalarInt {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.check_data();
+        if f.alternate() {
+            // Like regular ints, alternate flag adds leading `0x`.
+            write!(f, "0x")?;
+        }
         // Format as hex number wide enough to fit any value of the given `size`.
         // So data=20, size=1 will be "0x14", but with size=4 it'll be "0x00000014".
         // Using a block `{self.data}` here to force a copy instead of using `self.data`