From 2900ba15b3f2b17808b32af050c85f86ca98c136 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 7 Feb 2023 11:39:07 +0100 Subject: [PATCH] miri: fix ICE when running out of address space --- .../interpret/intrinsics/caller_location.rs | 7 ++-- .../rustc_const_eval/src/interpret/machine.rs | 6 ++-- .../rustc_const_eval/src/interpret/memory.rs | 12 +++---- .../rustc_const_eval/src/interpret/place.rs | 6 ++-- .../rustc_middle/src/mir/interpret/error.rs | 7 +++- src/tools/miri/src/intptrcast.rs | 35 ++++++++++++++----- src/tools/miri/src/machine.rs | 8 ++--- src/tools/miri/src/shims/backtrace.rs | 4 +-- src/tools/miri/src/shims/panic.rs | 2 +- 9 files changed, 54 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs b/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs index 77c7b4bacb8..5042c6bac99 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs @@ -78,13 +78,16 @@ pub(crate) fn alloc_caller_location( 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("", MemoryKind::CallerLocation, Mutability::Not) + self.allocate_str("", 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) }; @@ -95,8 +98,6 @@ pub(crate) fn alloc_caller_location( .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. diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 76ed7b80f8d..d8087a36a7c 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -291,7 +291,7 @@ fn extern_static_base_pointer( fn adjust_alloc_base_pointer( ecx: &InterpCx<'mir, 'tcx, Self>, ptr: Pointer, - ) -> Pointer; + ) -> InterpResult<'tcx, Pointer>; /// "Int-to-pointer cast" fn ptr_from_addr_cast( @@ -505,8 +505,8 @@ fn extern_static_base_pointer( fn adjust_alloc_base_pointer( _ecx: &InterpCx<$mir, $tcx, Self>, ptr: Pointer, - ) -> Pointer { - ptr + ) -> InterpResult<$tcx, Pointer> { + Ok(ptr) } #[inline(always)] diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index a87ce0053e8..cfad930b1e5 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -171,7 +171,7 @@ pub fn global_base_pointer( _ => {} } // 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( @@ -200,8 +200,7 @@ pub fn allocate_ptr( kind: MemoryKind, ) -> InterpResult<'tcx, Pointer> { 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( @@ -210,10 +209,9 @@ pub fn allocate_bytes_ptr( align: Align, kind: MemoryKind, mutability: Mutability, - ) -> Pointer { + ) -> InterpResult<'tcx, Pointer> { 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. @@ -230,7 +228,7 @@ pub fn allocate_raw_ptr( ); 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( diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 8d4d0420cda..86786d81266 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -754,8 +754,8 @@ pub fn allocate_str( str: &str, kind: 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) }; @@ -764,7 +764,7 @@ pub fn allocate_str( 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 discriminant of the given variant. diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index bd9cd53e115..f22c0dbc60d 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -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 { @@ -447,6 +449,9 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { 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") + } } } } diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs index 618cf9df7f3..dcb18790420 100644 --- a/src/tools/miri/src/intptrcast.rs +++ b/src/tools/miri/src/intptrcast.rs @@ -162,11 +162,14 @@ pub fn ptr_from_addr_cast( 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 @@ -181,7 +184,10 @@ fn alloc_base_addr(ecx: &MiriInterpCx<'mir, 'tcx>, alloc_id: AllocId) -> u64 { 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!( @@ -197,24 +203,33 @@ fn alloc_base_addr(ecx: &MiriInterpCx<'mir, 'tcx>, alloc_id: AllocId) -> u64 { // 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) -> u64 { + pub fn rel_ptr_to_addr( + ecx: &MiriInterpCx<'mir, 'tcx>, + ptr: Pointer, + ) -> 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 @@ -232,7 +247,9 @@ pub fn abs_ptr_to_rel( 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(); diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 01a3d7550e2..8e44d4d7ade 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -971,7 +971,7 @@ fn adjust_allocation<'b>( fn adjust_alloc_base_pointer( ecx: &MiriInterpCx<'mir, 'tcx>, ptr: Pointer, - ) -> Pointer { + ) -> InterpResult<'tcx, Pointer> { if cfg!(debug_assertions) { // The machine promises to never call us on thread-local or extern statics. let alloc_id = ptr.provenance; @@ -985,17 +985,17 @@ fn adjust_alloc_base_pointer( _ => {} } } - 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)] diff --git a/src/tools/miri/src/shims/backtrace.rs b/src/tools/miri/src/shims/backtrace.rs index 15987eee537..ed1c6ebfece 100644 --- a/src/tools/miri/src/shims/backtrace.rs +++ b/src/tools/miri/src/shims/backtrace.rs @@ -190,9 +190,9 @@ fn handle_miri_resolve_frame( 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), diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index db3e42facad..0ea1137200b 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -172,7 +172,7 @@ fn start_panic(&mut self, msg: &str, unwind: StackPopUnwind) -> InterpResult<'tc 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(); -- 2.44.0