Skip to content

Commit 29780f4

Browse files
Introduce the (WIP) THIR unsafety checker
1 parent d956122 commit 29780f4

File tree

6 files changed

+343
-1
lines changed

6 files changed

+343
-1
lines changed

compiler/rustc_interface/src/passes.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,11 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {
884884

885885
sess.time("MIR_effect_checking", || {
886886
for def_id in tcx.body_owners() {
887-
mir::transform::check_unsafety::check_unsafety(tcx, def_id);
887+
if tcx.sess.opts.debugging_opts.thir_unsafeck {
888+
tcx.ensure().thir_check_unsafety(def_id);
889+
} else {
890+
mir::transform::check_unsafety::check_unsafety(tcx, def_id);
891+
}
888892

889893
if tcx.hir().body_const_context(def_id).is_some() {
890894
tcx.ensure()

compiler/rustc_interface/src/tests.rs

+1
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,7 @@ fn test_debugging_options_tracking_hash() {
735735
tracked!(symbol_mangling_version, Some(SymbolManglingVersion::V0));
736736
tracked!(teach, true);
737737
tracked!(thinlto, Some(true));
738+
tracked!(thir_unsafeck, true);
738739
tracked!(tune_cpu, Some(String::from("abc")));
739740
tracked!(tls_model, Some(TlsModel::GeneralDynamic));
740741
tracked!(trap_unreachable, Some(false));

compiler/rustc_middle/src/query/mod.rs

+13
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,19 @@ rustc_queries! {
611611
}
612612
}
613613

614+
/// Unsafety-check this `LocalDefId` with THIR unsafeck. This should be
615+
/// used with `-Zthir-unsafeck`.
616+
query thir_check_unsafety(key: LocalDefId) {
617+
desc { |tcx| "unsafety-checking `{}`", tcx.def_path_str(key.to_def_id()) }
618+
cache_on_disk_if { true }
619+
}
620+
query thir_check_unsafety_for_const_arg(key: (LocalDefId, DefId)) {
621+
desc {
622+
|tcx| "unsafety-checking the const argument `{}`",
623+
tcx.def_path_str(key.0.to_def_id())
624+
}
625+
}
626+
614627
/// HACK: when evaluated, this reports a "unsafe derive on repr(packed)" error.
615628
///
616629
/// Unsafety checking is executed for each method separately, but we only want
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,319 @@
1+
use crate::thir::visit::{self, Visitor};
2+
use crate::thir::*;
3+
4+
use rustc_errors::struct_span_err;
5+
use rustc_hir as hir;
6+
use rustc_middle::ty::{self, TyCtxt};
7+
use rustc_session::lint::builtin::{UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE};
8+
use rustc_session::lint::Level;
9+
use rustc_span::def_id::{DefId, LocalDefId};
10+
use rustc_span::Span;
11+
12+
struct UnsafetyVisitor<'tcx> {
13+
tcx: TyCtxt<'tcx>,
14+
/// The `HirId` of the current scope, which would be the `HirId`
15+
/// of the current HIR node, modulo adjustments. Used for lint levels.
16+
hir_context: hir::HirId,
17+
/// The current "safety context". This notably tracks whether we are in an
18+
/// `unsafe` block, and whether it has been used.
19+
safety_context: SafetyContext,
20+
body_unsafety: BodyUnsafety,
21+
}
22+
23+
impl<'tcx> UnsafetyVisitor<'tcx> {
24+
fn requires_unsafe(&mut self, span: Span, kind: UnsafeOpKind) {
25+
let (description, note) = kind.description_and_note();
26+
let unsafe_op_in_unsafe_fn_allowed = self.unsafe_op_in_unsafe_fn_allowed();
27+
match self.safety_context {
28+
SafetyContext::UnsafeBlock { ref mut used, .. } => {
29+
if !self.body_unsafety.is_unsafe() || !unsafe_op_in_unsafe_fn_allowed {
30+
// Mark this block as useful
31+
*used = true;
32+
}
33+
}
34+
SafetyContext::UnsafeFn if unsafe_op_in_unsafe_fn_allowed => {}
35+
SafetyContext::UnsafeFn => {
36+
// unsafe_op_in_unsafe_fn is disallowed
37+
if kind == BorrowOfPackedField {
38+
// FIXME handle borrows of packed fields
39+
} else {
40+
struct_span_err!(
41+
self.tcx.sess,
42+
span,
43+
E0133,
44+
"{} is unsafe and requires unsafe block",
45+
description,
46+
)
47+
.span_label(span, description)
48+
.note(note)
49+
.emit();
50+
}
51+
}
52+
SafetyContext::Safe => {
53+
if kind == BorrowOfPackedField {
54+
// FIXME handle borrows of packed fields
55+
} else {
56+
let fn_sugg = if unsafe_op_in_unsafe_fn_allowed { " function or" } else { "" };
57+
struct_span_err!(
58+
self.tcx.sess,
59+
span,
60+
E0133,
61+
"{} is unsafe and requires unsafe{} block",
62+
description,
63+
fn_sugg,
64+
)
65+
.span_label(span, description)
66+
.note(note)
67+
.emit();
68+
}
69+
}
70+
}
71+
}
72+
73+
fn warn_unused_unsafe(
74+
&self,
75+
hir_id: hir::HirId,
76+
block_span: Span,
77+
enclosing_span: Option<Span>,
78+
) {
79+
let block_span = self.tcx.sess.source_map().guess_head_span(block_span);
80+
self.tcx.struct_span_lint_hir(UNUSED_UNSAFE, hir_id, block_span, |lint| {
81+
let msg = "unnecessary `unsafe` block";
82+
let mut db = lint.build(msg);
83+
db.span_label(block_span, msg);
84+
if let Some(enclosing_span) = enclosing_span {
85+
db.span_label(
86+
enclosing_span,
87+
format!("because it's nested under this `unsafe` block"),
88+
);
89+
}
90+
db.emit();
91+
});
92+
}
93+
94+
/// Whether the `unsafe_op_in_unsafe_fn` lint is `allow`ed at the current HIR node.
95+
fn unsafe_op_in_unsafe_fn_allowed(&self) -> bool {
96+
self.tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, self.hir_context).0 == Level::Allow
97+
}
98+
}
99+
100+
impl<'thir, 'tcx> Visitor<'thir, 'tcx> for UnsafetyVisitor<'tcx> {
101+
fn visit_block(&mut self, block: &Block<'thir, 'tcx>) {
102+
if let BlockSafety::ExplicitUnsafe(hir_id) = block.safety_mode {
103+
if let SafetyContext::UnsafeBlock { span: enclosing_span, .. } = self.safety_context {
104+
self.warn_unused_unsafe(
105+
hir_id,
106+
block.span,
107+
Some(self.tcx.sess.source_map().guess_head_span(enclosing_span)),
108+
);
109+
} else {
110+
let prev_context = self.safety_context;
111+
self.safety_context =
112+
SafetyContext::UnsafeBlock { span: block.span, hir_id, used: false };
113+
visit::walk_block(self, block);
114+
if let SafetyContext::UnsafeBlock { used: false, span, hir_id } =
115+
self.safety_context
116+
{
117+
self.warn_unused_unsafe(hir_id, span, self.body_unsafety.unsafe_fn_sig_span());
118+
}
119+
self.safety_context = prev_context;
120+
return;
121+
}
122+
}
123+
124+
visit::walk_block(self, block);
125+
}
126+
127+
fn visit_expr(&mut self, expr: &'thir Expr<'thir, 'tcx>) {
128+
match expr.kind {
129+
ExprKind::Scope { value, lint_level: LintLevel::Explicit(hir_id), .. } => {
130+
let prev_id = self.hir_context;
131+
self.hir_context = hir_id;
132+
self.visit_expr(value);
133+
self.hir_context = prev_id;
134+
return;
135+
}
136+
ExprKind::Call { fun, .. } => {
137+
if fun.ty.fn_sig(self.tcx).unsafety() == hir::Unsafety::Unsafe {
138+
self.requires_unsafe(expr.span, CallToUnsafeFunction);
139+
}
140+
}
141+
_ => {}
142+
}
143+
144+
visit::walk_expr(self, expr);
145+
}
146+
}
147+
148+
#[derive(Clone, Copy)]
149+
enum SafetyContext {
150+
Safe,
151+
UnsafeFn,
152+
UnsafeBlock { span: Span, hir_id: hir::HirId, used: bool },
153+
}
154+
155+
#[derive(Clone, Copy)]
156+
enum BodyUnsafety {
157+
/// The body is not unsafe.
158+
Safe,
159+
/// The body is an unsafe function. The span points to
160+
/// the signature of the function.
161+
Unsafe(Span),
162+
}
163+
164+
impl BodyUnsafety {
165+
/// Returns whether the body is unsafe.
166+
fn is_unsafe(&self) -> bool {
167+
matches!(self, BodyUnsafety::Unsafe(_))
168+
}
169+
170+
/// If the body is unsafe, returns the `Span` of its signature.
171+
fn unsafe_fn_sig_span(self) -> Option<Span> {
172+
match self {
173+
BodyUnsafety::Unsafe(span) => Some(span),
174+
BodyUnsafety::Safe => None,
175+
}
176+
}
177+
}
178+
179+
#[derive(Clone, Copy, PartialEq)]
180+
enum UnsafeOpKind {
181+
CallToUnsafeFunction,
182+
#[allow(dead_code)] // FIXME
183+
UseOfInlineAssembly,
184+
#[allow(dead_code)] // FIXME
185+
InitializingTypeWith,
186+
#[allow(dead_code)] // FIXME
187+
CastOfPointerToInt,
188+
#[allow(dead_code)] // FIXME
189+
BorrowOfPackedField,
190+
#[allow(dead_code)] // FIXME
191+
UseOfMutableStatic,
192+
#[allow(dead_code)] // FIXME
193+
UseOfExternStatic,
194+
#[allow(dead_code)] // FIXME
195+
DerefOfRawPointer,
196+
#[allow(dead_code)] // FIXME
197+
AssignToDroppingUnionField,
198+
#[allow(dead_code)] // FIXME
199+
AccessToUnionField,
200+
#[allow(dead_code)] // FIXME
201+
MutationOfLayoutConstrainedField,
202+
#[allow(dead_code)] // FIXME
203+
BorrowOfLayoutConstrainedField,
204+
#[allow(dead_code)] // FIXME
205+
CallToFunctionWith,
206+
}
207+
208+
use UnsafeOpKind::*;
209+
210+
impl UnsafeOpKind {
211+
pub fn description_and_note(&self) -> (&'static str, &'static str) {
212+
match self {
213+
CallToUnsafeFunction => (
214+
"call to unsafe function",
215+
"consult the function's documentation for information on how to avoid undefined \
216+
behavior",
217+
),
218+
UseOfInlineAssembly => (
219+
"use of inline assembly",
220+
"inline assembly is entirely unchecked and can cause undefined behavior",
221+
),
222+
InitializingTypeWith => (
223+
"initializing type with `rustc_layout_scalar_valid_range` attr",
224+
"initializing a layout restricted type's field with a value outside the valid \
225+
range is undefined behavior",
226+
),
227+
CastOfPointerToInt => {
228+
("cast of pointer to int", "casting pointers to integers in constants")
229+
}
230+
BorrowOfPackedField => (
231+
"borrow of packed field",
232+
"fields of packed structs might be misaligned: dereferencing a misaligned pointer \
233+
or even just creating a misaligned reference is undefined behavior",
234+
),
235+
UseOfMutableStatic => (
236+
"use of mutable static",
237+
"mutable statics can be mutated by multiple threads: aliasing violations or data \
238+
races will cause undefined behavior",
239+
),
240+
UseOfExternStatic => (
241+
"use of extern static",
242+
"extern statics are not controlled by the Rust type system: invalid data, \
243+
aliasing violations or data races will cause undefined behavior",
244+
),
245+
DerefOfRawPointer => (
246+
"dereference of raw pointer",
247+
"raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules \
248+
and cause data races: all of these are undefined behavior",
249+
),
250+
AssignToDroppingUnionField => (
251+
"assignment to union field that might need dropping",
252+
"the previous content of the field will be dropped, which causes undefined \
253+
behavior if the field was not properly initialized",
254+
),
255+
AccessToUnionField => (
256+
"access to union field",
257+
"the field may not be properly initialized: using uninitialized data will cause \
258+
undefined behavior",
259+
),
260+
MutationOfLayoutConstrainedField => (
261+
"mutation of layout constrained field",
262+
"mutating layout constrained fields cannot statically be checked for valid values",
263+
),
264+
BorrowOfLayoutConstrainedField => (
265+
"borrow of layout constrained field with interior mutability",
266+
"references to fields of layout constrained fields lose the constraints. Coupled \
267+
with interior mutability, the field can be changed to invalid values",
268+
),
269+
CallToFunctionWith => (
270+
"call to function with `#[target_feature]`",
271+
"can only be called if the required target features are available",
272+
),
273+
}
274+
}
275+
}
276+
277+
// FIXME: checking unsafety for closures should be handled by their parent body,
278+
// as they inherit their "safety context" from their declaration site.
279+
pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, thir: &Expr<'_, 'tcx>, hir_id: hir::HirId) {
280+
let body_unsafety = tcx.hir().fn_sig_by_hir_id(hir_id).map_or(BodyUnsafety::Safe, |fn_sig| {
281+
if fn_sig.header.unsafety == hir::Unsafety::Unsafe {
282+
BodyUnsafety::Unsafe(fn_sig.span)
283+
} else {
284+
BodyUnsafety::Safe
285+
}
286+
});
287+
let safety_context =
288+
if body_unsafety.is_unsafe() { SafetyContext::UnsafeFn } else { SafetyContext::Safe };
289+
let mut visitor = UnsafetyVisitor { tcx, safety_context, hir_context: hir_id, body_unsafety };
290+
visitor.visit_expr(thir);
291+
}
292+
293+
crate fn thir_check_unsafety_inner<'tcx>(
294+
tcx: TyCtxt<'tcx>,
295+
def: ty::WithOptConstParam<LocalDefId>,
296+
) {
297+
let hir_id = tcx.hir().local_def_id_to_hir_id(def.did);
298+
let body_id = tcx.hir().body_owned_by(hir_id);
299+
let body = tcx.hir().body(body_id);
300+
301+
let arena = Arena::default();
302+
let thir = cx::build_thir(tcx, def, &arena, &body.value);
303+
check_unsafety(tcx, thir, hir_id);
304+
}
305+
306+
crate fn thir_check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) {
307+
if let Some(def) = ty::WithOptConstParam::try_lookup(def_id, tcx) {
308+
tcx.thir_check_unsafety_for_const_arg(def)
309+
} else {
310+
thir_check_unsafety_inner(tcx, ty::WithOptConstParam::unknown(def_id))
311+
}
312+
}
313+
314+
crate fn thir_check_unsafety_for_const_arg<'tcx>(
315+
tcx: TyCtxt<'tcx>,
316+
(did, param_did): (LocalDefId, DefId),
317+
) {
318+
thir_check_unsafety_inner(tcx, ty::WithOptConstParam { did, const_param_did: Some(param_did) })
319+
}

compiler/rustc_mir_build/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ extern crate tracing;
1919
extern crate rustc_middle;
2020

2121
mod build;
22+
mod check_unsafety;
2223
mod lints;
2324
pub mod thir;
2425

@@ -28,4 +29,6 @@ pub fn provide(providers: &mut Providers) {
2829
providers.check_match = thir::pattern::check_match;
2930
providers.lit_to_const = thir::constant::lit_to_const;
3031
providers.mir_built = build::mir_built;
32+
providers.thir_check_unsafety = check_unsafety::thir_check_unsafety;
33+
providers.thir_check_unsafety_for_const_arg = check_unsafety::thir_check_unsafety_for_const_arg;
3134
}

compiler/rustc_session/src/options.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1251,6 +1251,8 @@ options! {
12511251
"select processor to schedule for (`rustc --print target-cpus` for details)"),
12521252
thinlto: Option<bool> = (None, parse_opt_bool, [TRACKED],
12531253
"enable ThinLTO when possible"),
1254+
thir_unsafeck: bool = (false, parse_bool, [TRACKED],
1255+
"use the work-in-progress THIR unsafety checker. NOTE: this is unsound (default: no)"),
12541256
/// We default to 1 here since we want to behave like
12551257
/// a sequential compiler for now. This'll likely be adjusted
12561258
/// in the future. Note that -Zthreads=0 is the way to get

0 commit comments

Comments
 (0)