stop leaking memory on closure calls

Author: oli-obk

Diff for: src/interpreter/mod.rs

@@ -82,6 +82,12 @@ pub struct Frame<'tcx> {
     /// Before being initialized, a local is simply marked as None.
     pub locals: Vec<Option<Value>>,
 
+    /// Temporaries 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
+    /// If they are Value::ByRef, their memory will be freed when the stackframe finishes
+    pub interpreter_temporaries: Vec<Value>,
+
     ////////////////////////////////////////////////////////////////////////////////
     // Current position within the function
     ////////////////////////////////////////////////////////////////////////////////
@@ -327,6 +333,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         substs: &'tcx Substs<'tcx>,
         return_lvalue: Lvalue<'tcx>,
         return_to_block: StackPopCleanup,
+        temporaries: Vec<Value>,
     ) -> EvalResult<'tcx, ()> {
         ::log_settings::settings().indentation += 1;
 
@@ -341,6 +348,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             return_to_block: return_to_block,
             return_lvalue: return_lvalue,
             locals: locals,
+            interpreter_temporaries: temporaries,
             span: span,
             def_id: def_id,
             substs: substs,
@@ -385,9 +393,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             StackPopCleanup::None => {},
         }
         // deallocate all locals that are backed by an allocation
-        for (i, local) in frame.locals.into_iter().enumerate() {
-            if let Some(Value::ByRef(ptr)) = local {
-                trace!("deallocating local {}: {:?}", i + 1, ptr);
+        for local in frame.locals.into_iter().filter_map(|l| l).chain(frame.interpreter_temporaries) {
+            if let Value::ByRef(ptr) = local {
                 self.memory.dump(ptr.alloc_id);
                 match self.memory.deallocate(ptr) {
                     // Any frozen memory means that it belongs to a constant or something referenced
@@ -1131,27 +1138,6 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         Ok(new_lvalue)
     }
 
-    // FIXME(solson): This method unnecessarily allocates and should not be necessary. We can
-    // remove it as soon as PrimVal can represent fat pointers.
-    fn value_to_ptr_dont_use(&mut self, value: Value, ty: Ty<'tcx>) -> EvalResult<'tcx, Pointer> {
-        match value {
-            Value::ByRef(ptr) => Ok(ptr),
-
-            Value::ByVal(primval) => {
-                let ptr = self.alloc_ptr(ty)?;
-                let kind = self.ty_to_primval_kind(ty)?;
-                self.memory.write_primval(ptr, primval, kind)?;
-                Ok(ptr)
-            }
-
-            Value::ByValPair(a, b) => {
-                let ptr = self.alloc_ptr(ty)?;
-                self.write_pair_to_ptr(a, b, ptr, ty)?;
-                Ok(ptr)
-            }
-        }
-    }
-
     /// ensures this Value is not a ByRef
     fn follow_by_ref_value(&mut self, value: Value, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> {
         match value {
@@ -1719,6 +1705,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
         tcx.intern_substs(&[]),
         Lvalue::from_ptr(Pointer::zst_ptr()),
         StackPopCleanup::None,
+        Vec::new(),
     ).expect("could not allocate first stack frame");
 
     loop {

Diff for: src/interpreter/step.rs

@@ -145,7 +145,7 @@ impl<'a, 'b, 'tcx> ConstantExtractor<'a, 'b, 'tcx> {
             } else {
                 StackPopCleanup::None
             };
-            this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Global(cid), cleanup)
+            this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Global(cid), cleanup, Vec::new())
         });
     }
     fn try<F: FnOnce(&mut Self) -> EvalResult<'tcx, ()>>(&mut self, f: F) {
@@ -194,7 +194,8 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> {
                                               mir,
                                               this.substs,
                                               Lvalue::Global(cid),
-                                              StackPopCleanup::Freeze)
+                                              StackPopCleanup::Freeze,
+                                              Vec::new())
                 });
             }
         }

Diff for: src/interpreter/terminator/mod.rs

@@ -164,6 +164,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 substs,
                 Lvalue::from_ptr(Pointer::zst_ptr()),
                 StackPopCleanup::None,
+                Vec::new(),
             )?;
             let mut arg_locals = self.frame().mir.args_iter();
             let first = arg_locals.next().expect("drop impl has self arg");
@@ -211,11 +212,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 }
 
                 // Only trait methods can have a Self parameter.
-                let (resolved_def_id, resolved_substs) =
+                let (resolved_def_id, resolved_substs, temporaries) =
                     if let Some(trait_id) = self.tcx.trait_of_item(def_id) {
                         self.trait_method(trait_id, def_id, substs, &mut args)?
                     } else {
-                        (def_id, substs)
+                        (def_id, substs, Vec::new())
                     };
 
                 let mir = self.load_mir(resolved_def_id)?;
@@ -235,6 +236,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     resolved_substs,
                     return_lvalue,
                     return_to_block,
+                    temporaries,
                 )?;
 
                 let arg_locals = self.frame().mir.args_iter();
@@ -430,7 +432,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>)> {
+    ) -> EvalResult<'tcx, (DefId, &'tcx Substs<'tcx>, Vec<Value>)> {
         let trait_ref = ty::TraitRef::from_method(self.tcx, trait_id, substs);
         let trait_ref = self.tcx.normalize_associated_type(&ty::Binder(trait_ref));
 
@@ -442,7 +444,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 // and those from the method:
                 let (did, substs) = find_method(self.tcx, substs, impl_did, vtable_impl.substs, mname);
 
-                Ok((did, substs))
+                Ok((did, substs, Vec::new()))
             }
 
             traits::VtableClosure(vtable_closure) => {
@@ -453,6 +455,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let closure_kind = self.tcx.closure_kind(vtable_closure.closure_def_id);
                 trace!("closures {:?}, {:?}", closure_kind, trait_closure_kind);
                 self.unpack_fn_args(args)?;
+                let mut temporaries = Vec::new();
                 match (closure_kind, trait_closure_kind) {
                     (ty::ClosureKind::Fn, ty::ClosureKind::Fn) |
                     (ty::ClosureKind::FnMut, ty::ClosureKind::FnMut) |
@@ -472,23 +475,36 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
                         // Interpreter magic: insert an intermediate pointer, so we can skip the
                         // intermediate function call.
-                        // FIXME: this is a memory leak, should probably add the pointer to the
-                        // current stack.
-                        let first = self.value_to_ptr_dont_use(args[0].0, args[0].1)?;
-                        args[0].0 = Value::ByVal(PrimVal::from_ptr(first));
+                        let ptr = match args[0].0 {
+                            Value::ByRef(ptr) => ptr,
+                            Value::ByVal(primval) => {
+                                let ptr = self.alloc_ptr(args[0].1)?;
+                                let kind = self.ty_to_primval_kind(args[0].1)?;
+                                self.memory.write_primval(ptr, primval, kind)?;
+                                temporaries.push(Value::ByRef(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(Value::ByRef(ptr));
+                                ptr
+                            },
+                        };
+                        args[0].0 = Value::ByVal(PrimVal::from_ptr(ptr));
                         args[0].1 = self.tcx.mk_mut_ptr(args[0].1);
                     }
 
                     _ => bug!("cannot convert {:?} to {:?}", closure_kind, trait_closure_kind),
                 }
-                Ok((vtable_closure.closure_def_id, vtable_closure.substs.substs))
+                Ok((vtable_closure.closure_def_id, vtable_closure.substs.substs, temporaries))
             }
 
             traits::VtableFnPointer(vtable_fn_ptr) => {
                 if let ty::TyFnDef(did, ref substs, _) = vtable_fn_ptr.fn_ty.sty {
                     args.remove(0);
                     self.unpack_fn_args(args)?;
-                    Ok((did, substs))
+                    Ok((did, substs, Vec::new()))
                 } else {
                     bug!("VtableFnPointer did not contain a concrete function: {:?}", vtable_fn_ptr)
                 }
@@ -504,7 +520,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     let fn_ptr = self.memory.read_ptr(vtable.offset(offset))?;
                     let (def_id, substs, _abi, sig) = self.memory.get_fn(fn_ptr.alloc_id)?;
                     *first_ty = sig.inputs[0];
-                    Ok((def_id, substs))
+                    Ok((def_id, substs, Vec::new()))
                 } else {
                     Err(EvalError::VtableForArgumentlessMethod)
                 }