1
Fork 0

Rollup merge of #61559 - spastorino:make-visitors-iterate, r=oli-obk

Make visitors iterate

r? @oli-obk

The second commit is not completely equivalent, unsure if the code is wrong or not. Tests pass though, otherwise we would need to iterate in the opposite direction as it happened in other parts of the code.
This commit is contained in:
Mazdak Farrokhzad 2019-06-06 22:39:12 +02:00 committed by GitHub
commit 3fd632aa7e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 86 additions and 76 deletions

View file

@ -154,51 +154,62 @@ impl<'mir, 'a: 'mir, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
context: PlaceContext,
location: Location) {
debug!("visit_place(place={:?}, context={:?})", place, context);
let mut context = context;
let cx = self.fx.cx;
if let mir::Place::Projection(ref proj) = *place {
// Allow uses of projections that are ZSTs or from scalar fields.
let is_consume = match context {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => true,
_ => false
};
if is_consume {
let base_ty = proj.base.ty(self.fx.mir, cx.tcx());
let base_ty = self.fx.monomorphize(&base_ty);
place.iterate(|place_base, place_projections| {
for proj in place_projections {
// Allow uses of projections that are ZSTs or from scalar fields.
let is_consume = match context {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => true,
_ => false
};
if is_consume {
let base_ty = proj.base.ty(self.fx.mir, cx.tcx());
let base_ty = self.fx.monomorphize(&base_ty);
// ZSTs don't require any actual memory access.
let elem_ty = base_ty
.projection_ty(cx.tcx(), &proj.elem)
.ty;
let elem_ty = self.fx.monomorphize(&elem_ty);
if cx.layout_of(elem_ty).is_zst() {
return;
}
if let mir::ProjectionElem::Field(..) = proj.elem {
let layout = cx.layout_of(base_ty.ty);
if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) {
// Recurse with the same context, instead of `Projection`,
// potentially stopping at non-operand projections,
// which would trigger `not_ssa` on locals.
self.visit_place(&proj.base, context, location);
// ZSTs don't require any actual memory access.
let elem_ty = base_ty
.projection_ty(cx.tcx(), &proj.elem)
.ty;
let elem_ty = self.fx.monomorphize(&elem_ty);
if cx.layout_of(elem_ty).is_zst() {
return;
}
if let mir::ProjectionElem::Field(..) = proj.elem {
let layout = cx.layout_of(base_ty.ty);
if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) {
// Recurse with the same context, instead of `Projection`,
// potentially stopping at non-operand projections,
// which would trigger `not_ssa` on locals.
continue;
}
}
}
// A deref projection only reads the pointer, never needs the place.
if let mir::ProjectionElem::Deref = proj.elem {
return self.visit_place(
&proj.base,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
location
);
}
context = if context.is_mutating_use() {
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
};
}
// A deref projection only reads the pointer, never needs the place.
if let mir::ProjectionElem::Deref = proj.elem {
return self.visit_place(
&proj.base,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
location
);
// Default base visit behavior
if let mir::PlaceBase::Local(local) = place_base {
self.visit_local(local, context, location);
}
}
self.super_place(place, context, location);
});
}
fn visit_local(&mut self,

View file

@ -199,11 +199,39 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
fn visit_place(&mut self,
place: &Place<'tcx>,
context: PlaceContext,
location: Location) {
match place {
&Place::Projection(box Projection {
ref base, ref elem
}) => {
_location: Location) {
place.iterate(|place_base, place_projections| {
match place_base {
PlaceBase::Local(..) => {
// Locals are safe.
}
PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. }) => {
bug!("unsafety checking should happen before promotion")
}
PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. }) => {
if self.tcx.is_mutable_static(*def_id) {
self.require_unsafe("use of mutable static",
"mutable statics can be mutated by multiple threads: aliasing \
violations or data races will cause undefined behavior",
UnsafetyViolationKind::General);
} else if self.tcx.is_foreign_item(*def_id) {
let source_info = self.source_info;
let lint_root =
self.source_scope_local_data[source_info.scope].lint_root;
self.register_violations(&[UnsafetyViolation {
source_info,
description: InternedString::intern("use of extern static"),
details: InternedString::intern(
"extern statics are not controlled by the Rust type system: \
invalid data, aliasing violations or data races will cause \
undefined behavior"),
kind: UnsafetyViolationKind::ExternStatic(lint_root)
}], &[]);
}
}
}
for proj in place_projections {
if context.is_borrow() {
if util::is_disaligned(self.tcx, self.mir, self.param_env, place) {
let source_info = self.source_info;
@ -220,7 +248,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
}], &[]);
}
}
let is_borrow_of_interior_mut = context.is_borrow() && !base
let is_borrow_of_interior_mut = context.is_borrow() && !proj.base
.ty(self.mir, self.tcx)
.ty
.is_freeze(self.tcx, self.param_env, self.source_info.span);
@ -236,7 +264,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
);
}
let old_source_info = self.source_info;
if let &Place::Base(PlaceBase::Local(local)) = base {
if let Place::Base(PlaceBase::Local(local)) = proj.base {
if self.mir.local_decls[local].internal {
// Internal locals are used in the `move_val_init` desugaring.
// We want to check unsafety against the source info of the
@ -244,7 +272,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
self.source_info = self.mir.local_decls[local].source_info;
}
}
let base_ty = base.ty(self.mir, self.tcx).ty;
let base_ty = proj.base.ty(self.mir, self.tcx).ty;
match base_ty.sty {
ty::RawPtr(..) => {
self.require_unsafe("dereference of raw pointer",
@ -260,8 +288,8 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
MutatingUseContext::AsmOutput
)
{
let elem_ty = match elem {
&ProjectionElem::Field(_, ty) => ty,
let elem_ty = match proj.elem {
ProjectionElem::Field(_, ty) => ty,
_ => span_bug!(
self.source_info.span,
"non-field projection {:?} from union?",
@ -292,36 +320,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
}
self.source_info = old_source_info;
}
&Place::Base(PlaceBase::Local(..)) => {
// locals are safe
}
&Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Promoted(_), .. })) => {
bug!("unsafety checking should happen before promotion")
}
&Place::Base(
PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. })
) => {
if self.tcx.is_mutable_static(def_id) {
self.require_unsafe("use of mutable static",
"mutable statics can be mutated by multiple threads: aliasing violations \
or data races will cause undefined behavior",
UnsafetyViolationKind::General);
} else if self.tcx.is_foreign_item(def_id) {
let source_info = self.source_info;
let lint_root =
self.source_scope_local_data[source_info.scope].lint_root;
self.register_violations(&[UnsafetyViolation {
source_info,
description: InternedString::intern("use of extern static"),
details: InternedString::intern(
"extern statics are not controlled by the Rust type system: invalid \
data, aliasing violations or data races will cause undefined behavior"),
kind: UnsafetyViolationKind::ExternStatic(lint_root)
}], &[]);
}
}
};
self.super_place(place, context, location);
});
}
}