various trace-related fixups

Author: nia-e

src/alloc/isolated_alloc.rs

@@ -318,15 +318,15 @@ impl IsolatedAlloc {
     ///
     /// SAFETY: Accessing memory after this point will result in a segfault
     /// unless it is first unprotected.
-    pub unsafe fn prepare_ffi(&mut self) -> Result<(), nix::errno::Errno> {
+    pub unsafe fn start_ffi(&mut self) -> Result<(), nix::errno::Errno> {
         let prot = mman::ProtFlags::PROT_NONE;
         unsafe { self.mprotect(prot) }
     }
 
     /// Deprotects all owned memory by setting it to RW. Erroring here is very
     /// likely unrecoverable, so it may panic if applying those permissions
     /// fails.
-    pub fn unprep_ffi(&mut self) {
+    pub fn end_ffi(&mut self) {
         let prot = mman::ProtFlags::PROT_READ | mman::ProtFlags::PROT_WRITE;
         unsafe {
             self.mprotect(prot).unwrap();

src/bin/miri.rs

@@ -227,8 +227,8 @@ impl rustc_driver::Callbacks for MiriCompilerCalls {
         } else {
             let return_code = miri::eval_entry(tcx, entry_def_id, entry_type, &config, None)
                 .unwrap_or_else(|| {
-                    //#[cfg(target_os = "linux")]
-                    //miri::native_lib::register_retcode_sv(rustc_driver::EXIT_FAILURE);
+                    #[cfg(target_os = "linux")]
+                    miri::native_lib::register_retcode_sv(rustc_driver::EXIT_FAILURE);
                     tcx.dcx().abort_if_errors();
                     rustc_driver::EXIT_FAILURE
                 });
@@ -804,7 +804,7 @@ fn main() {
         // thread in an async-signal-unsafe way such as by accessing shared
         // semaphores, etc.; the handler only calls `sleep()` and `exit()`, which
         // are async-signal-safe, as is accessing atomics
-        //let _ = unsafe { miri::native_lib::init_sv() };
+        let _ = unsafe { miri::native_lib::init_sv() };
     }
     run_compiler_and_exit(
         &rustc_args,

src/lib.rs

@@ -100,10 +100,10 @@ pub use rustc_const_eval::interpret::{self, AllocMap, Provenance as _};
 use rustc_middle::{bug, span_bug};
 use tracing::{info, trace};
 
-//#[cfg(target_os = "linux")]
-//pub mod native_lib {
-//    pub use crate::shims::{init_sv, register_retcode_sv};
-//}
+#[cfg(target_os = "linux")]
+pub mod native_lib {
+    pub use crate::shims::{init_sv, register_retcode_sv};
+}
 
 // Type aliases that set the provenance parameter.
 pub type Pointer = interpret::Pointer<Option<machine::Provenance>>;

src/shims/mod.rs

@@ -21,8 +21,8 @@ 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};
+#[cfg(target_os = "linux")]
+pub use self::native_lib::trace::{init_sv, register_retcode_sv};
 pub use self::unix::{DirTable, EpollInterestTable};
 
 /// What needs to be done after emulating an item (a shim or an intrinsic) is done.

src/shims/native_lib/mod.rs

@@ -1,8 +1,7 @@
 //! Implements calling functions from a native library.
 
-// FIXME: disabled since it fails to build on many targets.
-//#[cfg(target_os = "linux")]
-//pub mod trace;
+#[cfg(target_os = "linux")]
+pub mod trace;
 
 use std::ops::Deref;
 
@@ -13,13 +12,13 @@ use rustc_middle::mir::interpret::Pointer;
 use rustc_middle::ty::{self as ty, IntTy, UintTy};
 use rustc_span::Symbol;
 
-//#[cfg(target_os = "linux")]
-//use self::trace::Supervisor;
+#[cfg(target_os = "linux")]
+use self::trace::Supervisor;
 use crate::*;
 
-//#[cfg(target_os = "linux")]
-//type CallResult<'tcx> = InterpResult<'tcx, (ImmTy<'tcx>, Option<self::trace::messages::MemEvents>)>;
-//#[cfg(not(target_os = "linux"))]
+#[cfg(target_os = "linux")]
+type CallResult<'tcx> = InterpResult<'tcx, (ImmTy<'tcx>, Option<self::trace::messages::MemEvents>)>;
+#[cfg(not(target_os = "linux"))]
 type CallResult<'tcx> = InterpResult<'tcx, (ImmTy<'tcx>, Option<!>)>;
 
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
@@ -33,12 +32,12 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         libffi_args: Vec<libffi::high::Arg<'a>>,
     ) -> CallResult<'tcx> {
         let this = self.eval_context_mut();
-        //#[cfg(target_os = "linux")]
-        //let alloc = this.machine.allocator.as_ref().unwrap();
+        #[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, stack_ptr) = unsafe { Supervisor::start_ffi(alloc) };
+        #[cfg(target_os = "linux")]
+        let (guard, stack_ptr) = unsafe { Supervisor::start_ffi(alloc) };
 
         // Call the function (`ptr`) with arguments `libffi_args`, and obtain the return value
         // as the specified primitive integer type
@@ -112,9 +111,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // SAFETY: We got the guard and stack pointer from start_ffi, and
         // the allocator is the same
-        //#[cfg(target_os = "linux")]
-        //let events = unsafe { Supervisor::end_ffi(alloc, guard, stack_ptr) };
-        //#[cfg(not(target_os = "linux"))]
+        #[cfg(target_os = "linux")]
+        let events = unsafe { Supervisor::end_ffi(alloc, guard, stack_ptr) };
+        #[cfg(not(target_os = "linux"))]
         let events = None;
 
         interp_ok((res?, events))
@@ -214,9 +213,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 if !this.machine.native_call_mem_warned.replace(true) {
                     // Newly set, so first time we get here.
                     this.emit_diagnostic(NonHaltingDiagnostic::NativeCallSharedMem {
-                        //#[cfg(target_os = "linux")]
-                        //tracing: self::trace::Supervisor::is_enabled(),
-                        //#[cfg(not(target_os = "linux"))]
+                        #[cfg(target_os = "linux")]
+                        tracing: self::trace::Supervisor::is_enabled(),
+                        #[cfg(not(target_os = "linux"))]
                         tracing: false,
                     });
                 }

src/shims/native_lib/trace/child.rs

@@ -49,11 +49,11 @@ impl Supervisor {
     ) -> (std::sync::MutexGuard<'static, Option<Supervisor>>, Option<*mut [u8; CALLBACK_STACK_SIZE]>)
     {
         let mut sv_guard = SUPERVISOR.lock().unwrap();
-        // If the supervisor is not initialised for whatever reason, fast-fail.
+        // 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.take() else {
+        let Some(sv) = sv_guard.as_mut() else {
             return (sv_guard, None);
         };
 
@@ -68,9 +68,9 @@ impl Supervisor {
         // SAFETY: We do not access machine memory past this point until the
         // supervisor is ready to allow it.
         unsafe {
-            if alloc.borrow_mut().prepare_ffi().is_err() {
+            if alloc.borrow_mut().start_ffi().is_err() {
                 // Don't mess up unwinding by maybe leaving the memory partly protected
-                alloc.borrow_mut().unprep_ffi();
+                alloc.borrow_mut().end_ffi();
                 panic!("Cannot protect memory for FFI call!");
             }
         }
@@ -82,7 +82,6 @@ impl Supervisor {
         // enforce an ordering for these events.
         sv.message_tx.send(TraceRequest::StartFfi(start_info)).unwrap();
         sv.confirm_rx.recv().unwrap();
-        *sv_guard = Some(sv);
         // We need to be stopped for the supervisor to be able to make certain
         // modifications to our memory - simply waiting on the recv() doesn't
         // count.
@@ -113,20 +112,19 @@ impl Supervisor {
         signal::raise(signal::SIGUSR1).unwrap();
 
         // This is safe! It just sets memory to normal expected permissions.
-        alloc.borrow_mut().unprep_ffi();
+        alloc.borrow_mut().end_ffi();
 
         // If this is `None`, then `raw_stack_ptr` is None and does not need to
         // be deallocated (and there's no need to worry about the guard, since
         // it contains nothing).
-        let sv = sv_guard.take()?;
+        let sv = sv_guard.as_mut()?;
         // SAFETY: Caller upholds that this pointer was allocated as a box with
         // this type.
         unsafe {
             drop(Box::from_raw(raw_stack_ptr.unwrap()));
         }
         // On the off-chance something really weird happens, don't block forever.
-        let ret = sv
-            .event_rx
+        sv.event_rx
             .try_recv_timeout(std::time::Duration::from_secs(5))
             .map_err(|e| {
                 match e {
@@ -135,10 +133,7 @@ impl Supervisor {
                         eprintln!("Waiting for accesses from supervisor timed out!"),
                 }
             })
-            .ok();
-        // Do *not* leave the supervisor empty, or else we might get another fork...
-        *sv_guard = Some(sv);
-        ret
+            .ok()
     }
 }
 
@@ -148,11 +143,19 @@ impl Supervisor {
 /// receiving back events through `get_events`.
 ///
 /// # Safety
-/// The invariants for `fork()` must be upheld by the caller.
+/// The invariants for `fork()` must be upheld by the caller, namely either:
+/// - Other threads do not exist, or;
+/// - If they do exist, either those threads or the resulting child process
+///   only ever act in [async-signal-safe](https://www.man7.org/linux/man-pages/man7/signal-safety.7.html) ways.
 pub unsafe fn init_sv() -> Result<(), SvInitError> {
     // FIXME: Much of this could be reimplemented via the mitosis crate if we upstream the
     // relevant missing bits.
 
+    // Not on a properly supported architecture!
+    if cfg!(not(any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64"))) {
+        return Err(SvInitError);
+    }
+
     // On Linux, this will check whether ptrace is fully disabled by the Yama module.
     // If Yama isn't running or we're not on Linux, we'll still error later, but
     // this saves a very expensive fork call.
@@ -244,8 +247,7 @@ pub unsafe fn init_sv() -> Result<(), SvInitError> {
 /// given as a 0 if this is not used.
 pub fn register_retcode_sv(code: i32) {
     let mut sv_guard = SUPERVISOR.lock().unwrap();
-    if let Some(sv) = sv_guard.take() {
+    if let Some(sv) = sv_guard.as_mut() {
         sv.message_tx.send(TraceRequest::OverrideRetcode(code)).unwrap();
-        *sv_guard = Some(sv);
     }
 }

src/shims/native_lib/trace/messages.rs

@@ -41,7 +41,7 @@ pub enum TraceRequest {
 #[derive(serde::Serialize, serde::Deserialize, Debug, Clone)]
 pub struct StartFfiInfo {
     /// A vector of page addresses. These should have been automatically obtained
-    /// with `IsolatedAlloc::pages` and prepared with `IsolatedAlloc::prepare_ffi`.
+    /// with `IsolatedAlloc::pages` and prepared with `IsolatedAlloc::start_ffi`.
     pub page_ptrs: Vec<usize>,
     /// The address of an allocation that can serve as a temporary stack.
     /// This should be a leaked `Box<[u8; CALLBACK_STACK_SIZE]>` cast to an int.