Skip to content

Memleak #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 33 additions & 32 deletions src/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ pub struct Frame<'tcx> {
/// Temporary allocations introduced to save stackframes
/// This is pure interpreter magic and has nothing to do with how rustc does it
/// An example is calling an FnMut closure that has been converted to a FnOnce closure
/// The memory will be freed when the stackframe finishes
pub interpreter_temporaries: Vec<Pointer>,
/// The value's destructor will be called and the memory freed when the stackframe finishes
pub interpreter_temporaries: Vec<(Pointer, Ty<'tcx>)>,

////////////////////////////////////////////////////////////////////////////////
// Current position within the function
Expand Down Expand Up @@ -273,7 +273,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
substs: &'tcx Substs<'tcx>,
return_lvalue: Lvalue<'tcx>,
return_to_block: StackPopCleanup,
temporaries: Vec<Pointer>,
temporaries: Vec<(Pointer, Ty<'tcx>)>,
) -> EvalResult<'tcx> {
::log_settings::settings().indentation += 1;

Expand Down Expand Up @@ -347,11 +347,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
}
}
}
// deallocate all temporary allocations
for ptr in frame.interpreter_temporaries {
trace!("deallocating temporary allocation");
self.memory.dump_alloc(ptr.alloc_id);
self.memory.deallocate(ptr)?;
// drop and deallocate all temporary allocations
for (ptr, ty) in frame.interpreter_temporaries {
trace!("dropping temporary allocation");
let mut drops = Vec::new();
self.drop(Lvalue::from_ptr(ptr), ty, &mut drops)?;
self.eval_drop_impls(drops, frame.span)?;
}
Ok(())
}
Expand Down Expand Up @@ -456,7 +457,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {

General { discr, ref variants, .. } => {
if let mir::AggregateKind::Adt(adt_def, variant, _, _) = *kind {
let discr_val = adt_def.variants[variant].disr_val.to_u128_unchecked();
let discr_val = adt_def.variants[variant].disr_val;
let discr_size = discr.size().bytes();
if variants[variant].packed {
let ptr = self.force_allocation(dest)?.to_ptr_and_extra().0;
Expand Down Expand Up @@ -529,7 +530,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
CEnum { .. } => {
assert_eq!(operands.len(), 0);
if let mir::AggregateKind::Adt(adt_def, variant, _, _) = *kind {
let n = adt_def.variants[variant].disr_val.to_u128_unchecked();
let n = adt_def.variants[variant].disr_val;
self.write_primval(dest, PrimVal::Bytes(n), dest_ty)?;
} else {
bug!("tried to assign {:?} to Layout::CEnum", kind);
Expand Down Expand Up @@ -661,7 +662,20 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
}
}

InlineAsm { .. } => return Err(EvalError::InlineAsm),
Discriminant(ref lvalue) => {
let lval = self.eval_lvalue(lvalue)?;
let ty = self.lvalue_ty(lvalue);
let ptr = self.force_allocation(lval)?.to_ptr();
let discr_val = self.read_discriminant_value(ptr, ty)?;
if let ty::TyAdt(adt_def, _) = ty.sty {
if adt_def.variants.iter().all(|v| discr_val != v.disr_val) {
return Err(EvalError::InvalidDiscriminant);
}
} else {
bug!("rustc only generates Rvalue::Discriminant for enums");
}
self.write_primval(dest, PrimVal::Bytes(discr_val), dest_ty)?;
},
}

if log_enabled!(::log::LogLevel::Trace) {
Expand Down Expand Up @@ -915,26 +929,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
val: PrimVal,
dest_ty: Ty<'tcx>,
) -> EvalResult<'tcx> {
match dest {
Lvalue::Ptr { ptr, extra } => {
assert_eq!(extra, LvalueExtra::None);
let size = self.type_size(dest_ty)?.expect("dest type must be sized");
self.memory.write_primval(ptr, val, size)
}
Lvalue::Local { frame, local, field } => {
self.stack[frame].set_local(local, field.map(|(i, _)| i), Value::ByVal(val));
Ok(())
}
Lvalue::Global(cid) => {
let global_val = self.globals.get_mut(&cid).expect("global not cached");
if global_val.mutable {
global_val.value = Value::ByVal(val);
Ok(())
} else {
Err(EvalError::ModifiedConstantMemory)
}
}
}
self.write_value(Value::ByVal(val), dest, dest_ty)
}

pub(super) fn write_value(
Expand Down Expand Up @@ -1496,7 +1491,13 @@ pub fn eval_main<'a, 'tcx: 'a>(
loop {
match ecx.step() {
Ok(true) => {}
Ok(false) => return,
Ok(false) => {
let leaks = ecx.memory.leak_report();
if leaks != 0 {
tcx.sess.err("the evaluated program leaked memory");
}
return;
}
Err(e) => {
report(tcx, &ecx, e);
return;
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![feature(
btree_range,
collections,
field_init_shorthand,
i128_type,
pub_restricted,
rustc_private,
Expand Down
17 changes: 17 additions & 0 deletions src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,23 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
}
}
}

pub fn leak_report(&self) -> usize {
trace!("### LEAK REPORT ###");
let leaks: Vec<_> = self.alloc_map
.iter()
.filter_map(|(&key, val)| {
if val.static_kind == StaticKind::NotStatic {
Some(key)
} else {
None
}
})
.collect();
let n = leaks.len();
self.dump_allocs(leaks);
n
}
}

fn dump_fn_def<'tcx>(fn_def: FunctionDefinition<'tcx>) -> String {
Expand Down
2 changes: 2 additions & 0 deletions src/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
// Defined to do nothing. These are added by optimization passes, to avoid changing the
// size of MIR constantly.
Nop => {}

InlineAsm { .. } => return Err(EvalError::InlineAsm),
}

self.frame_mut().stmt += 1;
Expand Down
2 changes: 1 addition & 1 deletion src/terminator/drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
Layout::General { .. } => {
let discr_val = self.read_discriminant_value(adt_ptr, ty)? as u128;
let ptr = self.force_allocation(lval)?.to_ptr();
match adt_def.variants.iter().position(|v| discr_val == v.disr_val.to_u128_unchecked()) {
match adt_def.variants.iter().position(|v| discr_val == v.disr_val) {
Some(i) => {
lval = Lvalue::Ptr {
ptr,
Expand Down
39 changes: 8 additions & 31 deletions src/terminator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,16 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {

Goto { target } => self.goto_block(target),

If { ref cond, targets: (then_target, else_target) } => {
let cond_val = self.eval_operand_to_primval(cond)?.to_bool()?;
self.goto_block(if cond_val { then_target } else { else_target });
}

SwitchInt { ref discr, ref values, ref targets, .. } => {
let discr_val = self.eval_and_read_lvalue(discr)?;
let discr_ty = self.lvalue_ty(discr);
let discr_val = self.eval_operand(discr)?;
let discr_ty = self.operand_ty(discr);
let discr_prim = self.value_to_primval(discr_val, discr_ty)?;

// Branch to the `otherwise` case by default, if no match is found.
let mut target_block = targets[targets.len() - 1];

for (index, const_val) in values.iter().enumerate() {
let val = self.const_to_value(const_val)?;
let prim = self.value_to_primval(val, discr_ty)?;
for (index, const_int) in values.iter().enumerate() {
let prim = PrimVal::Bytes(const_int.to_u128_unchecked());
if discr_prim.to_bytes()? == prim.to_bytes()? {
target_block = targets[index];
break;
Expand All @@ -60,23 +54,6 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
self.goto_block(target_block);
}

Switch { ref discr, ref targets, adt_def } => {
// FIXME(solson)
let lvalue = self.eval_lvalue(discr)?;
let lvalue = self.force_allocation(lvalue)?;

let adt_ptr = lvalue.to_ptr();
let adt_ty = self.lvalue_ty(discr);
let discr_val = self.read_discriminant_value(adt_ptr, adt_ty)?;
let matching = adt_def.variants.iter()
.position(|v| discr_val == v.disr_val.to_u128_unchecked());

match matching {
Some(i) => self.goto_block(targets[i]),
None => return Err(EvalError::InvalidDiscriminant),
}
}

Call { ref func, ref args, ref destination, .. } => {
let destination = match *destination {
Some((ref lv, target)) => Some((self.eval_lvalue(lv)?, target)),
Expand Down Expand Up @@ -216,12 +193,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
trace!("layout({:?}) = {:#?}", dest_ty, dest_layout);
match *dest_layout {
Layout::Univariant { .. } => {
let disr_val = v.disr_val.to_u128_unchecked();
let disr_val = v.disr_val;
assert_eq!(disr_val, 0);
self.assign_fields(lvalue, dest_ty, args)?;
},
Layout::General { discr, ref variants, .. } => {
let disr_val = v.disr_val.to_u128_unchecked();
let disr_val = v.disr_val;
let discr_size = discr.size().bytes();
self.assign_discr_and_fields(
lvalue,
Expand All @@ -234,7 +211,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
)?;
},
Layout::StructWrappedNullablePointer { nndiscr, ref discrfield, .. } => {
let disr_val = v.disr_val.to_u128_unchecked();
let disr_val = v.disr_val;
if nndiscr as u128 == disr_val {
self.assign_fields(lvalue, dest_ty, args)?;
} else {
Expand Down Expand Up @@ -325,7 +302,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
}
}

fn read_discriminant_value(&self, adt_ptr: Pointer, adt_ty: Ty<'tcx>) -> EvalResult<'tcx, u128> {
pub fn read_discriminant_value(&self, adt_ptr: Pointer, adt_ty: Ty<'tcx>) -> EvalResult<'tcx, u128> {
use rustc::ty::layout::Layout::*;
let adt_layout = self.type_layout(adt_ty)?;
trace!("read_discriminant_value {:#?}", adt_layout);
Expand Down
5 changes: 2 additions & 3 deletions src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
def_id: DefId,
substs: &'tcx Substs<'tcx>,
args: &mut Vec<(Value, Ty<'tcx>)>,
) -> EvalResult<'tcx, (DefId, &'tcx Substs<'tcx>, Vec<Pointer>)> {
) -> EvalResult<'tcx, (DefId, &'tcx Substs<'tcx>, Vec<(Pointer, Ty<'tcx>)>)> {
let trait_ref = ty::TraitRef::from_method(self.tcx, trait_id, substs);
let trait_ref = self.tcx.normalize_associated_type(&ty::Binder(trait_ref));

Expand Down Expand Up @@ -72,16 +72,15 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
let ptr = self.alloc_ptr(args[0].1)?;
let size = self.type_size(args[0].1)?.expect("closures are sized");
self.memory.write_primval(ptr, primval, size)?;
temporaries.push(ptr);
ptr
},
Value::ByValPair(a, b) => {
let ptr = self.alloc_ptr(args[0].1)?;
self.write_pair_to_ptr(a, b, ptr, args[0].1)?;
temporaries.push(ptr);
ptr
},
};
temporaries.push((ptr, args[0].1));
args[0].0 = Value::ByVal(PrimVal::Ptr(ptr));
args[0].1 = self.tcx.mk_mut_ptr(args[0].1);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/compile-fail/memleak.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//error-pattern: the evaluated program leaked memory

fn main() {
std::mem::forget(Box::new(42));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test for an Rc cycle? There should be examples around back from before mem::forget was made safe, with the whole scoped threads discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
12 changes: 12 additions & 0 deletions tests/compile-fail/memleak_rc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//error-pattern: the evaluated program leaked memory

use std::rc::Rc;
use std::cell::RefCell;

struct Dummy(Rc<RefCell<Option<Dummy>>>);

fn main() {
let x = Dummy(Rc::new(RefCell::new(None)));
let y = Dummy(x.0.clone());
*x.0.borrow_mut() = Some(y);
}
15 changes: 5 additions & 10 deletions tests/run-pass/closure-drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,12 @@ fn f<T: FnOnce()>(t: T) {
fn main() {
let mut ran_drop = false;
{
// FIXME: v is a temporary hack to force the below closure to be a FnOnce-only closure
// (with sig fn(self)). Without it, the closure sig would be fn(&self) which requires a
// shim to call via FnOnce::call_once, and Miri's current shim doesn't correctly call
// destructors.
let v = vec![1];
let x = Foo(&mut ran_drop);
let g = move || {
let _ = x;
drop(v); // Force the closure to be FnOnce-only by using a capture by-value.
};
f(g);
// this closure never by val uses its captures
// so it's basically a fn(&self)
// the shim used to not drop the `x`
let x = move || { let _ = x; };
f(x);
}
assert!(ran_drop);
}
Expand Down
5 changes: 4 additions & 1 deletion tests/run-pass/option_box_transmute_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ fn option_box_deref() -> i32 {
let val = Some(Box::new(42));
unsafe {
let ptr: *const i32 = std::mem::transmute::<Option<Box<i32>>, *const i32>(val);
*ptr
let ret = *ptr;
// unleak memory
std::mem::transmute::<*const i32, Option<Box<i32>>>(ptr);
ret
}
}

Expand Down