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

CTFE validation: catch ReadPointerAsBytes and better error #82061

Merged
merged 1 commit into from
Feb 16, 2021
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
42 changes: 29 additions & 13 deletions compiler/rustc_mir/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::hash::Hash;

use super::{
CheckInAllocMsg, GlobalAlloc, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
ValueVisitor,
ScalarMaybeUninit, ValueVisitor,
};

macro_rules! throw_validation_failure {
Expand Down Expand Up @@ -378,7 +378,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
value: OpTy<'tcx, M::PointerTag>,
kind: &str,
) -> InterpResult<'tcx> {
let value = self.ecx.read_immediate(value)?;
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" },
);
// Handle wide pointers.
// Check metadata early, for better diagnostics
let place = try_validation!(
Expand Down Expand Up @@ -485,6 +489,17 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(())
}

fn read_scalar(
&self,
op: OpTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, ScalarMaybeUninit<M::PointerTag>> {
Ok(try_validation!(
self.ecx.read_scalar(op),
self.path,
err_unsup!(ReadPointerAsBytes) => { "(potentially part of) a pointer" } expected { "plain (non-pointer) bytes" },
))
}

/// Check if this is a value of primitive type, and if yes check the validity of the value
/// at that type. Return `true` if the type is indeed primitive.
fn try_visit_primitive(
Expand All @@ -495,7 +510,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
let ty = value.layout.ty;
match ty.kind() {
ty::Bool => {
let value = self.ecx.read_scalar(value)?;
let value = self.read_scalar(value)?;
try_validation!(
value.to_bool(),
self.path,
Expand All @@ -505,7 +520,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(true)
}
ty::Char => {
let value = self.ecx.read_scalar(value)?;
let value = self.read_scalar(value)?;
try_validation!(
value.to_char(),
self.path,
Expand All @@ -515,11 +530,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(true)
}
ty::Float(_) | ty::Int(_) | ty::Uint(_) => {
let value = try_validation!(
self.ecx.read_scalar(value),
self.path,
err_unsup!(ReadPointerAsBytes) => { "read of part of a pointer" },
);
let value = self.read_scalar(value)?;
// NOTE: Keep this in sync with the array optimization for int/float
// types below!
if self.ctfe_mode.is_some() {
Expand All @@ -541,9 +552,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// actually enforce the strict rules for raw pointers (mostly because
// that lets us re-use `ref_to_mplace`).
let place = try_validation!(
self.ecx.ref_to_mplace(self.ecx.read_immediate(value)?),
self.ecx.read_immediate(value).and_then(|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 @@ -569,9 +581,13 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(true)
}
ty::FnPtr(_sig) => {
let value = self.ecx.read_scalar(value)?;
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 _fn = try_validation!(
value.check_init().and_then(|ptr| self.ecx.memory.get_fn(ptr)),
value.to_scalar().and_then(|ptr| self.ecx.memory.get_fn(ptr)),
self.path,
err_ub!(DanglingIntPointer(..)) |
err_ub!(InvalidFunctionPointer(..)) |
Expand Down Expand Up @@ -615,7 +631,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
op: OpTy<'tcx, M::PointerTag>,
scalar_layout: &Scalar,
) -> InterpResult<'tcx> {
let value = self.ecx.read_scalar(op)?;
let value = self.read_scalar(op)?;
let valid_range = &scalar_layout.valid_range;
let (lo, hi) = valid_range.clone().into_inner();
// Determine the allowed range
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/issue-79690.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/issue-79690.rs:29:1
|
LL | const G: Fat = unsafe { Transmute { t: FOO }.u };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered read of part of a pointer at .1.<deref>.size.foo
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered (potentially part of) a pointer at .1.<deref>.size.foo, but expected plain (non-pointer) 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.

Expand Down