Merge pull request #142 from oli-obk/memleak

Memleak

Author: solson

src/eval_context.rs

@@ -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
@@ -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;
 
@@ -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(())
     }
@@ -928,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(
@@ -1509,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;

src/memory.rs

@@ -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 {

src/traits.rs

@@ -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));
 
@@ -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);
                     }

tests/compile-fail/memleak.rs

@@ -0,0 +1,5 @@
+//error-pattern: the evaluated program leaked memory
+
+fn main() {
+    std::mem::forget(Box::new(42));
+}

tests/compile-fail/memleak_rc.rs

@@ -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);
+}

tests/run-pass/closure-drop.rs

@@ -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);
 }

tests/run-pass/option_box_transmute_ptr.rs

@@ -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
     }
 }