Skip to content

Commit

Permalink
fix codegen of drops of fields of packed structs
Browse files Browse the repository at this point in the history
  • Loading branch information
arielb1 authored and Ariel Ben-Yehuda committed Nov 26, 2017
1 parent 3801c05 commit 06eb5a6
Show file tree
Hide file tree
Showing 8 changed files with 341 additions and 26 deletions.
5 changes: 4 additions & 1 deletion src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ use syntax_pos::Span;
use std::fmt;
use std::iter;

use transform::{add_call_guards, no_landing_pads, simplify};
use transform::{add_moves_for_packed_drops, add_call_guards};
use transform::{no_landing_pads, simplify};
use util::elaborate_drops::{self, DropElaborator, DropStyle, DropFlagMode};
use util::patch::MirPatch;

Expand Down Expand Up @@ -114,6 +115,8 @@ fn make_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
};
debug!("make_shim({:?}) = untransformed {:?}", instance, result);
add_moves_for_packed_drops::add_moves_for_packed_drops(
tcx, &mut result, instance.def_id());
no_landing_pads::no_landing_pads(tcx, &mut result);
simplify::simplify_cfg(&mut result);
add_call_guards::CriticalCallEdges.add_call_guards(&mut result);
Expand Down
141 changes: 141 additions & 0 deletions src/librustc_mir/transform/add_moves_for_packed_drops.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::hir::def_id::DefId;
use rustc::mir::*;
use rustc::ty::TyCtxt;

use transform::{MirPass, MirSource};
use util::patch::MirPatch;
use util;

// This pass moves values being dropped that are within a packed
// struct to a separate local before dropping them, to ensure that
// they are dropped from an aligned address.
//
// For example, if we have something like
// ```Rust
// #[repr(packed)]
// struct Foo {
// dealign: u8,
// data: Vec<u8>
// }
//
// let foo = ...;
// ```
//
// We want to call `drop_in_place::<Vec<u8>>` on `data` from an aligned
// address. This means we can't simply drop `foo.data` directly, because
// its address is not aligned.
//
// Instead, we move `foo.data` to a local and drop that:
// ```
// storage.live(drop_temp)
// drop_temp = foo.data;
// drop(drop_temp) -> next
// next:
// storage.dead(drop_temp)
// ```
//
// The storage instructions are required to avoid stack space
// blowup.

pub struct AddMovesForPackedDrops;

impl MirPass for AddMovesForPackedDrops {
fn run_pass<'a, 'tcx>(&self,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
src: MirSource,
mir: &mut Mir<'tcx>)
{
debug!("add_moves_for_packed_drops({:?} @ {:?})", src, mir.span);
add_moves_for_packed_drops(tcx, mir, src.def_id);
}
}

pub fn add_moves_for_packed_drops<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir: &mut Mir<'tcx>,
def_id: DefId)
{
let patch = add_moves_for_packed_drops_patch(tcx, mir, def_id);
patch.apply(mir);
}

fn add_moves_for_packed_drops_patch<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir: &Mir<'tcx>,
def_id: DefId)
-> MirPatch<'tcx>
{
let mut patch = MirPatch::new(mir);
let param_env = tcx.param_env(def_id);

for (bb, data) in mir.basic_blocks().iter_enumerated() {
let loc = Location { block: bb, statement_index: data.statements.len() };
let terminator = data.terminator();

match terminator.kind {
TerminatorKind::Drop { ref location, .. }
if util::is_disaligned(tcx, mir, param_env, location) =>
{
add_move_for_packed_drop(tcx, mir, &mut patch, terminator,
loc, data.is_cleanup);
}
TerminatorKind::DropAndReplace { .. } => {
span_bug!(terminator.source_info.span,
"replace in AddMovesForPackedDrops");
}
_ => {}
}
}

patch
}

fn add_move_for_packed_drop<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir: &Mir<'tcx>,
patch: &mut MirPatch<'tcx>,
terminator: &Terminator<'tcx>,
loc: Location,
is_cleanup: bool)
{
debug!("add_move_for_packed_drop({:?} @ {:?})", terminator, loc);
let (location, target, unwind) = match terminator.kind {
TerminatorKind::Drop { ref location, target, unwind } =>
(location, target, unwind),
_ => unreachable!()
};

let source_info = terminator.source_info;
let ty = location.ty(mir, tcx).to_ty(tcx);
let temp = patch.new_temp(ty, terminator.source_info.span);

let storage_dead_block = patch.new_block(BasicBlockData {
statements: vec![Statement {
source_info, kind: StatementKind::StorageDead(temp)
}],
terminator: Some(Terminator {
source_info, kind: TerminatorKind::Goto { target }
}),
is_cleanup
});

patch.add_statement(
loc, StatementKind::StorageLive(temp));
patch.add_assign(loc, Lvalue::Local(temp),
Rvalue::Use(Operand::Consume(location.clone())));
patch.patch_terminator(loc.block, TerminatorKind::Drop {
location: Lvalue::Local(temp),
target: storage_dead_block,
unwind
});
}
29 changes: 4 additions & 25 deletions src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::indexed_vec::IndexVec;

use rustc::ty::maps::Providers;
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::{self, TyCtxt};
use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::lint::builtin::{SAFE_EXTERN_STATICS, UNUSED_UNSAFE};
Expand All @@ -22,16 +22,13 @@ use rustc::mir::visit::{LvalueContext, Visitor};
use syntax::ast;

use std::rc::Rc;

use util;

pub struct UnsafetyChecker<'a, 'tcx: 'a> {
mir: &'a Mir<'tcx>,
visibility_scope_info: &'a IndexVec<VisibilityScope, VisibilityScopeInfo>,
violations: Vec<UnsafetyViolation>,
source_info: SourceInfo,
// true if an a part of this *memory block* of this expression
// is being borrowed, used for repr(packed) checking.
need_check_packed: bool,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
used_unsafe: FxHashSet<ast::NodeId>,
Expand All @@ -53,7 +50,6 @@ impl<'a, 'gcx, 'tcx> UnsafetyChecker<'a, 'tcx> {
},
tcx,
param_env,
need_check_packed: false,
used_unsafe: FxHashSet(),
inherited_blocks: vec![],
}
Expand Down Expand Up @@ -142,11 +138,9 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
lvalue: &Lvalue<'tcx>,
context: LvalueContext<'tcx>,
location: Location) {
let old_need_check_packed = self.need_check_packed;
if let LvalueContext::Borrow { .. } = context {
let ty = lvalue.ty(self.mir, self.tcx).to_ty(self.tcx);
if !self.has_align_1(ty) {
self.need_check_packed = true;
if util::is_disaligned(self.tcx, self.mir, self.param_env, lvalue) {
self.require_unsafe("borrow of packed field")
}
}

Expand All @@ -163,9 +157,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
self.source_info = self.mir.local_decls[local].source_info;
}
}
if let &ProjectionElem::Deref = elem {
self.need_check_packed = false;
}
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
match base_ty.sty {
ty::TyRawPtr(..) => {
Expand Down Expand Up @@ -194,9 +185,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
self.require_unsafe("access to union field")
}
}
if adt.repr.packed() && self.need_check_packed {
self.require_unsafe("borrow of packed field")
}
}
_ => {}
}
Expand All @@ -221,19 +209,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
}
};
self.super_lvalue(lvalue, context, location);
self.need_check_packed = old_need_check_packed;
}
}

impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {

fn has_align_1(&self, ty: Ty<'tcx>) -> bool {
self.tcx.at(self.source_info.span)
.layout_raw(self.param_env.and(ty))
.map(|layout| layout.align.abi() == 1)
.unwrap_or(false)
}

fn require_unsafe(&mut self,
description: &'static str)
{
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use syntax::ast;
use syntax_pos::Span;

pub mod add_validation;
pub mod add_moves_for_packed_drops;
pub mod clean_end_regions;
pub mod check_unsafety;
pub mod simplify_branches;
Expand Down Expand Up @@ -236,7 +237,12 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx
// an AllCallEdges pass right before it.
add_call_guards::AllCallEdges,
add_validation::AddValidation,
// AddMovesForPackedDrops needs to run after drop
// elaboration.
add_moves_for_packed_drops::AddMovesForPackedDrops,

simplify::SimplifyCfg::new("elaborate-drops"),

// No lifetime analysis based on borrowing can be done from here on out.

// From here on out, regions are gone.
Expand Down
74 changes: 74 additions & 0 deletions src/librustc_mir/util/alignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.


use rustc::ty::{self, TyCtxt};
use rustc::mir::*;

/// Return `true` if this lvalue is allowed to be less aligned
/// than its containing struct (because it is within a packed
/// struct).
pub fn is_disaligned<'a, 'tcx, L>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
local_decls: &L,
param_env: ty::ParamEnv<'tcx>,
lvalue: &Lvalue<'tcx>)
-> bool
where L: HasLocalDecls<'tcx>
{
debug!("is_disaligned({:?})", lvalue);
if !is_within_packed(tcx, local_decls, lvalue) {
debug!("is_disaligned({:?}) - not within packed", lvalue);
return false
}

let ty = lvalue.ty(local_decls, tcx).to_ty(tcx);
match tcx.layout_raw(param_env.and(ty)) {
Ok(layout) if layout.align.abi() == 1 => {
// if the alignment is 1, the type can't be further
// disaligned.
debug!("is_disaligned({:?}) - align = 1", lvalue);
false
}
_ => {
debug!("is_disaligned({:?}) - true", lvalue);
true
}
}
}

fn is_within_packed<'a, 'tcx, L>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
local_decls: &L,
lvalue: &Lvalue<'tcx>)
-> bool
where L: HasLocalDecls<'tcx>
{
let mut lvalue = lvalue;
while let &Lvalue::Projection(box Projection {
ref base, ref elem
}) = lvalue {
match *elem {
// encountered a Deref, which is ABI-aligned
ProjectionElem::Deref => break,
ProjectionElem::Field(..) => {
let ty = base.ty(local_decls, tcx).to_ty(tcx);
match ty.sty {
ty::TyAdt(def, _) if def.repr.packed() => {
return true
}
_ => {}
}
}
_ => {}
}
lvalue = base;
}

false
}
2 changes: 2 additions & 0 deletions src/librustc_mir/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ pub mod elaborate_drops;
pub mod def_use;
pub mod patch;

mod alignment;
mod graphviz;
mod pretty;
pub mod liveness;

pub use self::alignment::is_disaligned;
pub use self::pretty::{dump_enabled, dump_mir, write_mir_pretty, PassWhere};
pub use self::graphviz::{write_mir_graphviz};
pub use self::graphviz::write_node_label as write_graphviz_node_label;
Loading

0 comments on commit 06eb5a6

Please sign in to comment.