various minor cleanups

Author: RalfJung

src/bin/miri.rs

@@ -335,7 +335,7 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
 fn exit(exit_code: i32) -> ! {
     // Drop the tracing guard before exiting, so tracing calls are flushed correctly.
     deinit_loggers();
-    //eprintln!("Called exit!");
+    // Make sure the supervisor knows about the code code.
     #[cfg(target_os = "linux")]
     miri::native_lib::register_retcode_sv(exit_code);
     std::process::exit(exit_code);
@@ -357,7 +357,8 @@ fn run_compiler_and_exit(
     callbacks: &mut (dyn rustc_driver::Callbacks + Send),
 ) -> ! {
     // Install the ctrlc handler that sets `rustc_const_eval::CTRL_C_RECEIVED`, even if
-    // MIRI_BE_RUSTC is set.
+    // MIRI_BE_RUSTC is set. We do this late so that when `native_lib::init_sv` is called,
+    // there are no other threads.
     rustc_driver::install_ctrlc_handler();
 
     // Invoke compiler, catch any unwinding panics and handle return code.

src/shims/native_lib/mod.rs

@@ -22,18 +22,12 @@ pub mod trace;
 use crate::*;
 
 /// The final results of an FFI trace, containing every relevant event detected
-/// by the tracer. Sent by the supervisor after receiving a `SIGUSR1` signal.
-///
-/// The sender for this channel should live on the parent process.
+/// by the tracer.
 #[allow(dead_code)]
 #[cfg_attr(target_os = "linux", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Debug)]
 pub struct MemEvents {
-    /// An ordered list of memory accesses that occurred. These should be assumed
-    /// to be overcautious; that is, if the size of an access is uncertain it is
-    /// pessimistically rounded up, and if the type (read/write/both) is uncertain
-    /// it is reported as whatever would be safest to assume; i.e. a read + maybe-write
-    /// becomes a read + write, etc.
+    /// An list of memory accesses that occurred, in the order they occurred in.
     pub acc_events: Vec<AccessEvent>,
 }
 
@@ -76,9 +70,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // Placeholder value.
         let alloc = ();
 
-        // Call the function (`ptr`) with arguments `libffi_args`, and obtain the return value
-        // as the specified primitive integer type
-        let ffi_fn = || {
+        trace::Supervisor::do_ffi(alloc, || {
+            // Call the function (`ptr`) with arguments `libffi_args`, and obtain the return value
+            // as the specified primitive integer type
             let scalar = match dest.layout.ty.kind() {
                 // ints
                 ty::Int(IntTy::I8) => {
@@ -144,9 +138,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     .into(),
             };
             interp_ok(ImmTy::from_scalar(scalar, dest.layout))
-        };
-
-        trace::Supervisor::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/messages.rs

@@ -8,11 +8,11 @@
 //! the child process's POV is as follows:
 //! ```
 //! message_tx.send(TraceRequest::StartFfi);
-//! confirm_rx.recv();
+//! confirm_rx.recv(); // receives a `Confirmation`
 //! raise(SIGSTOP);
 //! /* do ffi call */
 //! raise(SIGUSR1); // morally equivalent to some kind of "TraceRequest::EndFfi"
-//! let events = event_rx.recv();
+//! let events = event_rx.recv(); // receives a `MemEvents`
 //! ```
 //! `TraceRequest::OverrideRetcode` can be sent at any point in the above, including
 //! before or after all of them. `confirm_rx.recv()` is to be called after, to ensure

src/shims/native_lib/trace/parent.rs

@@ -15,7 +15,7 @@ const WAIT_FLAGS: wait::WaitPidFlag =
 /// The default word size on a given platform, in bytes.
 #[cfg(target_arch = "x86")]
 const ARCH_WORD_SIZE: usize = 4;
-#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
+#[cfg(target_arch = "x86_64")]
 const ARCH_WORD_SIZE: usize = 8;
 
 /// The address of the page set to be edited, initialised to a sentinel null
@@ -65,17 +65,6 @@ impl ArchIndependentRegs for libc::user_regs_struct {
     fn set_sp(&mut self, sp: usize) { self.esp = sp.try_into().unwrap() }
 }
 
-#[cfg(target_arch = "aarch64")]
-#[rustfmt::skip]
-impl ArchIndependentRegs for libc::user_regs_struct {
-    #[inline]
-    fn ip(&self) -> usize { self.pc.try_into().unwrap() }
-    #[inline]
-    fn set_ip(&mut self, ip: usize) { self.pc = ip.try_into().unwrap() }
-    #[inline]
-    fn set_sp(&mut self, sp: usize) { self.sp = sp.try_into().unwrap() }
-}
-
 /// A unified event representing something happening on the child process. Wraps
 /// `nix`'s `WaitStatus` and our custom signals so it can all be done with one
 /// `match` statement.
@@ -324,8 +313,6 @@ fn get_disasm() -> capstone::Capstone {
         {cs_pre.x86().mode(arch::x86::ArchMode::Mode64)}
         #[cfg(target_arch = "x86")]
         {cs_pre.x86().mode(arch::x86::ArchMode::Mode32)}
-        #[cfg(target_arch = "aarch64")]
-        {cs_pre.arm64().mode(arch::arm64::ArchMode::Arm)}
     }
     .detail(true)
     .build()
@@ -401,13 +388,21 @@ fn handle_segfault(
         let ins_detail = cs.insn_detail(&insns[0])?;
         let arch_detail = ins_detail.arch_detail();
 
+        let mut found_mem_op = false;
+
         for op in arch_detail.operands() {
             match op {
                 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
                 arch::ArchOperand::X86Operand(x86_operand) => {
                     match x86_operand.op_type {
                         // We only care about memory accesses
                         arch::x86::X86OperandType::Mem(_) => {
+                            if found_mem_op {
+                                panic!(
+                                    "more than one memory operand in a single operation is not supported"
+                                );
+                            }
+
                             let push = AccessRange { addr, size: x86_operand.size.into() };
                             // It's called a "RegAccessType" but it also applies to memory
                             let acc_ty = x86_operand.access.unwrap();
@@ -418,7 +413,7 @@ fn handle_segfault(
                                 acc_events.push(AccessEvent::Write(push));
                             }
 
-                            return Ok(());
+                            found_mem_op = true;
                         }
                         _ => (),
                     }