Skip to content

Commit

Permalink
CTFE: more informative error message on ReadPointerAsBytes trouble
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Sep 2, 2022
1 parent 1ec9b66 commit c7f8e68
Show file tree
Hide file tree
Showing 21 changed files with 275 additions and 50 deletions.
13 changes: 12 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_span::{Span, Symbol};

use super::InterpCx;
use crate::interpret::{
struct_error, ErrorHandled, FrameInfo, InterpError, InterpErrorInfo, Machine, MachineStopType,
struct_error, ErrorHandled, FrameInfo, InterpError, InterpErrorInfo, Machine, MachineStopType, UnsupportedOpInfo,
};

/// The CTFE machine has some custom error kinds.
Expand Down Expand Up @@ -153,6 +153,17 @@ impl<'tcx> ConstEvalErr<'tcx> {
if let Some(span_msg) = span_msg {
err.span_label(self.span, span_msg);
}
// Add some more context for select error types.
match self.error {
InterpError::Unsupported(
UnsupportedOpInfo::ReadPointerAsBytes
| UnsupportedOpInfo::PartialPointerOverwrite(_)
) => {
err.help("this code performed an operation that depends on the underlying bytes representing a pointer");
err.help("the absolute address of a pointer is not known at compile-time, so such operations are not supported");
}
_ => {}
}
// Add spans for the stacktrace. Don't print a single-line backtrace though.
if self.stacktrace.len() > 1 {
// Helper closure to print duplicated lines.
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::interpret::eval_nullary_intrinsic;
use crate::interpret::{
intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId,
Immediate, InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking,
ScalarMaybeUninit, StackPopCleanup,
ScalarMaybeUninit, StackPopCleanup, InterpError,
};

use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -374,7 +374,9 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
ecx.tcx,
"it is undefined behavior to use this value",
|diag| {
diag.note(NOTE_ON_UNDEFINED_BEHAVIOR_ERROR);
if matches!(err.error, InterpError::UndefinedBehavior(_)) {
diag.note(NOTE_ON_UNDEFINED_BEHAVIOR_ERROR);
}
diag.note(&format!(
"the raw bytes of the constant ({}",
display_allocation(
Expand Down
44 changes: 10 additions & 34 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// 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.
Expand All @@ -348,11 +344,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
value: &OpTy<'tcx, M::Provenance>,
kind: &str,
) -> InterpResult<'tcx> {
let value = try_validation!(
self.ecx.read_immediate(value),
self.path,
err_unsup!(ReadPointerAsBytes) => { "part of a pointer" } expected { "a proper pointer or integer value" },
);
let value = self.ecx.read_immediate(value)?;
// Handle wide pointers.
// Check metadata early, for better diagnostics
let place = try_validation!(
Expand Down Expand Up @@ -458,22 +450,14 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
&self,
op: &OpTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx, ScalarMaybeUninit<M::Provenance>> {
Ok(try_validation!(
self.ecx.read_scalar(op),
self.path,
err_unsup!(ReadPointerAsBytes) => { "(potentially part of) a pointer" } expected { "plain (non-pointer) bytes" },
))
self.ecx.read_scalar(op)
}

fn read_immediate_forced(
&self,
op: &OpTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx, Immediate<M::Provenance>> {
Ok(*try_validation!(
self.ecx.read_immediate_raw(op, /*force*/ true),
self.path,
err_unsup!(ReadPointerAsBytes) => { "(potentially part of) a pointer" } expected { "plain (non-pointer) bytes" },
).unwrap())
Ok(*self.ecx.read_immediate_raw(op, /*force*/ true)?.unwrap())
}

/// Check if this is a value of primitive type, and if yes check the validity of the value
Expand Down Expand Up @@ -535,7 +519,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
self.ecx.read_immediate(value).and_then(|ref i| self.ecx.ref_to_mplace(i)),
self.path,
err_ub!(InvalidUninitBytes(None)) => { "uninitialized raw pointer" },
err_unsup!(ReadPointerAsBytes) => { "part of a pointer" } expected { "a proper pointer or integer value" },
);
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta, place.layout)?;
Expand All @@ -560,7 +543,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let value = try_validation!(
self.ecx.read_scalar(value).and_then(|v| v.check_init()),
self.path,
err_unsup!(ReadPointerAsBytes) => { "part of a pointer" } expected { "a proper pointer or integer value" },
err_ub!(InvalidUninitBytes(None)) => { "uninitialized bytes" } expected { "a proper pointer or integer value" },
);

Expand Down Expand Up @@ -715,8 +697,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
{ "{:x}", val } expected { "a valid enum tag" },
err_ub!(InvalidUninitBytes(None)) =>
{ "uninitialized bytes" } expected { "a valid enum tag" },
err_unsup!(ReadPointerAsBytes) =>
{ "a pointer" } expected { "a valid enum tag" },
)
.1)
})
Expand Down Expand Up @@ -853,7 +833,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
self.ecx.read_bytes_ptr(mplace.ptr, Size::from_bytes(len)),
self.path,
err_ub!(InvalidUninitBytes(..)) => { "uninitialized data in `str`" },
err_unsup!(ReadPointerAsBytes) => { "a pointer in `str`" },
);
}
ty::Array(tys, ..) | ty::Slice(tys)
Expand Down Expand Up @@ -925,9 +904,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),
Expand Down Expand Up @@ -968,14 +944,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),
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/test/ui/const-ptr/forbidden_slices.32bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/forbidden_slices.rs:27:1
|
LL | pub static S5: &[u8] = unsafe { from_raw_parts((&D3) as *const _ as _, size_of::<&u32>()) };
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= 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
= note: the raw bytes of the constant (size: 8, align: 4) {
╾─ALLOC_ID─╼ 04 00 00 00 │ ╾──╼....
}
Expand Down Expand Up @@ -170,9 +171,10 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/forbidden_slices.rs:57:1
|
LL | pub static R5: &[u8] = unsafe {
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= 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
= note: the raw bytes of the constant (size: 8, align: 4) {
╾ALLOC_ID─╼ 04 00 00 00 │ ╾──╼....
}
Expand Down
10 changes: 6 additions & 4 deletions src/test/ui/const-ptr/forbidden_slices.64bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/forbidden_slices.rs:27:1
|
LL | pub static S5: &[u8] = unsafe { from_raw_parts((&D3) as *const _ as _, size_of::<&u32>()) };
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= 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
= note: the raw bytes of the constant (size: 16, align: 8) {
╾───────ALLOC_ID───────╼ 08 00 00 00 00 00 00 00 │ ╾──────╼........
}
Expand Down Expand Up @@ -170,9 +171,10 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/forbidden_slices.rs:57:1
|
LL | pub static R5: &[u8] = unsafe {
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>: encountered a pointer, but expected plain (non-pointer) bytes
| ^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= 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
= note: the raw bytes of the constant (size: 16, align: 8) {
╾──────ALLOC_ID───────╼ 08 00 00 00 00 00 00 00 │ ╾──────╼........
}
Expand Down
Loading

0 comments on commit c7f8e68

Please sign in to comment.