Skip to content

Commit

Permalink
Rollup merge of rust-lang#107756 - RalfJung:miri-out-of-addresses, r=…
Browse files Browse the repository at this point in the history
…oli-obk

miri: fix ICE when running out of address space

Fixes rust-lang/miri#2769
r? `@oli-obk`

I didn't add a test since that requires oli-obk/ui_test#38 (host must be 64bit and target 32bit). Also the test takes ~30s, so I am not sure if we want to have it in the test suite?
  • Loading branch information
matthiaskrgr committed Feb 7, 2023
2 parents 505d05d + 2900ba1 commit d044c1b
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
col: u32,
) -> MPlaceTy<'tcx, M::Provenance> {
let loc_details = &self.tcx.sess.opts.unstable_opts.location_detail;
// This can fail if rustc runs out of memory right here. Trying to emit an error would be
// pointless, since that would require allocating more memory than these short strings.
let file = if loc_details.file {
self.allocate_str(filename.as_str(), MemoryKind::CallerLocation, Mutability::Not)
.unwrap()
} else {
// FIXME: This creates a new allocation each time. It might be preferable to
// perform this allocation only once, and re-use the `MPlaceTy`.
// See https://github.com/rust-lang/rust/pull/89920#discussion_r730012398
self.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not)
self.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not).unwrap()
};
let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) };
let col = if loc_details.column { Scalar::from_u32(col) } else { Scalar::from_u32(0) };
Expand All @@ -95,8 +98,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.bound_type_of(self.tcx.require_lang_item(LangItem::PanicLocation, None))
.subst(*self.tcx, self.tcx.mk_substs([self.tcx.lifetimes.re_erased.into()].iter()));
let loc_layout = self.layout_of(loc_ty).unwrap();
// This can fail if rustc runs out of memory right here. Trying to emit an error would be
// pointless, since that would require allocating more memory than a Location.
let location = self.allocate(loc_layout, MemoryKind::CallerLocation).unwrap();

// Initialize fields.
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
fn adjust_alloc_base_pointer(
ecx: &InterpCx<'mir, 'tcx, Self>,
ptr: Pointer,
) -> Pointer<Self::Provenance>;
) -> InterpResult<'tcx, Pointer<Self::Provenance>>;

/// "Int-to-pointer cast"
fn ptr_from_addr_cast(
Expand Down Expand Up @@ -505,8 +505,8 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
fn adjust_alloc_base_pointer(
_ecx: &InterpCx<$mir, $tcx, Self>,
ptr: Pointer<AllocId>,
) -> Pointer<AllocId> {
ptr
) -> InterpResult<$tcx, Pointer<AllocId>> {
Ok(ptr)
}

#[inline(always)]
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
_ => {}
}
// And we need to get the provenance.
Ok(M::adjust_alloc_base_pointer(self, ptr))
M::adjust_alloc_base_pointer(self, ptr)
}

pub fn create_fn_alloc_ptr(
Expand Down Expand Up @@ -200,8 +200,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
kind: MemoryKind<M::MemoryKind>,
) -> InterpResult<'tcx, Pointer<M::Provenance>> {
let alloc = Allocation::uninit(size, align, M::PANIC_ON_ALLOC_FAIL)?;
// We can `unwrap` since `alloc` contains no pointers.
Ok(self.allocate_raw_ptr(alloc, kind).unwrap())
self.allocate_raw_ptr(alloc, kind)
}

pub fn allocate_bytes_ptr(
Expand All @@ -210,10 +209,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
align: Align,
kind: MemoryKind<M::MemoryKind>,
mutability: Mutability,
) -> Pointer<M::Provenance> {
) -> InterpResult<'tcx, Pointer<M::Provenance>> {
let alloc = Allocation::from_bytes(bytes, align, mutability);
// We can `unwrap` since `alloc` contains no pointers.
self.allocate_raw_ptr(alloc, kind).unwrap()
self.allocate_raw_ptr(alloc, kind)
}

/// This can fail only of `alloc` contains provenance.
Expand All @@ -230,7 +228,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);
let alloc = M::adjust_allocation(self, id, Cow::Owned(alloc), Some(kind))?;
self.memory.alloc_map.insert(id, (kind, alloc.into_owned()));
Ok(M::adjust_alloc_base_pointer(self, Pointer::from(id)))
M::adjust_alloc_base_pointer(self, Pointer::from(id))
}

pub fn reallocate_ptr(
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -754,8 +754,8 @@ where
str: &str,
kind: MemoryKind<M::MemoryKind>,
mutbl: Mutability,
) -> MPlaceTy<'tcx, M::Provenance> {
let ptr = self.allocate_bytes_ptr(str.as_bytes(), Align::ONE, kind, mutbl);
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
let ptr = self.allocate_bytes_ptr(str.as_bytes(), Align::ONE, kind, mutbl)?;
let meta = Scalar::from_machine_usize(u64::try_from(str.len()).unwrap(), self);
let mplace = MemPlace { ptr: ptr.into(), meta: MemPlaceMeta::Meta(meta) };

Expand All @@ -764,7 +764,7 @@ where
ty::TypeAndMut { ty: self.tcx.types.str_, mutbl },
);
let layout = self.layout_of(ty).unwrap();
MPlaceTy { mplace, layout, align: layout.align.abi }
Ok(MPlaceTy { mplace, layout, align: layout.align.abi })
}

/// Writes the aggregate to the destination.
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,10 @@ pub enum ResourceExhaustionInfo {
///
/// The exact limit is set by the `const_eval_limit` attribute.
StepLimitReached,
/// There is not enough memory to perform an allocation.
/// There is not enough memory (on the host) to perform an allocation.
MemoryExhausted,
/// The address space (of the target) is full.
AddressSpaceFull,
}

impl fmt::Display for ResourceExhaustionInfo {
Expand All @@ -447,6 +449,9 @@ impl fmt::Display for ResourceExhaustionInfo {
MemoryExhausted => {
write!(f, "tried to allocate more memory than available to compiler")
}
AddressSpaceFull => {
write!(f, "there are no more free addresses in the address space")
}
}
}
}
Expand Down
35 changes: 26 additions & 9 deletions src/tools/miri/src/intptrcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,14 @@ impl<'mir, 'tcx> GlobalStateInner {
Ok(Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)))
}

fn alloc_base_addr(ecx: &MiriInterpCx<'mir, 'tcx>, alloc_id: AllocId) -> u64 {
fn alloc_base_addr(
ecx: &MiriInterpCx<'mir, 'tcx>,
alloc_id: AllocId,
) -> InterpResult<'tcx, u64> {
let mut global_state = ecx.machine.intptrcast.borrow_mut();
let global_state = &mut *global_state;

match global_state.base_addr.entry(alloc_id) {
Ok(match global_state.base_addr.entry(alloc_id) {
Entry::Occupied(entry) => *entry.get(),
Entry::Vacant(entry) => {
// There is nothing wrong with a raw pointer being cast to an integer only after
Expand All @@ -181,7 +184,10 @@ impl<'mir, 'tcx> GlobalStateInner {
rng.gen_range(0..16)
};
// From next_base_addr + slack, round up to adjust for alignment.
let base_addr = global_state.next_base_addr.checked_add(slack).unwrap();
let base_addr = global_state
.next_base_addr
.checked_add(slack)
.ok_or_else(|| err_exhaust!(AddressSpaceFull))?;
let base_addr = Self::align_addr(base_addr, align.bytes());
entry.insert(base_addr);
trace!(
Expand All @@ -197,24 +203,33 @@ impl<'mir, 'tcx> GlobalStateInner {
// of at least 1 to avoid two allocations having the same base address.
// (The logic in `alloc_id_from_addr` assumes unique addresses, and different
// function/vtable pointers need to be distinguishable!)
global_state.next_base_addr = base_addr.checked_add(max(size.bytes(), 1)).unwrap();
global_state.next_base_addr = base_addr
.checked_add(max(size.bytes(), 1))
.ok_or_else(|| err_exhaust!(AddressSpaceFull))?;
// Even if `Size` didn't overflow, we might still have filled up the address space.
if global_state.next_base_addr > ecx.machine_usize_max() {
throw_exhaust!(AddressSpaceFull);
}
// Given that `next_base_addr` increases in each allocation, pushing the
// corresponding tuple keeps `int_to_ptr_map` sorted
global_state.int_to_ptr_map.push((base_addr, alloc_id));

base_addr
}
}
})
}

/// Convert a relative (tcx) pointer to an absolute address.
pub fn rel_ptr_to_addr(ecx: &MiriInterpCx<'mir, 'tcx>, ptr: Pointer<AllocId>) -> u64 {
pub fn rel_ptr_to_addr(
ecx: &MiriInterpCx<'mir, 'tcx>,
ptr: Pointer<AllocId>,
) -> InterpResult<'tcx, u64> {
let (alloc_id, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id);
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();
dl.overflowing_offset(base_addr, offset.bytes()).0
Ok(dl.overflowing_offset(base_addr, offset.bytes()).0)
}

/// When a pointer is used for a memory access, this computes where in which allocation the
Expand All @@ -232,7 +247,9 @@ impl<'mir, 'tcx> GlobalStateInner {
GlobalStateInner::alloc_id_from_addr(ecx, addr.bytes())?
};

let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id);
// This cannot fail: since we already have a pointer with that provenance, rel_ptr_to_addr
// must have been called in the past.
let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id).unwrap();

// Wrapping "addr - base_addr"
let dl = ecx.data_layout();
Expand Down
8 changes: 4 additions & 4 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
fn adjust_alloc_base_pointer(
ecx: &MiriInterpCx<'mir, 'tcx>,
ptr: Pointer<AllocId>,
) -> Pointer<Provenance> {
) -> InterpResult<'tcx, Pointer<Provenance>> {
if cfg!(debug_assertions) {
// The machine promises to never call us on thread-local or extern statics.
let alloc_id = ptr.provenance;
Expand All @@ -985,17 +985,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
_ => {}
}
}
let absolute_addr = intptrcast::GlobalStateInner::rel_ptr_to_addr(ecx, ptr);
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()
};
Pointer::new(
Ok(Pointer::new(
Provenance::Concrete { alloc_id: ptr.provenance, tag },
Size::from_bytes(absolute_addr),
)
))
}

#[inline(always)]
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/shims/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
0 => {
// These are "mutable" allocations as we consider them to be owned by the callee.
let name_alloc =
this.allocate_str(&name, MiriMemoryKind::Rust.into(), Mutability::Mut);
this.allocate_str(&name, MiriMemoryKind::Rust.into(), Mutability::Mut)?;
let filename_alloc =
this.allocate_str(&filename, MiriMemoryKind::Rust.into(), Mutability::Mut);
this.allocate_str(&filename, MiriMemoryKind::Rust.into(), Mutability::Mut)?;

this.write_immediate(
name_alloc.to_ref(this),
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let this = self.eval_context_mut();

// First arg: message.
let msg = this.allocate_str(msg, MiriMemoryKind::Machine.into(), Mutability::Not);
let msg = this.allocate_str(msg, MiriMemoryKind::Machine.into(), Mutability::Not)?;

// Call the lang item.
let panic = this.tcx.lang_items().panic_fn().unwrap();
Expand Down

0 comments on commit d044c1b

Please sign in to comment.