Skip to content

Commit

Permalink
translate array drop glue using MIR
Browse files Browse the repository at this point in the history
This fixes leakage on panic with arrays & slices. I am using a C-style
for-loop instead of a pointer-based loop because that would be ugly-er
to implement.
  • Loading branch information
arielb1 committed May 28, 2017
1 parent 5d2512e commit 9da2aac
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 46 deletions.
9 changes: 9 additions & 0 deletions src/librustc/ty/util.rs
Expand Up @@ -584,6 +584,15 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
bug!("empty_substs_for_def_id: {:?} has type parameters", item_def_id)
})
}

pub fn const_usize(&self, val: usize) -> ConstInt {
match self.sess.target.uint_type {
ast::UintTy::U16 => ConstInt::Usize(ConstUsize::Us16(val as u16)),
ast::UintTy::U32 => ConstInt::Usize(ConstUsize::Us32(val as u32)),
ast::UintTy::U64 => ConstInt::Usize(ConstUsize::Us64(val as u64)),
_ => bug!(),
}
}
}

pub struct TypeIdHasher<'a, 'gcx: 'a+'tcx, 'tcx: 'a, W> {
Expand Down
135 changes: 130 additions & 5 deletions src/librustc_mir/util/elaborate_drops.rs
Expand Up @@ -11,7 +11,7 @@
use std::fmt;
use rustc::hir;
use rustc::mir::*;
use rustc::middle::const_val::ConstInt;
use rustc::middle::const_val::{ConstInt, ConstVal};
use rustc::middle::lang_items;
use rustc::ty::{self, Ty};
use rustc::ty::subst::{Kind, Substs};
Expand Down Expand Up @@ -535,6 +535,114 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
})
}

/// create a loop that drops an array:
///
/// loop-block:
/// can_go = index < len
/// if can_go then drop-block else succ
/// drop-block:
/// ptr = &mut LV[len]
/// index = index + 1
/// drop(ptr)
fn drop_loop(&mut self,
unwind: Option<BasicBlock>,
succ: BasicBlock,
index: &Lvalue<'tcx>,
length: &Lvalue<'tcx>,
ety: Ty<'tcx>,
is_cleanup: bool)
-> BasicBlock
{
let use_ = |lv: &Lvalue<'tcx>| Operand::Consume(lv.clone());
let tcx = self.tcx();

let ref_ty = tcx.mk_ref(tcx.types.re_erased, ty::TypeAndMut {
ty: ety,
mutbl: hir::Mutability::MutMutable
});
let ptr = &Lvalue::Local(self.new_temp(ref_ty));
let can_go = &Lvalue::Local(self.new_temp(tcx.types.bool));

let one = self.constant_usize(1);
let drop_block = self.elaborator.patch().new_block(BasicBlockData {
statements: vec![
Statement { source_info: self.source_info, kind: StatementKind::Assign(
ptr.clone(), Rvalue::Ref(
tcx.types.re_erased, BorrowKind::Mut,
self.lvalue.clone().index(use_(index))
),
)},
Statement { source_info: self.source_info, kind: StatementKind::Assign(
index.clone(), Rvalue::BinaryOp(BinOp::Add, use_(index), one)
)},
],
is_cleanup,
terminator: Some(Terminator {
source_info: self.source_info,
kind: TerminatorKind::Resume,
})
});

let loop_block = self.elaborator.patch().new_block(BasicBlockData {
statements: vec![
Statement { source_info: self.source_info, kind: StatementKind::Assign(
can_go.clone(), Rvalue::BinaryOp(BinOp::Lt, use_(index), use_(length))
)},
],
is_cleanup,
terminator: Some(Terminator {
source_info: self.source_info,
kind: TerminatorKind::if_(tcx, use_(can_go), drop_block, succ)
})
});

self.elaborator.patch().patch_terminator(drop_block, TerminatorKind::Drop {
location: ptr.clone().deref(),
target: loop_block,
unwind: unwind
});

loop_block
}

fn open_drop_for_array(&mut self, ety: Ty<'tcx>) -> BasicBlock {
debug!("open_drop_for_array({:?})", ety);
// FIXME: using an index instead of a pointer to avoid
// special-casing ZSTs.
let tcx = self.tcx();
let index = &Lvalue::Local(self.new_temp(tcx.types.usize));
let length = &Lvalue::Local(self.new_temp(tcx.types.usize));

let unwind = self.unwind.map(|unwind| {
self.drop_loop(None, unwind, index, length, ety, true)
});

let is_cleanup = self.is_cleanup;
let succ = self.succ; // FIXME(#6393)
let loop_block = self.drop_loop(unwind, succ, index, length, ety, is_cleanup);

let zero = self.constant_usize(0);
let drop_block = self.elaborator.patch().new_block(BasicBlockData {
statements: vec![
Statement { source_info: self.source_info, kind: StatementKind::Assign(
length.clone(), Rvalue::Len(self.lvalue.clone())
)},
Statement { source_info: self.source_info, kind: StatementKind::Assign(
index.clone(), Rvalue::Use(zero),
)},
],
is_cleanup,
terminator: Some(Terminator {
source_info: self.source_info,
kind: TerminatorKind::Goto { target: loop_block }
})
});

// FIXME(#34708): handle partially-dropped array/slice elements.
self.drop_flag_test_and_reset_block(
is_cleanup, Some(DropFlagMode::Deep), drop_block, succ)
}

/// The slow-path - create an "open", elaborated drop for a type
/// which is moved-out-of only partially, and patch `bb` to a jump
/// to it. This must not be called on ADTs with a destructor,
Expand Down Expand Up @@ -564,10 +672,8 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
ty::TyDynamic(..) => {
self.complete_drop(is_cleanup, Some(DropFlagMode::Deep), succ)
}
ty::TyArray(..) | ty::TySlice(..) => {
// FIXME(#34708): handle partially-dropped
// array/slice elements.
self.complete_drop(is_cleanup, Some(DropFlagMode::Deep), succ)
ty::TyArray(ety, _) | ty::TySlice(ety) => {
self.open_drop_for_array(ety)
}
_ => bug!("open drop from non-ADT `{:?}`", ty)
}
Expand All @@ -588,6 +694,17 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
debug!("complete_drop({:?},{:?})", self, drop_mode);

let drop_block = self.drop_block(is_cleanup, succ);
self.drop_flag_test_and_reset_block(is_cleanup, drop_mode, drop_block, succ)
}

fn drop_flag_test_and_reset_block(&mut self,
is_cleanup: bool,
drop_mode: Option<DropFlagMode>,
drop_block: BasicBlock,
succ: BasicBlock) -> BasicBlock
{
debug!("drop_flag_test_and_reset_block({:?},{:?})", self, drop_mode);

if let Some(mode) = drop_mode {
let block_start = Location { block: drop_block, statement_index: 0 };
self.elaborator.clear_drop_flag(block_start, self.path, mode);
Expand Down Expand Up @@ -691,4 +808,12 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
let mir = self.elaborator.mir();
self.elaborator.patch().terminator_loc(mir, bb)
}

fn constant_usize(&self, val: usize) -> Operand<'tcx> {
Operand::Constant(box Constant {
span: self.source_info.span,
ty: self.tcx().types.usize,
literal: Literal::Value { value: ConstVal::Integral(self.tcx().const_usize(val)) }
})
}
}
12 changes: 1 addition & 11 deletions src/librustc_trans/collector.rs
Expand Up @@ -612,17 +612,7 @@ fn visit_instance_use<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
output.push(create_fn_trans_item(instance));
}
}
ty::InstanceDef::DropGlue(_, Some(ty)) => {
match ty.sty {
ty::TyArray(ety, _) |
ty::TySlice(ety)
if is_direct_call =>
{
// drop of arrays/slices is translated in-line.
visit_drop_use(scx, ety, false, output);
}
_ => {}
};
ty::InstanceDef::DropGlue(_, Some(_)) => {
output.push(create_fn_trans_item(instance));
}
ty::InstanceDef::ClosureOnceShim { .. } |
Expand Down
31 changes: 1 addition & 30 deletions src/librustc_trans/mir/block.rs
Expand Up @@ -20,13 +20,12 @@ use base::{self, Lifetime};
use callee;
use builder::Builder;
use common::{self, Funclet};
use common::{C_bool, C_str_slice, C_struct, C_u32, C_uint, C_undef};
use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef};
use consts;
use machine::llalign_of_min;
use meth;
use monomorphize;
use type_of;
use tvec;
use type_::Type;

use rustc_data_structures::indexed_vec::IndexVec;
Expand Down Expand Up @@ -222,34 +221,6 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
let (drop_fn, need_extra) = match ty.sty {
ty::TyDynamic(..) => (meth::DESTRUCTOR.get_fn(&bcx, lvalue.llextra),
false),
ty::TyArray(ety, _) | ty::TySlice(ety) => {
// FIXME: handle panics
let drop_fn = monomorphize::resolve_drop_in_place(
bcx.ccx.shared(), ety);
let drop_fn = callee::get_fn(bcx.ccx, drop_fn);
let bcx = tvec::slice_for_each(
&bcx,
lvalue.project_index(&bcx, C_uint(bcx.ccx, 0u64)),
ety,
lvalue.len(bcx.ccx),
|bcx, llval, loop_bb| {
self.set_debug_loc(&bcx, terminator.source_info);
if let Some(unwind) = unwind {
bcx.invoke(
drop_fn,
&[llval],
loop_bb,
llblock(self, unwind),
cleanup_bundle
);
} else {
bcx.call(drop_fn, &[llval], cleanup_bundle);
bcx.br(loop_bb);
}
});
funclet_br(self, bcx, target);
return
}
_ => (callee::get_fn(bcx.ccx, drop_fn), lvalue.has_extra())
};
let args = &[lvalue.llval, lvalue.llextra][..1 + need_extra as usize];
Expand Down
11 changes: 11 additions & 0 deletions src/test/run-pass/dynamic-drop.rs
Expand Up @@ -125,6 +125,14 @@ fn union1(a: &Allocator) {
}
}

fn array_simple(a: &Allocator) {
let _x = [a.alloc(), a.alloc(), a.alloc(), a.alloc()];
}

fn vec_simple(a: &Allocator) {
let _x = vec![a.alloc(), a.alloc(), a.alloc(), a.alloc()];
}

fn run_test<F>(mut f: F)
where F: FnMut(&Allocator)
{
Expand Down Expand Up @@ -171,5 +179,8 @@ fn main() {
run_test(|a| assignment1(a, false));
run_test(|a| assignment1(a, true));

run_test(|a| array_simple(a));
run_test(|a| vec_simple(a));

run_test_nopanic(|a| union1(a));
}

0 comments on commit 9da2aac

Please sign in to comment.