Add basic support for "other" kinds of values for function pointers, determined by the machine instance.

RalfJung · RalfJung · commit 1297a274a37f · 2019-07-04T10:38:54.000+02:00
So far, however, calling such a function will fail.
diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs
@@ -316,6 +316,7 @@ impl interpret::MayLeak for ! {
 impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> {
     type MemoryKinds = !;
     type PointerTag = ();
+    type ExtraFnVal = !;
 
     type FrameExtra = ();
     type MemoryExtra = ();
diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs
@@ -11,7 +11,7 @@ use rustc::mir::interpret::{
 };
 use rustc::mir::CastKind;
 
-use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate};
+use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate, FnVal};
 
 impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     fn type_is_fat_ptr(&self, ty: Ty<'tcx>) -> bool {
@@ -86,7 +86,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                             def_id,
                             substs,
                         ).ok_or_else(|| InterpError::TooGeneric.into());
-                        let fn_ptr = self.memory.create_fn_alloc(instance?);
+                        let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance?));
                         self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?;
                     }
                     _ => bug!("reify fn pointer on {:?}", src.layout.ty),
@@ -115,7 +115,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                             substs,
                             ty::ClosureKind::FnOnce,
                         );
-                        let fn_ptr = self.memory.create_fn_alloc(instance);
+                        let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance));
                         let val = Immediate::Scalar(Scalar::Ptr(fn_ptr.into()).into());
                         self.write_immediate(val, dest)?;
                     }
diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
@@ -67,6 +67,11 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// The `default()` is used for pointers to consts, statics, vtables and functions.
     type PointerTag: ::std::fmt::Debug + Copy + Eq + Hash + 'static;
 
+    /// Machines can define extra (non-instance) things that represent values of function pointers.
+    /// For example, Miri uses this to return a fucntion pointer from `dlsym`
+    /// that can later be called to execute the right thing.
+    type ExtraFnVal: ::std::fmt::Debug + Copy;
+
     /// Extra data stored in every call frame.
     type FrameExtra;
 
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
@@ -10,7 +10,7 @@ use std::collections::VecDeque;
 use std::ptr;
 use std::borrow::Cow;
 
-use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt};
+use rustc::ty::{self, Instance, query::TyCtxtAt};
 use rustc::ty::layout::{Align, TargetDataLayout, Size, HasDataLayout};
 use rustc_data_structures::fx::{FxHashSet, FxHashMap};
 
@@ -54,6 +54,26 @@ pub enum AllocCheck {
     MaybeDead,
 }
 
+/// The value of a function pointer.
+#[derive(Debug, Copy, Clone)]
+pub enum FnVal<'tcx, Other> {
+    Instance(Instance<'tcx>),
+    Other(Other),
+}
+
+impl<'tcx, Other> FnVal<'tcx, Other> {
+    pub fn as_instance(self) -> InterpResult<'tcx, Instance<'tcx>> {
+        match self {
+            FnVal::Instance(instance) =>
+                Ok(instance),
+            FnVal::Other(_) =>
+                err!(MachineError(
+                    format!("Expected instance function pointer, got 'other' pointer")
+                )),
+        }
+    }
+}
+
 // `Memory` has to depend on the `Machine` because some of its operations
 // (e.g., `get`) call a `Machine` hook.
 pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
@@ -69,16 +89,20 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
     // FIXME: this should not be public, but interning currently needs access to it
     pub(super) alloc_map: M::MemoryMap,
 
+    /// Map for "extra" function pointers.
+    extra_fn_ptr_map: FxHashMap<AllocId, M::ExtraFnVal>,
+
     /// To be able to compare pointers with NULL, and to check alignment for accesses
     /// to ZSTs (where pointers may dangle), we keep track of the size even for allocations
     /// that do not exist any more.
+    // FIXME: this should not be public, but interning currently needs access to it
     pub(super) dead_alloc_map: FxHashMap<AllocId, (Size, Align)>,
 
     /// Extra data added by the machine.
     pub extra: M::MemoryExtra,
 
     /// Lets us implement `HasDataLayout`, which is awfully convenient.
-    pub(super) tcx: TyCtxtAt<'tcx>,
+    pub tcx: TyCtxtAt<'tcx>,
 }
 
 impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M> {
@@ -98,6 +122,7 @@ where
     fn clone(&self) -> Self {
         Memory {
             alloc_map: self.alloc_map.clone(),
+            extra_fn_ptr_map: self.extra_fn_ptr_map.clone(),
             dead_alloc_map: self.dead_alloc_map.clone(),
             extra: (),
             tcx: self.tcx,
@@ -109,6 +134,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
     pub fn new(tcx: TyCtxtAt<'tcx>) -> Self {
         Memory {
             alloc_map: M::MemoryMap::default(),
+            extra_fn_ptr_map: FxHashMap::default(),
             dead_alloc_map: FxHashMap::default(),
             extra: M::MemoryExtra::default(),
             tcx,
@@ -120,8 +146,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         ptr.with_tag(M::tag_static_base_pointer(ptr.alloc_id, &self))
     }
 
-    pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer<M::PointerTag> {
-        let id = self.tcx.alloc_map.lock().create_fn_alloc(instance);
+    pub fn create_fn_alloc(
+        &mut self,
+        fn_val: FnVal<'tcx, M::ExtraFnVal>,
+    ) -> Pointer<M::PointerTag>
+    {
+        let id = match fn_val {
+            FnVal::Instance(instance) => self.tcx.alloc_map.lock().create_fn_alloc(instance),
+            FnVal::Other(extra) => {
+                // TODO: Should we have a cache here?
+                let id = self.tcx.alloc_map.lock().reserve();
+                let old = self.extra_fn_ptr_map.insert(id, extra);
+                assert!(old.is_none());
+                id
+            }
+        };
         self.tag_static_base_pointer(Pointer::from(id))
     }
 
@@ -495,56 +534,50 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         id: AllocId,
         liveness: AllocCheck,
     ) -> InterpResult<'static, (Size, Align)> {
+        // Regular allocations.
         if let Ok(alloc) = self.get(id) {
-            return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align));
+            Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align))
         }
-        // can't do this in the match argument, we may get cycle errors since the lock would get
-        // dropped after the match.
-        let alloc = self.tcx.alloc_map.lock().get(id);
-        // Could also be a fn ptr or extern static
-        match alloc {
-            Some(GlobalAlloc::Function(..)) => {
-                if let AllocCheck::Dereferencable = liveness {
-                    // The caller requested no function pointers.
-                    err!(DerefFunctionPointer)
-                } else {
-                    Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
-                }
+        // Function pointers.
+        else if let Ok(_) = self.get_fn_alloc(id) {
+            if let AllocCheck::Dereferencable = liveness {
+                // The caller requested no function pointers.
+                err!(DerefFunctionPointer)
+            } else {
+                Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
             }
-            // `self.get` would also work, but can cause cycles if a static refers to itself
-            Some(GlobalAlloc::Static(did)) => {
-                // The only way `get` couldn't have worked here is if this is an extern static
-                assert!(self.tcx.is_foreign_item(did));
-                // Use size and align of the type
-                let ty = self.tcx.type_of(did);
-                let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap();
-                Ok((layout.size, layout.align.abi))
+        }
+        // The rest must be dead.
+        else if let AllocCheck::MaybeDead = liveness {
+            // Deallocated pointers are allowed, we should be able to find
+            // them in the map.
+            Ok(*self.dead_alloc_map.get(&id)
+                .expect("deallocated pointers should all be recorded in `dead_alloc_map`"))
+        } else {
+            err!(DanglingPointerDeref)
+        }
+    }
+
+    fn get_fn_alloc(&self, id: AllocId) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> {
+        trace!("reading fn ptr: {}", id);
+        if let Some(extra) = self.extra_fn_ptr_map.get(&id) {
+            Ok(FnVal::Other(*extra))
+        } else {
+            match self.tcx.alloc_map.lock().get(id) {
+                Some(GlobalAlloc::Function(instance)) => Ok(FnVal::Instance(instance)),
+                _ => Err(InterpError::ExecuteMemory.into()),
             }
-            _ => {
-                if let Ok(alloc) = self.get(id) {
-                    Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align))
-                }
-                else if let AllocCheck::MaybeDead = liveness {
-                    // Deallocated pointers are allowed, we should be able to find
-                    // them in the map.
-                    Ok(*self.dead_alloc_map.get(&id)
-                        .expect("deallocated pointers should all be recorded in `dead_alloc_map`"))
-                } else {
-                    err!(DanglingPointerDeref)
-                }
-            },
         }
     }
 
-    pub fn get_fn(&self, ptr: Pointer<M::PointerTag>) -> InterpResult<'tcx, Instance<'tcx>> {
+    pub fn get_fn(
+        &self,
+        ptr: Pointer<M::PointerTag>,
+    ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> {
         if ptr.offset.bytes() != 0 {
             return err!(InvalidFunctionPointer);
         }
-        trace!("reading fn ptr: {}", ptr.alloc_id);
-        match self.tcx.alloc_map.lock().get(ptr.alloc_id) {
-            Some(GlobalAlloc::Function(instance)) => Ok(instance),
-            _ => Err(InterpError::ExecuteMemory.into()),
-        }
+        self.get_fn_alloc(ptr.alloc_id)
     }
 
     pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> {
diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs
@@ -24,7 +24,7 @@ pub use self::eval_context::{
 
 pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy};
 
-pub use self::memory::{Memory, MemoryKind, AllocCheck};
+pub use self::memory::{Memory, MemoryKind, AllocCheck, FnVal};
 
 pub use self::machine::{Machine, AllocMap, MayLeak};
 
diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs
@@ -6,9 +6,9 @@ use rustc::ty::layout::{self, TyLayout, LayoutOf};
 use syntax::source_map::Span;
 use rustc_target::spec::abi::Abi;
 
-use rustc::mir::interpret::{InterpResult, PointerArithmetic, InterpError, Scalar};
 use super::{
-    InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup
+    InterpResult, PointerArithmetic, InterpError, Scalar,
+    InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup, FnVal,
 };
 
 impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
@@ -76,16 +76,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 };
 
                 let func = self.eval_operand(func, None)?;
-                let (fn_def, abi) = match func.layout.ty.sty {
+                let (fn_val, abi) = match func.layout.ty.sty {
                     ty::FnPtr(sig) => {
                         let caller_abi = sig.abi();
                         let fn_ptr = self.force_ptr(self.read_scalar(func)?.not_undef()?)?;
-                        let instance = self.memory.get_fn(fn_ptr)?;
-                        (instance, caller_abi)
+                        let fn_val = self.memory.get_fn(fn_ptr)?;
+                        (fn_val, caller_abi)
                     }
                     ty::FnDef(def_id, substs) => {
                         let sig = func.layout.ty.fn_sig(*self.tcx);
-                        (self.resolve(def_id, substs)?, sig.abi())
+                        (FnVal::Instance(self.resolve(def_id, substs)?), sig.abi())
                     },
                     _ => {
                         let msg = format!("can't handle callee of type {:?}", func.layout.ty);
@@ -94,7 +94,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 };
                 let args = self.eval_operands(args)?;
                 self.eval_fn_call(
-                    fn_def,
+                    fn_val,
                     terminator.source_info.span,
                     abi,
                     &args[..],
@@ -228,14 +228,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Call this function -- pushing the stack frame and initializing the arguments.
     fn eval_fn_call(
         &mut self,
-        instance: ty::Instance<'tcx>,
+        fn_val: FnVal<'tcx, M::ExtraFnVal>,
         span: Span,
         caller_abi: Abi,
         args: &[OpTy<'tcx, M::PointerTag>],
         dest: Option<PlaceTy<'tcx, M::PointerTag>>,
         ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx> {
-        trace!("eval_fn_call: {:#?}", instance);
+        trace!("eval_fn_call: {:#?}", fn_val);
+
+        let instance = fn_val.as_instance()?;
 
         match instance.def {
             ty::InstanceDef::Intrinsic(..) => {
@@ -432,7 +434,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 )?.expect("cannot be a ZST");
                 let fn_ptr = self.memory.get(vtable_slot.alloc_id)?
                     .read_ptr_sized(self, vtable_slot)?.to_ptr()?;
-                let instance = self.memory.get_fn(fn_ptr)?;
+                let drop_fn = self.memory.get_fn(fn_ptr)?;
 
                 // `*mut receiver_place.layout.ty` is almost the layout that we
                 // want for args[0]: We have to project to field 0 because we want
@@ -447,7 +449,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 });
                 trace!("Patched self operand to {:#?}", args[0]);
                 // recurse with concrete function
-                self.eval_fn_call(instance, span, caller_abi, &args, dest, ret)
+                self.eval_fn_call(drop_fn, span, caller_abi, &args, dest, ret)
             }
         }
     }
@@ -482,7 +484,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let dest = MPlaceTy::dangling(self.layout_of(ty)?, self);
 
         self.eval_fn_call(
-            instance,
+            FnVal::Instance(instance),
             span,
             Abi::Rust,
             &[arg.into()],
diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs
@@ -2,7 +2,7 @@ use rustc::ty::{self, Ty, Instance};
 use rustc::ty::layout::{Size, Align, LayoutOf};
 use rustc::mir::interpret::{Scalar, Pointer, InterpResult, PointerArithmetic};
 
-use super::{InterpCx, InterpError, Machine, MemoryKind};
+use super::{InterpCx, InterpError, Machine, MemoryKind, FnVal};
 
 impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Creates a dynamic vtable for the given type and vtable origin. This is used only for
@@ -56,7 +56,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let tcx = &*self.tcx;
 
         let drop = Instance::resolve_drop_in_place(*tcx, ty);
-        let drop = self.memory.create_fn_alloc(drop);
+        let drop = self.memory.create_fn_alloc(FnVal::Instance(drop));
 
         // no need to do any alignment checks on the memory accesses below, because we know the
         // allocation is correctly aligned as we created it above. Also we're only offsetting by
@@ -84,7 +84,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     def_id,
                     substs,
                 ).ok_or_else(|| InterpError::TooGeneric)?;
-                let fn_ptr = self.memory.create_fn_alloc(instance);
+                let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance));
                 let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?;
                 self.memory
                     .get_mut(method_ptr.alloc_id)?
@@ -113,7 +113,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             .get(vtable.alloc_id)?
             .read_ptr_sized(self, vtable)?
             .to_ptr()?;
-        let drop_instance = self.memory.get_fn(drop_fn)?;
+        // We *need* an instance here, no other kind of function value, to be able
+        // to determine the type.
+        let drop_instance = self.memory.get_fn(drop_fn)?.as_instance()?;
         trace!("Found drop fn: {:?}", drop_instance);
         let fn_sig = drop_instance.ty(*self.tcx).fn_sig(*self.tcx);
         let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, &fn_sig);
