review 3

Author: nia-e

src/shims/mod.rs

@@ -21,6 +21,7 @@ pub mod time;
 pub mod tls;
 
 pub use self::files::FdTable;
+#[cfg(target_os = "linux")]
 pub use self::native_lib::trace::{init_sv, register_retcode_sv};
 pub use self::unix::{DirTable, EpollInterestTable};
 

src/shims/native_lib/mod.rs

@@ -15,11 +15,10 @@ use rustc_span::Symbol;
         target_env = "gnu",
         any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64")
     )),
-    path = "trace_stub.rs"
+    path = "trace/stub.rs"
 )]
 pub mod trace;
 
-use self::trace::Supervisor;
 use crate::*;
 
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
@@ -35,14 +34,13 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
         #[cfg(target_os = "linux")]
         let alloc = this.machine.allocator.as_ref().unwrap();
-
-        // SAFETY: We don't touch the machine memory past this point.
-        #[cfg(target_os = "linux")]
-        let guard = unsafe { Supervisor::start_ffi(alloc) };
+        #[cfg(not(target_os = "linux"))]
+        // Placeholder value.
+        let alloc = ();
 
         // Call the function (`ptr`) with arguments `libffi_args`, and obtain the return value
         // as the specified primitive integer type
-        let res = 'res: {
+        let ffi_fn = || {
             let scalar = match dest.layout.ty.kind() {
                 // ints
                 ty::Int(IntTy::I8) => {
@@ -93,15 +91,15 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 // have the output_type `Tuple([])`.
                 ty::Tuple(t_list) if (*t_list).deref().is_empty() => {
                     unsafe { ffi::call::<()>(ptr, libffi_args.as_slice()) };
-                    break 'res interp_ok(ImmTy::uninit(dest.layout));
+                    return interp_ok(ImmTy::uninit(dest.layout));
                 }
                 ty::RawPtr(..) => {
                     let x = unsafe { ffi::call::<*const ()>(ptr, libffi_args.as_slice()) };
                     let ptr = Pointer::new(Provenance::Wildcard, Size::from_bytes(x.addr()));
                     Scalar::from_pointer(ptr, this)
                 }
                 _ =>
-                    break 'res Err(err_unsup_format!(
+                    return Err(err_unsup_format!(
                         "unsupported return type for native call: {:?}",
                         link_name
                     ))
@@ -110,11 +108,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             interp_ok(ImmTy::from_scalar(scalar, dest.layout))
         };
 
-        // SAFETY: We got the guard and stack pointer from start_ffi, and
-        // the allocator is the same
-        let events = unsafe { Supervisor::end_ffi(guard) };
-
-        interp_ok((res?, events))
+        trace::do_ffi(alloc, ffi_fn)
     }
 
     /// Get the pointer to the function of the specified name in the shared object file,

src/shims/native_lib/trace/child.rs

@@ -15,6 +15,9 @@ use crate::alloc::isolated_alloc::IsolatedAlloc;
 /// need to ensure that either (a) only one `MiriMachine` is performing an FFI call
 /// at any given time, or (b) there are distinct supervisor and child processes for
 /// each machine. The former was chosen here.
+///
+/// This should only contain a `None` if the supervisor has not (yet) been initialised;
+/// otherwise, if `init_sv` was called and did not error, this will always be nonempty.
 static SUPERVISOR: std::sync::Mutex<Option<Supervisor>> = std::sync::Mutex::new(None);
 
 /// The main means of communication between the child and parent process,
@@ -55,21 +58,14 @@ impl Supervisor {
     ///
     /// SAFETY: The resulting guard must be dropped *via `end_ffi`* immediately
     /// after the desired call has concluded.
-    pub unsafe fn start_ffi(
-        alloc: &Rc<RefCell<IsolatedAlloc>>,
-    ) -> SvFfiGuard<'_>
-    {
+    pub unsafe fn start_ffi(alloc: &Rc<RefCell<IsolatedAlloc>>) -> SvFfiGuard<'_> {
         let mut sv_guard = SUPERVISOR.lock().unwrap();
         // If the supervisor is not initialised for whatever reason, fast-return.
         // This might be desired behaviour, as even on platforms where ptracing
         // is not implemented it enables us to enforce that only one FFI call
         // happens at a time.
         let Some(sv) = sv_guard.as_mut() else {
-            return SvFfiGuard {
-                alloc,
-                sv_guard,
-                cb_stack: None,
-            };
+            return SvFfiGuard { alloc, sv_guard, cb_stack: None };
         };
 
         // Get pointers to all the pages the supervisor must allow accesses in
@@ -101,11 +97,7 @@ impl Supervisor {
         // modifications to our memory - simply waiting on the recv() doesn't
         // count.
         signal::raise(signal::SIGSTOP).unwrap();
-        SvFfiGuard {
-            alloc,
-            sv_guard,
-            cb_stack: Some(raw_stack_ptr),
-        }
+        SvFfiGuard { alloc, sv_guard, cb_stack: Some(raw_stack_ptr) }
     }
 
     /// Undoes FFI-related preparations, allowing Miri to continue as normal, then
@@ -116,9 +108,7 @@ impl Supervisor {
     /// SAFETY: The `sv_guard` and `raw_stack_ptr` passed must be the same ones
     /// received by a prior call to `start_ffi`, and the allocator must be the
     /// one passed to it also.
-    pub unsafe fn end_ffi(
-        guard: SvFfiGuard<'_>,
-    ) -> Option<MemEvents> {
+    pub unsafe fn end_ffi(guard: SvFfiGuard<'_>) -> Option<MemEvents> {
         let alloc = guard.alloc;
         let mut sv_guard = guard.sv_guard;
         let cb_stack = guard.cb_stack;
@@ -162,10 +152,6 @@ impl Supervisor {
 /// supervisor process could not be created successfully; else, the caller
 /// is now the child process and can communicate via `start_ffi`/`end_ffi`,
 /// receiving back events through `get_events`.
-/// 
-/// When forking to initialise the supervisor, the child raises a `SIGSTOP`; if
-/// the parent successfully ptraces the child, it will allow it to resume. Else,
-/// the child will be killed by the parent.
 ///
 /// # Safety
 /// The invariants for `fork()` must be upheld by the caller, namely either:

src/shims/native_lib/trace/messages.rs

@@ -1,5 +1,8 @@
 //! Houses the types that are directly sent across the IPC channels.
-//! 
+//!
+//! When forking to initialise the supervisor during `init_sv`, the child raises
+//! a `SIGSTOP`; if the parent successfully ptraces the child, it will allow it
+//! to resume. Else, the child will be killed by the parent.
 //!
 //! After initialisation is done, the overall structure of a traced FFI call from
 //! the child process's POV is as follows:

src/shims/native_lib/trace/mod.rs

@@ -2,14 +2,34 @@ mod child;
 pub mod messages;
 mod parent;
 
+use std::cell::RefCell;
+use std::rc::Rc;
+
+use rustc_const_eval::interpret::InterpResult;
+
 pub use self::child::{Supervisor, init_sv, register_retcode_sv};
+use crate::alloc::isolated_alloc::IsolatedAlloc;
 
 /// The size of the temporary stack we use for callbacks that the server executes in the client.
 /// This should be big enough that `mempr_on` and `mempr_off` can safely be jumped into with the
 /// stack pointer pointing to a "stack" of this size without overflowing it.
 const CALLBACK_STACK_SIZE: usize = 1024;
 
-pub(super) type CallResult<'tcx> = rustc_const_eval::interpret::InterpResult<
-    'tcx,
-    (crate::ImmTy<'tcx>, Option<messages::MemEvents>),
->;
+/// Wrapper type for what we get back from an FFI call; the former is its actual
+/// return value, and the latter is the list of memory accesses that occurred during
+/// this call.
+pub type CallResult<'tcx> = InterpResult<'tcx, (crate::ImmTy<'tcx>, Option<messages::MemEvents>)>;
+
+/// Performs an arbitrary FFI call, enabling tracing from the supervisor.
+pub fn do_ffi<'tcx>(
+    alloc: &Rc<RefCell<IsolatedAlloc>>,
+    f: impl FnOnce() -> InterpResult<'tcx, crate::ImmTy<'tcx>>,
+) -> CallResult<'tcx> {
+    // SAFETY: We don't touch the machine memory past this point.
+    let guard = unsafe { Supervisor::start_ffi(alloc) };
+
+    f().map(|v| {
+        let memevents = unsafe { Supervisor::end_ffi(guard) };
+        (v, memevents)
+    })
+}

src/shims/native_lib/trace_stub.rs renamed to src/shims/native_lib/trace/stub.rs

@@ -1,5 +1,6 @@
-pub type CallResult<'tcx> =
-    rustc_const_eval::interpret::InterpResult<'tcx, (crate::ImmTy<'tcx>, Option<!>)>;
+use rustc_const_eval::interpret::InterpResult;
+
+pub type CallResult<'tcx> = InterpResult<'tcx, (crate::ImmTy<'tcx>, Option<!>)>;
 
 pub struct Supervisor;
 
@@ -8,14 +9,13 @@ impl Supervisor {
     pub fn is_enabled() -> bool {
         false
     }
+}
 
-    #[inline(always)]
-    pub unsafe fn start_ffi<T>(_: T) {}
-
-    #[inline(always)]
-    pub unsafe fn end_ffi<T>(_: T) -> Option<!> {
-        None
-    }
+pub fn do_ffi<'tcx, T>(
+    _: T,
+    f: impl FnOnce() -> InterpResult<'tcx, crate::ImmTy<'tcx>>,
+) -> CallResult<'tcx> {
+    f().map(|v| (v, None))
 }
 
 #[expect(clippy::missing_safety_doc)]