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,
@@ -162,10 +165,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, rc::Rc};
+
+use rustc_const_eval::interpret::InterpResult;
+
+use crate::alloc::isolated_alloc::IsolatedAlloc;
+
 pub use self::child::{Supervisor, init_sv, register_retcode_sv};
 
 /// 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<
+/// 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

@@ -8,14 +8,12 @@ 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)]