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.

Sign up for GitHub

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: use AllocRange in UninitByteAccess #98979

Merged
merged 1 commit into from
Jul 7, 2022
Merged

interpret: use AllocRange in UninitByteAccess #98979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/validity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
err_ub!(DanglingIntPointer(0, _)) =>
{ "a null {kind}" },
err_ub!(DanglingIntPointer(i, _)) =>
{ "a dangling {kind} (address 0x{i:x} is unallocated)" },
{ "a dangling {kind} (address {i:#x} is unallocated)" },
err_ub!(PointerOutOfBounds { .. }) =>
{ "a dangling {kind} (going beyond the bounds of its allocation)" },
// This cannot happen during const-eval (because interning already detects
Expand Down Expand Up @@ -941,7 +941,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// element that byte belongs to so we can
// provide an index.
let i = usize::try_from(
access.uninit_offset.bytes() / layout.size.bytes(),
access.uninit.start.bytes() / layout.size.bytes(),
)
.unwrap();
self.path.push(PathElem::ArrayElem(i));
Expand Down
21 changes: 12 additions & 9 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ pub fn alloc_range(start: Size, size: Size) -> AllocRange {
}

impl AllocRange {
#[inline]
pub fn from(r: Range<Size>) -> Self {
alloc_range(r.start, r.end - r.start) // `Size` subtraction (overflow-checked)
}

#[inline(always)]
pub fn end(self) -> Size {
self.start + self.size // This does overflow checking.
Expand Down Expand Up @@ -1095,17 +1100,17 @@ impl InitMask {
/// Returns `Ok(())` if it's initialized. Otherwise returns a range of byte
/// indexes for the first contiguous span of the uninitialized access.
#[inline]
pub fn is_range_initialized(&self, start: Size, end: Size) -> Result<(), Range<Size>> {
pub fn is_range_initialized(&self, start: Size, end: Size) -> Result<(), AllocRange> {
if end > self.len {
return Err(self.len..end);
return Err(AllocRange::from(self.len..end));
}

let uninit_start = self.find_bit(start, end, false);

match uninit_start {
Some(uninit_start) => {
let uninit_end = self.find_bit(uninit_start, end, true).unwrap_or(end);
Err(uninit_start..uninit_end)
Err(AllocRange::from(uninit_start..uninit_end))
}
None => Ok(()),
}
Expand Down Expand Up @@ -1176,19 +1181,17 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
///
/// Returns `Ok(())` if it's initialized. Otherwise returns the range of byte
/// indexes of the first contiguous uninitialized access.
fn is_init(&self, range: AllocRange) -> Result<(), Range<Size>> {
fn is_init(&self, range: AllocRange) -> Result<(), AllocRange> {
self.init_mask.is_range_initialized(range.start, range.end()) // `Size` addition
}

/// Checks that a range of bytes is initialized. If not, returns the `InvalidUninitBytes`
/// error which will report the first range of bytes which is uninitialized.
fn check_init(&self, range: AllocRange) -> AllocResult {
self.is_init(range).map_err(|idx_range| {
self.is_init(range).map_err(|uninit_range| {
AllocError::InvalidUninitBytes(Some(UninitBytesAccess {
access_offset: range.start,
access_size: range.size,
uninit_offset: idx_range.start,
uninit_size: idx_range.end - idx_range.start, // `Size` subtraction
access: range,
uninit: uninit_range,
}))
})
}
Expand Down
90 changes: 39 additions & 51 deletions compiler/rustc_middle/src/mir/interpret/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{AllocId, ConstAlloc, Pointer, Scalar};
use super::{AllocId, AllocRange, ConstAlloc, Pointer, Scalar};

use crate::mir::interpret::ConstValue;
use crate::ty::{layout, query::TyCtxtAt, tls, FnSig, Ty, ValTree};
Expand Down Expand Up @@ -162,9 +162,9 @@ impl fmt::Display for InvalidProgramInfo<'_> {
AlreadyReported(ErrorGuaranteed { .. }) => {
write!(f, "encountered constants with type errors, stopping evaluation")
}
Layout(ref err) => write!(f, "{}", err),
FnAbiAdjustForForeignAbi(ref err) => write!(f, "{}", err),
SizeOfUnsizedType(ty) => write!(f, "size_of called on unsized type `{}`", ty),
Layout(ref err) => write!(f, "{err}"),
FnAbiAdjustForForeignAbi(ref err) => write!(f, "{err}"),
SizeOfUnsizedType(ty) => write!(f, "size_of called on unsized type `{ty}`"),
}
}
}
Expand Down Expand Up @@ -205,14 +205,10 @@ impl fmt::Display for CheckInAllocMsg {
/// Details of an access to uninitialized bytes where it is not allowed.
#[derive(Debug)]
pub struct UninitBytesAccess {
/// Location of the original memory access.
pub access_offset: Size,
/// Size of the original memory access.
pub access_size: Size,
/// Location of the first uninitialized byte that was accessed.
pub uninit_offset: Size,
/// Number of consecutive uninitialized bytes that were accessed.
pub uninit_size: Size,
/// Range of the original memory access.
pub access: AllocRange,
/// Range of the uninit memory that was encountered. (Might not be maximal.)
pub uninit: AllocRange,
}

/// Information about a size mismatch.
Expand Down Expand Up @@ -308,30 +304,28 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use UndefinedBehaviorInfo::*;
match self {
Ub(msg) => write!(f, "{}", msg),
Ub(msg) => write!(f, "{msg}"),
Unreachable => write!(f, "entering unreachable code"),
BoundsCheckFailed { ref len, ref index } => {
write!(f, "indexing out of bounds: the len is {} but the index is {}", len, index)
write!(f, "indexing out of bounds: the len is {len} but the index is {index}")
}
DivisionByZero => write!(f, "dividing by zero"),
RemainderByZero => write!(f, "calculating the remainder with a divisor of zero"),
DivisionOverflow => write!(f, "overflow in signed division (dividing MIN by -1)"),
RemainderOverflow => write!(f, "overflow in signed remainder (dividing MIN by -1)"),
PointerArithOverflow => write!(f, "overflowing in-bounds pointer arithmetic"),
InvalidMeta(msg) => write!(f, "invalid metadata in wide pointer: {}", msg),
InvalidMeta(msg) => write!(f, "invalid metadata in wide pointer: {msg}"),
InvalidVtableDropFn(sig) => write!(
f,
"invalid drop function signature: got {}, expected exactly one argument which must be a pointer type",
sig
"invalid drop function signature: got {sig}, expected exactly one argument which must be a pointer type",
),
InvalidVtableSize => {
write!(f, "invalid vtable: size is bigger than largest supported object")
}
InvalidVtableAlignment(msg) => write!(f, "invalid vtable: alignment {}", msg),
InvalidVtableAlignment(msg) => write!(f, "invalid vtable: alignment {msg}"),
UnterminatedCString(p) => write!(
f,
"reading a null-terminated string starting at {:?} with no null found before end of allocation",
p,
"reading a null-terminated string starting at {p:?} with no null found before end of allocation",
),
PointerUseAfterFree(a) => {
write!(f, "pointer to {a:?} was dereferenced after this allocation got freed")
Expand Down Expand Up @@ -359,41 +353,36 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
}
AlignmentCheckFailed { required, has } => write!(
f,
"accessing memory with alignment {}, but alignment {} is required",
has.bytes(),
required.bytes()
"accessing memory with alignment {has}, but alignment {required} is required",
has = has.bytes(),
required = required.bytes()
),
WriteToReadOnly(a) => write!(f, "writing to {a:?} which is read-only"),
DerefFunctionPointer(a) => write!(f, "accessing {a:?} which contains a function"),
ValidationFailure { path: None, msg } => {
write!(f, "constructing invalid value: {}", msg)
write!(f, "constructing invalid value: {msg}")
}
ValidationFailure { path: Some(path), msg } => {
write!(f, "constructing invalid value at {}: {}", path, msg)
write!(f, "constructing invalid value at {path}: {msg}")
}
InvalidBool(b) => {
write!(f, "interpreting an invalid 8-bit value as a bool: 0x{:02x}", b)
write!(f, "interpreting an invalid 8-bit value as a bool: 0x{b:02x}")
}
InvalidChar(c) => {
write!(f, "interpreting an invalid 32-bit value as a char: 0x{:08x}", c)
write!(f, "interpreting an invalid 32-bit value as a char: 0x{c:08x}")
}
InvalidTag(val) => write!(f, "enum value has invalid tag: {:x}", val),
InvalidTag(val) => write!(f, "enum value has invalid tag: {val:x}"),
InvalidFunctionPointer(p) => {
write!(f, "using {:?} as function pointer but it does not point to a function", p)
write!(f, "using {p:?} as function pointer but it does not point to a function")
}
InvalidStr(err) => write!(f, "this string is not valid UTF-8: {}", err),
InvalidUninitBytes(Some((alloc, access))) => write!(
InvalidStr(err) => write!(f, "this string is not valid UTF-8: {err}"),
InvalidUninitBytes(Some((alloc, info))) => write!(
f,
"reading {} byte{} of memory starting at {:?}, \
but {} byte{} {} uninitialized starting at {:?}, \
"reading memory at {alloc:?}{access:?}, \
but memory is uninitialized at {uninit:?}, \
and this operation requires initialized memory",
access.access_size.bytes(),
pluralize!(access.access_size.bytes()),
Pointer::new(*alloc, access.access_offset),
access.uninit_size.bytes(),
pluralize!(access.uninit_size.bytes()),
pluralize!("is", access.uninit_size.bytes()),
Pointer::new(*alloc, access.uninit_offset),
access = info.access,
uninit = info.uninit,
),
InvalidUninitBytes(None) => write!(
f,
Expand All @@ -402,8 +391,7 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
DeadLocal => write!(f, "accessing a dead local variable"),
ScalarSizeMismatch(self::ScalarSizeMismatch { target_size, data_size }) => write!(
f,
"scalar size mismatch: expected {} bytes but got {} bytes instead",
target_size, data_size
"scalar size mismatch: expected {target_size} bytes but got {data_size} bytes instead",
),
UninhabitedEnumVariantWritten => {
write!(f, "writing discriminant of an uninhabited enum")
Expand Down Expand Up @@ -437,13 +425,13 @@ impl fmt::Display for UnsupportedOpInfo {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use UnsupportedOpInfo::*;
match self {
Unsupported(ref msg) => write!(f, "{}", msg),
Unsupported(ref msg) => write!(f, "{msg}"),
ReadPointerAsBytes => write!(f, "unable to turn pointer into raw bytes"),
PartialPointerOverwrite(ptr) => {
write!(f, "unable to overwrite parts of a pointer in memory at {:?}", ptr)
write!(f, "unable to overwrite parts of a pointer in memory at {ptr:?}")
}
ThreadLocalStatic(did) => write!(f, "cannot access thread local static ({:?})", did),
ReadExternStatic(did) => write!(f, "cannot read from extern static ({:?})", did),
ThreadLocalStatic(did) => write!(f, "cannot access thread local static ({did:?})"),
ReadExternStatic(did) => write!(f, "cannot read from extern static ({did:?})"),
}
}
}
Expand Down Expand Up @@ -526,11 +514,11 @@ impl fmt::Display for InterpError<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use InterpError::*;
match *self {
Unsupported(ref msg) => write!(f, "{}", msg),
InvalidProgram(ref msg) => write!(f, "{}", msg),
UndefinedBehavior(ref msg) => write!(f, "{}", msg),
ResourceExhaustion(ref msg) => write!(f, "{}", msg),
MachineStop(ref msg) => write!(f, "{}", msg),
Unsupported(ref msg) => write!(f, "{msg}"),
InvalidProgram(ref msg) => write!(f, "{msg}"),
UndefinedBehavior(ref msg) => write!(f, "{msg}"),
ResourceExhaustion(ref msg) => write!(f, "{msg}"),
MachineStop(ref msg) => write!(f, "{msg}"),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/intrinsics/intrinsic-raw_eq-const-padding.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0080]: evaluation of constant value failed
--> $DIR/intrinsic-raw_eq-const-padding.rs:6:5
|
LL | std::intrinsics::raw_eq(&(1_u8, 2_u16), &(1_u8, 2_u16))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading 4 bytes of memory starting at alloc3, but 1 byte is uninitialized starting at alloc3+0x1, and this operation requires initialized memory
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading memory at alloc3[0x0..0x4], but memory is uninitialized at [0x1..0x2], and this operation requires initialized memory
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I wasn't quite sure if we want the second range to repeat the alloc3 or not? It will always be the same allocation as for the first range.

Copy link
Contributor

Choose a reason for hiding this comment

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

nope, no repeating is better. I even stumbled over memory being mentioned twice, but just using it isn't better either, so let's just do this.


error: aborting due to previous error

Expand Down