Skip to content

Commit 4af0952

Browse files
committed
Call is_freeze less in unsafety-checking
This is to avoid cycles when calling `is_freeze` on an opaque type.
1 parent 7f41cf4 commit 4af0952

File tree

2 files changed

+58
-19
lines changed

2 files changed

+58
-19
lines changed

src/librustc_mir/transform/check_unsafety.rs

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,16 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
178178
}
179179

180180
fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
181+
// prevent
182+
// * `&mut x.field`
183+
// * `x.field = y;`
184+
// * `&x.field` if `field`'s type has interior mutability
185+
// because either of these would allow modifying the layout constrained field and
186+
// insert values that violate the layout constraints.
187+
if context.is_mutating_use() || context.is_borrow() {
188+
self.check_mut_borrowing_layout_constrained_field(place, context.is_mutating_use());
189+
}
190+
181191
for (i, elem) in place.projection.iter().enumerate() {
182192
let proj_base = &place.projection[..i];
183193

@@ -198,24 +208,9 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
198208
);
199209
}
200210
}
201-
let is_borrow_of_interior_mut = context.is_borrow()
202-
&& !Place::ty_from(place.local, proj_base, self.body, self.tcx).ty.is_freeze(
203-
self.tcx,
204-
self.param_env,
205-
self.source_info.span,
206-
);
207-
// prevent
208-
// * `&mut x.field`
209-
// * `x.field = y;`
210-
// * `&x.field` if `field`'s type has interior mutability
211-
// because either of these would allow modifying the layout constrained field and
212-
// insert values that violate the layout constraints.
213-
if context.is_mutating_use() || is_borrow_of_interior_mut {
214-
self.check_mut_borrowing_layout_constrained_field(place, context.is_mutating_use());
215-
}
216211
let old_source_info = self.source_info;
217-
if let (local, []) = (&place.local, proj_base) {
218-
let decl = &self.body.local_decls[*local];
212+
if let [] = proj_base {
213+
let decl = &self.body.local_decls[place.local];
219214
if decl.internal {
220215
if let LocalInfo::StaticRef { def_id, .. } = decl.local_info {
221216
if self.tcx.is_mutable_static(def_id) {
@@ -240,7 +235,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
240235
// Internal locals are used in the `move_val_init` desugaring.
241236
// We want to check unsafety against the source info of the
242237
// desugaring, rather than the source info of the RHS.
243-
self.source_info = self.body.local_decls[*local].source_info;
238+
self.source_info = self.body.local_decls[place.local].source_info;
244239
}
245240
}
246241
}
@@ -396,6 +391,9 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
396391
cursor = proj_base;
397392

398393
match elem {
394+
// Modifications behind a dereference don't affect the value of
395+
// the pointer.
396+
ProjectionElem::Deref => return,
399397
ProjectionElem::Field(..) => {
400398
let ty =
401399
Place::ty_from(place.local, proj_base, &self.body.local_decls, self.tcx).ty;
@@ -409,14 +407,23 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
409407
"mutating layout constrained fields cannot statically be \
410408
checked for valid values",
411409
)
412-
} else {
410+
411+
// Check `is_freeze` as late as possible to avoid cycle errors
412+
// with opaque types.
413+
} else if !place.ty(self.body, self.tcx).ty.is_freeze(
414+
self.tcx,
415+
self.param_env,
416+
self.source_info.span,
417+
) {
413418
(
414419
"borrow of layout constrained field with interior \
415420
mutability",
416421
"references to fields of layout constrained fields \
417422
lose the constraints. Coupled with interior mutability, \
418423
the field can be changed to invalid values",
419424
)
425+
} else {
426+
continue;
420427
};
421428
self.require_unsafe(
422429
description,
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Ensure that we don't get a cycle error from trying to determine whether an
2+
// opaque type implements `Freeze` in safety checking, when it doesn't matter.
3+
4+
// check-pass
5+
6+
#![feature(rustc_attrs)]
7+
8+
struct AnyValue<T>(T);
9+
10+
// No need to check for `Freeze` here, there's no
11+
// `rustc_layout_scalar_valid_range_start` involved.
12+
fn not_restricted(c: bool) -> impl Sized {
13+
if c {
14+
let x = AnyValue(not_restricted(false));
15+
&x.0;
16+
}
17+
2u32
18+
}
19+
20+
#[rustc_layout_scalar_valid_range_start(1)]
21+
struct NonZero<T>(T);
22+
23+
// No need to check for `Freeze` here, we're not borrowing the field.
24+
fn not_field(c: bool) -> impl Sized {
25+
if c {
26+
let x = unsafe { NonZero(not_field(false)) };
27+
&x;
28+
}
29+
5u32
30+
}
31+
32+
fn main() {}

0 commit comments

Comments
 (0)