Skip to content

Commit

Permalink
Rollup merge of rust-lang#61559 - spastorino:make-visitors-iterate, r…
Browse files Browse the repository at this point in the history 
…=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.
  • Loading branch information
@Centril
Centril committed Jun 6, 2019
2 parents 4c74056 + 0cfaa28 commit 3fd632a
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 77 deletions.
85 changes: 48 additions & 37 deletions src/librustc_codegen_ssa/mir/analyze.rs
View file
Expand Up @@ -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);

// 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);
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.
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
);
// 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)
};
}
}

self.super_place(place, context, location);
// Default base visit behavior
if let mir::PlaceBase::Local(local) = place_base {
self.visit_local(local, context, location);
}
});
}

fn visit_local(&mut self,
Expand Down
79 changes: 39 additions & 40 deletions src/librustc_mir/transform/check_unsafety.rs
View file
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -236,15 +264,15 @@ 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
// desugaring, rather than the source info of the RHS.
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",
Expand All @@ -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?",
Expand Down Expand Up @@ -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);
});
}
}

Expand Down

0 comments on commit 3fd632a

Please sign in to comment.