Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MIR dump: print pointers consistently with Miri output #71590

Merged
merged 5 commits into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/librustc_middle/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,17 @@ pub enum LitToConstError {
#[derive(Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct AllocId(pub u64);

// We want the `Debug` output to be readable as it is used by `derive(Debug)` for
// all the Miri types.
impl fmt::Debug for AllocId {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self, fmt)
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if f.alternate() { write!(f, "a{}", self.0) } else { write!(f, "alloc{}", self.0) }
}
}

impl fmt::Display for AllocId {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "alloc{}", self.0)
fmt::Debug::fmt(self, f)
}
}

Expand Down
22 changes: 20 additions & 2 deletions src/librustc_middle/mir/interpret/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,33 @@ pub struct Pointer<Tag = (), Id = AllocId> {

static_assert_size!(Pointer, 16);

// We want the `Debug` output to be readable as it is used by `derive(Debug)` for
// all the Miri types.
// We have to use `Debug` output for the tag, because `()` does not implement
// `Display` so we cannot specialize that.
impl<Tag: fmt::Debug, Id: fmt::Debug> fmt::Debug for Pointer<Tag, Id> {
default fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}+0x{:x}[{:?}]", self.alloc_id, self.offset.bytes(), self.tag)
if f.alternate() {
write!(f, "{:#?}+0x{:x}[{:?}]", self.alloc_id, self.offset.bytes(), self.tag)
} else {
write!(f, "{:?}+0x{:x}[{:?}]", self.alloc_id, self.offset.bytes(), self.tag)
}
}
}
// Specialization for no tag
impl<Id: fmt::Debug> fmt::Debug for Pointer<(), Id> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}+0x{:x}", self.alloc_id, self.offset.bytes())
if f.alternate() {
write!(f, "{:#?}+0x{:x}", self.alloc_id, self.offset.bytes())
} else {
write!(f, "{:?}+0x{:x}", self.alloc_id, self.offset.bytes())
}
}
}

impl<Tag: fmt::Debug> fmt::Display for Pointer<Tag> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}

Expand Down
14 changes: 9 additions & 5 deletions src/librustc_middle/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ pub enum Scalar<Tag = (), Id = AllocId> {
#[cfg(target_arch = "x86_64")]
static_assert_size!(Scalar, 24);

// We want the `Debug` output to be readable as it is used by `derive(Debug)` for
// all the Miri types.
impl<Tag: fmt::Debug, Id: fmt::Debug> fmt::Debug for Scalar<Tag, Id> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Expand All @@ -125,11 +127,11 @@ impl<Tag: fmt::Debug, Id: fmt::Debug> fmt::Debug for Scalar<Tag, Id> {
}
}

impl<Tag> fmt::Display for Scalar<Tag> {
impl<Tag: fmt::Debug> fmt::Display for Scalar<Tag> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Scalar::Ptr(_) => write!(f, "a pointer"),
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
Scalar::Raw { data, .. } => write!(f, "{}", data),
Scalar::Ptr(ptr) => write!(f, "pointer to {}", ptr),
Scalar::Raw { .. } => fmt::Debug::fmt(self, f),
}
}
}
Expand Down Expand Up @@ -554,16 +556,18 @@ impl<Tag> From<Pointer<Tag>> for ScalarMaybeUndef<Tag> {
}
}

// We want the `Debug` output to be readable as it is used by `derive(Debug)` for
// all the Miri types.
impl<Tag: fmt::Debug, Id: fmt::Debug> fmt::Debug for ScalarMaybeUndef<Tag, Id> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ScalarMaybeUndef::Undef => write!(f, "Undef"),
ScalarMaybeUndef::Undef => write!(f, "<uninitialized>"),
ScalarMaybeUndef::Scalar(s) => write!(f, "{:?}", s),
}
}
}

impl<Tag> fmt::Display for ScalarMaybeUndef<Tag> {
impl<Tag: fmt::Debug> fmt::Display for ScalarMaybeUndef<Tag> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ScalarMaybeUndef::Undef => write!(f, "uninitialized bytes"),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub trait Machine<'mir, 'tcx>: Sized {
/// Tag tracked alongside every pointer. This is used to implement "Stacked Borrows"
/// <https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html>.
/// The `default()` is used for pointers to consts, statics, vtables and functions.
/// The `Debug` formatting is used for displaying pointers; we cannot use `Display`
/// as `()` does not implement that, but it should be "nice" output.
type PointerTag: ::std::fmt::Debug + Copy + Eq + Hash + 'static;

/// Machines can define extra (non-instance) things that represent values of function pointers.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
/// control for this.
pub fn dump_allocs(&self, mut allocs: Vec<AllocId>) {
// Cannot be a closure because it is generic in `Tag`, `Extra`.
fn write_allocation_track_relocs<'tcx, Tag, Extra>(
fn write_allocation_track_relocs<'tcx, Tag: Copy + fmt::Debug, Extra>(
tcx: TyCtxtAt<'tcx>,
allocs_to_print: &mut VecDeque<AllocId>,
alloc: &Allocation<Tag, Extra>,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,11 @@ impl<Tag: Copy> std::fmt::Display for ImmTy<'tcx, Tag> {
p(cx, s, ty)?;
return Ok(());
}
write!(f, "{:?}: {}", s.erase_tag(), self.layout.ty)
write!(f, "{}: {}", s.erase_tag(), self.layout.ty)
}
Immediate::ScalarPair(a, b) => {
// FIXME(oli-obk): at least print tuples and slices nicely
write!(f, "({:?}, {:?}): {}", a.erase_tag(), b.erase_tag(), self.layout.ty,)
write!(f, "({}, {}): {}", a.erase_tag(), b.erase_tag(), self.layout.ty,)
}
}
})
Expand Down
29 changes: 18 additions & 11 deletions src/librustc_mir/util/pretty.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
use std::collections::BTreeSet;
use std::fmt::Write as _;
use std::fmt::{Debug, Display};
use std::fs;
use std::io::{self, Write};
use std::path::{Path, PathBuf};

use super::graphviz::write_mir_fn_graphviz;
use crate::transform::MirSource;
use either::Either;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_index::vec::Idx;
use rustc_middle::mir::interpret::{
read_target_uint, AllocId, Allocation, ConstValue, GlobalAlloc,
read_target_uint, AllocId, Allocation, ConstValue, GlobalAlloc, Pointer,
};
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, TyCtxt, TypeFoldable, TypeVisitor};
use rustc_target::abi::Size;
use std::collections::BTreeSet;
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
use std::fmt::Display;
use std::fmt::Write as _;
use std::fs;
use std::io::{self, Write};
use std::path::{Path, PathBuf};

const INDENT: &str = " ";
/// Alignment for lining up comments following MIR statements
Expand Down Expand Up @@ -635,7 +636,7 @@ pub fn write_allocations<'tcx>(
/// After the hex dump, an ascii dump follows, replacing all unprintable characters (control
/// characters or characters whose value is larger than 127) with a `.`
/// This also prints relocations adequately.
pub fn write_allocation<Tag, Extra>(
pub fn write_allocation<Tag: Copy + Debug, Extra>(
tcx: TyCtxt<'tcx>,
alloc: &Allocation<Tag, Extra>,
w: &mut dyn Write,
Expand Down Expand Up @@ -679,7 +680,7 @@ fn write_allocation_newline(
/// The `prefix` argument allows callers to add an arbitrary prefix before each line (even if there
/// is only one line). Note that your prefix should contain a trailing space as the lines are
/// printed directly after it.
fn write_allocation_bytes<Tag, Extra>(
fn write_allocation_bytes<Tag: Copy + Debug, Extra>(
tcx: TyCtxt<'tcx>,
alloc: &Allocation<Tag, Extra>,
w: &mut dyn Write,
Expand Down Expand Up @@ -715,14 +716,20 @@ fn write_allocation_bytes<Tag, Extra>(
if i != line_start {
write!(w, " ")?;
}
if let Some(&(_, target_id)) = alloc.relocations().get(&i) {
if let Some(&(tag, target_id)) = alloc.relocations().get(&i) {
// Memory with a relocation must be defined
let j = i.bytes_usize();
let offset =
alloc.inspect_with_undef_and_ptr_outside_interpreter(j..j + ptr_size.bytes_usize());
let offset = read_target_uint(tcx.data_layout.endian, offset).unwrap();
let offset = Size::from_bytes(offset);
let relocation_width = |bytes| bytes * 3;
let mut target = format!("{}+{}", target_id, offset);
let ptr = Pointer::new_with_tag(target_id, offset, tag);
let mut target = format!("{:?}", ptr);
if target.len() > relocation_width(ptr_size.bytes_usize() - 1) {
// This is too long, try to save some space.
target = format!("{:#?}", ptr);
}
if ((i - line_start) + ptr_size).bytes_usize() > BYTES_PER_LINE {
// This branch handles the situation where a relocation starts in the current line
// but ends in the next one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 8, align: 4) {
alloc17+0╼ 03 00 00 00 │ ╾──╼....
─a17+0x0─╼ 03 00 00 00 │ ╾──╼....
}

alloc17 (size: 48, align: 4) {
0x00 │ 00 00 00 00 __ __ __ __ ╾alloc4+0─╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾alloc8+0─╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc13+0╼ 03 00 00 00 │ ....*...╾──╼....
0x00 │ 00 00 00 00 __ __ __ __ ╾─a4+0x0──╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾─a8+0x0──╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾─a13+0x0─╼ 03 00 00 00 │ ....*...╾──╼....
Comment on lines +37 to +39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmpf, for the common case (offset is 0, or otherwise small), this seems unnecessarily noisy compared to decimal (due to the prefix).

I know @RalfJung doesn't like it, but my preferred approach of printing arbitrary integers involves using a decimal digit when unambiguous (i.e. <= 9), and +0 feels unnecessary so I would skip it too.

But maybe this won't matter long-term, if we improve our ty::Const printing to move beyond the need for these (otherwise really cool!) allocation dumps. (We would need to detect if the allocations are entirely reproducible from the pretty value, and don't have e.g. initialized padding, to avoid losing information)

Copy link
Member Author

@RalfJung RalfJung May 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay introducing a special case for 0, to not print the offset at all.

But alloc13+4 and elsewhere alloc13+0x10 seems oddly inconsistent to me.

}

alloc4 (size: 0, align: 4) {}

alloc8 (size: 16, align: 4) {
alloc7+0─╼ 03 00 00 00 ╾alloc9+0─╼ 03 00 00 00 │ ╾──╼....╾──╼....
─a7+0x0──╼ 03 00 00 00 ╾─a9+0x0──╼ 03 00 00 00 │ ╾──╼....╾──╼....
}

alloc7 (size: 3, align: 1) {
Expand All @@ -54,8 +54,8 @@ alloc9 (size: 3, align: 1) {
}

alloc13 (size: 24, align: 4) {
0x00 │ ╾alloc12+0╼ 03 00 00 00 ╾alloc14+0╼ 03 00 00 00 │ ╾──╼....╾──╼....
0x10 │ ╾alloc15+0╼ 04 00 00 00 │ ╾──╼....
0x00 │ ╾─a12+0x0─╼ 03 00 00 00 ╾─a14+0x0─╼ 03 00 00 00 │ ╾──╼....╾──╼....
0x10 │ ╾─a15+0x0─╼ 04 00 00 00 │ ╾──╼....
}

alloc12 (size: 3, align: 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,22 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 16, align: 8) {
╾─────alloc17+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
╾─────alloc17+0x0─────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
}

alloc17 (size: 72, align: 8) {
0x00 │ 00 00 00 00 __ __ __ __ ╾─────alloc4+0───────╼ │ ....░░░░╾──────╼
0x00 │ 00 00 00 00 __ __ __ __ ╾─────alloc4+0x0──────╼ │ ....░░░░╾──────╼
0x10 │ 00 00 00 00 00 00 00 00 00 00 00 00 __ __ __ __ │ ............░░░░
0x20 │ ╾─────alloc8+0───────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾─────alloc13+0──────╼ │ ....*...╾──────╼
0x20 │ ╾─────alloc8+0x0──────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾─────alloc13+0x0─────╼ │ ....*...╾──────╼
0x40 │ 03 00 00 00 00 00 00 00 │ ........
}

alloc4 (size: 0, align: 8) {}

alloc8 (size: 32, align: 8) {
0x00 │ ╾─────alloc7+0───────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x10 │ ╾─────alloc9+0───────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x00 │ ╾─────alloc7+0x0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x10 │ ╾─────alloc9+0x0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
}

alloc7 (size: 3, align: 1) {
Expand All @@ -57,9 +57,9 @@ alloc9 (size: 3, align: 1) {
}

alloc13 (size: 48, align: 8) {
0x00 │ ╾─────alloc12+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x10 │ ╾─────alloc14+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x20 │ ╾─────alloc15+0──────╼ 04 00 00 00 00 00 00 00 │ ╾──────╼........
0x00 │ ╾─────alloc12+0x0─────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x10 │ ╾─────alloc14+0x0─────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
0x20 │ ╾─────alloc15+0x0─────╼ 04 00 00 00 00 00 00 00 │ ╾──────╼........
}

alloc12 (size: 3, align: 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 8, align: 4) {
alloc21+0╼ 03 00 00 00 │ ╾──╼....
─a21+0x0─╼ 03 00 00 00 │ ╾──╼....
}

alloc21 (size: 48, align: 4) {
0x00 │ 00 00 00 00 __ __ __ __ ╾alloc4+0─╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾alloc9+0─╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾alloc19+0╼ 03 00 00 00 │ ....*...╾──╼....
0x00 │ 00 00 00 00 __ __ __ __ ╾─a4+0x0──╼ 00 00 00 00 │ ....░░░░╾──╼....
0x10 │ 00 00 00 00 __ __ __ __ ╾─a9+0x0──╼ 02 00 00 00 │ ....░░░░╾──╼....
0x20 │ 01 00 00 00 2a 00 00 00 ╾─a19+0x0─╼ 03 00 00 00 │ ....*...╾──╼....
}

alloc4 (size: 0, align: 4) {}

alloc9 (size: 8, align: 4) {
alloc7+0─╼ ╾alloc8+0─╼ │ ╾──╼╾──╼
─a7+0x0──╼ ╾─a8+0x0──╼ │ ╾──╼╾──╼
}

alloc7 (size: 1, align: 1) {
Expand All @@ -54,7 +54,7 @@ alloc8 (size: 1, align: 1) {
}

alloc19 (size: 12, align: 4) {
alloc15+3╼ ╾alloc16+0╼ ╾alloc18+2╼ │ ╾──╼╾──╼╾──╼
─a15+0x3─╼ ╾─a16+0x0─╼ ╾─a18+0x2─╼ │ ╾──╼╾──╼╾──╼
}

alloc15 (size: 4, align: 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,21 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 16, align: 8) {
╾─────alloc21+0──────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
╾─────alloc21+0x0─────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........
}

alloc21 (size: 72, align: 8) {
0x00 │ 00 00 00 00 __ __ __ __ ╾─────alloc4+0───────╼ │ ....░░░░╾──────╼
0x00 │ 00 00 00 00 __ __ __ __ ╾─────alloc4+0x0──────╼ │ ....░░░░╾──────╼
0x10 │ 00 00 00 00 00 00 00 00 00 00 00 00 __ __ __ __ │ ............░░░░
0x20 │ ╾─────alloc9+0───────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾─────alloc19+0──────╼ │ ....*...╾──────╼
0x20 │ ╾─────alloc9+0x0──────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........
0x30 │ 01 00 00 00 2a 00 00 00 ╾─────alloc19+0x0─────╼ │ ....*...╾──────╼
0x40 │ 03 00 00 00 00 00 00 00 │ ........
}

alloc4 (size: 0, align: 8) {}

alloc9 (size: 16, align: 8) {
╾─────alloc7+0──────╼ ╾─────alloc8+0───────╼ │ ╾──────╼╾──────╼
╾─────alloc7+0x0──────╼ ╾─────alloc8+0x0──────╼ │ ╾──────╼╾──────╼
}

alloc7 (size: 1, align: 1) {
Expand All @@ -56,8 +56,8 @@ alloc8 (size: 1, align: 1) {
}

alloc19 (size: 24, align: 8) {
0x00 │ ╾─────alloc15+3─────╼ ╾─────alloc16+0──────╼ │ ╾──────╼╾──────╼
0x10 │ ╾─────alloc18+2──────╼ │ ╾──────╼
0x00 │ ╾─────alloc15+0x3─────╼ ╾─────alloc16+0x0─────╼ │ ╾──────╼╾──────╼
0x10 │ ╾─────alloc18+0x2─────╼ │ ╾──────╼
}

alloc15 (size: 4, align: 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,20 @@ fn main() -> () {
}

alloc0 (static: FOO, size: 4, align: 4) {
alloc9+0─╼ │ ╾──╼
─a9+0x0──╼ │ ╾──╼
}

alloc9 (size: 168, align: 1) {
0x00 │ ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab ab │ ................
0x10 │ ab ab ab ab ab ab ab ab ab ab ab ab ╾alloc4+0─╼ │ ............╾──╼
0x10 │ ab ab ab ab ab ab ab ab ab ab ab ab ╾─a4+0x0──╼ │ ............╾──╼
0x20 │ 01 ef cd ab 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
0x30 │ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
0x40 │ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
0x50 │ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
0x60 │ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
0x70 │ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
0x80 │ 00 00 00 00 00 00 00 00 00 00 ╾alloc6+0─╼ 00 00 │ ..........╾──╼..
0x90 │ ╾alloc7+99╼ 00 00 00 00 00 00 00 00 00 00 00 00 │ ╾──╼............
0x80 │ 00 00 00 00 00 00 00 00 00 00 ╾─a6+0x0──╼ 00 00 │ ..........╾──╼..
0x90 │ ╾─a7+0x63─╼ 00 00 00 00 00 00 00 00 00 00 00 00 │ ╾──╼............
0xa0 │ 00 00 00 00 00 00 00 00 │ ........
}

Expand Down
Loading