bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #101101 - RalfJung:read-pointer-as-bytes, r=oli-obk

Browse source 
interpret: make read-pointer-as-bytes a CTFE-only error with extra information

Next step in the reaction to https://github.com/rust-lang/rust/issues/99923. Also teaches Miri to implicitly strip provenance in more situations when transmuting pointers to integers, which fixes https://github.com/rust-lang/miri/issues/2456.

Pointer-to-int transmutation during CTFE now produces a message like this:
```
   = help: this code performed an operation that depends on the underlying bytes representing a pointer
   = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
```

r? ``@oli-obk``
This commit is contained in:
Dylan DPC 2022-08-30 11:26:51 +05:30 • committed by GitHub
commit 81f3841cfb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
46 changed files with 631 additions and 336 deletions
Download patch file Download diff file Expand all files Collapse all files

72
compiler/rustc_const_eval/src/interpret/validity.rs
View file

@ -20,9 +20,11 @@ use rustc_target::abi::{Abi, Scalar as ScalarAbi, Size, VariantIdx, Variants, Wr
use std::hash::Hash;
// for the validation errors
use super::UndefinedBehaviorInfo::*;
use super::{
alloc_range, CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy,
Machine, MemPlaceMeta, OpTy, Scalar, ValueVisitor,
CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy, Machine,
MemPlaceMeta, OpTy, Scalar, ValueVisitor,
};
macro_rules! throw_validation_failure {
@ -60,6 +62,7 @@ macro_rules! throw_validation_failure {
/// });
/// ```
///
/// The patterns must be of type `UndefinedBehaviorInfo`.
/// An additional expected parameter can also be added to the failure message:
///
/// ```
@ -87,7 +90,7 @@ macro_rules! try_validation {
// allocation here as this can only slow down builds that fail anyway.
Err(e) => match e.kind() {
$(
$($p)|+ =>
InterpError::UndefinedBehavior($($p)|+) =>
throw_validation_failure!(
$where,
{ $( $what_fmt ),+ } $( expected { $( $expected_fmt ),+ } )?
@ -313,8 +316,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(try_validation!(
self.ecx.read_immediate(op),
self.path,
err_unsup!(ReadPointerAsBytes) => { "(potentially part of) a pointer" } expected { "{expected}" },
err_ub!(InvalidUninitBytes(None)) => { "uninitialized memory" } expected { "{expected}" }
InvalidUninitBytes(None) => { "uninitialized memory" } expected { "{expected}" }
))
}
@ -339,18 +341,14 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let (_ty, _trait) = try_validation!(
self.ecx.get_ptr_vtable(vtable),
self.path,
err_ub!(DanglingIntPointer(..)) |
err_ub!(InvalidVTablePointer(..)) =>
DanglingIntPointer(..) |
InvalidVTablePointer(..) =>
{ "{vtable}" } expected { "a vtable pointer" },
);
// FIXME: check if the type/trait match what ty::Dynamic says?
}
ty::Slice(..) | ty::Str => {
let _len = try_validation!(
meta.unwrap_meta().to_machine_usize(self.ecx),
self.path,
err_unsup!(ReadPointerAsBytes) => { "non-integer slice length in wide pointer" },
);
let _len = meta.unwrap_meta().to_machine_usize(self.ecx)?;
// We do not check that `len * elem_size <= isize::MAX`:
// that is only required for references, and there it falls out of the
// "dereferenceable" check performed by Stacked Borrows.
@ -380,7 +378,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let size_and_align = try_validation!(
self.ecx.size_and_align_of_mplace(&place),
self.path,
err_ub!(InvalidMeta(msg)) => { "invalid {} metadata: {}", kind, msg },
InvalidMeta(msg) => { "invalid {} metadata: {}", kind, msg },
);
let (size, align) = size_and_align
// for the purpose of validity, consider foreign types to have
@ -396,21 +394,21 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
CheckInAllocMsg::InboundsTest, // will anyway be replaced by validity message
),
self.path,
err_ub!(AlignmentCheckFailed { required, has }) =>
AlignmentCheckFailed { required, has } =>
{
"an unaligned {kind} (required {} byte alignment but found {})",
required.bytes(),
has.bytes()
},
err_ub!(DanglingIntPointer(0, _)) =>
DanglingIntPointer(0, _) =>
{ "a null {kind}" },
err_ub!(DanglingIntPointer(i, _)) =>
DanglingIntPointer(i, _) =>
{ "a dangling {kind} (address {i:#x} is unallocated)" },
err_ub!(PointerOutOfBounds { .. }) =>
PointerOutOfBounds { .. } =>
{ "a dangling {kind} (going beyond the bounds of its allocation)" },
// This cannot happen during const-eval (because interning already detects
// dangling pointers), but it can happen in Miri.
err_ub!(PointerUseAfterFree(..)) =>
PointerUseAfterFree(..) =>
{ "a dangling {kind} (use-after-free)" },
);
// Do not allow pointers to uninhabited types.
@ -498,7 +496,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
try_validation!(
value.to_bool(),
self.path,
err_ub!(InvalidBool(..)) =>
InvalidBool(..) =>
{ "{:x}", value } expected { "a boolean" },
);
Ok(true)
@ -508,7 +506,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
try_validation!(
value.to_char(),
self.path,
err_ub!(InvalidChar(..)) =>
InvalidChar(..) =>
{ "{:x}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" },
);
Ok(true)
@ -567,8 +565,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let _fn = try_validation!(
self.ecx.get_ptr_fn(ptr),
self.path,
err_ub!(DanglingIntPointer(..)) |
err_ub!(InvalidFunctionPointer(..)) =>
DanglingIntPointer(..) |
InvalidFunctionPointer(..) =>
{ "{ptr}" } expected { "a function pointer" },
);
// FIXME: Check if the signature matches
@ -683,12 +681,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
Ok(try_validation!(
this.ecx.read_discriminant(op),
this.path,
err_ub!(InvalidTag(val)) =>
InvalidTag(val) =>
{ "{:x}", val } expected { "a valid enum tag" },
err_ub!(InvalidUninitBytes(None)) =>
InvalidUninitBytes(None) =>
{ "uninitialized bytes" } expected { "a valid enum tag" },
err_unsup!(ReadPointerAsBytes) =>
{ "a pointer" } expected { "a valid enum tag" },
)
.1)
})
@ -828,10 +824,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
let mplace = op.assert_mem_place(); // strings are unsized and hence never immediate
let len = mplace.len(self.ecx)?;
try_validation!(
self.ecx.read_bytes_ptr(mplace.ptr, Size::from_bytes(len)),
self.ecx.read_bytes_ptr_strip_provenance(mplace.ptr, Size::from_bytes(len)),
self.path,
err_ub!(InvalidUninitBytes(..)) => { "uninitialized data in `str`" },
err_unsup!(ReadPointerAsBytes) => { "a pointer in `str`" },
InvalidUninitBytes(..) => { "uninitialized data in `str`" },
);
}
ty::Array(tys, ..) | ty::Slice(tys)
@ -879,9 +874,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// We also accept uninit, for consistency with the slow path.
let alloc = self.ecx.get_ptr_alloc(mplace.ptr, size, mplace.align)?.expect("we already excluded size 0");
match alloc.check_bytes(alloc_range(Size::ZERO, size)) {
match alloc.get_bytes_strip_provenance() {
// In the happy case, we needn't check anything else.
Ok(()) => {}
Ok(_) => {}
// Some error happened, try to provide a more detailed description.
Err(err) => {
// For some errors we might be able to provide extra information.
@ -899,9 +894,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
throw_validation_failure!(self.path, { "uninitialized bytes" })
}
err_unsup!(ReadPointerAsBytes) => {
throw_validation_failure!(self.path, { "a pointer" } expected { "plain (non-pointer) bytes" })
}
// Propagate upwards (that will also check for unexpected errors).
_ => return Err(err),
@ -942,14 +934,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ok(()) => Ok(()),
// Pass through validation failures.
Err(err) if matches!(err.kind(), err_ub!(ValidationFailure { .. })) => Err(err),
// Also pass through InvalidProgram, those just indicate that we could not
// validate and each caller will know best what to do with them.
Err(err) if matches!(err.kind(), InterpError::InvalidProgram(_)) => Err(err),
// Avoid other errors as those do not show *where* in the value the issue lies.
Err(err) => {
// Complain about any other kind of UB error -- those are bad because we'd like to
// report them in a way that shows *where* in the value the issue lies.
Err(err) if matches!(err.kind(), InterpError::UndefinedBehavior(_)) => {
err.print_backtrace();
bug!("Unexpected error during validation: {}", err);
bug!("Unexpected Undefined Behavior error during validation: {}", err);
}
// Pass through everything else.
Err(err) => Err(err),
}
}