refactor dlsym: dispatch symbols via the normal shim mechanism

Author: RalfJung

src/tools/miri/src/helpers.rs

@@ -960,7 +960,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         self.check_abi(abi, exp_abi)?;
         if let Some((body, instance)) = self.eval_context_mut().lookup_exported_symbol(link_name)? {
             // If compiler-builtins is providing the symbol, then don't treat it as a clash.
-            // We'll use our built-in implementation in `emulate_foreign_item_by_name` for increased
+            // We'll use our built-in implementation in `emulate_foreign_item_inner` for increased
             // performance. Note that this means we won't catch any undefined behavior in
             // compiler-builtins when running other crates, but Miri can still be run on
             // compiler-builtins itself (or any crate that uses it as a normal dependency)

src/tools/miri/src/lib.rs

@@ -8,6 +8,7 @@
 #![feature(yeet_expr)]
 #![feature(nonzero_ops)]
 #![feature(round_ties_even)]
+#![feature(let_chains)]
 #![feature(lint_reasons)]
 #![feature(trait_upcasting)]
 // Configure clippy and other lints
@@ -86,9 +87,8 @@ pub use rustc_const_eval::interpret::*;
 // Resolve ambiguity.
 pub use rustc_const_eval::interpret::{self, AllocMap, PlaceTy, Provenance as _};
 
-pub use crate::shims::dlsym::{Dlsym, EvalContextExt as _};
 pub use crate::shims::env::{EnvVars, EvalContextExt as _};
-pub use crate::shims::foreign_items::EvalContextExt as _;
+pub use crate::shims::foreign_items::{DynSym, EvalContextExt as _};
 pub use crate::shims::intrinsics::EvalContextExt as _;
 pub use crate::shims::os_str::EvalContextExt as _;
 pub use crate::shims::panic::{CatchUnwindData, EvalContextExt as _};

src/tools/miri/src/machine.rs

@@ -709,9 +709,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
             "android" => {
                 // "signal"
                 let layout = this.machine.layouts.const_raw_ptr;
-                let dlsym = Dlsym::from_str("signal".as_bytes(), &this.tcx.sess.target.os)?
-                    .expect("`signal` must be an actual dlsym on android");
-                let ptr = this.fn_ptr(FnVal::Other(dlsym));
+                let ptr = this.fn_ptr(FnVal::Other(DynSym::from_str("signal")));
                 let val = ImmTy::from_scalar(Scalar::from_pointer(ptr, this), layout);
                 Self::alloc_extern_static(this, "signal", val)?;
                 // A couple zero-initialized pointer-sized extern statics.
@@ -867,7 +865,7 @@ impl<'mir, 'tcx> MiriInterpCxExt<'mir, 'tcx> for MiriInterpCx<'mir, 'tcx> {
 /// Machine hook implementations.
 impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     type MemoryKind = MiriMemoryKind;
-    type ExtraFnVal = Dlsym;
+    type ExtraFnVal = DynSym;
 
     type FrameExtra = FrameExtra<'tcx>;
     type AllocExtra = AllocExtra<'tcx>;
@@ -939,15 +937,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     #[inline(always)]
     fn call_extra_fn(
         ecx: &mut MiriInterpCx<'mir, 'tcx>,
-        fn_val: Dlsym,
+        fn_val: DynSym,
         abi: Abi,
         args: &[FnArg<'tcx, Provenance>],
         dest: &PlaceTy<'tcx, Provenance>,
         ret: Option<mir::BasicBlock>,
-        _unwind: mir::UnwindAction,
+        unwind: mir::UnwindAction,
     ) -> InterpResult<'tcx> {
         let args = ecx.copy_fn_args(args)?; // FIXME: Should `InPlace` arguments be reset to uninit?
-        ecx.call_dlsym(fn_val, abi, &args, dest, ret)
+        ecx.emulate_dyn_sym(fn_val, abi, &args, dest, ret, unwind)
     }
 
     #[inline(always)]

src/tools/miri/src/shims/dlsym.rs

@@ -6,7 +6,7 @@ use rustc_apfloat::Float;
 use rustc_ast::expand::allocator::AllocatorKind;
 use rustc_hir::{
     def::DefKind,
-    def_id::{CrateNum, DefId, LOCAL_CRATE},
+    def_id::{CrateNum, LOCAL_CRATE},
 };
 use rustc_middle::middle::{
     codegen_fn_attrs::CodegenFnAttrFlags, dependency_format::Linkage,
@@ -25,7 +25,18 @@ use super::backtrace::EvalContextExt as _;
 use crate::helpers::target_os_is_unix;
 use crate::*;
 
-/// Returned by `emulate_foreign_item_by_name`.
+/// Type of dynamic symbols (for `dlsym` et al)
+#[derive(Debug, Copy, Clone)]
+pub struct DynSym(Symbol);
+
+#[allow(clippy::should_implement_trait)]
+impl DynSym {
+    pub fn from_str(name: &str) -> Self {
+        DynSym(Symbol::intern(name))
+    }
+}
+
+/// Returned by `emulate_foreign_item_inner`.
 pub enum EmulateByNameResult<'mir, 'tcx> {
     /// The caller is expected to jump to the return block.
     NeedsJumping,
@@ -254,15 +265,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     /// is delegated to another function.
     fn emulate_foreign_item(
         &mut self,
-        def_id: DefId,
+        link_name: Symbol,
         abi: Abi,
         args: &[OpTy<'tcx, Provenance>],
         dest: &PlaceTy<'tcx, Provenance>,
         ret: Option<mir::BasicBlock>,
         unwind: mir::UnwindAction,
     ) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>> {
         let this = self.eval_context_mut();
-        let link_name = this.item_link_name(def_id);
         let tcx = this.tcx.tcx;
 
         // First: functions that diverge.
@@ -322,7 +332,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         };
 
         // Second: functions that return immediately.
-        match this.emulate_foreign_item_by_name(link_name, abi, args, dest)? {
+        match this.emulate_foreign_item_inner(link_name, abi, args, dest)? {
             EmulateByNameResult::NeedsJumping => {
                 trace!("{:?}", this.dump_place(dest));
                 this.go_to_block(ret);
@@ -345,6 +355,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(None)
     }
 
+    /// Emulates a call to a `DynSym`.
+    fn emulate_dyn_sym(
+        &mut self,
+        sym: DynSym,
+        abi: Abi,
+        args: &[OpTy<'tcx, Provenance>],
+        dest: &PlaceTy<'tcx, Provenance>,
+        ret: Option<mir::BasicBlock>,
+        unwind: mir::UnwindAction,
+    ) -> InterpResult<'tcx> {
+        let res = self.emulate_foreign_item(sym.0, abi, args, dest, ret, unwind)?;
+        assert!(res.is_none(), "DynSyms that delegate are not supported");
+        Ok(())
+    }
+
     /// Emulates calling the internal __rust_* allocator functions
     fn emulate_allocator(
         &mut self,
@@ -373,8 +398,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         }
     }
 
-    /// Emulates calling a foreign item using its name.
-    fn emulate_foreign_item_by_name(
+    fn emulate_foreign_item_inner(
         &mut self,
         link_name: Symbol,
         abi: Abi,
@@ -1045,11 +1069,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             _ =>
                 return match this.tcx.sess.target.os.as_ref() {
                     target_os if target_os_is_unix(target_os) =>
-                        shims::unix::foreign_items::EvalContextExt::emulate_foreign_item_by_name(
+                        shims::unix::foreign_items::EvalContextExt::emulate_foreign_item_inner(
                             this, link_name, abi, args, dest,
                         ),
                     "windows" =>
-                        shims::windows::foreign_items::EvalContextExt::emulate_foreign_item_by_name(
+                        shims::windows::foreign_items::EvalContextExt::emulate_foreign_item_inner(
                             this, link_name, abi, args, dest,
                         ),
                     _ => Ok(EmulateByNameResult::NotSupported),

src/tools/miri/src/shims/foreign_items.rs

@@ -9,7 +9,6 @@ pub mod unix;
 pub mod windows;
 mod x86;
 
-pub mod dlsym;
 pub mod env;
 pub mod os_str;
 pub mod panic;
@@ -58,7 +57,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             // foreign function
             // Any needed call to `goto_block` will be performed by `emulate_foreign_item`.
             let args = this.copy_fn_args(args)?; // FIXME: Should `InPlace` arguments be reset to uninit?
-            return this.emulate_foreign_item(instance.def_id(), abi, &args, dest, ret, unwind);
+            let link_name = this.item_link_name(instance.def_id());
+            return this.emulate_foreign_item(link_name, abi, &args, dest, ret, unwind);
         }
 
         // Otherwise, load the MIR.

src/tools/miri/src/shims/mod.rs

@@ -6,17 +6,26 @@ use shims::foreign_items::EmulateByNameResult;
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 
+pub fn is_dyn_sym(name: &str) -> bool {
+    matches!(name, "signal")
+}
+
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
-    fn emulate_foreign_item_by_name(
+    fn emulate_foreign_item_inner(
         &mut self,
         link_name: Symbol,
-        _abi: Abi,
-        _args: &[OpTy<'tcx, Provenance>],
-        _dest: &PlaceTy<'tcx, Provenance>,
+        abi: Abi,
+        args: &[OpTy<'tcx, Provenance>],
+        dest: &PlaceTy<'tcx, Provenance>,
     ) -> InterpResult<'tcx, EmulateByNameResult<'mir, 'tcx>> {
-        let _this = self.eval_context_mut();
-        #[allow(clippy::match_single_binding)]
+        let this = self.eval_context_mut();
+
         match link_name.as_str() {
+            "signal" if this.frame_in_std() => {
+                let [_sig, _func] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                this.write_null(dest)?;
+            }
             _ => return Ok(EmulateByNameResult::NotSupported),
         }
 

src/tools/miri/src/shims/unix/android/dlsym.rs

@@ -1,2 +1 @@
-pub mod dlsym;
 pub mod foreign_items;