Rollup merge of rust-lang#62245 - RalfJung:miri-extra-fn, r=eddyb,zackmdavis

Miri engine: support extra function (pointer) values

We want to add basic support for `dlsym` in Miri (needed to run the latest version of `getrandom`). For that to work, `dlsym` needs to return *something* that can be stored in a function pointer and later called.

So we add a new `ExtraFnVal` type to the `Machine` trait, and enable Miri's memory to associate allocation IDs with such values, so that `create_fn_alloc` and `get_fn` can work on *both* `Instance` (this is used for "normal" function pointers) and `ExtraFnVal`.

Cc @oli-obk

Author: Centril

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 = ();
@@ -370,6 +371,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         }))
     }
 
+    fn call_extra_fn(
+        _ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        fn_val: !,
+        _args: &[OpTy<'tcx>],
+        _dest: Option<PlaceTy<'tcx>>,
+        _ret: Option<mir::BasicBlock>,
+    ) -> InterpResult<'tcx> {
+        match fn_val {}
+    }
+
     fn call_intrinsic(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         instance: ty::Instance<'tcx>,

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

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;
 
@@ -119,6 +124,16 @@ pub trait Machine<'mir, 'tcx>: Sized {
         ret: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>;
 
+    /// Execute `fn_val`.  it is the hook's responsibility to advance the instruction
+    /// pointer as appropriate.
+    fn call_extra_fn(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        fn_val: Self::ExtraFnVal,
+        args: &[OpTy<'tcx, Self::PointerTag>],
+        dest: Option<PlaceTy<'tcx, Self::PointerTag>>,
+        ret: Option<mir::BasicBlock>,
+    ) -> InterpResult<'tcx>;
+
     /// Directly process an intrinsic without pushing a stack frame.
     /// If this returns successfully, the engine will take care of jumping to the next block.
     fn call_intrinsic(

src/librustc_mir/interpret/memory.rs

@@ -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>, extra: M::MemoryExtra) -> Self {
         Memory {
             alloc_map: M::MemoryMap::default(),
+            extra_fn_ptr_map: FxHashMap::default(),
             dead_alloc_map: FxHashMap::default(),
             extra,
             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) => {
+                // FIXME(RalfJung): 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,65 @@ 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));
         }
-        // can't do this in the match argument, we may get cycle errors since the lock would get
-        // dropped after the match.
+        // Function pointers.
+        if let Ok(_) = self.get_fn_alloc(id) {
+            return if let AllocCheck::Dereferencable = liveness {
+                // The caller requested no function pointers.
+                err!(DerefFunctionPointer)
+            } else {
+                Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
+            };
+        }
+        // Foreign statics.
+        // Can't do this in the match argument, we may get cycle errors since the lock would
+        // be held throughout 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()))
-                }
-            }
-            // `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))
+                return Ok((layout.size, layout.align.abi));
             }
-            _ => {
-                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)
-                }
-            },
+            _ => {}
+        }
+        // The rest must be dead.
+        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>> {
+    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()),
+            }
+        }
+    }
+
+    pub fn get_fn(
+        &self,
+        ptr: Scalar<M::PointerTag>,
+    ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> {
+        let ptr = self.force_ptr(ptr)?; // We definitely need a pointer value.
         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> {

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

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_ptr = self.read_scalar(func)?.not_undef()?;
+                        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,21 @@ 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 = match fn_val {
+            FnVal::Instance(instance) => instance,
+            FnVal::Other(extra) => {
+                return M::call_extra_fn(self, extra, args, dest, ret);
+            }
+        };
 
         match instance.def {
             ty::InstanceDef::Intrinsic(..) => {
@@ -431,8 +438,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     self.tcx.data_layout.pointer_align.abi,
                 )?.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)?;
+                    .read_ptr_sized(self, vtable_slot)?.not_undef()?;
+                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 +454,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 +489,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()],