Skip to content

Commit 9d3d57c

Browse files
1c3t3a1c3t3a
committed
Refactor PointerFinder into a separate module
This also parameterize the "excluded pointee types" and exposes a general method for inserting checks on pointers. This is a preparation for adding a NullCheck that makes use of the same code.
1 parent eedc229 commit 9d3d57c

File tree

3 files changed

+168
-128
lines changed

3 files changed

+168
-128
lines changed

‎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>>,

‎compiler/rustc_mir_transform/src/check_pointers.rs

+161
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
use rustc_hir::lang_items::LangItem;
2+
use rustc_index::IndexVec;
3+
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
4+
use rustc_middle::mir::*;
5+
use rustc_middle::ty::{self, Ty, TyCtxt};
6+
use tracing::{debug, trace};
7+
8+
pub(crate) fn check_pointers<'a, 'tcx, F>(
9+
tcx: TyCtxt<'tcx>,
10+
body: &mut Body<'tcx>,
11+
excluded_pointees: &'a [Ty<'tcx>],
12+
on_finding: F,
13+
) where
14+
F: Fn(
15+
TyCtxt<'tcx>,
16+
&mut IndexVec<Local, LocalDecl<'tcx>>,
17+
&mut BasicBlockData<'tcx>,
18+
Place<'tcx>,
19+
Ty<'tcx>,
20+
SourceInfo,
21+
BasicBlock,
22+
),
23+
{
24+
// This pass emits new panics. If for whatever reason we do not have a panic
25+
// implementation, running this pass may cause otherwise-valid code to not compile.
26+
if tcx.lang_items().get(LangItem::PanicImpl).is_none() {
27+
return;
28+
}
29+
30+
let typing_env = body.typing_env(tcx);
31+
let basic_blocks = body.basic_blocks.as_mut();
32+
let local_decls = &mut body.local_decls;
33+
34+
// This operation inserts new blocks. Each insertion changes the Location for all
35+
// statements/blocks after. Iterating or visiting the MIR in order would require updating
36+
// our current location after every insertion. By iterating backwards, we dodge this issue:
37+
// The only Locations that an insertion changes have already been handled.
38+
for block in (0..basic_blocks.len()).rev() {
39+
let block = block.into();
40+
for statement_index in (0..basic_blocks[block].statements.len()).rev() {
41+
let location = Location { block, statement_index };
42+
let statement = &basic_blocks[block].statements[statement_index];
43+
let source_info = statement.source_info;
44+
45+
let mut finder = PointerFinder::new(tcx, local_decls, typing_env, excluded_pointees);
46+
finder.visit_statement(statement, location);
47+
48+
for (local, ty) in finder.into_found_pointers() {
49+
debug!("Inserting check for {:?}", ty);
50+
let new_block = split_block(basic_blocks, location);
51+
on_finding(
52+
tcx,
53+
local_decls,
54+
&mut basic_blocks[block],
55+
local,
56+
ty,
57+
source_info,
58+
new_block,
59+
);
60+
}
61+
}
62+
}
63+
}
64+
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+
excluded_pointees: &'a [Ty<'tcx>],
71+
}
72+
73+
impl<'a, 'tcx> PointerFinder<'a, 'tcx> {
74+
fn new(
75+
tcx: TyCtxt<'tcx>,
76+
local_decls: &'a mut LocalDecls<'tcx>,
77+
typing_env: ty::TypingEnv<'tcx>,
78+
excluded_pointees: &'a [Ty<'tcx>],
79+
) -> Self {
80+
PointerFinder { tcx, local_decls, typing_env, excluded_pointees, pointers: Vec::new() }
81+
}
82+
83+
fn into_found_pointers(self) -> Vec<(Place<'tcx>, Ty<'tcx>)> {
84+
self.pointers
85+
}
86+
}
87+
88+
impl<'a, 'tcx> Visitor<'tcx> for PointerFinder<'a, 'tcx> {
89+
fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) {
90+
// We want to only check reads and writes to Places, so we specifically exclude
91+
// Borrow and RawBorrow.
92+
match context {
93+
PlaceContext::MutatingUse(
94+
MutatingUseContext::Store
95+
| MutatingUseContext::AsmOutput
96+
| MutatingUseContext::Call
97+
| MutatingUseContext::Yield
98+
| MutatingUseContext::Drop,
99+
) => {}
100+
PlaceContext::NonMutatingUse(
101+
NonMutatingUseContext::Copy | NonMutatingUseContext::Move,
102+
) => {}
103+
_ => {
104+
return;
105+
}
106+
}
107+
108+
if !place.is_indirect() {
109+
return;
110+
}
111+
112+
// Since Deref projections must come first and only once, the pointer for an indirect place
113+
// is the Local that the Place is based on.
114+
let pointer = Place::from(place.local);
115+
let pointer_ty = self.local_decls[place.local].ty;
116+
117+
// We only want to check places based on unsafe pointers
118+
if !pointer_ty.is_unsafe_ptr() {
119+
trace!("Indirect, but not based on an unsafe ptr, not checking {:?}", place);
120+
return;
121+
}
122+
123+
let pointee_ty =
124+
pointer_ty.builtin_deref(true).expect("no builtin_deref for an unsafe pointer");
125+
// Ideally we'd support this in the future, but for now we are limited to sized types.
126+
if !pointee_ty.is_sized(self.tcx, self.typing_env) {
127+
debug!("Unsafe pointer, but pointee is not known to be sized: {:?}", pointer_ty);
128+
return;
129+
}
130+
131+
// We don't need to look for str and slices, we already rejected unsized types above
132+
let element_ty = match pointee_ty.kind() {
133+
ty::Array(ty, _) => *ty,
134+
_ => pointee_ty,
135+
};
136+
if self.excluded_pointees.contains(&element_ty) {
137+
debug!("Skipping pointer for type: {:?}", pointee_ty);
138+
return;
139+
}
140+
141+
self.pointers.push((pointer, pointee_ty));
142+
143+
self.super_place(place, context, location);
144+
}
145+
}
146+
147+
fn split_block(
148+
basic_blocks: &mut IndexVec<BasicBlock, BasicBlockData<'_>>,
149+
location: Location,
150+
) -> BasicBlock {
151+
let block_data = &mut basic_blocks[location.block];
152+
153+
// Drain every statement after this one and move the current terminator to a new basic block
154+
let new_block = BasicBlockData {
155+
statements: block_data.statements.split_off(location.statement_index),
156+
terminator: block_data.terminator.take(),
157+
is_cleanup: block_data.is_cleanup,
158+
};
159+
160+
basic_blocks.push(new_block)
161+
}

‎compiler/rustc_mir_transform/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use std::sync::LazyLock;
4444

4545
use pass_manager::{self as pm, Lint, MirLint, MirPass, WithMinOptLevel};
4646

47+
mod check_pointers;
4748
mod cost_checker;
4849
mod cross_crate_inline;
4950
mod deduce_param_attrs;

0 commit comments

Comments
 (0)