squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Miri fn ptr check: don't use conservative null check

Browse source
This commit is contained in:
Ralf Jung 2022-02-24 19:38:37 -05:00
parent 3d127e2040
commit d8064d7d49
7 changed files with 43 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
compiler/rustc_const_eval/src/const_eval/machine.rs
View file

@ -217,8 +217,9 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
// Comparisons of abstract pointers with null pointers are known if the pointer
// is in bounds, because if they are in bounds, the pointer can't be null.
// Inequality with integers other than null can never be known for sure.
(Scalar::Int(int), Scalar::Ptr(ptr, _)) | (Scalar::Ptr(ptr, _), Scalar::Int(int)) => {
int.is_null() && !self.memory.ptr_may_be_null(ptr.into())
(Scalar::Int(int), ptr @ Scalar::Ptr(..))
| (ptr @ Scalar::Ptr(..), Scalar::Int(int)) => {
int.is_null() && !self.scalar_may_be_null(ptr)
}
// FIXME: return `true` for at least some comparisons where we can reliably
// determine the result of runtime inequality tests at compile-time.

29
compiler/rustc_const_eval/src/interpret/eval_context.rs
View file

@ -22,9 +22,9 @@ use rustc_span::{Pos, Span};
use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayout};
use super::{
AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine, MemPlace,
MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, Pointer, Provenance, Scalar,
ScalarMaybeUninit, StackPopJump,
AllocCheck, AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine,
MemPlace, MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, Pointer, Provenance,
Scalar, ScalarMaybeUninit, StackPopJump,
};
use crate::transform::validate::equal_up_to_regions;
@ -440,6 +440,29 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.memory.scalar_to_ptr(scalar)
}
/// Test if this value might be null.
/// If the machine does not support ptr-to-int casts, this is conservative.
pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> bool {
match scalar.try_to_int() {
Ok(int) => int.is_null(),
Err(_) => {
let ptr = self.scalar_to_ptr(scalar);
match self.memory.ptr_try_get_alloc(ptr) {
Ok((alloc_id, offset, _)) => {
let (size, _align) = self
.memory
.get_size_and_align(alloc_id, AllocCheck::MaybeDead)
.expect("alloc info with MaybeDead cannot fail");
// If the pointer is out-of-bounds, it may be null.
// Note that one-past-the-end (offset == size) is still inbounds, and never null.
offset > size
}
Err(offset) => offset == 0,
}
}
}
}
/// Call this to turn untagged "global" pointers (obtained via `tcx`) into
/// the machine pointer to the allocation. Must never be used
/// for any other pointers, nor for TLS statics.

15
compiler/rustc_const_eval/src/interpret/memory.rs
View file

@ -483,21 +483,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
}
})
}
/// Test if the pointer might be null.
pub fn ptr_may_be_null(&self, ptr: Pointer<Option<M::PointerTag>>) -> bool {
match self.ptr_try_get_alloc(ptr) {
Ok((alloc_id, offset, _)) => {
let (size, _align) = self
.get_size_and_align(alloc_id, AllocCheck::MaybeDead)
.expect("alloc info with MaybeDead cannot fail");
// If the pointer is out-of-bounds, it may be null.
// Note that one-past-the-end (offset == size) is still inbounds, and never null.
offset > size
}
Err(offset) => offset == 0,
}
}
}
/// Allocation accessors

3
compiler/rustc_const_eval/src/interpret/operand.rs
View file

@ -720,12 +720,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Err(dbg_val) => {
// So this is a pointer then, and casting to an int failed.
// Can only happen during CTFE.
let ptr = self.scalar_to_ptr(tag_val);
// The niche must be just 0, and the ptr not null, then we know this is
// okay. Everything else, we conservatively reject.
let ptr_valid = niche_start == 0
&& variants_start == variants_end
&& !self.memory.ptr_may_be_null(ptr);
&& !self.scalar_may_be_null(tag_val);
if !ptr_valid {
throw_ub!(InvalidTag(dbg_val))
}

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

@ -572,21 +572,25 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
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" },
);
let ptr = self.ecx.scalar_to_ptr(value);
// Ensure the pointer is non-null.
if self.ecx.memory.ptr_may_be_null(ptr) {
throw_validation_failure!(self.path, { "a potentially null function pointer" });
}
// If we check references recursively, also check that this points to a function.
if let Some(_) = self.ref_tracking {
let ptr = self.ecx.scalar_to_ptr(value);
let _fn = try_validation!(
self.ecx.memory.get_fn(ptr),
self.path,
err_ub!(DanglingIntPointer(0, _)) =>
{ "a null function pointer" },
err_ub!(DanglingIntPointer(..)) |
err_ub!(InvalidFunctionPointer(..)) =>
{ "{:x}", value } expected { "a function pointer" },
);
// FIXME: Check if the signature matches
} else {
// Otherwise (for standalone Miri), we have to still check it to be non-null.
if self.ecx.scalar_may_be_null(value) {
throw_validation_failure!(self.path, { "a null function pointer" });
}
}
Ok(true)
}
@ -644,10 +648,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Err(_) => {
// So this is a pointer then, and casting to an int failed.
// Can only happen during CTFE.
let ptr = self.ecx.scalar_to_ptr(value);
if start == 1 && end == max_value {
// Only null is the niche. So make sure the ptr is NOT null.
if self.ecx.memory.ptr_may_be_null(ptr) {
if self.ecx.scalar_may_be_null(value) {
throw_validation_failure!(self.path,
{ "a potentially null pointer" }
expected {
@ -758,7 +761,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
fn visit_value(&mut self, op: &OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> {
trace!("visit_value: {:?}, {:?}", *op, op.layout);
// Check primitive types -- the leafs of our recursive descend.
// Check primitive types -- the leaves of our recursive descent.
if self.try_visit_primitive(op)? {
return Ok(());
}

2
src/test/ui/consts/const-eval/ub-ref-ptr.32bit.stderr
View file

@ -112,7 +112,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-ref-ptr.rs:49:1
|
LL | const NULL_FN_PTR: fn() = unsafe { mem::transmute(0usize) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a potentially null function pointer
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a null function pointer
|
= 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.
= note: the raw bytes of the constant (size: 4, align: 4) {

2
src/test/ui/consts/const-eval/ub-ref-ptr.64bit.stderr
View file

@ -112,7 +112,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-ref-ptr.rs:49:1
|
LL | const NULL_FN_PTR: fn() = unsafe { mem::transmute(0usize) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a potentially null function pointer
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a null function pointer
|
= 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.
= note: the raw bytes of the constant (size: 8, align: 8) {