Skip to content

Commit

Permalink
add AllocRange Debug impl; remove redundant AllocId Display impl
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jul 2, 2022
1 parent 0075bb4 commit c36572c
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 50 deletions.
32 changes: 9 additions & 23 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
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!(
Expand All @@ -289,10 +289,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// 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)),
}
Expand All @@ -302,21 +302,19 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
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,
"deallocating {alloc_id:?}, which is {} memory, using {} deallocation operation",
alloc_kind,
kind
);
}
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(),
Expand Down Expand Up @@ -815,7 +813,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> std::fmt::Debug for DumpAllocs<'a,
continue;
}

write!(fmt, "{}", id)?;
write!(fmt, "{id:?}")?;
match self.ecx.memory.alloc_map.get(id) {
Some(&(kind, ref alloc)) => {
// normal alloc
Expand Down Expand Up @@ -865,13 +863,7 @@ impl<'tcx, 'a, Tag: Provenance, Extra> AllocRefMut<'a, 'tcx, Tag, Extra> {
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)
Expand Down Expand Up @@ -906,13 +898,7 @@ impl<'tcx, 'a, Tag: Provenance, Extra> AllocRef<'a, 'tcx, Tag, Extra> {
.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)
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,18 @@ impl AllocError {
}

/// 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 {
Expand Down
20 changes: 7 additions & 13 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,45 +334,39 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
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}{:#x} is not a valid pointer", i)
}
AlignmentCheckFailed { required, has } => write!(
f,
"accessing memory with alignment {}, but alignment {} is required",
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)
}
Expand Down
12 changes: 4 additions & 8 deletions compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,7 @@ impl fmt::Debug for AllocId {
}
}

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 {
Expand Down Expand Up @@ -470,7 +466,7 @@ impl<'tcx> TyCtxt<'tcx> {
return alloc_id;
}
let id = alloc_map.reserve();
debug!("creating alloc {:?} with id {}", alloc, id);
debug!("creating alloc {:?} with id {:?}", alloc, id);
alloc_map.alloc_map.insert(id, alloc.clone());
alloc_map.dedup.insert(alloc, id);
id
Expand Down Expand Up @@ -538,15 +534,15 @@ impl<'tcx> TyCtxt<'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:?}"),
}
}

/// Freezes an `AllocId` created with `reserve` by pointing it at an `Allocation`. Trying to
/// 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:#?}");
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/interpret/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl Provenance for AllocId {
}
// 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(())
}
Expand Down Expand Up @@ -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()),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,12 +716,12 @@ pub fn write_allocations<'tcx>(
}
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) => {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/consts/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,10 @@ impl fmt::Debug for ScalarInt {
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`
Expand Down

0 comments on commit c36572c

Please sign in to comment.