Skip to content

Commit b088861

Browse files
FabianWolffFabianWolff
committed
Implement Mutation- and BorrowOfLayoutConstrainedField in thir-unsafeck
1 parent a84d1b2 commit b088861

30 files changed

+512
-59
lines changed

‎compiler/rustc_mir_build/src/build/expr/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,6 @@ mod as_operand;
6565
pub mod as_place;
6666
mod as_rvalue;
6767
mod as_temp;
68-
mod category;
68+
pub mod category;
6969
mod into;
7070
mod stmt;

‎compiler/rustc_mir_build/src/build/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1131,3 +1131,5 @@ mod expr;
11311131
mod matches;
11321132
mod misc;
11331133
mod scope;
1134+
1135+
pub(crate) use expr::category::Category as ExprCategory;

‎compiler/rustc_mir_build/src/check_unsafety.rs

+150-47
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
use crate::build::ExprCategory;
12
use crate::thir::visit::{self, Visitor};
23

34
use rustc_errors::struct_span_err;
45
use rustc_hir as hir;
6+
use rustc_middle::mir::BorrowKind;
57
use rustc_middle::thir::*;
6-
use rustc_middle::ty::{self, TyCtxt};
8+
use rustc_middle::ty::{self, ParamEnv, TyCtxt};
79
use rustc_session::lint::builtin::{UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE};
810
use rustc_session::lint::Level;
911
use rustc_span::def_id::{DefId, LocalDefId};
@@ -28,6 +30,8 @@ struct UnsafetyVisitor<'a, 'tcx> {
2830
is_const: bool,
2931
in_possible_lhs_union_assign: bool,
3032
in_union_destructure: bool,
33+
param_env: ParamEnv<'tcx>,
34+
inside_adt: bool,
3135
}
3236

3337
impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
@@ -134,6 +138,50 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
134138
}
135139
}
136140

141+
// Searches for accesses to layout constrained fields.
142+
struct LayoutConstrainedPlaceVisitor<'a, 'tcx> {
143+
found: bool,
144+
thir: &'a Thir<'tcx>,
145+
tcx: TyCtxt<'tcx>,
146+
}
147+
148+
impl<'a, 'tcx> LayoutConstrainedPlaceVisitor<'a, 'tcx> {
149+
fn new(thir: &'a Thir<'tcx>, tcx: TyCtxt<'tcx>) -> Self {
150+
Self { found: false, thir, tcx }
151+
}
152+
}
153+
154+
impl<'a, 'tcx> Visitor<'a, 'tcx> for LayoutConstrainedPlaceVisitor<'a, 'tcx> {
155+
fn thir(&self) -> &'a Thir<'tcx> {
156+
self.thir
157+
}
158+
159+
fn visit_expr(&mut self, expr: &Expr<'tcx>) {
160+
match expr.kind {
161+
ExprKind::Field { lhs, .. } => {
162+
if let ty::Adt(adt_def, _) = self.thir[lhs].ty.kind() {
163+
if (Bound::Unbounded, Bound::Unbounded)
164+
!= self.tcx.layout_scalar_valid_range(adt_def.did)
165+
{
166+
self.found = true;
167+
}
168+
}
169+
visit::walk_expr(self, expr);
170+
}
171+
172+
// Keep walking through the expression as long as we stay in the same
173+
// place, i.e. the expression is a place expression and not a dereference
174+
// (since dereferencing something leads us to a different place).
175+
ExprKind::Deref { .. } => {}
176+
ref kind if ExprCategory::of(kind).map_or(true, |cat| cat == ExprCategory::Place) => {
177+
visit::walk_expr(self, expr);
178+
}
179+
180+
_ => {}
181+
}
182+
}
183+
}
184+
137185
impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
138186
fn thir(&self) -> &'a Thir<'tcx> {
139187
&self.thir
@@ -161,60 +209,82 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
161209
}
162210

163211
fn visit_pat(&mut self, pat: &Pat<'tcx>) {
164-
use PatKind::*;
165-
166212
if self.in_union_destructure {
167213
match *pat.kind {
168214
// binding to a variable allows getting stuff out of variable
169-
Binding { .. }
215+
PatKind::Binding { .. }
170216
// match is conditional on having this value
171-
| Constant { .. }
172-
| Variant { .. }
173-
| Leaf { .. }
174-
| Deref { .. }
175-
| Range { .. }
176-
| Slice { .. }
177-
| Array { .. } => {
217+
| PatKind::Constant { .. }
218+
| PatKind::Variant { .. }
219+
| PatKind::Leaf { .. }
220+
| PatKind::Deref { .. }
221+
| PatKind::Range { .. }
222+
| PatKind::Slice { .. }
223+
| PatKind::Array { .. } => {
178224
self.requires_unsafe(pat.span, AccessToUnionField);
179-
return; // don't walk pattern
225+
return; // we can return here since this already requires unsafe
180226
}
181227
// wildcard doesn't take anything
182-
Wild |
228+
PatKind::Wild |
183229
// these just wrap other patterns
184-
Or { .. } |
185-
AscribeUserType { .. } => {}
230+
PatKind::Or { .. } |
231+
PatKind::AscribeUserType { .. } => {}
186232
}
187233
};
188234

189-
if let ty::Adt(adt_def, _) = pat.ty.kind() {
190-
// check for extracting values from union via destructuring
191-
if adt_def.is_union() {
192-
match *pat.kind {
193-
// assigning the whole union is okay
194-
// let x = Union { ... };
195-
// let y = x; // safe
196-
Binding { .. } |
197-
// binding to wildcard is okay since that never reads anything and stops double errors
198-
// with implict wildcard branches from `if let`s
199-
Wild |
200-
// doesn't have any effect on semantics
201-
AscribeUserType { .. } |
202-
// creating a union literal
203-
Constant { .. } => {},
204-
Leaf { .. } | Or { .. } => {
205-
// pattern matching with a union and not doing something like v = Union { bar: 5 }
206-
self.in_union_destructure = true;
235+
match &*pat.kind {
236+
PatKind::Leaf { .. } => {
237+
if let ty::Adt(adt_def, ..) = pat.ty.kind() {
238+
if adt_def.is_union() {
239+
let old_in_union_destructure =
240+
std::mem::replace(&mut self.in_union_destructure, true);
241+
visit::walk_pat(self, pat);
242+
self.in_union_destructure = old_in_union_destructure;
243+
} else if (Bound::Unbounded, Bound::Unbounded)
244+
!= self.tcx.layout_scalar_valid_range(adt_def.did)
245+
{
246+
let old_inside_adt = std::mem::replace(&mut self.inside_adt, true);
247+
visit::walk_pat(self, pat);
248+
self.inside_adt = old_inside_adt;
249+
} else {
207250
visit::walk_pat(self, pat);
208-
self.in_union_destructure = false;
209-
return; // don't walk pattern
210251
}
211-
Variant { .. } | Deref { .. } | Range { .. } | Slice { .. } | Array { .. } =>
212-
unreachable!("impossible union destructuring type"),
252+
} else {
253+
visit::walk_pat(self, pat);
213254
}
214255
}
256+
PatKind::Binding { mode: BindingMode::ByRef(borrow_kind), ty, .. } => {
257+
if self.inside_adt {
258+
if let ty::Ref(_, ty, _) = ty.kind() {
259+
match borrow_kind {
260+
BorrowKind::Shallow | BorrowKind::Shared | BorrowKind::Unique => {
261+
if !ty.is_freeze(self.tcx.at(pat.span), self.param_env) {
262+
self.requires_unsafe(pat.span, BorrowOfLayoutConstrainedField);
263+
}
264+
}
265+
BorrowKind::Mut { .. } => {
266+
self.requires_unsafe(pat.span, MutationOfLayoutConstrainedField);
267+
}
268+
}
269+
} else {
270+
span_bug!(
271+
pat.span,
272+
"BindingMode::ByRef in pattern, but found non-reference type {}",
273+
ty
274+
);
275+
}
276+
}
277+
visit::walk_pat(self, pat);
278+
}
279+
PatKind::Deref { .. } => {
280+
let old_inside_adt = std::mem::replace(&mut self.inside_adt, false);
281+
visit::walk_pat(self, pat);
282+
self.inside_adt = old_inside_adt;
283+
}
284+
_ => {
285+
visit::walk_pat(self, pat);
286+
}
215287
}
216-
217-
visit::walk_pat(self, pat);
218288
}
219289

220290
fn visit_expr(&mut self, expr: &Expr<'tcx>) {
@@ -361,15 +431,46 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
361431
}
362432
}
363433
}
364-
// don't have any special handling for AssignOp since it causes a read *and* write to lhs
365-
ExprKind::Assign { lhs, rhs } => {
366-
// assigning to a union is safe, check here so it doesn't get treated as a read later
367-
self.in_possible_lhs_union_assign = true;
368-
visit::walk_expr(self, &self.thir()[lhs]);
369-
self.in_possible_lhs_union_assign = false;
370-
visit::walk_expr(self, &self.thir()[rhs]);
371-
return; // don't visit the whole expression
434+
ExprKind::Assign { lhs, rhs } | ExprKind::AssignOp { lhs, rhs, .. } => {
435+
// First, check whether we are mutating a layout constrained field
436+
let mut visitor = LayoutConstrainedPlaceVisitor::new(self.thir, self.tcx);
437+
visit::walk_expr(&mut visitor, &self.thir[lhs]);
438+
if visitor.found {
439+
self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField);
440+
}
441+
442+
// Second, check for accesses to union fields
443+
// don't have any special handling for AssignOp since it causes a read *and* write to lhs
444+
if matches!(expr.kind, ExprKind::Assign { .. }) {
445+
// assigning to a union is safe, check here so it doesn't get treated as a read later
446+
self.in_possible_lhs_union_assign = true;
447+
visit::walk_expr(self, &self.thir()[lhs]);
448+
self.in_possible_lhs_union_assign = false;
449+
visit::walk_expr(self, &self.thir()[rhs]);
450+
return; // we have already visited everything by now
451+
}
372452
}
453+
ExprKind::Borrow { borrow_kind, arg } => match borrow_kind {
454+
BorrowKind::Shallow | BorrowKind::Shared | BorrowKind::Unique => {
455+
if !self.thir[arg]
456+
.ty
457+
.is_freeze(self.tcx.at(self.thir[arg].span), self.param_env)
458+
{
459+
let mut visitor = LayoutConstrainedPlaceVisitor::new(self.thir, self.tcx);
460+
visit::walk_expr(&mut visitor, expr);
461+
if visitor.found {
462+
self.requires_unsafe(expr.span, BorrowOfLayoutConstrainedField);
463+
}
464+
}
465+
}
466+
BorrowKind::Mut { .. } => {
467+
let mut visitor = LayoutConstrainedPlaceVisitor::new(self.thir, self.tcx);
468+
visit::walk_expr(&mut visitor, expr);
469+
if visitor.found {
470+
self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField);
471+
}
472+
}
473+
},
373474
_ => {}
374475
}
375476
visit::walk_expr(self, expr);
@@ -541,6 +642,8 @@ pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam<LocalD
541642
is_const,
542643
in_possible_lhs_union_assign: false,
543644
in_union_destructure: false,
645+
param_env: tcx.param_env(def.did),
646+
inside_adt: false,
544647
};
545648
visitor.visit_expr(&thir[expr]);
546649
}

‎src/test/ui/unsafe/ranged_ints2.stderr renamed to ‎src/test/ui/unsafe/ranged_ints2.mirunsafeck.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
2-
--> $DIR/ranged_ints2.rs:8:13
2+
--> $DIR/ranged_ints2.rs:11:13
33
|
44
LL | let y = &mut x.0;
55
| ^^^^^^^^ mutation of layout constrained field

‎src/test/ui/unsafe/ranged_ints2.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// revisions: mirunsafeck thirunsafeck
2+
// [thirunsafeck]compile-flags: -Z thir-unsafeck
3+
14
#![feature(rustc_attrs)]
25

36
#[rustc_layout_scalar_valid_range_start(1)]

‎src/test/ui/unsafe/ranged_ints2.thirunsafeck.stderr

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
2+
--> $DIR/ranged_ints2.rs:11:13
3+
|
4+
LL | let y = &mut x.0;
5+
| ^^^^^^^^ mutation of layout constrained field
6+
|
7+
= note: mutating layout constrained fields cannot statically be checked for valid values
8+
9+
error: aborting due to previous error
10+
11+
For more information about this error, try `rustc --explain E0133`.

‎src/test/ui/unsafe/ranged_ints2_const.stderr renamed to ‎src/test/ui/unsafe/ranged_ints2_const.mirunsafeck.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0658]: mutable references are not allowed in constant functions
2-
--> $DIR/ranged_ints2_const.rs:11:13
2+
--> $DIR/ranged_ints2_const.rs:14:13
33
|
44
LL | let y = &mut x.0;
55
| ^^^^^^^^
@@ -8,7 +8,7 @@ LL | let y = &mut x.0;
88
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
99

1010
error[E0658]: mutable references are not allowed in constant functions
11-
--> $DIR/ranged_ints2_const.rs:18:22
11+
--> $DIR/ranged_ints2_const.rs:21:22
1212
|
1313
LL | let y = unsafe { &mut x.0 };
1414
| ^^^^^^^^
@@ -17,7 +17,7 @@ LL | let y = unsafe { &mut x.0 };
1717
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
1818

1919
error[E0658]: mutable references are not allowed in constant functions
20-
--> $DIR/ranged_ints2_const.rs:24:22
20+
--> $DIR/ranged_ints2_const.rs:27:22
2121
|
2222
LL | unsafe { let y = &mut x.0; }
2323
| ^^^^^^^^
@@ -26,7 +26,7 @@ LL | unsafe { let y = &mut x.0; }
2626
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
2727

2828
error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
29-
--> $DIR/ranged_ints2_const.rs:11:13
29+
--> $DIR/ranged_ints2_const.rs:14:13
3030
|
3131
LL | let y = &mut x.0;
3232
| ^^^^^^^^ mutation of layout constrained field

‎src/test/ui/unsafe/ranged_ints2_const.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// revisions: mirunsafeck thirunsafeck
2+
// [thirunsafeck]compile-flags: -Z thir-unsafeck
3+
14
#![feature(rustc_attrs)]
25

36
#[rustc_layout_scalar_valid_range_start(1)]

‎src/test/ui/unsafe/ranged_ints2_const.thirunsafeck.stderr

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
2+
--> $DIR/ranged_ints2_const.rs:14:13
3+
|
4+
LL | let y = &mut x.0;
5+
| ^^^^^^^^ mutation of layout constrained field
6+
|
7+
= note: mutating layout constrained fields cannot statically be checked for valid values
8+
9+
error[E0658]: mutable references are not allowed in constant functions
10+
--> $DIR/ranged_ints2_const.rs:14:13
11+
|
12+
LL | let y = &mut x.0;
13+
| ^^^^^^^^
14+
|
15+
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
16+
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
17+
18+
error[E0658]: mutable references are not allowed in constant functions
19+
--> $DIR/ranged_ints2_const.rs:21:22
20+
|
21+
LL | let y = unsafe { &mut x.0 };
22+
| ^^^^^^^^
23+
|
24+
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
25+
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
26+
27+
error[E0658]: mutable references are not allowed in constant functions
28+
--> $DIR/ranged_ints2_const.rs:27:22
29+
|
30+
LL | unsafe { let y = &mut x.0; }
31+
| ^^^^^^^^
32+
|
33+
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
34+
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
35+
36+
error: aborting due to 4 previous errors
37+
38+
Some errors have detailed explanations: E0133, E0658.
39+
For more information about an error, try `rustc --explain E0133`.

‎src/test/ui/unsafe/ranged_ints3.stderr renamed to ‎src/test/ui/unsafe/ranged_ints3.mirunsafeck.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0133]: borrow of layout constrained field with interior mutability is unsafe and requires unsafe function or block
2-
--> $DIR/ranged_ints3.rs:10:13
2+
--> $DIR/ranged_ints3.rs:13:13
33
|
44
LL | let y = &x.0;
55
| ^^^^ borrow of layout constrained field with interior mutability

‎src/test/ui/unsafe/ranged_ints3.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// revisions: mirunsafeck thirunsafeck
2+
// [thirunsafeck]compile-flags: -Z thir-unsafeck
3+
14
#![feature(rustc_attrs)]
25

36
use std::cell::Cell;

0 commit comments

Comments
 (0)