From de78cb56b2fa62b7a5cbb036d05e638dd474c6c3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 29 Jul 2024 16:40:21 +0200 Subject: [PATCH 1/3] on a signed deref check, mention the right pointer in the error --- compiler/rustc_const_eval/messages.ftl | 35 +++++--- .../src/const_eval/machine.rs | 4 +- compiler/rustc_const_eval/src/errors.rs | 31 ++++--- .../src/interpret/intrinsics.rs | 4 +- .../rustc_const_eval/src/interpret/machine.rs | 13 ++- .../rustc_const_eval/src/interpret/memory.rs | 86 ++++++++++++------- .../rustc_const_eval/src/interpret/place.rs | 10 +-- .../src/interpret/validity.rs | 2 +- .../rustc_middle/src/mir/interpret/error.rs | 7 +- .../rustc_middle/src/mir/interpret/pointer.rs | 9 +- src/tools/miri/src/alloc_addresses/mod.rs | 14 ++- .../src/borrow_tracker/stacked_borrows/mod.rs | 4 +- .../src/borrow_tracker/tree_borrows/mod.rs | 2 +- src/tools/miri/src/concurrency/data_race.rs | 4 +- src/tools/miri/src/concurrency/weak_memory.rs | 8 +- src/tools/miri/src/machine.rs | 14 +-- src/tools/miri/src/shims/backtrace.rs | 2 +- src/tools/miri/src/shims/foreign_items.rs | 18 ++-- .../miri/tests/fail-dep/libc/affinity.stderr | 4 +- .../issue-miri-1050-1.stack.stderr | 4 +- .../issue-miri-1050-1.tree.stderr | 4 +- .../out_of_bounds_project.stderr | 4 +- .../intrinsics/out_of_bounds_ptr_1.stderr | 4 +- .../fail/intrinsics/out_of_bounds_ptr_3.rs | 2 +- .../intrinsics/out_of_bounds_ptr_3.stderr | 4 +- .../ptr_offset_from_different_ints.stderr | 4 +- tests/ui/const-ptr/forbidden_slices.stderr | 4 +- tests/ui/consts/const-compare-bytes-ub.stderr | 4 +- .../consts/const-eval/raw-pointer-ub.stderr | 2 +- tests/ui/consts/const-eval/ub-nonnull.stderr | 2 +- tests/ui/consts/offset_from_ub.rs | 4 +- tests/ui/consts/offset_from_ub.stderr | 8 +- tests/ui/consts/offset_ub.rs | 2 +- tests/ui/consts/offset_ub.stderr | 10 +-- 34 files changed, 192 insertions(+), 141 deletions(-) diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index 43f405b223504..e02d6ebb18302 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/compiler/rustc_const_eval/messages.ftl @@ -88,10 +88,18 @@ const_eval_exact_div_has_remainder = exact_div: {$a} cannot be divided by {$b} without remainder const_eval_expected_inbounds_pointer = - expected {$inbounds_size -> - [0] a pointer to some allocation - [1] a pointer to 1 byte of memory - *[x] a pointer to {$inbounds_size} bytes of memory + expected a pointer to {$inbounds_size_abs -> + [0] some allocation + *[x] {$inbounds_size_is_neg -> + [false] {$inbounds_size_abs -> + [1] 1 byte of memory + *[x] {$inbounds_size_abs} bytes of memory + } + *[true] the end of {$inbounds_size_abs -> + [1] 1 byte of memory + *[x] {$inbounds_size_abs} bytes of memory + } + } } const_eval_extern_static = @@ -243,7 +251,7 @@ const_eval_offset_from_different_allocations = const_eval_offset_from_overflow = `{$name}` called when first pointer is too far ahead of second const_eval_offset_from_test = - out-of-bounds `offset_from` + out-of-bounds `offset_from` origin const_eval_offset_from_underflow = `{$name}` called when first pointer is too far before second const_eval_offset_from_unsigned_overflow = @@ -274,12 +282,19 @@ const_eval_pointer_arithmetic_test = out-of-bounds pointer arithmetic const_eval_pointer_out_of_bounds = {$bad_pointer_message}: {const_eval_expected_inbounds_pointer}, but got {$pointer} {$ptr_offset_is_neg -> [true] which points to before the beginning of the allocation - *[false] {$alloc_size_minus_ptr_offset -> - [0] which is at or beyond the end of the allocation of size {$alloc_size -> - [1] 1 byte - *[x] {$alloc_size} bytes + *[false] {$inbounds_size_is_neg -> + [true] {$ptr_offset_abs -> + [0] which is at the beginning of the allocation + *[other] which does not have enough space to the beginning of the allocation + } + *[false] {$alloc_size_minus_ptr_offset -> + [0] which is at or beyond the end of the allocation of size {$alloc_size -> + [1] 1 byte + *[x] {$alloc_size} bytes + } + [1] which is only 1 byte from the end of the allocation + *[x] which is only {$alloc_size_minus_ptr_offset} bytes from the end of the allocation } - *[x] and there are only {$alloc_size_minus_ptr_offset} bytes starting at that pointer } } const_eval_pointer_use_after_free = diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 65cbeab24ec00..901149825bfd6 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -295,7 +295,7 @@ impl<'tcx> CompileTimeInterpCx<'tcx> { ); } - match self.ptr_try_get_alloc_id(ptr) { + match self.ptr_try_get_alloc_id(ptr, 0) { Ok((alloc_id, offset, _extra)) => { let (_size, alloc_align, _kind) = self.get_alloc_info(alloc_id); @@ -510,7 +510,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { // If an allocation is created in an another const, // we don't deallocate it. - let (alloc_id, _, _) = ecx.ptr_get_alloc_id(ptr)?; + let (alloc_id, _, _) = ecx.ptr_get_alloc_id(ptr, 0)?; let is_allocated_in_another_const = matches!( ecx.tcx.try_get_global_alloc(alloc_id), Some(interpret::GlobalAlloc::Memory(_)) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 2dd8640009a69..7afb92c08ec9a 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::fmt::Write; use either::Either; use rustc_errors::codes::*; @@ -15,7 +16,7 @@ use rustc_middle::mir::interpret::{ use rustc_middle::ty::{self, Mutability, Ty}; use rustc_span::Span; use rustc_target::abi::call::AdjustForForeignAbiError; -use rustc_target::abi::{Size, WrappingRange}; +use rustc_target::abi::WrappingRange; use crate::interpret::InternKind; @@ -575,18 +576,21 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { .arg("bad_pointer_message", bad_pointer_message(msg, dcx)); } PointerOutOfBounds { alloc_id, alloc_size, ptr_offset, inbounds_size, msg } => { - diag.arg("alloc_size", alloc_size.bytes()) - .arg("inbounds_size", inbounds_size.bytes()) - .arg("bad_pointer_message", bad_pointer_message(msg, dcx)); - diag.arg( - "pointer", - Pointer::new( - Some(CtfeProvenance::from(alloc_id)), - Size::from_bytes(ptr_offset as u64), - ) - .to_string(), - ); + diag.arg("alloc_size", alloc_size.bytes()); + diag.arg("bad_pointer_message", bad_pointer_message(msg, dcx)); + diag.arg("pointer", { + let mut out = format!("{:?}", alloc_id); + if ptr_offset > 0 { + write!(out, "+{:#x}", ptr_offset).unwrap(); + } else if ptr_offset < 0 { + write!(out, "-{:#x}", ptr_offset.unsigned_abs()).unwrap(); + } + out + }); + diag.arg("inbounds_size_is_neg", inbounds_size < 0); + diag.arg("inbounds_size_abs", inbounds_size.unsigned_abs()); diag.arg("ptr_offset_is_neg", ptr_offset < 0); + diag.arg("ptr_offset_abs", ptr_offset.unsigned_abs()); diag.arg( "alloc_size_minus_ptr_offset", alloc_size.bytes().saturating_sub(ptr_offset as u64), @@ -600,7 +604,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { ); } - diag.arg("inbounds_size", inbounds_size.bytes()); + diag.arg("inbounds_size_is_neg", inbounds_size < 0); + diag.arg("inbounds_size_abs", inbounds_size.unsigned_abs()); diag.arg("bad_pointer_message", bad_pointer_message(msg, dcx)); } AlignmentCheckFailed(Misalignment { required, has }, msg) => { diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 1e3de224380b9..7db376a4a3de9 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -243,7 +243,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { let (a_offset, b_offset, is_addr) = if M::Provenance::OFFSET_IS_ADDR { (a.addr().bytes(), b.addr().bytes(), /*is_addr*/ true) } else { - match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) { + match (self.ptr_try_get_alloc_id(a, 0), self.ptr_try_get_alloc_id(b, 0)) { (Err(a), Err(b)) => { // Neither pointer points to an allocation, so they are both absolute. (a, b, /*is_addr*/ true) @@ -312,7 +312,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { }; // Check that the memory between them is dereferenceable at all, starting from the - // base pointer: `dist` is `a - b`, so it is based on `b`. + // origin pointer: `dist` is `a - b`, so it is based on `b`. self.check_ptr_access_signed(b, dist, CheckInAllocMsg::OffsetFromTest)?; // Then check that this is also dereferenceable from `a`. This ensures that they are // derived from the same allocation. diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index a82209514ec8f..4620b15d8d985 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -321,15 +321,21 @@ pub trait Machine<'tcx>: Sized { ptr: Pointer, ) -> InterpResult<'tcx>; - /// Convert a pointer with provenance into an allocation-offset pair - /// and extra provenance info. + /// Convert a pointer with provenance into an allocation-offset pair and extra provenance info. + /// `size` says how many bytes of memory are expected at that pointer. The *sign* of `size` can + /// be used to disambiguate situations where a wildcard pointer sits right in between two + /// allocations. /// - /// The returned `AllocId` must be the same as `ptr.provenance.get_alloc_id()`. + /// If `ptr.provenance.get_alloc_id()` is `Some(p)`, the returned `AllocId` must be `p`. + /// The resulting `AllocId` will just be used for that one step and the forgotten again + /// (i.e., we'll never turn the data returned here back into a `Pointer` that might be + /// stored in machine state). /// /// When this fails, that means the pointer does not point to a live allocation. fn ptr_get_alloc( ecx: &InterpCx<'tcx, Self>, ptr: Pointer, + size: i64, ) -> Option<(AllocId, Size, Self::ProvenanceExtra)>; /// Called to adjust global allocations to the Provenance and AllocExtra of this machine. @@ -658,6 +664,7 @@ pub macro compile_time_machine(<$tcx: lifetime>) { fn ptr_get_alloc( _ecx: &InterpCx<$tcx, Self>, ptr: Pointer, + _size: i64, ) -> Option<(AllocId, Size, Self::ProvenanceExtra)> { // We know `offset` is relative to the allocation, so we can use `into_parts`. let (prov, offset) = ptr.into_parts(); diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 859f30137dc4e..754bd00413caf 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -261,7 +261,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { new_align: Align, kind: MemoryKind, ) -> InterpResult<'tcx, Pointer> { - let (alloc_id, offset, _prov) = self.ptr_get_alloc_id(ptr)?; + let (alloc_id, offset, _prov) = self.ptr_get_alloc_id(ptr, 0)?; if offset.bytes() != 0 { throw_ub_custom!( fluent::const_eval_realloc_or_alloc_with_offset, @@ -291,7 +291,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { old_size_and_align: Option<(Size, Align)>, kind: MemoryKind, ) -> InterpResult<'tcx> { - let (alloc_id, offset, prov) = self.ptr_get_alloc_id(ptr)?; + let (alloc_id, offset, prov) = self.ptr_get_alloc_id(ptr, 0)?; trace!("deallocating: {alloc_id:?}"); if offset.bytes() != 0 { @@ -383,6 +383,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { ptr: Pointer>, size: Size, ) -> InterpResult<'tcx, Option<(AllocId, Size, M::ProvenanceExtra)>> { + let size = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes self.check_and_deref_ptr( ptr, size, @@ -404,6 +405,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { size: Size, msg: CheckInAllocMsg, ) -> InterpResult<'tcx> { + let size = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes self.check_and_deref_ptr(ptr, size, msg, |alloc_id, _, _| { let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?; Ok((size, align, ())) @@ -420,19 +422,17 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { size: i64, msg: CheckInAllocMsg, ) -> InterpResult<'tcx> { - if let Ok(size) = u64::try_from(size) { - self.check_ptr_access(ptr, Size::from_bytes(size), msg) - } else { - // Compute the pointer at the beginning of the range, and do the standard - // dereferenceability check from there. - let begin_ptr = ptr.wrapping_signed_offset(size, self); - self.check_ptr_access(begin_ptr, Size::from_bytes(size.unsigned_abs()), msg) - } + self.check_and_deref_ptr(ptr, size, msg, |alloc_id, _, _| { + let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?; + Ok((size, align, ())) + })?; + Ok(()) } /// Low-level helper function to check if a ptr is in-bounds and potentially return a reference /// to the allocation it points to. Supports both shared and mutable references, as the actual - /// checking is offloaded to a helper closure. + /// checking is offloaded to a helper closure. Supports signed sizes for checks "to the left" of + /// a pointer. /// /// `alloc_size` will only get called for non-zero-sized accesses. /// @@ -440,7 +440,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { fn check_and_deref_ptr( &self, ptr: Pointer>, - size: Size, + size: i64, msg: CheckInAllocMsg, alloc_size: impl FnOnce( AllocId, @@ -449,24 +449,31 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { ) -> InterpResult<'tcx, (Size, Align, T)>, ) -> InterpResult<'tcx, Option> { // Everything is okay with size 0. - if size.bytes() == 0 { + if size == 0 { return Ok(None); } - Ok(match self.ptr_try_get_alloc_id(ptr) { + Ok(match self.ptr_try_get_alloc_id(ptr, size) { Err(addr) => { // We couldn't get a proper allocation. throw_ub!(DanglingIntPointer { addr, inbounds_size: size, msg }); } Ok((alloc_id, offset, prov)) => { let (alloc_size, _alloc_align, ret_val) = alloc_size(alloc_id, offset, prov)?; - // Test bounds. - // It is sufficient to check this for the end pointer. Also check for overflow! - if offset.checked_add(size, &self.tcx).is_none_or(|end| end > alloc_size) { + let offset = offset.bytes(); + // Compute absolute begin and end of the range. + let (begin, end) = if size >= 0 { + (Some(offset), offset.checked_add(size as u64)) + } else { + (offset.checked_sub(size.unsigned_abs()), Some(offset)) + }; + // Ensure both are within bounds. + let in_bounds = begin.is_some() && end.is_some_and(|e| e <= alloc_size.bytes()); + if !in_bounds { throw_ub!(PointerOutOfBounds { alloc_id, alloc_size, - ptr_offset: self.target_usize_to_isize(offset.bytes()), + ptr_offset: self.target_usize_to_isize(offset), inbounds_size: size, msg, }) @@ -498,7 +505,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } #[inline] - fn offset_misalignment(offset: u64, align: Align) -> Option { + fn is_offset_misaligned(offset: u64, align: Align) -> Option { if offset % align.bytes() == 0 { None } else { @@ -508,8 +515,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } } - match self.ptr_try_get_alloc_id(ptr) { - Err(addr) => offset_misalignment(addr, align), + match self.ptr_try_get_alloc_id(ptr, 0) { + Err(addr) => is_offset_misaligned(addr, align), Ok((alloc_id, offset, _prov)) => { let (_size, alloc_align, kind) = self.get_alloc_info(alloc_id); if let Some(misalign) = @@ -517,14 +524,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { { Some(misalign) } else if M::Provenance::OFFSET_IS_ADDR { - // `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true. - offset_misalignment(ptr.addr().bytes(), align) + is_offset_misaligned(ptr.addr().bytes(), align) } else { // Check allocation alignment and offset alignment. if alloc_align.bytes() < align.bytes() { Some(Misalignment { has: alloc_align, required: align }) } else { - offset_misalignment(offset.bytes(), align) + is_offset_misaligned(offset.bytes(), align) } } } @@ -660,9 +666,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { size: Size, ) -> InterpResult<'tcx, Option>> { + let size_i64 = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes let ptr_and_alloc = self.check_and_deref_ptr( ptr, - size, + size_i64, CheckInAllocMsg::MemoryAccessTest, |alloc_id, offset, prov| { let alloc = self.get_alloc_raw(alloc_id)?; @@ -673,7 +680,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // accesses. That means we cannot rely on the closure above or the `Some` branch below. We // do this after `check_and_deref_ptr` to ensure some basic sanity has already been checked. if !self.memory.validation_in_progress.get() { - if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(ptr) { + if let Ok((alloc_id, ..)) = self.ptr_try_get_alloc_id(ptr, size_i64) { M::before_alloc_read(self, alloc_id)?; } } @@ -894,7 +901,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { ptr: Pointer>, ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { trace!("get_ptr_fn({:?})", ptr); - let (alloc_id, offset, _prov) = self.ptr_get_alloc_id(ptr)?; + let (alloc_id, offset, _prov) = self.ptr_get_alloc_id(ptr, 0)?; if offset.bytes() != 0 { throw_ub!(InvalidFunctionPointer(Pointer::new(alloc_id, offset))) } @@ -910,7 +917,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { expected_trait: Option<&'tcx ty::List>>, ) -> InterpResult<'tcx, Ty<'tcx>> { trace!("get_ptr_vtable({:?})", ptr); - let (alloc_id, offset, _tag) = self.ptr_get_alloc_id(ptr)?; + let (alloc_id, offset, _tag) = self.ptr_get_alloc_id(ptr, 0)?; if offset.bytes() != 0 { throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset))) } @@ -1391,7 +1398,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { Err(_) => { // Can only happen during CTFE. let ptr = scalar.to_pointer(self)?; - match self.ptr_try_get_alloc_id(ptr) { + match self.ptr_try_get_alloc_id(ptr, 0) { Ok((alloc_id, offset, _)) => { let (size, _align, _kind) = self.get_alloc_info(alloc_id); // If the pointer is out-of-bounds, it may be null. @@ -1407,6 +1414,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { /// Turning a "maybe pointer" into a proper pointer (and some information /// about where it points), or an absolute address. /// + /// `size` says how many bytes of memory are expected at that pointer. This is largely only used + /// for error messages; however, the *sign* of `size` can be used to disambiguate situations + /// where a wildcard pointer sits right in between two allocations. + /// It is almost always okay to just set the size to 0; this will be treated like a positive size + /// for handling wildcard pointers. + /// /// The result must be used immediately; it is not allowed to convert /// the returned data back into a `Pointer` and store that in machine state. /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and @@ -1414,9 +1427,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { pub fn ptr_try_get_alloc_id( &self, ptr: Pointer>, + size: i64, ) -> Result<(AllocId, Size, M::ProvenanceExtra), u64> { match ptr.into_pointer_or_addr() { - Ok(ptr) => match M::ptr_get_alloc(self, ptr) { + Ok(ptr) => match M::ptr_get_alloc(self, ptr, size) { Some((alloc_id, offset, extra)) => Ok((alloc_id, offset, extra)), None => { assert!(M::Provenance::OFFSET_IS_ADDR); @@ -1430,6 +1444,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { /// Turning a "maybe pointer" into a proper pointer (and some information about where it points). /// + /// `size` says how many bytes of memory are expected at that pointer. This is largely only used + /// for error messages; however, the *sign* of `size` can be used to disambiguate situations + /// where a wildcard pointer sits right in between two allocations. + /// It is almost always okay to just set the size to 0; this will be treated like a positive size + /// for handling wildcard pointers. + /// /// The result must be used immediately; it is not allowed to convert /// the returned data back into a `Pointer` and store that in machine state. /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and @@ -1438,12 +1458,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { pub fn ptr_get_alloc_id( &self, ptr: Pointer>, + size: i64, ) -> InterpResult<'tcx, (AllocId, Size, M::ProvenanceExtra)> { - self.ptr_try_get_alloc_id(ptr).map_err(|offset| { + self.ptr_try_get_alloc_id(ptr, size).map_err(|offset| { err_ub!(DanglingIntPointer { addr: offset, - // We don't know the actually required size. - inbounds_size: Size::ZERO, + inbounds_size: size, msg: CheckInAllocMsg::InboundsTest }) .into() diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 9f79f4c55be85..4572c4dd2205c 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -13,10 +13,9 @@ use rustc_target::abi::{Abi, Align, HasDataLayout, Size}; use tracing::{instrument, trace}; use super::{ - alloc_range, mir_assign_valid_types, throw_ub, AllocRef, AllocRefMut, CheckAlignMsg, - CtfeProvenance, ImmTy, Immediate, InterpCx, InterpResult, Machine, MemoryKind, Misalignment, - OffsetMode, OpTy, Operand, Pointer, PointerArithmetic, Projectable, Provenance, Readable, - Scalar, + alloc_range, mir_assign_valid_types, AllocRef, AllocRefMut, CheckAlignMsg, CtfeProvenance, + ImmTy, Immediate, InterpCx, InterpResult, Machine, MemoryKind, Misalignment, OffsetMode, OpTy, + Operand, Pointer, Projectable, Provenance, Readable, Scalar, }; #[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)] @@ -85,9 +84,6 @@ impl MemPlace { !meta.has_meta() || self.meta.has_meta(), "cannot use `offset_with_meta` to add metadata to a place" ); - if offset > ecx.data_layout().max_size_of_val() { - throw_ub!(PointerArithOverflow); - } let ptr = match mode { OffsetMode::Inbounds => { ecx.ptr_offset_inbounds(self.ptr, offset.bytes().try_into().unwrap())? diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index adb6ebabd73af..c8d59c5648da0 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -455,7 +455,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { }; // Proceed recursively even for ZST, no reason to skip them! // `!` is a ZST and we want to validate it. - if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr()) { + if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr(), 0) { let mut skip_recursive_check = false; if let Some(GlobalAlloc::Static(did)) = self.ecx.tcx.try_get_global_alloc(alloc_id) { diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index d2d91333ffe33..69ce3e087350b 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -334,14 +334,15 @@ pub enum UndefinedBehaviorInfo<'tcx> { alloc_size: Size, ptr_offset: i64, /// The size of the memory range that was expected to be in-bounds. - inbounds_size: Size, + inbounds_size: i64, msg: CheckInAllocMsg, }, /// Using an integer as a pointer in the wrong way. DanglingIntPointer { addr: u64, - /// The size of the memory range that was expected to be in-bounds (or 0 if we don't know). - inbounds_size: Size, + /// The size of the memory range that was expected to be in-bounds (or 0 if we need an + /// allocation but not any actual memory there, e.g. for function pointers). + inbounds_size: i64, msg: CheckInAllocMsg, }, /// Used a pointer with bad alignment. diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index 42f30c14cea88..faacc2457873a 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -181,12 +181,9 @@ impl Provenance for CtfeProvenance { fn fmt(ptr: &Pointer, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Print AllocId. fmt::Debug::fmt(&ptr.provenance.alloc_id(), f)?; // propagates `alternate` flag - // Print offset only if it is non-zero. Print it signed. - let signed_offset = ptr.offset.bytes() as i64; - if signed_offset > 0 { - write!(f, "+{:#x}", signed_offset)?; - } else if signed_offset < 0 { - write!(f, "-{:#x}", signed_offset.unsigned_abs())?; + // Print offset only if it is non-zero. + if ptr.offset.bytes() > 0 { + write!(f, "+{:#x}", ptr.offset.bytes())?; } // Print immutable status. if ptr.provenance.immutable() { diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index d0f977f81433f..81facdd34b62b 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -105,15 +105,17 @@ impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // Returns the exposed `AllocId` that corresponds to the specified addr, // or `None` if the addr is out of bounds - fn alloc_id_from_addr(&self, addr: u64) -> Option { + fn alloc_id_from_addr(&self, addr: u64, size: i64) -> Option { let ecx = self.eval_context_ref(); let global_state = ecx.machine.alloc_addresses.borrow(); assert!(global_state.provenance_mode != ProvenanceMode::Strict); + // We always search the allocation to the right of this address. So if the size is structly + // negative, we have to search for `addr-1` instead. + let addr = if size >= 0 { addr } else { addr.saturating_sub(1) }; let pos = global_state.int_to_ptr_map.binary_search_by_key(&addr, |(addr, _)| *addr); // Determine the in-bounds provenance for this pointer. - // (This is only called on an actual access, so in-bounds is the only possible kind of provenance.) let alloc_id = match pos { Ok(pos) => Some(global_state.int_to_ptr_map[pos].1), Err(0) => None, @@ -318,7 +320,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// When a pointer is used for a memory access, this computes where in which allocation the /// access is going. - fn ptr_get_alloc(&self, ptr: interpret::Pointer) -> Option<(AllocId, Size)> { + fn ptr_get_alloc( + &self, + ptr: interpret::Pointer, + size: i64, + ) -> Option<(AllocId, Size)> { let ecx = self.eval_context_ref(); let (tag, addr) = ptr.into_parts(); // addr is absolute (Tag provenance) @@ -327,7 +333,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { alloc_id } else { // A wildcard pointer. - ecx.alloc_id_from_addr(addr.bytes())? + ecx.alloc_id_from_addr(addr.bytes(), size)? }; // This cannot fail: since we already have a pointer with that provenance, adjust_alloc_root_pointer diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 1d75486a78189..3f9c991df6af6 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -673,7 +673,7 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> { // attempt to use it for a non-zero-sized access. // Dangling slices are a common case here; it's valid to get their length but with raw // pointer tagging for example all calls to get_unchecked on them are invalid. - if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr()) { + if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr(), 0) { log_creation(this, Some((alloc_id, base_offset, orig_tag)))?; // Still give it the new provenance, it got retagged after all. return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); @@ -685,7 +685,7 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> { } } - let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr())?; + let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr(), 0)?; log_creation(this, Some((alloc_id, base_offset, orig_tag)))?; trace!( diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 123d4b407fb4c..44f42d5fb9cd9 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -223,7 +223,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; trace!("Reborrow of size {:?}", ptr_size); - let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr()) { + let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr(), 0) { Ok(data) => { // Unlike SB, we *do* a proper retag for size 0 if can identify the allocation. // After all, the pointer may be lazily initialized outside this initial range. diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 2baa09bec168b..9df0d95f1f275 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -1180,7 +1180,7 @@ trait EvalContextPrivExt<'tcx>: MiriInterpCxExt<'tcx> { // We avoid `get_ptr_alloc` since we do *not* want to run the access hooks -- the actual // access will happen later. let (alloc_id, _offset, _prov) = this - .ptr_try_get_alloc_id(place.ptr()) + .ptr_try_get_alloc_id(place.ptr(), 0) .expect("there are no zero-sized atomic accesses"); if this.get_alloc_mutability(alloc_id)? == Mutability::Not { // See if this is fine. @@ -1307,7 +1307,7 @@ trait EvalContextPrivExt<'tcx>: MiriInterpCxExt<'tcx> { if let Some(data_race) = &this.machine.data_race { if data_race.race_detecting() { let size = place.layout.size; - let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr())?; + let (alloc_id, base_offset, _prov) = this.ptr_get_alloc_id(place.ptr(), 0)?; // Load and log the atomic operation. // Note that atomic loads are possible even from read-only allocations, so `get_alloc_extra_mut` is not an option. let alloc_meta = this.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap(); diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs index e92eaa8f91f75..6f4171584a8f7 100644 --- a/src/tools/miri/src/concurrency/weak_memory.rs +++ b/src/tools/miri/src/concurrency/weak_memory.rs @@ -468,7 +468,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { init: Scalar, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?; + let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr(), 0)?; if let ( crate::AllocExtra { weak_memory: Some(alloc_buffers), .. }, crate::MiriMachine { data_race: Some(global), threads, .. }, @@ -495,7 +495,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_ref(); if let Some(global) = &this.machine.data_race { - let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?; + let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr(), 0)?; if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() { if atomic == AtomicReadOrd::SeqCst { global.sc_read(&this.machine.threads); @@ -535,7 +535,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { init: Scalar, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr())?; + let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr(), 0)?; if let ( crate::AllocExtra { weak_memory: Some(alloc_buffers), .. }, crate::MiriMachine { data_race: Some(global), threads, .. }, @@ -585,7 +585,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { global.sc_read(&this.machine.threads); } let size = place.layout.size; - let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr())?; + let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr(), 0)?; if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() { let buffer = alloc_buffers .get_or_create_store_buffer(alloc_range(base_offset, size), init)?; diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index e492793a65135..5f4aa9d2f5d21 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1198,19 +1198,23 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { } } - /// Convert a pointer with provenance into an allocation-offset pair, - /// or a `None` with an absolute address if that conversion is not possible. + /// Convert a pointer with provenance into an allocation-offset pair and extra provenance info. + /// `size` says how many bytes of memory are expected at that pointer. The *sign* of `size` can + /// be used to disambiguate situations where a wildcard pointer sits right in between two + /// allocations. /// - /// This is called when a pointer is about to be used for memory access, - /// an in-bounds check, or anything else that requires knowing which allocation it points to. + /// If `ptr.provenance.get_alloc_id()` is `Some(p)`, the returned `AllocId` must be `p`. /// The resulting `AllocId` will just be used for that one step and the forgotten again /// (i.e., we'll never turn the data returned here back into a `Pointer` that might be /// stored in machine state). + /// + /// When this fails, that means the pointer does not point to a live allocation. fn ptr_get_alloc( ecx: &MiriInterpCx<'tcx>, ptr: StrictPointer, + size: i64, ) -> Option<(AllocId, Size, Self::ProvenanceExtra)> { - let rel = ecx.ptr_get_alloc(ptr); + let rel = ecx.ptr_get_alloc(ptr, size); rel.map(|(alloc_id, size)| { let tag = match ptr.provenance { diff --git a/src/tools/miri/src/shims/backtrace.rs b/src/tools/miri/src/shims/backtrace.rs index 24a4b5f26a950..42babb4c78dc5 100644 --- a/src/tools/miri/src/shims/backtrace.rs +++ b/src/tools/miri/src/shims/backtrace.rs @@ -116,7 +116,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let ptr = this.read_pointer(ptr)?; // Take apart the pointer, we need its pieces. The offset encodes the span. - let (alloc_id, offset, _prov) = this.ptr_get_alloc_id(ptr)?; + let (alloc_id, offset, _prov) = this.ptr_get_alloc_id(ptr, 0)?; // This has to be an actual global fn ptr, not a dlsym function. let fn_instance = if let Some(GlobalAlloc::Function { instance, .. }) = diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 9004f7efc8b5e..f0d8cc9a1cc23 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -278,7 +278,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { "miri_get_alloc_id" => { let [ptr] = this.check_shim(abi, Abi::Rust, link_name, args)?; let ptr = this.read_pointer(ptr)?; - let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| { + let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr, 0).map_err(|_e| { err_machine_stop!(TerminationInfo::Abort(format!( "pointer passed to `miri_get_alloc_id` must not be dangling, got {ptr:?}" ))) @@ -311,7 +311,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { "miri_static_root" => { let [ptr] = this.check_shim(abi, Abi::Rust, link_name, args)?; let ptr = this.read_pointer(ptr)?; - let (alloc_id, offset, _) = this.ptr_get_alloc_id(ptr)?; + let (alloc_id, offset, _) = this.ptr_get_alloc_id(ptr, 0)?; if offset != Size::ZERO { throw_unsup_format!( "pointer passed to `miri_static_root` must point to beginning of an allocated block" @@ -392,7 +392,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { "`miri_promise_symbolic_alignment`: pointer is not actually aligned" ); } - if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) { + if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr, 0) { let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id); // If the newly promised alignment is bigger than the native alignment of this // allocation, and bigger than the previously promised alignment, then set it. @@ -584,8 +584,8 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let n = Size::from_bytes(this.read_target_usize(n)?); // C requires that this must always be a valid pointer (C18 §7.1.4). - this.ptr_get_alloc_id(left)?; - this.ptr_get_alloc_id(right)?; + this.ptr_get_alloc_id(left, 0)?; + this.ptr_get_alloc_id(right, 0)?; let result = { let left_bytes = this.read_bytes_ptr_strip_provenance(left, n)?; @@ -612,7 +612,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let val = val as u8; // C requires that this must always be a valid pointer (C18 §7.1.4). - this.ptr_get_alloc_id(ptr)?; + this.ptr_get_alloc_id(ptr, 0)?; if let Some(idx) = this .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))? @@ -639,7 +639,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let val = val as u8; // C requires that this must always be a valid pointer (C18 §7.1.4). - this.ptr_get_alloc_id(ptr)?; + this.ptr_get_alloc_id(ptr, 0)?; let idx = this .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))? @@ -681,8 +681,8 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // C requires that this must always be a valid pointer, even if `n` is zero, so we better check that. // (This is more than Rust requires, so `mem_copy` is not sufficient.) - this.ptr_get_alloc_id(ptr_dest)?; - this.ptr_get_alloc_id(ptr_src)?; + this.ptr_get_alloc_id(ptr_dest, 0)?; + this.ptr_get_alloc_id(ptr_src, 0)?; this.mem_copy(ptr_src, ptr_dest, Size::from_bytes(n), true)?; this.write_pointer(ptr_dest, dest)?; diff --git a/src/tools/miri/tests/fail-dep/libc/affinity.stderr b/src/tools/miri/tests/fail-dep/libc/affinity.stderr index b9f79fdda89c4..38414623ccb94 100644 --- a/src/tools/miri/tests/fail-dep/libc/affinity.stderr +++ b/src/tools/miri/tests/fail-dep/libc/affinity.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: memory access failed: expected a pointer to 129 bytes of memory, but got ALLOC and there are only 128 bytes starting at that pointer +error: Undefined Behavior: memory access failed: expected a pointer to 129 bytes of memory, but got ALLOC which is only 128 bytes from the end of the allocation --> $DIR/affinity.rs:LL:CC | LL | let err = unsafe { sched_setaffinity(PID, size_of::() + 1, &cpuset) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 129 bytes of memory, but got ALLOC and there are only 128 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 129 bytes of memory, but got ALLOC which is only 128 bytes from the end of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.stack.stderr b/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.stack.stderr index 07bb3293989e7..64bbbfcd848b6 100644 --- a/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC and there are only 2 bytes starting at that pointer +error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC which is only 2 bytes from the end of the allocation --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC and there are only 2 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC which is only 2 bytes from the end of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.tree.stderr b/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.tree.stderr index 07bb3293989e7..64bbbfcd848b6 100644 --- a/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.tree.stderr +++ b/src/tools/miri/tests/fail/both_borrows/issue-miri-1050-1.tree.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC and there are only 2 bytes starting at that pointer +error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC which is only 2 bytes from the end of the allocation --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC and there are only 2 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: expected a pointer to 4 bytes of memory, but got ALLOC which is only 2 bytes from the end of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_project.stderr b/src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_project.stderr index bdd245e184948..4bfac8f96578d 100644 --- a/src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_project.stderr +++ b/src/tools/miri/tests/fail/dangling_pointers/out_of_bounds_project.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC and there are only 4 bytes starting at that pointer +error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC which is only 4 bytes from the end of the allocation --> $DIR/out_of_bounds_project.rs:LL:CC | LL | let _field = addr_of!((*ptr).2); - | ^^^^^^^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC and there are only 4 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC which is only 4 bytes from the end of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr index ddc5ae8efbc01..c6a1e4710a8cd 100644 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr +++ b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 5 bytes of memory, but got ALLOC and there are only 4 bytes starting at that pointer +error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 5 bytes of memory, but got ALLOC which is only 4 bytes from the end of the allocation --> $DIR/out_of_bounds_ptr_1.rs:LL:CC | LL | let x = unsafe { x.offset(5) }; - | ^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 5 bytes of memory, but got ALLOC and there are only 4 bytes starting at that pointer + | ^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 5 bytes of memory, but got ALLOC which is only 4 bytes from the end of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.rs b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.rs index fc9fb3d35d610..bd1d5c064c065 100644 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.rs +++ b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.rs @@ -1,6 +1,6 @@ fn main() { let v = [0i8; 4]; let x = &v as *const i8; - let x = unsafe { x.offset(-1) }; //~ERROR: expected a pointer to 1 byte of memory + let x = unsafe { x.offset(-1) }; //~ERROR: expected a pointer to the end of 1 byte of memory panic!("this should never print: {:?}", x); } diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr index 88963e712f4cc..ba7615da1deaa 100644 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr +++ b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 1 byte of memory, but got ALLOC-0x1 which points to before the beginning of the allocation +error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to the end of 1 byte of memory, but got ALLOC which is at the beginning of the allocation --> $DIR/out_of_bounds_ptr_3.rs:LL:CC | LL | let x = unsafe { x.offset(-1) }; - | ^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 1 byte of memory, but got ALLOC-0x1 which points to before the beginning of the allocation + | ^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to the end of 1 byte of memory, but got ALLOC which is at the beginning of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr index 649075dbc55de..bf36c54ac781c 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: out-of-bounds `offset_from`: expected a pointer to 1 byte of memory, but got 0xa[noalloc] which is a dangling pointer (it has no provenance) +error: Undefined Behavior: out-of-bounds `offset_from` origin: expected a pointer to the end of 1 byte of memory, but got 0xb[noalloc] which is a dangling pointer (it has no provenance) --> $DIR/ptr_offset_from_different_ints.rs:LL:CC | LL | let _ = p1.byte_offset_from(p2); - | ^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: expected a pointer to 1 byte of memory, but got 0xa[noalloc] which is a dangling pointer (it has no provenance) + | ^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from` origin: expected a pointer to the end of 1 byte of memory, but got 0xb[noalloc] which is a dangling pointer (it has no provenance) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/tests/ui/const-ptr/forbidden_slices.stderr b/tests/ui/const-ptr/forbidden_slices.stderr index 034e8bd185264..fad078ad2b2bd 100644 --- a/tests/ui/const-ptr/forbidden_slices.stderr +++ b/tests/ui/const-ptr/forbidden_slices.stderr @@ -118,7 +118,7 @@ LL | pub static R1: &[()] = unsafe { from_ptr_range(ptr::null()..ptr::null()) }; error[E0080]: could not evaluate static initializer --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC10 and there are only 4 bytes starting at that pointer + = note: out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC10 which is only 4 bytes from the end of the allocation | note: inside `std::ptr::const_ptr::::add` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -177,7 +177,7 @@ LL | pub static R7: &[u16] = unsafe { error[E0080]: could not evaluate static initializer --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC11+0x1 and there are only 7 bytes starting at that pointer + = note: out-of-bounds pointer arithmetic: expected a pointer to 8 bytes of memory, but got ALLOC11+0x1 which is only 7 bytes from the end of the allocation | note: inside `std::ptr::const_ptr::::add` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL diff --git a/tests/ui/consts/const-compare-bytes-ub.stderr b/tests/ui/consts/const-compare-bytes-ub.stderr index 8a923779a5bb4..7f83dee640969 100644 --- a/tests/ui/consts/const-compare-bytes-ub.stderr +++ b/tests/ui/consts/const-compare-bytes-ub.stderr @@ -20,13 +20,13 @@ error[E0080]: evaluation of constant value failed --> $DIR/const-compare-bytes-ub.rs:22:9 | LL | compare_bytes([1, 2, 3].as_ptr(), [1, 2, 3, 4].as_ptr(), 4) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 4 bytes of memory, but got ALLOC0 and there are only 3 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 4 bytes of memory, but got ALLOC0 which is only 3 bytes from the end of the allocation error[E0080]: evaluation of constant value failed --> $DIR/const-compare-bytes-ub.rs:26:9 | LL | compare_bytes([1, 2, 3, 4].as_ptr(), [1, 2, 3].as_ptr(), 4) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 4 bytes of memory, but got ALLOC1 and there are only 3 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: expected a pointer to 4 bytes of memory, but got ALLOC1 which is only 3 bytes from the end of the allocation error[E0080]: evaluation of constant value failed --> $DIR/const-compare-bytes-ub.rs:30:9 diff --git a/tests/ui/consts/const-eval/raw-pointer-ub.stderr b/tests/ui/consts/const-eval/raw-pointer-ub.stderr index 5fce25701bdd1..aeb46725c0642 100644 --- a/tests/ui/consts/const-eval/raw-pointer-ub.stderr +++ b/tests/ui/consts/const-eval/raw-pointer-ub.stderr @@ -35,7 +35,7 @@ error[E0080]: evaluation of constant value failed --> $DIR/raw-pointer-ub.rs:41:16 | LL | let _val = *ptr; - | ^^^^ memory access failed: expected a pointer to 8 bytes of memory, but got ALLOC0 and there are only 4 bytes starting at that pointer + | ^^^^ memory access failed: expected a pointer to 8 bytes of memory, but got ALLOC0 which is only 4 bytes from the end of the allocation error: aborting due to 5 previous errors diff --git a/tests/ui/consts/const-eval/ub-nonnull.stderr b/tests/ui/consts/const-eval/ub-nonnull.stderr index fe3060dda17df..0e4926eb49edf 100644 --- a/tests/ui/consts/const-eval/ub-nonnull.stderr +++ b/tests/ui/consts/const-eval/ub-nonnull.stderr @@ -13,7 +13,7 @@ error[E0080]: evaluation of constant value failed --> $DIR/ub-nonnull.rs:20:29 | LL | let out_of_bounds_ptr = &ptr[255]; - | ^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 255 bytes of memory, but got ALLOC1 and there are only 1 bytes starting at that pointer + | ^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 255 bytes of memory, but got ALLOC1 which is only 1 byte from the end of the allocation error[E0080]: it is undefined behavior to use this value --> $DIR/ub-nonnull.rs:24:1 diff --git a/tests/ui/consts/offset_from_ub.rs b/tests/ui/consts/offset_from_ub.rs index 66bb056ceb049..7efc5dd3e28ba 100644 --- a/tests/ui/consts/offset_from_ub.rs +++ b/tests/ui/consts/offset_from_ub.rs @@ -1,4 +1,4 @@ -//@ normalize-stderr-test: "to \d+ bytes of memory" -> "to $$BYTES bytes of memory" +//@ normalize-stderr-test: "\d+ bytes" -> "$$BYTES bytes" #![feature(const_ptr_sub_ptr)] #![feature(core_intrinsics)] @@ -55,7 +55,7 @@ const OUT_OF_BOUNDS_2: isize = { let end_ptr = (start_ptr).wrapping_add(length); // Second ptr is out of bounds unsafe { ptr_offset_from(start_ptr, end_ptr) } //~ERROR evaluation of constant value failed - //~| expected a pointer to 10 bytes of memory + //~| expected a pointer to the end of 10 bytes of memory }; pub const DIFFERENT_ALLOC_UNSIGNED: usize = { diff --git a/tests/ui/consts/offset_from_ub.stderr b/tests/ui/consts/offset_from_ub.stderr index f2f27735630fe..ac4597ff0119a 100644 --- a/tests/ui/consts/offset_from_ub.stderr +++ b/tests/ui/consts/offset_from_ub.stderr @@ -27,19 +27,19 @@ error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:39:14 | LL | unsafe { ptr_offset_from(ptr2, ptr1) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: expected a pointer to $BYTES bytes of memory, but got 0x8[noalloc] which is a dangling pointer (it has no provenance) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from` origin: expected a pointer to $BYTES bytes of memory, but got 0x8[noalloc] which is a dangling pointer (it has no provenance) error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:48:14 | LL | unsafe { ptr_offset_from(end_ptr, start_ptr) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: expected a pointer to $BYTES bytes of memory, but got ALLOC0 and there are only 4 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from` origin: expected a pointer to $BYTES bytes of memory, but got ALLOC0 which is only $BYTES bytes from the end of the allocation error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:57:14 | LL | unsafe { ptr_offset_from(start_ptr, end_ptr) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: expected a pointer to $BYTES bytes of memory, but got ALLOC1 and there are only 4 bytes starting at that pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from` origin: expected a pointer to the end of $BYTES bytes of memory, but got ALLOC1+0xa which does not have enough space to the beginning of the allocation error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:66:14 @@ -80,7 +80,7 @@ LL | unsafe { ptr_offset_from_unsigned(ptr2, ptr1) } error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds `offset_from`: expected a pointer to $BYTES bytes of memory, but got a null pointer + = note: out-of-bounds `offset_from` origin: expected a pointer to $BYTES bytes of memory, but got a null pointer | note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL diff --git a/tests/ui/consts/offset_ub.rs b/tests/ui/consts/offset_ub.rs index b239b91e11cf6..5026d9a271322 100644 --- a/tests/ui/consts/offset_ub.rs +++ b/tests/ui/consts/offset_ub.rs @@ -2,7 +2,7 @@ use std::ptr; //@ normalize-stderr-test: "0xf+" -> "0xf..f" //@ normalize-stderr-test: "0x7f+" -> "0x7f..f" -//@ normalize-stderr-test: "to \d+ bytes of memory" -> "to $$BYTES bytes of memory" +//@ normalize-stderr-test: "\d+ bytes" -> "$$BYTES bytes" pub const BEFORE_START: *const u8 = unsafe { (&0u8 as *const u8).offset(-1) }; //~NOTE diff --git a/tests/ui/consts/offset_ub.stderr b/tests/ui/consts/offset_ub.stderr index b42d9482f8a01..4a9ffb46a3feb 100644 --- a/tests/ui/consts/offset_ub.stderr +++ b/tests/ui/consts/offset_ub.stderr @@ -14,7 +14,7 @@ LL | pub const BEFORE_START: *const u8 = unsafe { (&0u8 as *const u8).offset(-1) error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC0 and there are only 1 bytes starting at that pointer + = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC0 which is only 1 byte from the end of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -27,7 +27,7 @@ LL | pub const AFTER_END: *const u8 = unsafe { (&0u8 as *const u8).offset(2) }; error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC1 and there are only 100 bytes starting at that pointer + = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC1 which is only $BYTES bytes from the end of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -92,7 +92,7 @@ LL | pub const UNDERFLOW_ADDRESS_SPACE: *const u8 = unsafe { (1 as *const u8).of error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC2-0x4 which points to before the beginning of the allocation + = note: out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got ALLOC2-0x2 which points to before the beginning of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -105,7 +105,7 @@ LL | pub const NEGATIVE_OFFSET: *const u8 = unsafe { [0u8; 1].as_ptr().wrapping_ error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to 1 byte of memory, but got ALLOC3 which is at or beyond the end of the allocation of size 0 bytes + = note: out-of-bounds pointer arithmetic: expected a pointer to 1 byte of memory, but got ALLOC3 which is at or beyond the end of the allocation of size $BYTES bytes | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -131,7 +131,7 @@ LL | pub const DANGLING: *const u8 = unsafe { ptr::NonNull::::dangling().as_ error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got 0x7f..f[noalloc] which is a dangling pointer (it has no provenance) + = note: out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got 0xf..f[noalloc] which is a dangling pointer (it has no provenance) | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL From 5d5c97aad7cd2803b6dbecdd34293616065dc6b2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Aug 2024 10:19:13 +0200 Subject: [PATCH 2/3] interpret: simplify pointer arithmetic logic --- compiler/rustc_abi/src/lib.rs | 6 +- .../src/interpret/eval_context.rs | 11 --- .../src/interpret/intrinsics.rs | 9 +- .../rustc_const_eval/src/interpret/memory.rs | 2 +- .../rustc_const_eval/src/interpret/place.rs | 6 +- .../rustc_const_eval/src/interpret/step.rs | 2 +- compiler/rustc_lint/src/types.rs | 6 +- .../rustc_middle/src/mir/interpret/pointer.rs | 98 +++---------------- .../rustc_middle/src/mir/interpret/value.rs | 2 +- compiler/rustc_middle/src/ty/consts/int.rs | 4 +- compiler/rustc_middle/src/ty/util.rs | 2 +- src/tools/miri/src/alloc_addresses/mod.rs | 24 ++--- src/tools/miri/src/helpers.rs | 6 +- src/tools/miri/src/shims/foreign_items.rs | 4 +- src/tools/miri/src/shims/unix/env.rs | 2 +- src/tools/miri/src/shims/unix/fs.rs | 2 +- src/tools/miri/src/shims/unix/linux/mem.rs | 2 +- .../fail/intrinsics/out_of_bounds_ptr_2.rs | 6 -- .../intrinsics/out_of_bounds_ptr_2.stderr | 15 --- .../intrinsics/ptr_offset_int_plus_ptr.rs | 2 +- ...s_ptr_1.rs => ptr_offset_out_of_bounds.rs} | 1 - ...stderr => ptr_offset_out_of_bounds.stderr} | 6 +- ...r_3.rs => ptr_offset_out_of_bounds_neg.rs} | 0 ...rr => ptr_offset_out_of_bounds_neg.stderr} | 6 +- .../fail/intrinsics/ptr_offset_overflow.rs | 9 +- .../intrinsics/ptr_offset_overflow.stderr | 13 ++- tests/ui/consts/offset_ub.stderr | 14 +-- 27 files changed, 73 insertions(+), 187 deletions(-) delete mode 100644 src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.rs delete mode 100644 src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.stderr rename src/tools/miri/tests/fail/intrinsics/{out_of_bounds_ptr_1.rs => ptr_offset_out_of_bounds.rs} (72%) rename src/tools/miri/tests/fail/intrinsics/{out_of_bounds_ptr_1.stderr => ptr_offset_out_of_bounds.stderr} (85%) rename src/tools/miri/tests/fail/intrinsics/{out_of_bounds_ptr_3.rs => ptr_offset_out_of_bounds_neg.rs} (100%) rename src/tools/miri/tests/fail/intrinsics/{out_of_bounds_ptr_3.stderr => ptr_offset_out_of_bounds_neg.stderr} (84%) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 378af8af50ec1..3dc548c4554a3 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -516,7 +516,7 @@ impl Size { /// Truncates `value` to `self` bits and then sign-extends it to 128 bits /// (i.e., if it is negative, fill with 1's on the left). #[inline] - pub fn sign_extend(self, value: u128) -> u128 { + pub fn sign_extend(self, value: u128) -> i128 { let size = self.bits(); if size == 0 { // Truncated until nothing is left. @@ -526,7 +526,7 @@ impl Size { let shift = 128 - size; // Shift the unsigned value to the left, then shift back to the right as signed // (essentially fills with sign bit on the left). - (((value << shift) as i128) >> shift) as u128 + ((value << shift) as i128) >> shift } /// Truncates `value` to `self` bits. @@ -544,7 +544,7 @@ impl Size { #[inline] pub fn signed_int_min(&self) -> i128 { - self.sign_extend(1_u128 << (self.bits() - 1)) as i128 + self.sign_extend(1_u128 << (self.bits() - 1)) } #[inline] diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 85f9b2341d91c..fc063b8bab055 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -560,17 +560,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { self.frame().body } - #[inline(always)] - pub fn sign_extend(&self, value: u128, ty: TyAndLayout<'_>) -> u128 { - assert!(ty.abi.is_signed()); - ty.size.sign_extend(value) - } - - #[inline(always)] - pub fn truncate(&self, value: u128, ty: TyAndLayout<'_>) -> u128 { - ty.size.truncate(value) - } - #[inline] pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool { ty.is_freeze(*self.tcx, self.param_env) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 7db376a4a3de9..3cf3bd87d3ddb 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -206,7 +206,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } else { (val_bits >> shift_bits) | (val_bits << inv_shift_bits) }; - let truncated_bits = self.truncate(result_bits, layout_val); + let truncated_bits = layout_val.size.truncate(result_bits); let result = Scalar::from_uint(truncated_bits, layout_val.size); self.write_scalar(result, dest)?; } @@ -580,13 +580,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { ptr: Pointer>, offset_bytes: i64, ) -> InterpResult<'tcx, Pointer>> { - // We first compute the pointer with overflow checks, to get a specific error for when it - // overflows (though technically this is redundant with the following inbounds check). - let result = ptr.signed_offset(offset_bytes, self)?; // The offset must be in bounds starting from `ptr`. self.check_ptr_access_signed(ptr, offset_bytes, CheckInAllocMsg::PointerArithmeticTest)?; - // Done. - Ok(result) + // This also implies that there is no overflow, so we are done. + Ok(ptr.wrapping_signed_offset(offset_bytes, self)) } /// Copy `count*size_of::()` many bytes from `*src` to `*dst`. diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 754bd00413caf..b71e6ed8d2b65 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -473,7 +473,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { throw_ub!(PointerOutOfBounds { alloc_id, alloc_size, - ptr_offset: self.target_usize_to_isize(offset), + ptr_offset: self.sign_extend_to_target_isize(offset), inbounds_size: size, msg, }) diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 4572c4dd2205c..242f36363a58e 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -285,10 +285,8 @@ impl<'tcx, Prov: Provenance> Projectable<'tcx, Prov> for PlaceTy<'tcx, Prov> { // projections are type-checked and bounds-checked. assert!(offset + layout.size <= self.layout.size); - let new_offset = Size::from_bytes( - ecx.data_layout() - .offset(old_offset.unwrap_or(Size::ZERO).bytes(), offset.bytes())?, - ); + // Size `+`, ensures no overflow. + let new_offset = old_offset.unwrap_or(Size::ZERO) + offset; PlaceTy { place: Place::Local { local, offset: Some(new_offset), locals_addr }, diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 48433d95c5113..211a7b2300222 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -362,7 +362,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // of the first element. let elem_size = first.layout.size; let first_ptr = first.ptr(); - let rest_ptr = first_ptr.offset(elem_size, self)?; + let rest_ptr = first_ptr.wrapping_offset(elem_size, self); // No alignment requirement since `copy_op` above already checked it. self.mem_copy_repeatedly( first_ptr, diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index e9f44f3af0272..f3196cfed533e 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -309,11 +309,7 @@ fn report_bin_hex_error( ) { let (t, actually) = match ty { attr::IntType::SignedInt(t) => { - let actually = if negative { - -(size.sign_extend(val) as i128) - } else { - size.sign_extend(val) as i128 - }; + let actually = if negative { -(size.sign_extend(val)) } else { size.sign_extend(val) }; (t.name_str(), actually.to_string()) } attr::IntType::UnsignedInt(t) => { diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index faacc2457873a..6cfd07d699c3e 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -5,7 +5,7 @@ use rustc_data_structures::static_assert_size; use rustc_macros::{HashStable, TyDecodable, TyEncodable}; use rustc_target::abi::{HasDataLayout, Size}; -use super::{AllocId, InterpResult}; +use super::AllocId; //////////////////////////////////////////////////////////////////////////////// // Pointer arithmetic @@ -40,62 +40,13 @@ pub trait PointerArithmetic: HasDataLayout { } #[inline] - fn target_usize_to_isize(&self, val: u64) -> i64 { - let val = val as i64; - // Now wrap-around into the machine_isize range. - if val > self.target_isize_max() { - // This can only happen if the ptr size is < 64, so we know max_usize_plus_1 fits into - // i64. - debug_assert!(self.pointer_size().bits() < 64); - let max_usize_plus_1 = 1u128 << self.pointer_size().bits(); - val - i64::try_from(max_usize_plus_1).unwrap() - } else { - val - } - } - - /// Helper function: truncate given value-"overflowed flag" pair to pointer size and - /// update "overflowed flag" if there was an overflow. - /// This should be called by all the other methods before returning! - #[inline] - fn truncate_to_ptr(&self, (val, over): (u64, bool)) -> (u64, bool) { - let val = u128::from(val); - let max_ptr_plus_1 = 1u128 << self.pointer_size().bits(); - (u64::try_from(val % max_ptr_plus_1).unwrap(), over || val >= max_ptr_plus_1) - } - - #[inline] - fn overflowing_offset(&self, val: u64, i: u64) -> (u64, bool) { - // We do not need to check if i fits in a machine usize. If it doesn't, - // either the wrapping_add will wrap or res will not fit in a pointer. - let res = val.overflowing_add(i); - self.truncate_to_ptr(res) - } - - #[inline] - fn overflowing_signed_offset(&self, val: u64, i: i64) -> (u64, bool) { - // We need to make sure that i fits in a machine isize. - let n = i.unsigned_abs(); - if i >= 0 { - let (val, over) = self.overflowing_offset(val, n); - (val, over || i > self.target_isize_max()) - } else { - let res = val.overflowing_sub(n); - let (val, over) = self.truncate_to_ptr(res); - (val, over || i < self.target_isize_min()) - } - } - - #[inline] - fn offset<'tcx>(&self, val: u64, i: u64) -> InterpResult<'tcx, u64> { - let (res, over) = self.overflowing_offset(val, i); - if over { throw_ub!(PointerArithOverflow) } else { Ok(res) } + fn truncate_to_target_usize(&self, val: u64) -> u64 { + self.pointer_size().truncate(val.into()).try_into().unwrap() } #[inline] - fn signed_offset<'tcx>(&self, val: u64, i: i64) -> InterpResult<'tcx, u64> { - let (res, over) = self.overflowing_signed_offset(val, i); - if over { throw_ub!(PointerArithOverflow) } else { Ok(res) } + fn sign_extend_to_target_isize(&self, val: u64) -> i64 { + self.pointer_size().sign_extend(val.into()).try_into().unwrap() } } @@ -331,7 +282,7 @@ impl Pointer> { } } -impl<'tcx, Prov> Pointer { +impl Pointer { #[inline(always)] pub fn new(provenance: Prov, offset: Size) -> Self { Pointer { provenance, offset } @@ -349,43 +300,16 @@ impl<'tcx, Prov> Pointer { Pointer { provenance: f(self.provenance), ..self } } - #[inline] - pub fn offset(self, i: Size, cx: &impl HasDataLayout) -> InterpResult<'tcx, Self> { - Ok(Pointer { - offset: Size::from_bytes(cx.data_layout().offset(self.offset.bytes(), i.bytes())?), - ..self - }) - } - - #[inline] - pub fn overflowing_offset(self, i: Size, cx: &impl HasDataLayout) -> (Self, bool) { - let (res, over) = cx.data_layout().overflowing_offset(self.offset.bytes(), i.bytes()); - let ptr = Pointer { offset: Size::from_bytes(res), ..self }; - (ptr, over) - } - #[inline(always)] pub fn wrapping_offset(self, i: Size, cx: &impl HasDataLayout) -> Self { - self.overflowing_offset(i, cx).0 - } - - #[inline] - pub fn signed_offset(self, i: i64, cx: &impl HasDataLayout) -> InterpResult<'tcx, Self> { - Ok(Pointer { - offset: Size::from_bytes(cx.data_layout().signed_offset(self.offset.bytes(), i)?), - ..self - }) - } - - #[inline] - pub fn overflowing_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> (Self, bool) { - let (res, over) = cx.data_layout().overflowing_signed_offset(self.offset.bytes(), i); - let ptr = Pointer { offset: Size::from_bytes(res), ..self }; - (ptr, over) + let res = + cx.data_layout().truncate_to_target_usize(self.offset.bytes().wrapping_add(i.bytes())); + Pointer { offset: Size::from_bytes(res), ..self } } #[inline(always)] pub fn wrapping_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> Self { - self.overflowing_signed_offset(i, cx).0 + // It's wrapping anyway, so we can just cast to `u64`. + self.wrapping_offset(Size::from_bytes(i as u64), cx) } } diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index 491d7cbcfe09c..84c17b39a623e 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -393,7 +393,7 @@ impl<'tcx, Prov: Provenance> Scalar { #[inline] pub fn to_int(self, size: Size) -> InterpResult<'tcx, i128> { let b = self.to_bits(size)?; - Ok(size.sign_extend(b) as i128) + Ok(size.sign_extend(b)) } /// Converts the scalar to produce an `i8`. Fails if the scalar is a pointer. diff --git a/compiler/rustc_middle/src/ty/consts/int.rs b/compiler/rustc_middle/src/ty/consts/int.rs index 6bfdb3d973607..0024a2ae756ea 100644 --- a/compiler/rustc_middle/src/ty/consts/int.rs +++ b/compiler/rustc_middle/src/ty/consts/int.rs @@ -234,7 +234,7 @@ impl ScalarInt { let data = i.into(); // `into` performed sign extension, we have to truncate let r = Self::raw(size.truncate(data as u128), size); - (r, size.sign_extend(r.data) as i128 != data) + (r, size.sign_extend(r.data) != data) } #[inline] @@ -335,7 +335,7 @@ impl ScalarInt { #[inline] pub fn to_int(self, size: Size) -> i128 { let b = self.to_bits(size); - size.sign_extend(b) as i128 + size.sign_extend(b) } /// Converts the `ScalarInt` to i8. diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 8b6c9a4a10d65..3cf8531bb62d5 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -79,7 +79,7 @@ impl<'tcx> Discr<'tcx> { let (val, oflo) = if signed { let min = size.signed_int_min(); let max = size.signed_int_max(); - let val = size.sign_extend(self.val) as i128; + let val = size.sign_extend(self.val); assert!(n < (i128::MAX as u128)); let n = n as i128; let oflo = val > max - n; diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index 81facdd34b62b..ed955e78c3e9a 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -11,7 +11,7 @@ use rand::Rng; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_span::Span; -use rustc_target::abi::{Align, HasDataLayout, Size}; +use rustc_target::abi::{Align, Size}; use crate::{concurrency::VClock, *}; @@ -307,15 +307,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (prov, offset) = ptr.into_parts(); // offset is relative (AllocId provenance) let alloc_id = prov.alloc_id(); - let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?; - // Add offset with the right kind of pointer-overflowing arithmetic. - let dl = ecx.data_layout(); - let absolute_addr = dl.overflowing_offset(base_addr, offset.bytes()).0; - Ok(interpret::Pointer::new( + // Get a pointer to the beginning of this allocation. + let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?; + let base_ptr = interpret::Pointer::new( Provenance::Concrete { alloc_id, tag }, - Size::from_bytes(absolute_addr), - )) + Size::from_bytes(base_addr), + ); + // Add offset with the right kind of pointer-overflowing arithmetic. + Ok(base_ptr.wrapping_offset(offset, ecx)) } /// When a pointer is used for a memory access, this computes where in which allocation the @@ -341,12 +341,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let base_addr = *ecx.machine.alloc_addresses.borrow().base_addr.get(&alloc_id).unwrap(); // Wrapping "addr - base_addr" - #[allow(clippy::cast_possible_wrap)] // we want to wrap here - let neg_base_addr = (base_addr as i64).wrapping_neg(); - Some(( - alloc_id, - Size::from_bytes(ecx.overflowing_signed_offset(addr.bytes(), neg_base_addr).0), - )) + let rel_offset = ecx.truncate_to_target_usize(addr.bytes().wrapping_sub(base_addr)); + Some((alloc_id, Size::from_bytes(rel_offset))) } } diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index ba094c988e5ab..8bc8188f05314 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -606,7 +606,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } // The part between the end_ptr and the end of the place is also frozen. // So pretend there is a 0-sized `UnsafeCell` at the end. - unsafe_cell_action(&place.ptr().offset(size, this)?, Size::ZERO)?; + unsafe_cell_action(&place.ptr().wrapping_offset(size, this), Size::ZERO)?; // Done! return Ok(()); @@ -975,7 +975,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { loop { // FIXME: We are re-getting the allocation each time around the loop. // Would be nice if we could somehow "extend" an existing AllocRange. - let alloc = this.get_ptr_alloc(ptr.offset(len, this)?, size1)?.unwrap(); // not a ZST, so we will get a result + let alloc = this.get_ptr_alloc(ptr.wrapping_offset(len, this), size1)?.unwrap(); // not a ZST, so we will get a result let byte = alloc.read_integer(alloc_range(Size::ZERO, size1))?.to_u8()?; if byte == 0 { break; @@ -1039,7 +1039,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { break; } else { wchars.push(wchar_int.try_into().unwrap()); - ptr = ptr.offset(size, this)?; + ptr = ptr.wrapping_offset(size, this); } } diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index f0d8cc9a1cc23..7f6f63ff5e79b 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -622,7 +622,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { { let idx = u64::try_from(idx).unwrap(); #[allow(clippy::arithmetic_side_effects)] // idx < num, so this never wraps - let new_ptr = ptr.offset(Size::from_bytes(num - idx - 1), this)?; + let new_ptr = ptr.wrapping_offset(Size::from_bytes(num - idx - 1), this); this.write_pointer(new_ptr, dest)?; } else { this.write_null(dest)?; @@ -646,7 +646,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { .iter() .position(|&c| c == val); if let Some(idx) = idx { - let new_ptr = ptr.offset(Size::from_bytes(idx as u64), this)?; + let new_ptr = ptr.wrapping_offset(Size::from_bytes(idx as u64), this); this.write_pointer(new_ptr, dest)?; } else { this.write_null(dest)?; diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index 3b8ad65195b8a..08b9b4e8fa340 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -82,7 +82,7 @@ impl<'tcx> UnixEnvVars<'tcx> { }; // The offset is used to strip the "{name}=" part of the string. let var_ptr = var_ptr - .offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx)?; + .wrapping_offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx); Ok(Some(var_ptr)) } diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 6923b39733f0b..f5695713dd33f 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -996,7 +996,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { &this.ptr_to_mplace(entry, dirent64_layout), )?; - let name_ptr = entry.offset(Size::from_bytes(d_name_offset), this)?; + let name_ptr = entry.wrapping_offset(Size::from_bytes(d_name_offset), this); this.write_bytes_ptr(name_ptr, name_bytes.iter().copied())?; Some(entry) diff --git a/src/tools/miri/src/shims/unix/linux/mem.rs b/src/tools/miri/src/shims/unix/linux/mem.rs index c430eff0180ea..3b32612e8baf0 100644 --- a/src/tools/miri/src/shims/unix/linux/mem.rs +++ b/src/tools/miri/src/shims/unix/linux/mem.rs @@ -53,7 +53,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // We just allocated this, the access is definitely in-bounds and fits into our address space. // mmap guarantees new mappings are zero-init. this.write_bytes_ptr( - ptr.offset(Size::from_bytes(old_size), this).unwrap().into(), + ptr.wrapping_offset(Size::from_bytes(old_size), this).into(), std::iter::repeat(0u8).take(usize::try_from(increase).unwrap()), ) .unwrap(); diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.rs b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.rs deleted file mode 100644 index 0d4eea9a5bdea..0000000000000 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.rs +++ /dev/null @@ -1,6 +0,0 @@ -fn main() { - let v = [0i8; 4]; - let x = &v as *const i8; - let x = unsafe { x.offset(isize::MIN) }; //~ERROR: overflowing in-bounds pointer arithmetic - panic!("this should never print: {:?}", x); -} diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.stderr b/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.stderr deleted file mode 100644 index 97fa1f19af106..0000000000000 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_2.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error: Undefined Behavior: overflowing in-bounds pointer arithmetic - --> $DIR/out_of_bounds_ptr_2.rs:LL:CC - | -LL | let x = unsafe { x.offset(isize::MIN) }; - | ^^^^^^^^^^^^^^^^^^^^ overflowing in-bounds pointer arithmetic - | - = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior - = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - = note: BACKTRACE: - = note: inside `main` at $DIR/out_of_bounds_ptr_2.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_int_plus_ptr.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_int_plus_ptr.rs index c4b6f69dd2be6..29bf61e25c896 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_int_plus_ptr.rs +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_int_plus_ptr.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-permissive-provenance -//@normalize-stderr-test: "to \d+ bytes of memory" -> "to $$BYTES bytes of memory" +//@normalize-stderr-test: "\d+ bytes" -> "$$BYTES bytes" fn main() { let ptr = Box::into_raw(Box::new(0u32)); diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds.rs similarity index 72% rename from src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.rs rename to src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds.rs index f337090aa1e36..905fc678f6d50 100644 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.rs +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds.rs @@ -1,7 +1,6 @@ fn main() { let v = [0i8; 4]; let x = &v as *const i8; - // The error is inside another function, so we cannot match it by line let x = unsafe { x.offset(5) }; //~ERROR: expected a pointer to 5 bytes of memory panic!("this should never print: {:?}", x); } diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds.stderr similarity index 85% rename from src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr rename to src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds.stderr index c6a1e4710a8cd..c4548200f0596 100644 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_1.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 5 bytes of memory, but got ALLOC which is only 4 bytes from the end of the allocation - --> $DIR/out_of_bounds_ptr_1.rs:LL:CC + --> $DIR/ptr_offset_out_of_bounds.rs:LL:CC | LL | let x = unsafe { x.offset(5) }; | ^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 5 bytes of memory, but got ALLOC which is only 4 bytes from the end of the allocation @@ -7,12 +7,12 @@ LL | let x = unsafe { x.offset(5) }; = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information help: ALLOC was allocated here: - --> $DIR/out_of_bounds_ptr_1.rs:LL:CC + --> $DIR/ptr_offset_out_of_bounds.rs:LL:CC | LL | let v = [0i8; 4]; | ^ = note: BACKTRACE (of the first span): - = note: inside `main` at $DIR/out_of_bounds_ptr_1.rs:LL:CC + = note: inside `main` at $DIR/ptr_offset_out_of_bounds.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds_neg.rs similarity index 100% rename from src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.rs rename to src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds_neg.rs diff --git a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds_neg.stderr similarity index 84% rename from src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr rename to src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds_neg.stderr index ba7615da1deaa..8041e1542c68d 100644 --- a/src/tools/miri/tests/fail/intrinsics/out_of_bounds_ptr_3.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_out_of_bounds_neg.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to the end of 1 byte of memory, but got ALLOC which is at the beginning of the allocation - --> $DIR/out_of_bounds_ptr_3.rs:LL:CC + --> $DIR/ptr_offset_out_of_bounds_neg.rs:LL:CC | LL | let x = unsafe { x.offset(-1) }; | ^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to the end of 1 byte of memory, but got ALLOC which is at the beginning of the allocation @@ -7,12 +7,12 @@ LL | let x = unsafe { x.offset(-1) }; = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information help: ALLOC was allocated here: - --> $DIR/out_of_bounds_ptr_3.rs:LL:CC + --> $DIR/ptr_offset_out_of_bounds_neg.rs:LL:CC | LL | let v = [0i8; 4]; | ^ = note: BACKTRACE (of the first span): - = note: inside `main` at $DIR/out_of_bounds_ptr_3.rs:LL:CC + = note: inside `main` at $DIR/ptr_offset_out_of_bounds_neg.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.rs index c3db1e23b9bfb..6839431223270 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.rs +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.rs @@ -1,5 +1,8 @@ +//@normalize-stderr-test: "\d+ bytes" -> "$$BYTES bytes" + fn main() { - let v = [1i8, 2]; - let x = &v[1] as *const i8; - let _val = unsafe { x.offset(isize::MIN) }; //~ERROR: overflowing in-bounds pointer arithmetic + let v = [0i8; 4]; + let x = &v as *const i8; + let x = unsafe { x.offset(isize::MIN) }; //~ERROR: out-of-bounds pointer arithmetic + panic!("this should never print: {:?}", x); } diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.stderr index 122529c30490a..ee5aebc6eaeef 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_overflow.stderr @@ -1,12 +1,17 @@ -error: Undefined Behavior: overflowing in-bounds pointer arithmetic +error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got ALLOC which is at the beginning of the allocation --> $DIR/ptr_offset_overflow.rs:LL:CC | -LL | let _val = unsafe { x.offset(isize::MIN) }; - | ^^^^^^^^^^^^^^^^^^^^ overflowing in-bounds pointer arithmetic +LL | let x = unsafe { x.offset(isize::MIN) }; + | ^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got ALLOC which is at the beginning of the allocation | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - = note: BACKTRACE: +help: ALLOC was allocated here: + --> $DIR/ptr_offset_overflow.rs:LL:CC + | +LL | let v = [0i8; 4]; + | ^ + = note: BACKTRACE (of the first span): = note: inside `main` at $DIR/ptr_offset_overflow.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/tests/ui/consts/offset_ub.stderr b/tests/ui/consts/offset_ub.stderr index 4a9ffb46a3feb..29327569323f9 100644 --- a/tests/ui/consts/offset_ub.stderr +++ b/tests/ui/consts/offset_ub.stderr @@ -1,7 +1,7 @@ error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: overflowing in-bounds pointer arithmetic + = note: out-of-bounds pointer arithmetic: expected a pointer to the end of 1 byte of memory, but got ALLOC0 which is at the beginning of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -14,7 +14,7 @@ LL | pub const BEFORE_START: *const u8 = unsafe { (&0u8 as *const u8).offset(-1) error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC0 which is only 1 byte from the end of the allocation + = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC1 which is only 1 byte from the end of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -27,7 +27,7 @@ LL | pub const AFTER_END: *const u8 = unsafe { (&0u8 as *const u8).offset(2) }; error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC1 which is only $BYTES bytes from the end of the allocation + = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got ALLOC2 which is only $BYTES bytes from the end of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -66,7 +66,7 @@ LL | pub const UNDERFLOW: *const u16 = unsafe { [0u16; 1].as_ptr().offset(isize: error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: overflowing in-bounds pointer arithmetic + = note: out-of-bounds pointer arithmetic: expected a pointer to $BYTES bytes of memory, but got 0xf..f[noalloc] which is a dangling pointer (it has no provenance) | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -79,7 +79,7 @@ LL | pub const OVERFLOW_ADDRESS_SPACE: *const u8 = unsafe { (usize::MAX as *cons error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: overflowing in-bounds pointer arithmetic + = note: out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got 0x1[noalloc] which is a dangling pointer (it has no provenance) | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -92,7 +92,7 @@ LL | pub const UNDERFLOW_ADDRESS_SPACE: *const u8 = unsafe { (1 as *const u8).of error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got ALLOC2-0x2 which points to before the beginning of the allocation + = note: out-of-bounds pointer arithmetic: expected a pointer to the end of $BYTES bytes of memory, but got ALLOC3-0x2 which points to before the beginning of the allocation | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -105,7 +105,7 @@ LL | pub const NEGATIVE_OFFSET: *const u8 = unsafe { [0u8; 1].as_ptr().wrapping_ error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds pointer arithmetic: expected a pointer to 1 byte of memory, but got ALLOC3 which is at or beyond the end of the allocation of size $BYTES bytes + = note: out-of-bounds pointer arithmetic: expected a pointer to 1 byte of memory, but got ALLOC4 which is at or beyond the end of the allocation of size $BYTES bytes | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL From db1652e07b7e6f81692795e7cb3b8c02798b1756 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Aug 2024 14:38:58 +0200 Subject: [PATCH 3/3] fix the way we detect overflow for inbounds arithmetic (and tweak the error message) --- compiler/rustc_const_eval/messages.ftl | 2 +- .../src/interpret/operator.rs | 22 ++++++++++++------- tests/ui/consts/offset_ub.stderr | 4 ++-- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index e02d6ebb18302..94cdd021d8d9f 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/compiler/rustc_const_eval/messages.ftl @@ -277,7 +277,7 @@ const_eval_partial_pointer_copy = const_eval_partial_pointer_overwrite = unable to overwrite parts of a pointer in memory at {$ptr} const_eval_pointer_arithmetic_overflow = - overflowing in-bounds pointer arithmetic + overflowing pointer arithmetic: the total offset in bytes does not fit in an `isize` const_eval_pointer_arithmetic_test = out-of-bounds pointer arithmetic const_eval_pointer_out_of_bounds = {$bad_pointer_message}: {const_eval_expected_inbounds_pointer}, but got {$pointer} {$ptr_offset_is_neg -> diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index 3263c90ec7265..fe5869ad7fa7a 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -7,7 +7,7 @@ use rustc_middle::{bug, mir, span_bug}; use rustc_span::symbol::sym; use tracing::trace; -use super::{err_ub, throw_ub, ImmTy, InterpCx, Machine, MemPlaceMeta}; +use super::{throw_ub, ImmTy, InterpCx, Machine, MemPlaceMeta}; impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { fn three_way_compare(&self, lhs: T, rhs: T) -> ImmTy<'tcx, M::Provenance> { @@ -298,17 +298,23 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // Pointer ops that are always supported. Offset => { let ptr = left.to_scalar().to_pointer(self)?; - let offset_count = right.to_scalar().to_target_isize(self)?; let pointee_ty = left.layout.ty.builtin_deref(true).unwrap(); + let pointee_layout = self.layout_of(pointee_ty)?; + assert!(pointee_layout.abi.is_sized()); // We cannot overflow i64 as a type's size must be <= isize::MAX. - let pointee_size = i64::try_from(self.layout_of(pointee_ty)?.size.bytes()).unwrap(); - // The computed offset, in bytes, must not overflow an isize. - // `checked_mul` enforces a too small bound, but no actual allocation can be big enough for - // the difference to be noticeable. - let offset_bytes = - offset_count.checked_mul(pointee_size).ok_or(err_ub!(PointerArithOverflow))?; + let pointee_size = i64::try_from(pointee_layout.size.bytes()).unwrap(); + let pointee_size = ImmTy::from_int(pointee_size, right.layout); + // Multiply element size and element count. + let (val, overflowed) = self + .binary_op(mir::BinOp::MulWithOverflow, right, &pointee_size)? + .to_scalar_pair(); + // This must not overflow. + if overflowed.to_bool()? { + throw_ub!(PointerArithOverflow) + } + let offset_bytes = val.to_target_isize(self)?; let offset_ptr = self.ptr_offset_inbounds(ptr, offset_bytes)?; Ok(ImmTy::from_scalar(Scalar::from_maybe_pointer(offset_ptr, self), left.layout)) } diff --git a/tests/ui/consts/offset_ub.stderr b/tests/ui/consts/offset_ub.stderr index 29327569323f9..779cb9654f4d2 100644 --- a/tests/ui/consts/offset_ub.stderr +++ b/tests/ui/consts/offset_ub.stderr @@ -40,7 +40,7 @@ LL | pub const AFTER_ARRAY: *const u8 = unsafe { [0u8; 100].as_ptr().offset(101) error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: overflowing in-bounds pointer arithmetic + = note: overflowing pointer arithmetic: the total offset in bytes does not fit in an `isize` | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -53,7 +53,7 @@ LL | pub const OVERFLOW: *const u16 = unsafe { [0u16; 1].as_ptr().offset(isize:: error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: overflowing in-bounds pointer arithmetic + = note: overflowing pointer arithmetic: the total offset in bytes does not fit in an `isize` | note: inside `std::ptr::const_ptr::::offset` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL