Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

interpret: on a signed deref check, mention the right pointer in the error #128482

Merged
merged 3 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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]
Expand Down
37 changes: 26 additions & 11 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand All @@ -269,17 +277,24 @@ 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 ->
[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 =
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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(_))
Expand Down
31 changes: 18 additions & 13 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::borrow::Cow;
use std::fmt::Write;

use either::Either;
use rustc_errors::codes::*;
Expand All @@ -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;

Expand Down Expand Up @@ -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),
Expand All @@ -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) => {
Expand Down
11 changes: 0 additions & 11 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 5 additions & 8 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -580,13 +580,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
ptr: Pointer<Option<M::Provenance>>,
offset_bytes: i64,
) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
// 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::<T>()` many bytes from `*src` to `*dst`.
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,15 +321,21 @@ pub trait Machine<'tcx>: Sized {
ptr: Pointer<Self::Provenance>,
) -> 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<Self::Provenance>,
size: i64,
) -> Option<(AllocId, Size, Self::ProvenanceExtra)>;

/// Called to adjust global allocations to the Provenance and AllocExtra of this machine.
Expand Down Expand Up @@ -658,6 +664,7 @@ pub macro compile_time_machine(<$tcx: lifetime>) {
fn ptr_get_alloc(
_ecx: &InterpCx<$tcx, Self>,
ptr: Pointer<CtfeProvenance>,
_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();
Expand Down
Loading
Loading