Skip to content

Commit 61e98dc

Browse files
committed
Auto merge of #134424 - 1c3t3a:null-checks, r=<try>
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: rust-lang/rust-project-goals#177. r? `@saethlin`
2 parents 604d669 + 52b1360 commit 61e98dc

File tree

26 files changed

+364
-132
lines changed

26 files changed

+364
-132
lines changed

compiler/rustc_codegen_cranelift/src/base.rs

+10
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,16 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
417417
Some(source_info.span),
418418
);
419419
}
420+
AssertKind::NullPointerDereference => {
421+
let location = fx.get_caller_location(source_info).load_scalar(fx);
422+
423+
codegen_panic_inner(
424+
fx,
425+
rustc_hir::LangItem::PanicNullPointerDereference,
426+
&[location],
427+
Some(source_info.span),
428+
)
429+
}
420430
_ => {
421431
let location = fx.get_caller_location(source_info).load_scalar(fx);
422432

compiler/rustc_codegen_ssa/src/mir/block.rs

+5
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
713713
// and `#[track_caller]` adds an implicit third argument.
714714
(LangItem::PanicMisalignedPointerDereference, vec![required, found, location])
715715
}
716+
AssertKind::NullPointerDereference => {
717+
// It's `fn panic_null_pointer_dereference()`, and
718+
// `#[track_caller]` adds an implicit third argument.
719+
(LangItem::PanicNullPointerDereference, vec![location])
720+
}
716721
_ => {
717722
// It's `pub fn panic_...()` and `#[track_caller]` adds an implicit argument.
718723
(msg.panic_function(), vec![location])

compiler/rustc_const_eval/src/const_eval/machine.rs

+1
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
509509
found: eval_to_int(found)?,
510510
}
511511
}
512+
NullPointerDereference => NullPointerDereference,
512513
};
513514
Err(ConstEvalErrKind::AssertFailure(err)).into()
514515
}

compiler/rustc_hir/src/lang_items.rs

+1
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ language_item_table! {
317317
PanicAsyncFnResumedPanic, sym::panic_const_async_fn_resumed_panic, panic_const_async_fn_resumed_panic, Target::Fn, GenericRequirement::None;
318318
PanicAsyncGenFnResumedPanic, sym::panic_const_async_gen_fn_resumed_panic, panic_const_async_gen_fn_resumed_panic, Target::Fn, GenericRequirement::None;
319319
PanicGenFnNonePanic, sym::panic_const_gen_fn_none_panic, panic_const_gen_fn_none_panic, Target::Fn, GenericRequirement::None;
320+
PanicNullPointerDereference, sym::panic_null_pointer_dereference, panic_null_pointer_dereference, Target::Fn, GenericRequirement::None;
320321
/// libstd panic entry point. Necessary for const eval to be able to catch it
321322
BeginPanic, sym::begin_panic, begin_panic_fn, Target::Fn, GenericRequirement::None;
322323

compiler/rustc_middle/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ middle_assert_gen_resume_after_panic = `gen` fn or block cannot be further itera
1717
middle_assert_misaligned_ptr_deref =
1818
misaligned pointer dereference: address must be a multiple of {$required} but is {$found}
1919
20+
middle_assert_null_ptr_deref =
21+
null pointer dereference occurred
22+
2023
middle_assert_op_overflow =
2124
attempt to compute `{$left} {$op} {$right}`, which would overflow
2225

compiler/rustc_middle/src/mir/syntax.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1012,6 +1012,7 @@ pub enum AssertKind<O> {
10121012
ResumedAfterReturn(CoroutineKind),
10131013
ResumedAfterPanic(CoroutineKind),
10141014
MisalignedPointerDereference { required: O, found: O },
1015+
NullPointerDereference,
10151016
}
10161017

10171018
#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)]

compiler/rustc_middle/src/mir/terminator.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ impl<O> AssertKind<O> {
195195
ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::Gen, _)) => {
196196
LangItem::PanicGenFnNonePanic
197197
}
198+
NullPointerDereference => LangItem::PanicNullPointerDereference,
198199

199200
BoundsCheck { .. } | MisalignedPointerDereference { .. } => {
200201
bug!("Unexpected AssertKind")
@@ -260,6 +261,7 @@ impl<O> AssertKind<O> {
260261
"\"misaligned pointer dereference: address must be a multiple of {{}} but is {{}}\", {required:?}, {found:?}"
261262
)
262263
}
264+
NullPointerDereference => write!(f, "\"null pointer dereference occured\""),
263265
ResumedAfterReturn(CoroutineKind::Coroutine(_)) => {
264266
write!(f, "\"coroutine resumed after completion\"")
265267
}
@@ -330,7 +332,7 @@ impl<O> AssertKind<O> {
330332
ResumedAfterPanic(CoroutineKind::Coroutine(_)) => {
331333
middle_assert_coroutine_resume_after_panic
332334
}
333-
335+
NullPointerDereference => middle_assert_null_ptr_deref,
334336
MisalignedPointerDereference { .. } => middle_assert_misaligned_ptr_deref,
335337
}
336338
}
@@ -363,7 +365,7 @@ impl<O> AssertKind<O> {
363365
add!("left", format!("{left:#?}"));
364366
add!("right", format!("{right:#?}"));
365367
}
366-
ResumedAfterReturn(_) | ResumedAfterPanic(_) => {}
368+
ResumedAfterReturn(_) | ResumedAfterPanic(_) | NullPointerDereference => {}
367369
MisalignedPointerDereference { required, found } => {
368370
add!("required", format!("{required:#?}"));
369371
add!("found", format!("{found:#?}"));

compiler/rustc_middle/src/mir/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ macro_rules! make_mir_visitor {
636636
OverflowNeg(op) | DivisionByZero(op) | RemainderByZero(op) => {
637637
self.visit_operand(op, location);
638638
}
639-
ResumedAfterReturn(_) | ResumedAfterPanic(_) => {
639+
ResumedAfterReturn(_) | ResumedAfterPanic(_) | NullPointerDereference => {
640640
// Nothing to visit
641641
}
642642
MisalignedPointerDereference { required, found } => {

compiler/rustc_mir_transform/src/check_alignment.rs

+6-128
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
use rustc_hir::lang_items::LangItem;
21
use rustc_index::IndexVec;
32
use rustc_middle::mir::interpret::Scalar;
4-
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
53
use rustc_middle::mir::*;
6-
use rustc_middle::ty::{self, Ty, TyCtxt};
4+
use rustc_middle::ty::{Ty, TyCtxt};
75
use rustc_session::Session;
8-
use tracing::{debug, trace};
6+
7+
use crate::check_pointers::check_pointers;
98

109
pub(super) struct CheckAlignment;
1110

@@ -19,133 +18,12 @@ impl<'tcx> crate::MirPass<'tcx> for CheckAlignment {
1918
}
2019

2120
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
22-
// This pass emits new panics. If for whatever reason we do not have a panic
23-
// implementation, running this pass may cause otherwise-valid code to not compile.
24-
if tcx.lang_items().get(LangItem::PanicImpl).is_none() {
25-
return;
26-
}
27-
28-
let typing_env = body.typing_env(tcx);
29-
let basic_blocks = body.basic_blocks.as_mut();
30-
let local_decls = &mut body.local_decls;
31-
32-
// This pass inserts new blocks. Each insertion changes the Location for all
33-
// statements/blocks after. Iterating or visiting the MIR in order would require updating
34-
// our current location after every insertion. By iterating backwards, we dodge this issue:
35-
// The only Locations that an insertion changes have already been handled.
36-
for block in (0..basic_blocks.len()).rev() {
37-
let block = block.into();
38-
for statement_index in (0..basic_blocks[block].statements.len()).rev() {
39-
let location = Location { block, statement_index };
40-
let statement = &basic_blocks[block].statements[statement_index];
41-
let source_info = statement.source_info;
42-
43-
let mut finder =
44-
PointerFinder { tcx, local_decls, typing_env, pointers: Vec::new() };
45-
finder.visit_statement(statement, location);
46-
47-
for (local, ty) in finder.pointers {
48-
debug!("Inserting alignment check for {:?}", ty);
49-
let new_block = split_block(basic_blocks, location);
50-
insert_alignment_check(
51-
tcx,
52-
local_decls,
53-
&mut basic_blocks[block],
54-
local,
55-
ty,
56-
source_info,
57-
new_block,
58-
);
59-
}
60-
}
61-
}
21+
// Skip trivially aligned place types.
22+
let excluded_pointees = [tcx.types.bool, tcx.types.i8, tcx.types.u8];
23+
check_pointers(tcx, body, &excluded_pointees, insert_alignment_check);
6224
}
6325
}
6426

65-
struct PointerFinder<'a, 'tcx> {
66-
tcx: TyCtxt<'tcx>,
67-
local_decls: &'a mut LocalDecls<'tcx>,
68-
typing_env: ty::TypingEnv<'tcx>,
69-
pointers: Vec<(Place<'tcx>, Ty<'tcx>)>,
70-
}
71-
72-
impl<'a, 'tcx> Visitor<'tcx> for PointerFinder<'a, 'tcx> {
73-
fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) {
74-
// We want to only check reads and writes to Places, so we specifically exclude
75-
// Borrow and RawBorrow.
76-
match context {
77-
PlaceContext::MutatingUse(
78-
MutatingUseContext::Store
79-
| MutatingUseContext::AsmOutput
80-
| MutatingUseContext::Call
81-
| MutatingUseContext::Yield
82-
| MutatingUseContext::Drop,
83-
) => {}
84-
PlaceContext::NonMutatingUse(
85-
NonMutatingUseContext::Copy | NonMutatingUseContext::Move,
86-
) => {}
87-
_ => {
88-
return;
89-
}
90-
}
91-
92-
if !place.is_indirect() {
93-
return;
94-
}
95-
96-
// Since Deref projections must come first and only once, the pointer for an indirect place
97-
// is the Local that the Place is based on.
98-
let pointer = Place::from(place.local);
99-
let pointer_ty = self.local_decls[place.local].ty;
100-
101-
// We only want to check places based on unsafe pointers
102-
if !pointer_ty.is_unsafe_ptr() {
103-
trace!("Indirect, but not based on an unsafe ptr, not checking {:?}", place);
104-
return;
105-
}
106-
107-
let pointee_ty =
108-
pointer_ty.builtin_deref(true).expect("no builtin_deref for an unsafe pointer");
109-
// Ideally we'd support this in the future, but for now we are limited to sized types.
110-
if !pointee_ty.is_sized(self.tcx, self.typing_env) {
111-
debug!("Unsafe pointer, but pointee is not known to be sized: {:?}", pointer_ty);
112-
return;
113-
}
114-
115-
// Try to detect types we are sure have an alignment of 1 and skip the check
116-
// We don't need to look for str and slices, we already rejected unsized types above
117-
let element_ty = match pointee_ty.kind() {
118-
ty::Array(ty, _) => *ty,
119-
_ => pointee_ty,
120-
};
121-
if [self.tcx.types.bool, self.tcx.types.i8, self.tcx.types.u8].contains(&element_ty) {
122-
debug!("Trivially aligned place type: {:?}", pointee_ty);
123-
return;
124-
}
125-
126-
// Ensure that this place is based on an aligned pointer.
127-
self.pointers.push((pointer, pointee_ty));
128-
129-
self.super_place(place, context, location);
130-
}
131-
}
132-
133-
fn split_block(
134-
basic_blocks: &mut IndexVec<BasicBlock, BasicBlockData<'_>>,
135-
location: Location,
136-
) -> BasicBlock {
137-
let block_data = &mut basic_blocks[location.block];
138-
139-
// Drain every statement after this one and move the current terminator to a new basic block
140-
let new_block = BasicBlockData {
141-
statements: block_data.statements.split_off(location.statement_index),
142-
terminator: block_data.terminator.take(),
143-
is_cleanup: block_data.is_cleanup,
144-
};
145-
146-
basic_blocks.push(new_block)
147-
}
148-
14927
fn insert_alignment_check<'tcx>(
15028
tcx: TyCtxt<'tcx>,
15129
local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
use rustc_index::IndexVec;
2+
use rustc_middle::mir::interpret::Scalar;
3+
use rustc_middle::mir::*;
4+
use rustc_middle::ty::{Ty, TyCtxt};
5+
use rustc_session::Session;
6+
7+
use crate::check_pointers::check_pointers;
8+
9+
pub(super) struct CheckNull;
10+
11+
impl<'tcx> crate::MirPass<'tcx> for CheckNull {
12+
fn is_enabled(&self, sess: &Session) -> bool {
13+
sess.ub_checks()
14+
}
15+
16+
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
17+
check_pointers(tcx, body, &[], insert_null_check);
18+
}
19+
}
20+
21+
fn insert_null_check<'tcx>(
22+
tcx: TyCtxt<'tcx>,
23+
local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>,
24+
block_data: &mut BasicBlockData<'tcx>,
25+
pointer: Place<'tcx>,
26+
_: Ty<'tcx>,
27+
source_info: SourceInfo,
28+
new_block: BasicBlock,
29+
) {
30+
let const_raw_ptr = Ty::new_imm_ptr(tcx, tcx.types.unit);
31+
let rvalue = Rvalue::Cast(CastKind::PtrToPtr, Operand::Copy(pointer), const_raw_ptr);
32+
let thin_ptr = local_decls.push(LocalDecl::with_source_info(const_raw_ptr, source_info)).into();
33+
block_data
34+
.statements
35+
.push(Statement { source_info, kind: StatementKind::Assign(Box::new((thin_ptr, rvalue))) });
36+
37+
// Transmute the pointer to a usize (equivalent to `ptr.addr()`)
38+
let rvalue = Rvalue::Cast(CastKind::Transmute, Operand::Copy(thin_ptr), tcx.types.usize);
39+
let addr = local_decls.push(LocalDecl::with_source_info(tcx.types.usize, source_info)).into();
40+
block_data
41+
.statements
42+
.push(Statement { source_info, kind: StatementKind::Assign(Box::new((addr, rvalue))) });
43+
44+
// Check if the pointer is null.
45+
let is_ok = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
46+
let zero = Operand::Constant(Box::new(ConstOperand {
47+
span: source_info.span,
48+
user_ty: None,
49+
const_: Const::Val(ConstValue::Scalar(Scalar::from_target_usize(0, &tcx)), tcx.types.usize),
50+
}));
51+
block_data.statements.push(Statement {
52+
source_info,
53+
kind: StatementKind::Assign(Box::new((
54+
is_ok,
55+
Rvalue::BinaryOp(BinOp::Ne, Box::new((Operand::Copy(addr), zero))),
56+
))),
57+
});
58+
59+
// Set this block's terminator to our assert, continuing to new_block if we pass
60+
block_data.terminator = Some(Terminator {
61+
source_info,
62+
kind: TerminatorKind::Assert {
63+
cond: Operand::Copy(is_ok),
64+
expected: true,
65+
target: new_block,
66+
msg: Box::new(AssertKind::NullPointerDereference),
67+
// This calls panic_misaligned_pointer_dereference, which is #[rustc_nounwind].
68+
// We never want to insert an unwind into unsafe code, because unwinding could
69+
// make a failing UB check turn into much worse UB when we start unwinding.
70+
unwind: UnwindAction::Unreachable,
71+
},
72+
});
73+
}

0 commit comments

Comments
 (0)