Skip to content

Commit

Permalink
Call is_freeze less in unsafety-checking
Browse files Browse the repository at this point in the history
This is to avoid cycles when calling `is_freeze` on an opaque type.
  • Loading branch information
matthewjasper committed Feb 14, 2020
1 parent 7f41cf4 commit 4af0952
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 19 deletions.
45 changes: 26 additions & 19 deletions src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,16 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
}

fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
// prevent
// * `&mut x.field`
// * `x.field = y;`
// * `&x.field` if `field`'s type has interior mutability
// because either of these would allow modifying the layout constrained field and
// insert values that violate the layout constraints.
if context.is_mutating_use() || context.is_borrow() {
self.check_mut_borrowing_layout_constrained_field(place, context.is_mutating_use());
}

for (i, elem) in place.projection.iter().enumerate() {
let proj_base = &place.projection[..i];

Expand All @@ -198,24 +208,9 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
);
}
}
let is_borrow_of_interior_mut = context.is_borrow()
&& !Place::ty_from(place.local, proj_base, self.body, self.tcx).ty.is_freeze(
self.tcx,
self.param_env,
self.source_info.span,
);
// prevent
// * `&mut x.field`
// * `x.field = y;`
// * `&x.field` if `field`'s type has interior mutability
// because either of these would allow modifying the layout constrained field and
// insert values that violate the layout constraints.
if context.is_mutating_use() || is_borrow_of_interior_mut {
self.check_mut_borrowing_layout_constrained_field(place, context.is_mutating_use());
}
let old_source_info = self.source_info;
if let (local, []) = (&place.local, proj_base) {
let decl = &self.body.local_decls[*local];
if let [] = proj_base {
let decl = &self.body.local_decls[place.local];
if decl.internal {
if let LocalInfo::StaticRef { def_id, .. } = decl.local_info {
if self.tcx.is_mutable_static(def_id) {
Expand All @@ -240,7 +235,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
// Internal locals are used in the `move_val_init` desugaring.
// We want to check unsafety against the source info of the
// desugaring, rather than the source info of the RHS.
self.source_info = self.body.local_decls[*local].source_info;
self.source_info = self.body.local_decls[place.local].source_info;
}
}
}
Expand Down Expand Up @@ -396,6 +391,9 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
cursor = proj_base;

match elem {
// Modifications behind a dereference don't affect the value of
// the pointer.
ProjectionElem::Deref => return,
ProjectionElem::Field(..) => {
let ty =
Place::ty_from(place.local, proj_base, &self.body.local_decls, self.tcx).ty;
Expand All @@ -409,14 +407,23 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
"mutating layout constrained fields cannot statically be \
checked for valid values",
)
} else {

// Check `is_freeze` as late as possible to avoid cycle errors
// with opaque types.
} else if !place.ty(self.body, self.tcx).ty.is_freeze(
self.tcx,
self.param_env,
self.source_info.span,
) {
(
"borrow of layout constrained field with interior \
mutability",
"references to fields of layout constrained fields \
lose the constraints. Coupled with interior mutability, \
the field can be changed to invalid values",
)
} else {
continue;
};
self.require_unsafe(
description,
Expand Down
32 changes: 32 additions & 0 deletions src/test/ui/impl-trait/unsafety-checking-cycle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Ensure that we don't get a cycle error from trying to determine whether an
// opaque type implements `Freeze` in safety checking, when it doesn't matter.

// check-pass

#![feature(rustc_attrs)]

struct AnyValue<T>(T);

// No need to check for `Freeze` here, there's no
// `rustc_layout_scalar_valid_range_start` involved.
fn not_restricted(c: bool) -> impl Sized {
if c {
let x = AnyValue(not_restricted(false));
&x.0;
}
2u32
}

#[rustc_layout_scalar_valid_range_start(1)]
struct NonZero<T>(T);

// No need to check for `Freeze` here, we're not borrowing the field.
fn not_field(c: bool) -> impl Sized {
if c {
let x = unsafe { NonZero(not_field(false)) };
&x;
}
5u32
}

fn main() {}

0 comments on commit 4af0952

Please sign in to comment.