Merge pull request rust-lang#115 from oli-obk/master

fix turning function items into closure trait objects

Author: solson

src/error.rs

@@ -3,7 +3,7 @@ use std::fmt;
 use rustc::mir;
 use rustc::ty::{BareFnTy, Ty, FnSig, layout};
 use syntax::abi::Abi;
-use memory::Pointer;
+use memory::{Pointer, Function};
 use rustc_const_math::ConstMathErr;
 use syntax::codemap::Span;
 
@@ -53,6 +53,9 @@ pub enum EvalError<'tcx> {
     DeallocatedFrozenMemory,
     Layout(layout::LayoutError<'tcx>),
     Unreachable,
+    ExpectedConcreteFunction(Function<'tcx>),
+    ExpectedDropGlue(Function<'tcx>),
+    ManuallyCalledDropGlue,
 }
 
 pub type EvalResult<'tcx, T = ()> = Result<T, EvalError<'tcx>>;
@@ -125,6 +128,12 @@ impl<'tcx> Error for EvalError<'tcx> {
                 "attempted to get length of a null terminated string, but no null found before end of allocation",
             EvalError::Unreachable =>
                 "entered unreachable code",
+            EvalError::ExpectedConcreteFunction(_) =>
+                "tried to use glue function as function",
+            EvalError::ExpectedDropGlue(_) =>
+                "tried to use non-drop-glue function as drop glue",
+            EvalError::ManuallyCalledDropGlue =>
+                "tried to manually invoke drop glue",
         }
     }
 

src/eval_context.rs

@@ -659,9 +659,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                         ty::TyFnPtr(unsafe_fn_ty) => {
                             let src = self.eval_operand(operand)?;
                             let ptr = src.read_ptr(&self.memory)?;
-                            let (def_id, substs, _, _) = self.memory.get_fn(ptr.alloc_id)?;
+                            let fn_def = self.memory.get_fn(ptr.alloc_id)?.expect_concrete()?;
                             let unsafe_fn_ty = self.tcx.erase_regions(&unsafe_fn_ty);
-                            let fn_ptr = self.memory.create_fn_ptr(self.tcx, def_id, substs, unsafe_fn_ty);
+                            let fn_ptr = self.memory.create_fn_ptr(self.tcx, fn_def.def_id, fn_def.substs, unsafe_fn_ty);
                             self.write_value(Value::ByVal(PrimVal::Ptr(fn_ptr)), dest, dest_ty)?;
                         },
                         ref other => bug!("fn to unsafe fn cast on {:?}", other),

src/memory.rs

@@ -94,14 +94,47 @@ impl Pointer {
     }
 }
 
-#[derive(Debug, Clone, Hash, Eq, PartialEq)]
-struct FunctionDefinition<'tcx> {
+#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
+/// Identifies a specific monomorphized function
+pub struct FunctionDefinition<'tcx> {
     pub def_id: DefId,
     pub substs: &'tcx Substs<'tcx>,
     pub abi: Abi,
     pub sig: &'tcx ty::FnSig<'tcx>,
 }
 
+/// Either a concrete function, or a glue function
+#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
+pub enum Function<'tcx> {
+    /// A function or method created by compiling code
+    Concrete(FunctionDefinition<'tcx>),
+    /// Glue required to call a regular function through a Fn(Mut|Once) trait object
+    FnDefAsTraitObject(FunctionDefinition<'tcx>),
+    /// Glue required to call the actual drop impl's `drop` method.
+    /// Drop glue takes the `self` by value, while `Drop::drop` take `&mut self`
+    DropGlue(FunctionDefinition<'tcx>),
+    /// Glue required to treat the ptr part of a fat pointer
+    /// as a function pointer
+    FnPtrAsTraitObject(&'tcx ty::FnSig<'tcx>),
+    /// Glue for Closures
+    Closure(FunctionDefinition<'tcx>),
+}
+
+impl<'tcx> Function<'tcx> {
+    pub fn expect_concrete(self) -> EvalResult<'tcx, FunctionDefinition<'tcx>> {
+        match self {
+            Function::Concrete(fn_def) => Ok(fn_def),
+            other => Err(EvalError::ExpectedConcreteFunction(other)),
+        }
+    }
+    pub fn expect_drop_glue(self) -> EvalResult<'tcx, FunctionDefinition<'tcx>> {
+        match self {
+            Function::DropGlue(fn_def) => Ok(fn_def),
+            other => Err(EvalError::ExpectedDropGlue(other)),
+        }
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Top-level interpreter memory
 ////////////////////////////////////////////////////////////////////////////////
@@ -115,9 +148,9 @@ pub struct Memory<'a, 'tcx> {
     memory_size: u64,
     /// Function "allocations". They exist solely so pointers have something to point to, and
     /// we can figure out what they point to.
-    functions: HashMap<AllocId, FunctionDefinition<'tcx>>,
+    functions: HashMap<AllocId, Function<'tcx>>,
     /// Inverse map of `functions` so we don't allocate a new pointer every time we need one
-    function_alloc_cache: HashMap<FunctionDefinition<'tcx>, AllocId>,
+    function_alloc_cache: HashMap<Function<'tcx>, AllocId>,
     next_id: AllocId,
     pub layout: &'a TargetDataLayout,
     /// List of memory regions containing packed structures
@@ -160,35 +193,61 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
             abi: fn_ty.abi,
             sig: fn_ty.sig,
         });
-        self.create_fn_alloc(FunctionDefinition {
+        self.create_fn_alloc(Function::Closure(FunctionDefinition {
             def_id,
             substs: substs.substs,
             abi: fn_ty.abi,
             // FIXME: why doesn't this compile?
             //sig: tcx.erase_late_bound_regions(&fn_ty.sig),
             sig: fn_ty.sig.skip_binder(),
-        })
+        }))
+    }
+
+    pub fn create_fn_as_trait_glue(&mut self, _tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId, substs: &'tcx Substs<'tcx>, fn_ty: &'tcx BareFnTy<'tcx>) -> Pointer {
+        self.create_fn_alloc(Function::FnDefAsTraitObject(FunctionDefinition {
+            def_id,
+            substs,
+            abi: fn_ty.abi,
+            // FIXME: why doesn't this compile?
+            //sig: tcx.erase_late_bound_regions(&fn_ty.sig),
+            sig: fn_ty.sig.skip_binder(),
+        }))
+    }
+
+    pub fn create_fn_ptr_as_trait_glue(&mut self, fn_ty: &'tcx BareFnTy<'tcx>) -> Pointer {
+        self.create_fn_alloc(Function::FnPtrAsTraitObject(fn_ty.sig.skip_binder()))
+    }
+
+    pub fn create_drop_glue(&mut self, _tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId, substs: &'tcx Substs<'tcx>, fn_ty: &'tcx BareFnTy<'tcx>) -> Pointer {
+        self.create_fn_alloc(Function::DropGlue(FunctionDefinition {
+            def_id,
+            substs,
+            abi: fn_ty.abi,
+            // FIXME: why doesn't this compile?
+            //sig: tcx.erase_late_bound_regions(&fn_ty.sig),
+            sig: fn_ty.sig.skip_binder(),
+        }))
     }
 
     pub fn create_fn_ptr(&mut self, _tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId, substs: &'tcx Substs<'tcx>, fn_ty: &'tcx BareFnTy<'tcx>) -> Pointer {
-        self.create_fn_alloc(FunctionDefinition {
+        self.create_fn_alloc(Function::Concrete(FunctionDefinition {
             def_id,
             substs,
             abi: fn_ty.abi,
             // FIXME: why doesn't this compile?
             //sig: tcx.erase_late_bound_regions(&fn_ty.sig),
             sig: fn_ty.sig.skip_binder(),
-        })
+        }))
     }
 
-    fn create_fn_alloc(&mut self, def: FunctionDefinition<'tcx>) -> Pointer {
+    fn create_fn_alloc(&mut self, def: Function<'tcx>) -> Pointer {
         if let Some(&alloc_id) = self.function_alloc_cache.get(&def) {
             return Pointer::new(alloc_id, 0);
         }
         let id = self.next_id;
         debug!("creating fn ptr: {}", id);
         self.next_id.0 += 1;
-        self.functions.insert(id, def.clone());
+        self.functions.insert(id, def);
         self.function_alloc_cache.insert(def, id);
         Pointer::new(id, 0)
     }
@@ -381,15 +440,10 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
         }
     }
 
-    pub fn get_fn(&self, id: AllocId) -> EvalResult<'tcx, (DefId, &'tcx Substs<'tcx>, Abi, &'tcx ty::FnSig<'tcx>)> {
+    pub fn get_fn(&self, id: AllocId) -> EvalResult<'tcx, Function<'tcx>> {
         debug!("reading fn ptr: {}", id);
         match self.functions.get(&id) {
-            Some(&FunctionDefinition {
-                def_id,
-                substs,
-                abi,
-                sig,
-            }) => Ok((def_id, substs, abi, sig)),
+            Some(&fndef) => Ok(fndef),
             None => match self.alloc_map.get(&id) {
                 Some(_) => Err(EvalError::ExecuteMemory),
                 None => Err(EvalError::InvalidFunctionPointer),
@@ -418,14 +472,24 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
 
             let alloc = match (self.alloc_map.get(&id), self.functions.get(&id)) {
                 (Some(a), None) => a,
-                (None, Some(fn_def)) => {
-                    let name = ty::tls::with(|tcx| tcx.item_path_str(fn_def.def_id));
-                    let abi = if fn_def.abi == Abi::Rust {
-                        format!("")
-                    } else {
-                        format!("extern {} ", fn_def.abi)
-                    };
-                    trace!("{} function pointer: {}: {}{}", msg, name, abi, fn_def.sig);
+                (None, Some(&Function::Concrete(fn_def))) => {
+                    trace!("{} {}", msg, dump_fn_def(fn_def));
+                    continue;
+                },
+                (None, Some(&Function::DropGlue(fn_def))) => {
+                    trace!("{} drop glue for {}", msg, dump_fn_def(fn_def));
+                    continue;
+                },
+                (None, Some(&Function::FnDefAsTraitObject(fn_def))) => {
+                    trace!("{} fn as Fn glue for {}", msg, dump_fn_def(fn_def));
+                    continue;
+                },
+                (None, Some(&Function::FnPtrAsTraitObject(fn_def))) => {
+                    trace!("{} fn ptr as Fn glue (signature: {:?})", msg, fn_def);
+                    continue;
+                },
+                (None, Some(&Function::Closure(fn_def))) => {
+                    trace!("{} closure glue for {}", msg, dump_fn_def(fn_def));
                     continue;
                 },
                 (None, None) => {
@@ -476,6 +540,16 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
     }
 }
 
+fn dump_fn_def<'tcx>(fn_def: FunctionDefinition<'tcx>) -> String {
+    let name = ty::tls::with(|tcx| tcx.item_path_str(fn_def.def_id));
+    let abi = if fn_def.abi == Abi::Rust {
+        format!("")
+    } else {
+        format!("extern {} ", fn_def.abi)
+    };
+    format!("function pointer: {}: {}{}", name, abi, fn_def.sig)
+}
+
 /// Byte accessors
 impl<'a, 'tcx> Memory<'a, 'tcx> {
     fn get_bytes_unchecked(&self, ptr: Pointer, size: u64, align: u64) -> EvalResult<'tcx, &[u8]> {

src/terminator/mod.rs

@@ -6,12 +6,12 @@ use rustc::ty::layout::{Layout, Size};
 use rustc::ty::subst::{Substs, Kind};
 use rustc::ty::{self, Ty, TyCtxt, BareFnTy};
 use syntax::codemap::{DUMMY_SP, Span};
-use syntax::{ast, attr};
+use syntax::{ast, attr, abi};
 
 use error::{EvalError, EvalResult};
 use eval_context::{EvalContext, IntegerExt, StackPopCleanup, monomorphize_field_ty, is_inhabited};
 use lvalue::{Lvalue, LvalueExtra};
-use memory::Pointer;
+use memory::{Pointer, FunctionDefinition, Function};
 use value::PrimVal;
 use value::Value;
 
@@ -89,7 +89,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 match func_ty.sty {
                     ty::TyFnPtr(bare_fn_ty) => {
                         let fn_ptr = self.eval_operand_to_primval(func)?.to_ptr()?;
-                        let (def_id, substs, abi, sig) = self.memory.get_fn(fn_ptr.alloc_id)?;
+                        let FunctionDefinition {def_id, substs, abi, sig} = self.memory.get_fn(fn_ptr.alloc_id)?.expect_concrete()?;
                         let bare_sig = self.tcx.erase_late_bound_regions_and_normalize(&bare_fn_ty.sig);
                         let bare_sig = self.tcx.erase_regions(&bare_sig);
                         // transmuting function pointers in miri is fine as long as the number of
@@ -313,6 +313,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 )?;
 
                 let arg_locals = self.frame().mir.args_iter();
+                assert_eq!(self.frame().mir.arg_count, args.len());
                 for (arg_local, (arg_val, arg_ty)) in arg_locals.zip(args) {
                     let dest = self.eval_lvalue(&mir::Lvalue::Local(arg_local))?;
                     self.write_value(arg_val, dest, arg_ty)?;
@@ -624,17 +625,48 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
             traits::VtableObject(ref data) => {
                 let idx = self.tcx.get_vtable_index_of_object_method(data, def_id) as u64;
-                if let Some(&mut(ref mut first_arg, ref mut first_ty)) = args.get_mut(0) {
-                    let (self_ptr, vtable) = first_arg.expect_ptr_vtable_pair(&self.memory)?;
-                    *first_arg = Value::ByVal(PrimVal::Ptr(self_ptr));
-                    let idx = idx + 3;
-                    let offset = idx * self.memory.pointer_size();
-                    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, Vec::new()))
-                } else {
-                    Err(EvalError::VtableForArgumentlessMethod)
+                if args.is_empty() {
+                    return Err(EvalError::VtableForArgumentlessMethod);
+                }
+                let (self_ptr, vtable) = args[0].0.expect_ptr_vtable_pair(&self.memory)?;
+                let idx = idx + 3;
+                let offset = idx * self.memory.pointer_size();
+                let fn_ptr = self.memory.read_ptr(vtable.offset(offset))?;
+                trace!("args: {:#?}", args);
+                match self.memory.get_fn(fn_ptr.alloc_id)? {
+                    Function::FnDefAsTraitObject(fn_def) => {
+                        trace!("sig: {:#?}", fn_def.sig);
+                        assert!(fn_def.abi != abi::Abi::RustCall);
+                        assert_eq!(args.len(), 2);
+                        // a function item turned into a closure trait object
+                        // the first arg is just there to give use the vtable
+                        args.remove(0);
+                        self.unpack_fn_args(args)?;
+                        Ok((fn_def.def_id, fn_def.substs, Vec::new()))
+                    },
+                    Function::DropGlue(_) => Err(EvalError::ManuallyCalledDropGlue),
+                    Function::Concrete(fn_def) => {
+                        trace!("sig: {:#?}", fn_def.sig);
+                        args[0] = (
+                            Value::ByVal(PrimVal::Ptr(self_ptr)),
+                            fn_def.sig.inputs()[0],
+                        );
+                        Ok((fn_def.def_id, fn_def.substs, Vec::new()))
+                    },
+                    Function::Closure(fn_def) => {
+                        self.unpack_fn_args(args)?;
+                        Ok((fn_def.def_id, fn_def.substs, Vec::new()))
+                    }
+                    Function::FnPtrAsTraitObject(sig) => {
+                        trace!("sig: {:#?}", sig);
+                        // the first argument was the fat ptr
+                        args.remove(0);
+                        self.unpack_fn_args(args)?;
+                        let fn_ptr = self.memory.read_ptr(self_ptr)?;
+                        let fn_def = self.memory.get_fn(fn_ptr.alloc_id)?.expect_concrete()?;
+                        assert_eq!(sig, fn_def.sig);
+                        Ok((fn_def.def_id, fn_def.substs, Vec::new()))
+                    }
                 }
             },
             vtable => bug!("resolved vtable bad vtable {:?} in trans", vtable),
@@ -768,7 +800,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let drop_fn = self.memory.read_ptr(vtable)?;
                 // some values don't need to call a drop impl, so the value is null
                 if drop_fn != Pointer::from_int(0) {
-                    let (def_id, substs, _abi, sig) = self.memory.get_fn(drop_fn.alloc_id)?;
+                    let FunctionDefinition {def_id, substs, sig, ..} = self.memory.get_fn(drop_fn.alloc_id)?.expect_drop_glue()?;
                     let real_ty = sig.inputs()[0];
                     self.drop(Lvalue::from_ptr(ptr), real_ty, drop)?;
                     drop.push((def_id, Value::ByVal(PrimVal::Ptr(ptr)), substs));

src/vtable.rs

@@ -55,12 +55,16 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     vec![Some(self.memory.create_closure_ptr(self.tcx, closure_def_id, substs, closure_type))].into_iter()
                 }
 
+                // turn a function definition into a Fn trait object
                 traits::VtableFnPointer(traits::VtableFnPointerData { fn_ty, .. }) => {
                     match fn_ty.sty {
                         ty::TyFnDef(did, substs, bare_fn_ty) => {
-                            vec![Some(self.memory.create_fn_ptr(self.tcx, did, substs, bare_fn_ty))].into_iter()
+                            vec![Some(self.memory.create_fn_as_trait_glue(self.tcx, did, substs, bare_fn_ty))].into_iter()
                         },
-                        _ => bug!("bad VtableFnPointer fn_ty: {:?}", fn_ty),
+                        ty::TyFnPtr(bare_fn_ty) => {
+                            vec![Some(self.memory.create_fn_ptr_as_trait_glue(bare_fn_ty))].into_iter()
+                        },
+                        _ => bug!("bad VtableFnPointer fn_ty: {:#?}", fn_ty.sty),
                     }
                 }
 
@@ -94,7 +98,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     ty::TyFnDef(_, _, fn_ty) => self.tcx.erase_regions(&fn_ty),
                     _ => bug!("drop method is not a TyFnDef"),
                 };
-                let fn_ptr = self.memory.create_fn_ptr(self.tcx, drop_def_id, substs, fn_ty);
+                let fn_ptr = self.memory.create_drop_glue(self.tcx, drop_def_id, substs, fn_ty);
                 self.memory.write_ptr(vtable, fn_ptr)?;
             }
         }