Insert null checks for pointer dereferences when debug assertions are enabled

Similar to how the alignment is already checked, this adds a check
for null pointer dereferences in debug mode. It is implemented similarly
to the alignment check as a MirPass.

This is related to a 2025H1 project goal for better UB checks in debug
mode: https://github.com/rust-lang/rust-project-goals/pull/177.
This commit is contained in:
Bastian Kersting 2024-12-17 13:00:22 +00:00
parent 851322b74d
commit b151b513ba
32 changed files with 281 additions and 6 deletions

View file

@ -417,6 +417,16 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
Some(source_info.span),
);
}
AssertKind::NullPointerDereference => {
let location = fx.get_caller_location(source_info).load_scalar(fx);
codegen_panic_inner(
fx,
rustc_hir::LangItem::PanicNullPointerDereference,
&[location],
Some(source_info.span),
)
}
_ => {
let location = fx.get_caller_location(source_info).load_scalar(fx);

View file

@ -713,6 +713,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// and `#[track_caller]` adds an implicit third argument.
(LangItem::PanicMisalignedPointerDereference, vec![required, found, location])
}
AssertKind::NullPointerDereference => {
// It's `fn panic_null_pointer_dereference()`,
// `#[track_caller]` adds an implicit argument.
(LangItem::PanicNullPointerDereference, vec![location])
}
_ => {
// It's `pub fn panic_...()` and `#[track_caller]` adds an implicit argument.
(msg.panic_function(), vec![location])

View file

@ -508,6 +508,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
found: eval_to_int(found)?,
}
}
NullPointerDereference => NullPointerDereference,
};
Err(ConstEvalErrKind::AssertFailure(err)).into()
}

View file

@ -316,6 +316,7 @@ language_item_table! {
PanicAsyncFnResumedPanic, sym::panic_const_async_fn_resumed_panic, panic_const_async_fn_resumed_panic, Target::Fn, GenericRequirement::None;
PanicAsyncGenFnResumedPanic, sym::panic_const_async_gen_fn_resumed_panic, panic_const_async_gen_fn_resumed_panic, Target::Fn, GenericRequirement::None;
PanicGenFnNonePanic, sym::panic_const_gen_fn_none_panic, panic_const_gen_fn_none_panic, Target::Fn, GenericRequirement::None;
PanicNullPointerDereference, sym::panic_null_pointer_dereference, panic_null_pointer_dereference, Target::Fn, GenericRequirement::None;
/// libstd panic entry point. Necessary for const eval to be able to catch it
BeginPanic, sym::begin_panic, begin_panic_fn, Target::Fn, GenericRequirement::None;

View file

@ -17,6 +17,9 @@ middle_assert_gen_resume_after_panic = `gen` fn or block cannot be further itera
middle_assert_misaligned_ptr_deref =
misaligned pointer dereference: address must be a multiple of {$required} but is {$found}
middle_assert_null_ptr_deref =
null pointer dereference occurred
middle_assert_op_overflow =
attempt to compute `{$left} {$op} {$right}`, which would overflow

View file

@ -1076,6 +1076,7 @@ pub enum AssertKind<O> {
ResumedAfterReturn(CoroutineKind),
ResumedAfterPanic(CoroutineKind),
MisalignedPointerDereference { required: O, found: O },
NullPointerDereference,
}
#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]

View file

@ -206,6 +206,7 @@ impl<O> AssertKind<O> {
ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::Gen, _)) => {
LangItem::PanicGenFnNonePanic
}
NullPointerDereference => LangItem::PanicNullPointerDereference,
BoundsCheck { .. } | MisalignedPointerDereference { .. } => {
bug!("Unexpected AssertKind")
@ -271,6 +272,7 @@ impl<O> AssertKind<O> {
"\"misaligned pointer dereference: address must be a multiple of {{}} but is {{}}\", {required:?}, {found:?}"
)
}
NullPointerDereference => write!(f, "\"null pointer dereference occured\""),
ResumedAfterReturn(CoroutineKind::Coroutine(_)) => {
write!(f, "\"coroutine resumed after completion\"")
}
@ -341,7 +343,7 @@ impl<O> AssertKind<O> {
ResumedAfterPanic(CoroutineKind::Coroutine(_)) => {
middle_assert_coroutine_resume_after_panic
}
NullPointerDereference => middle_assert_null_ptr_deref,
MisalignedPointerDereference { .. } => middle_assert_misaligned_ptr_deref,
}
}
@ -374,7 +376,7 @@ impl<O> AssertKind<O> {
add!("left", format!("{left:#?}"));
add!("right", format!("{right:#?}"));
}
ResumedAfterReturn(_) | ResumedAfterPanic(_) => {}
ResumedAfterReturn(_) | ResumedAfterPanic(_) | NullPointerDereference => {}
MisalignedPointerDereference { required, found } => {
add!("required", format!("{required:#?}"));
add!("found", format!("{found:#?}"));

View file

@ -636,7 +636,7 @@ macro_rules! make_mir_visitor {
OverflowNeg(op) | DivisionByZero(op) | RemainderByZero(op) => {
self.visit_operand(op, location);
}
ResumedAfterReturn(_) | ResumedAfterPanic(_) => {
ResumedAfterReturn(_) | ResumedAfterPanic(_) | NullPointerDereference => {
// Nothing to visit
}
MisalignedPointerDereference { required, found } => {

View file

@ -0,0 +1,110 @@
use rustc_index::IndexVec;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::*;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_session::Session;
use crate::check_pointers::{BorrowCheckMode, PointerCheck, check_pointers};
pub(super) struct CheckNull;
impl<'tcx> crate::MirPass<'tcx> for CheckNull {
fn is_enabled(&self, sess: &Session) -> bool {
sess.ub_checks()
}
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
check_pointers(tcx, body, &[], insert_null_check, BorrowCheckMode::IncludeBorrows);
}
fn is_required(&self) -> bool {
true
}
}
fn insert_null_check<'tcx>(
tcx: TyCtxt<'tcx>,
pointer: Place<'tcx>,
pointee_ty: Ty<'tcx>,
local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>,
stmts: &mut Vec<Statement<'tcx>>,
source_info: SourceInfo,
) -> PointerCheck<'tcx> {
// Cast the pointer to a *const ().
let const_raw_ptr = Ty::new_imm_ptr(tcx, tcx.types.unit);
let rvalue = Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(pointer), const_raw_ptr);
let thin_ptr = local_decls.push(LocalDecl::with_source_info(const_raw_ptr, source_info)).into();
stmts
.push(Statement { source_info, kind: StatementKind::Assign(Box::new((thin_ptr, rvalue))) });
// Transmute the pointer to a usize (equivalent to `ptr.addr()`).
let rvalue = Rvalue::Cast(CastKind::Transmute, Operand::Copy(thin_ptr), tcx.types.usize);
let addr = local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into();
stmts.push(Statement { source_info, kind: StatementKind::Assign(Box::new((addr, rvalue))) });
// Get size of the pointee (zero-sized reads and writes are allowed).
let rvalue = Rvalue::NullaryOp(NullOp::SizeOf, pointee_ty);
let sizeof_pointee =
local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into();
stmts.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((sizeof_pointee, rvalue))),
});
// Check that the pointee is not a ZST.
let zero = Operand::Constant(Box::new(ConstOperand {
span: source_info.span,
user_ty: None,
const_: Const::Val(ConstValue::Scalar(Scalar::from_target_usize(0, &tcx)), tcx.types.usize),
}));
let is_pointee_no_zst =
local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
stmts.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
is_pointee_no_zst,
Rvalue::BinaryOp(BinOp::Ne, Box::new((Operand::Copy(sizeof_pointee), zero.clone()))),
))),
});
// Check whether the pointer is null.
let is_null = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
stmts.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
is_null,
Rvalue::BinaryOp(BinOp::Eq, Box::new((Operand::Copy(addr), zero))),
))),
});
// We want to throw an exception if the pointer is null and doesn't point to a ZST.
let should_throw_exception =
local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
stmts.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
should_throw_exception,
Rvalue::BinaryOp(
BinOp::BitAnd,
Box::new((Operand::Copy(is_null), Operand::Copy(is_pointee_no_zst))),
),
))),
});
// The final condition whether this pointer usage is ok or not.
let is_ok = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
stmts.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
is_ok,
Rvalue::UnaryOp(UnOp::Not, Operand::Copy(should_throw_exception)),
))),
});
// Emit a PointerCheck that asserts on the condition and otherwise triggers
// a AssertKind::NullPointerDereference.
PointerCheck {
cond: Operand::Copy(is_ok),
assert_kind: Box::new(AssertKind::NullPointerDereference),
}
}

View file

@ -17,6 +17,7 @@ pub(crate) struct PointerCheck<'tcx> {
/// [NonMutatingUseContext::SharedBorrow].
#[derive(Copy, Clone)]
pub(crate) enum BorrowCheckMode {
IncludeBorrows,
ExcludeBorrows,
}
@ -168,7 +169,7 @@ impl<'a, 'tcx> PointerFinder<'a, 'tcx> {
) => true,
PlaceContext::MutatingUse(MutatingUseContext::Borrow)
| PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) => {
!matches!(self.borrow_check_mode, BorrowCheckMode::ExcludeBorrows)
matches!(self.borrow_check_mode, BorrowCheckMode::IncludeBorrows)
}
_ => false,
}

View file

@ -119,6 +119,7 @@ declare_passes! {
mod check_call_recursion : CheckCallRecursion, CheckDropRecursion;
mod check_alignment : CheckAlignment;
mod check_const_item_mutation : CheckConstItemMutation;
mod check_null : CheckNull;
mod check_packed_ref : CheckPackedRef;
mod check_undefined_transmutes : CheckUndefinedTransmutes;
// This pass is public to allow external drivers to perform MIR cleanup
@ -643,6 +644,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&[
// Add some UB checks before any UB gets optimized away.
&check_alignment::CheckAlignment,
&check_null::CheckNull,
// Before inlining: trim down MIR with passes to reduce inlining work.
// Has to be done before inlining, otherwise actual call will be almost always inlined.

View file

@ -814,6 +814,9 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
mir::AssertKind::MisalignedPointerDereference { .. } => {
push_mono_lang_item(self, LangItem::PanicMisalignedPointerDereference);
}
mir::AssertKind::NullPointerDereference => {
push_mono_lang_item(self, LangItem::PanicNullPointerDereference);
}
_ => {
push_mono_lang_item(self, msg.panic_function());
}

View file

@ -496,6 +496,9 @@ impl<'tcx> Stable<'tcx> for mir::AssertMessage<'tcx> {
found: found.stable(tables),
}
}
AssertKind::NullPointerDereference => {
stable_mir::mir::AssertMessage::NullPointerDereference
}
}
}
}

View file

@ -1476,6 +1476,7 @@ symbols! {
panic_location,
panic_misaligned_pointer_dereference,
panic_nounwind,
panic_null_pointer_dereference,
panic_runtime,
panic_str_2015,
panic_unwind,

View file

@ -251,6 +251,7 @@ pub enum AssertMessage {
ResumedAfterReturn(CoroutineKind),
ResumedAfterPanic(CoroutineKind),
MisalignedPointerDereference { required: Operand, found: Operand },
NullPointerDereference,
}
impl AssertMessage {
@ -306,6 +307,7 @@ impl AssertMessage {
AssertMessage::MisalignedPointerDereference { .. } => {
Ok("misaligned pointer dereference")
}
AssertMessage::NullPointerDereference => Ok("null pointer dereference occured"),
}
}
}

View file

@ -298,6 +298,9 @@ fn pretty_assert_message<W: Write>(writer: &mut W, msg: &AssertMessage) -> io::R
"\"misaligned pointer dereference: address must be a multiple of {{}} but is {{}}\",{pretty_required}, {pretty_found}"
)
}
AssertMessage::NullPointerDereference => {
write!(writer, "\"null pointer dereference occured.\"")
}
AssertMessage::ResumedAfterReturn(_) | AssertMessage::ResumedAfterPanic(_) => {
write!(writer, "{}", msg.description().unwrap())
}

View file

@ -426,7 +426,10 @@ pub trait MirVisitor {
| AssertMessage::RemainderByZero(op) => {
self.visit_operand(op, location);
}
AssertMessage::ResumedAfterReturn(_) | AssertMessage::ResumedAfterPanic(_) => { //nothing to visit
AssertMessage::ResumedAfterReturn(_)
| AssertMessage::ResumedAfterPanic(_)
| AssertMessage::NullPointerDereference => {
//nothing to visit
}
AssertMessage::MisalignedPointerDereference { required, found } => {
self.visit_operand(required, location);

View file

@ -291,6 +291,22 @@ fn panic_misaligned_pointer_dereference(required: usize, found: usize) -> ! {
)
}
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold, optimize(size))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
#[cfg_attr(not(bootstrap), lang = "panic_null_pointer_dereference")] // needed by codegen for panic on null pointer deref
#[rustc_nounwind] // `CheckNull` MIR pass requires this function to never unwind
fn panic_null_pointer_dereference() -> ! {
if cfg!(feature = "panic_immediate_abort") {
super::intrinsics::abort()
}
panic_nounwind_fmt(
format_args!("null pointer dereference occured"),
/* force_no_backtrace */ false,
)
}
/// Panics because we cannot unwind out of a function.
///
/// This is a separate function to avoid the codesize impact of each crate containing the string to

View file

@ -166,6 +166,8 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
// MIR passes can be enabled / disabled separately, we should figure out, what passes to
// use for Clippy.
config.opts.unstable_opts.mir_opt_level = Some(0);
config.opts.unstable_opts.mir_enable_passes =
vec![("CheckNull".to_owned(), false), ("CheckAlignment".to_owned(), false)];
// Disable flattening and inlining of format_args!(), so the HIR matches with the AST.
config.opts.unstable_opts.flatten_format_args = false;

View file

@ -168,7 +168,7 @@ pub const MIRI_DEFAULT_ARGS: &[&str] = &[
"-Zmir-emit-retag",
"-Zmir-keep-place-mention",
"-Zmir-opt-level=0",
"-Zmir-enable-passes=-CheckAlignment",
"-Zmir-enable-passes=-CheckAlignment,-CheckNull",
// Deduplicating diagnostics means we miss events when tracking what happens during an
// execution. Let's not do that.
"-Zdeduplicate-diagnostics=no",

View file

@ -411,6 +411,7 @@ language_item_table! {
PanicLocation, sym::panic_location, panic_location, Target::Struct, GenericRequirement::None;
PanicImpl, sym::panic_impl, panic_impl, Target::Fn, GenericRequirement::None;
PanicCannotUnwind, sym::panic_cannot_unwind, panic_cannot_unwind, Target::Fn, GenericRequirement::Exact(0);
PanicNullPointerDereference, sym::panic_null_pointer_dereference, panic_null_pointer_dereference, Target::Fn, GenericRequirement::None;
/// libstd panic entry point. Necessary for const eval to be able to catch it
BeginPanic, sym::begin_panic, begin_panic_fn, Target::Fn, GenericRequirement::None;

View file

@ -363,6 +363,7 @@ define_symbols! {
panic_location,
panic_misaligned_pointer_dereference,
panic_nounwind,
panic_null_pointer_dereference,
panic,
Param,
parse,

View file

@ -0,0 +1,16 @@
//@ compile-flags: -C debug-assertions
struct Null {
a: u32,
}
fn main() {
// CHECK-LABEL: fn main(
// CHECK-NOT: {{assert.*}}
let val: u32 = 42;
let val_ref: &u32 = &val;
let _access1: &u32 = &*val_ref;
let val = Null { a: 42 };
let _access2: &u32 = &val.a;
}

View file

@ -1,5 +1,6 @@
//@ run-pass
//@ needs-subprocess
//@ compile-flags: -Zub-checks=no -Zmir-enable-passes=-CheckNull
//@ ignore-fuchsia must translate zircon signal to SIGSEGV/SIGBUS, FIXME (#58590)
#![feature(rustc_private)]

View file

@ -0,0 +1,14 @@
// Make sure that we don't insert a check for `addr_of!`.
//@ run-pass
//@ compile-flags: -C debug-assertions
struct Field {
a: u32,
}
fn main() {
unsafe {
let ptr: *const Field = std::ptr::null();
let _ptr = core::ptr::addr_of!((*ptr).a);
}
}

View file

@ -0,0 +1,8 @@
//@ run-fail
//@ compile-flags: -C debug-assertions
//@ error-pattern: null pointer dereference occured
fn main() {
let ptr: *mut u32 = std::ptr::null_mut();
let _ptr: &mut u32 = unsafe { &mut *ptr };
}

View file

@ -0,0 +1,8 @@
//@ run-fail
//@ compile-flags: -C debug-assertions
//@ error-pattern: null pointer dereference occured
fn main() {
let ptr: *const u32 = std::ptr::null();
let _ptr: &u32 = unsafe { &*ptr };
}

View file

@ -0,0 +1,10 @@
//@ run-fail
//@ compile-flags: -C debug-assertions
//@ error-pattern: null pointer dereference occured
fn main() {
let ptr: *mut u32 = std::ptr::null_mut();
unsafe {
*(ptr) = 42;
}
}

View file

@ -0,0 +1,10 @@
//@ run-fail
//@ compile-flags: -C debug-assertions
//@ error-pattern: null pointer dereference occured
fn main() {
let ptr: *mut u32 = std::ptr::null_mut();
unsafe {
let _v = *ptr;
}
}

View file

@ -0,0 +1,10 @@
// Make sure that we don't insert a check for places that do not read.
//@ run-pass
//@ compile-flags: -C debug-assertions
fn main() {
let ptr: *const u16 = std::ptr::null();
unsafe {
let _ = *ptr;
}
}

View file

@ -0,0 +1,12 @@
//@ run-fail
//@ compile-flags: -C debug-assertions
//@ error-pattern: null pointer dereference occured
fn main() {
let ptr = std::ptr::null();
let mut dest = 0u32;
let dest_ptr = &mut dest as *mut u32;
unsafe {
*dest_ptr = *(ptr);
}
}

View file

@ -0,0 +1,15 @@
// Make sure that we don't insert a check for zero-sized reads or writes to
// null, because they are allowed.
//@ run-pass
//@ compile-flags: -C debug-assertions
fn main() {
let ptr: *mut () = std::ptr::null_mut();
unsafe {
*(ptr) = ();
}
let ptr1: *const () = std::ptr::null_mut();
unsafe {
let _ptr = *ptr1;
}
}