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

make InterpResult a dedicated type to avoid accidentally discarding the error

Browse source
This commit is contained in:
Ralf Jung 2024-09-29 11:53:23 +02:00
parent 4b8a5bd511
commit c4ce8c114b
102 changed files with 1588 additions and 1466 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -18,7 +18,7 @@ use rustc_middle::bug;
use rustc_middle::mir::interpret::ValidationErrorKind::{self, *};
use rustc_middle::mir::interpret::{
ExpectedKind, InterpError, InterpErrorInfo, InvalidMetaKind, Misalignment, PointerKind,
Provenance, UnsupportedOpInfo, ValidationErrorInfo, alloc_range,
Provenance, UnsupportedOpInfo, ValidationErrorInfo, alloc_range, interp_ok,
};
use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, Ty};
@ -32,7 +32,7 @@ use super::machine::AllocMap;
use super::{
AllocId, AllocKind, CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx, InterpResult,
MPlaceTy, Machine, MemPlaceMeta, PlaceTy, Pointer, Projectable, Scalar, ValueVisitor, err_ub,
format_interp_error, throw_ub,
format_interp_error,
};
// for the validation errors
@ -42,7 +42,7 @@ use super::InterpError::Unsupported as Unsup;
use super::UndefinedBehaviorInfo::*;
use super::UnsupportedOpInfo::*;
macro_rules! throw_validation_failure {
macro_rules! err_validation_failure {
($where:expr, $kind: expr) => {{
let where_ = &$where;
let path = if !where_.is_empty() {
@ -53,10 +53,16 @@ macro_rules! throw_validation_failure {
None
};
throw_ub!(ValidationError(ValidationErrorInfo { path, kind: $kind }))
err_ub!(ValidationError(ValidationErrorInfo { path, kind: $kind }))
}};
}
macro_rules! throw_validation_failure {
($where:expr, $kind: expr) => {
do yeet err_validation_failure!($where, $kind)
};
}
/// If $e throws an error matching the pattern, throw a validation failure.
/// Other errors are passed back to the caller, unchanged -- and if they reach the root of
/// the visitor, we make sure only validation errors and `InvalidProgram` errors are left.
@ -91,25 +97,22 @@ macro_rules! try_validation {
($e:expr, $where:expr,
$( $( $p:pat_param )|+ => $kind: expr ),+ $(,)?
) => {{
match $e {
Ok(x) => x,
$e.map_err(|e| {
// We catch the error and turn it into a validation failure. We are okay with
// allocation here as this can only slow down builds that fail anyway.
Err(e) => {
let (kind, backtrace) = e.into_parts();
match kind {
$(
$($p)|+ => {
throw_validation_failure!(
$where,
$kind
)
}
),+,
_ => Err::<!, _>(InterpErrorInfo::from_parts(kind, backtrace))?,
}
let (kind, backtrace) = e.into_parts();
match kind {
$(
$($p)|+ => {
err_validation_failure!(
$where,
$kind
).into()
}
),+,
_ => InterpErrorInfo::from_parts(kind, backtrace),
}
}
})?
}};
}
@ -381,7 +384,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
// Undo changes
self.path.truncate(path_len);
// Done
Ok(r)
interp_ok(r)
}
fn read_immediate(
@ -389,7 +392,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
val: &PlaceTy<'tcx, M::Provenance>,
expected: ExpectedKind,
) -> InterpResult<'tcx, ImmTy<'tcx, M::Provenance>> {
Ok(try_validation!(
interp_ok(try_validation!(
self.ecx.read_immediate(val),
self.path,
Ub(InvalidUninitBytes(None)) =>
@ -407,7 +410,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
val: &PlaceTy<'tcx, M::Provenance>,
expected: ExpectedKind,
) -> InterpResult<'tcx, Scalar<M::Provenance>> {
Ok(self.read_immediate(val, expected)?.to_scalar())
interp_ok(self.read_immediate(val, expected)?.to_scalar())
}
fn deref_pointer(
@ -472,7 +475,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
_ => bug!("Unexpected unsized type tail: {:?}", tail),
}
Ok(())
interp_ok(())
}
/// Check a reference or `Box`.
@ -635,7 +638,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
}
// Potentially skip recursive check.
if skip_recursive_check {
return Ok(());
return interp_ok(());
}
} else {
// This is not CTFE, so it's Miri with recursive checking.
@ -644,7 +647,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
// FIXME: should we also skip `UnsafeCell` behind shared references? Currently that is not
// needed since validation reads bypass Stacked Borrows and data race checks.
if matches!(ptr_kind, PointerKind::Box) {
return Ok(());
return interp_ok(());
}
}
let path = &self.path;
@ -657,7 +660,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
new_path
});
}
Ok(())
interp_ok(())
}
/// Check if this is a value of primitive type, and if yes check the validity of the value
@ -684,7 +687,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
self.ecx.clear_provenance(value)?;
self.add_data_range_place(value);
}
Ok(true)
interp_ok(true)
}
ty::Char => {
let scalar = self.read_scalar(value, ExpectedKind::Char)?;
@ -699,7 +702,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
self.ecx.clear_provenance(value)?;
self.add_data_range_place(value);
}
Ok(true)
interp_ok(true)
}
ty::Float(_) | ty::Int(_) | ty::Uint(_) => {
// NOTE: Keep this in sync with the array optimization for int/float
@ -716,18 +719,18 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
self.ecx.clear_provenance(value)?;
self.add_data_range_place(value);
}
Ok(true)
interp_ok(true)
}
ty::RawPtr(..) => {
let place = self.deref_pointer(value, ExpectedKind::RawPtr)?;
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta(), place.layout)?;
}
Ok(true)
interp_ok(true)
}
ty::Ref(_, _ty, mutbl) => {
self.check_safe_pointer(value, PointerKind::Ref(*mutbl))?;
Ok(true)
interp_ok(true)
}
ty::FnPtr(..) => {
let scalar = self.read_scalar(value, ExpectedKind::FnPtr)?;
@ -756,12 +759,12 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
}
self.add_data_range_place(value);
}
Ok(true)
interp_ok(true)
}
ty::Never => throw_validation_failure!(self.path, NeverVal),
ty::Foreign(..) | ty::FnDef(..) => {
// Nothing to check.
Ok(true)
interp_ok(true)
}
// The above should be all the primitive types. The rest is compound, we
// check them by visiting their fields/variants.
@ -774,7 +777,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
| ty::Closure(..)
| ty::Pat(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(..) => Ok(false),
| ty::Coroutine(..) => interp_ok(false),
// Some types only occur during typechecking, they have no layout.
// We should not see them here and we could not check them anyway.
ty::Error(_)
@ -811,11 +814,11 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
max_value
})
} else {
return Ok(());
return interp_ok(());
}
} else if scalar_layout.is_always_valid(self.ecx) {
// Easy. (This is reachable if `enforce_number_validity` is set.)
return Ok(());
return interp_ok(());
} else {
// Conservatively, we reject, because the pointer *could* have a bad
// value.
@ -828,7 +831,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
};
// Now compare.
if valid_range.contains(bits) {
Ok(())
interp_ok(())
} else {
throw_validation_failure!(self.path, OutOfRange {
value: format!("{bits}"),
@ -887,7 +890,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
}
fn reset_padding(&mut self, place: &PlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
let Some(data_bytes) = self.data_bytes.as_mut() else { return Ok(()) };
let Some(data_bytes) = self.data_bytes.as_mut() else { return interp_ok(()) };
// Our value must be in memory, otherwise we would not have set up `data_bytes`.
let mplace = self.ecx.force_allocation(place)?;
// Determine starting offset and size.
@ -899,14 +902,14 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
// If there is no padding at all, we can skip the rest: check for
// a single data range covering the entire value.
if data_bytes.0 == &[(start_offset, size)] {
return Ok(());
return interp_ok(());
}
// Get a handle for the allocation. Do this only once, to avoid looking up the same
// allocation over and over again. (Though to be fair, iterating the value already does
// exactly that.)
let Some(mut alloc) = self.ecx.get_ptr_alloc_mut(mplace.ptr(), size)? else {
// A ZST, no padding to clear.
return Ok(());
return interp_ok(());
};
// Add a "finalizer" data range at the end, so that the iteration below finds all gaps
// between ranges.
@ -933,7 +936,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
padding_cleared_until = offset + size;
}
assert!(padding_cleared_until == start_offset + size);
Ok(())
interp_ok(())
}
/// Computes the data range of this union type:
@ -1073,7 +1076,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
val: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx, VariantIdx> {
self.with_elem(PathElem::EnumTag, move |this| {
Ok(try_validation!(
interp_ok(try_validation!(
this.ecx.read_discriminant(val),
this.path,
Ub(InvalidTag(val)) => InvalidEnumTag {
@ -1137,7 +1140,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
data_bytes.add_range(base_offset + offset, size);
}
}
Ok(())
interp_ok(())
}
#[inline]
@ -1147,7 +1150,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
val: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx> {
self.check_safe_pointer(val, PointerKind::Box)?;
Ok(())
interp_ok(())
}
#[inline]
@ -1160,7 +1163,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
// We assume that the Scalar validity range does not restrict these values
// any further than `try_visit_primitive` does!
if self.try_visit_primitive(val)? {
return Ok(());
return interp_ok(());
}
// Special check preventing `UnsafeCell` in the inner part of constants
@ -1207,7 +1210,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
// If the size is 0, there is nothing to check.
// (`size` can only be 0 if `len` is 0, and empty arrays are always valid.)
if size == Size::ZERO {
return Ok(());
return interp_ok(());
}
// Now that we definitely have a non-ZST array, we know it lives in memory -- except it may
// be an uninitialized local variable, those are also "immediate".
@ -1227,37 +1230,33 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
// No need for an alignment check here, this is not an actual memory access.
let alloc = self.ecx.get_ptr_alloc(mplace.ptr(), size)?.expect("we already excluded size 0");
match alloc.get_bytes_strip_provenance() {
// In the happy case, we needn't check anything else.
Ok(_) => {}
alloc.get_bytes_strip_provenance().map_err(|err| {
// Some error happened, try to provide a more detailed description.
Err(err) => {
// For some errors we might be able to provide extra information.
// (This custom logic does not fit the `try_validation!` macro.)
let (kind, backtrace) = err.into_parts();
match kind {
Ub(InvalidUninitBytes(Some((_alloc_id, access)))) | Unsup(ReadPointerAsInt(Some((_alloc_id, access)))) => {
// Some byte was uninitialized, determine which
// element that byte belongs to so we can
// provide an index.
let i = usize::try_from(
access.bad.start.bytes() / layout.size.bytes(),
)
.unwrap();
self.path.push(PathElem::ArrayElem(i));
// For some errors we might be able to provide extra information.
// (This custom logic does not fit the `try_validation!` macro.)
let (kind, backtrace) = err.into_parts();
match kind {
Ub(InvalidUninitBytes(Some((_alloc_id, access)))) | Unsup(ReadPointerAsInt(Some((_alloc_id, access)))) => {
// Some byte was uninitialized, determine which
// element that byte belongs to so we can
// provide an index.
let i = usize::try_from(
access.bad.start.bytes() / layout.size.bytes(),
)
.unwrap();
self.path.push(PathElem::ArrayElem(i));
if matches!(kind, Ub(InvalidUninitBytes(_))) {
throw_validation_failure!(self.path, Uninit { expected })
} else {
throw_validation_failure!(self.path, PointerAsInt { expected })
}
if matches!(kind, Ub(InvalidUninitBytes(_))) {
err_validation_failure!(self.path, Uninit { expected }).into()
} else {
err_validation_failure!(self.path, PointerAsInt { expected }).into()
}
// Propagate upwards (that will also check for unexpected errors).
_ => return Err(InterpErrorInfo::from_parts(kind, backtrace)),
}
// Propagate upwards (that will also check for unexpected errors).
_ => return InterpErrorInfo::from_parts(kind, backtrace),
}
}
})?;
// Don't forget that these are all non-pointer types, and thus do not preserve
// provenance.
@ -1335,7 +1334,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
}
}
Ok(())
interp_ok(())
}
}
@ -1351,7 +1350,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
trace!("validate_operand_internal: {:?}, {:?}", *val, val.layout.ty);
// Run the visitor.
match self.run_for_validation(|ecx| {
self.run_for_validation(|ecx| {
let reset_padding = reset_provenance_and_padding && {
// Check if `val` is actually stored in memory. If not, padding is not even
// represented and we need not reset it.
@ -1367,29 +1366,22 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
};
v.visit_value(val)?;
v.reset_padding(val)?;
InterpResult::Ok(())
}) {
Ok(()) => Ok(()),
// Pass through validation failures and "invalid program" issues.
Err(err)
if matches!(
err.kind(),
err_ub!(ValidationError { .. })
| InterpError::InvalidProgram(_)
| InterpError::Unsupported(UnsupportedOpInfo::ExternTypeField)
) =>
{
Err(err)
}
// Complain about any other kind of 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) => {
interp_ok(())
})
.map_err(|err| {
if !matches!(
err.kind(),
err_ub!(ValidationError { .. })
| InterpError::InvalidProgram(_)
| InterpError::Unsupported(UnsupportedOpInfo::ExternTypeField)
) {
bug!(
"Unexpected error during validation: {}",
format_interp_error(self.tcx.dcx(), err)
);
}
}
err
})
}
/// This function checks the data at `op` to be const-valid.
@ -1460,6 +1452,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
/*reset_provenance_and_padding*/ false,
)?;
}
Ok(())
interp_ok(())
}
}