address review

Author: nia-e

build.rs

@@ -1,20 +1,20 @@
 use std::alloc::Layout;
 use std::borrow::Cow;
 use std::{alloc, slice};
-#[cfg(trace)]
+#[cfg(target_os = "linux")]
 use std::{cell::RefCell, rc::Rc};
 
 use rustc_abi::{Align, Size};
 use rustc_middle::mir::interpret::AllocBytes;
 
-#[cfg(trace)]
+#[cfg(target_os = "linux")]
 use crate::alloc::isolated_alloc::IsolatedAlloc;
 use crate::helpers::ToU64 as _;
 
 #[derive(Clone, Debug)]
 pub enum MiriAllocParams {
     Global,
-    #[cfg(trace)]
+    #[cfg(target_os = "linux")]
     Isolated(Rc<RefCell<IsolatedAlloc>>),
 }
 
@@ -56,7 +56,7 @@ impl Drop for MiriAllocBytes {
         unsafe {
             match self.params.clone() {
                 MiriAllocParams::Global => alloc::dealloc(self.ptr, alloc_layout),
-                #[cfg(trace)]
+                #[cfg(target_os = "linux")]
                 MiriAllocParams::Isolated(alloc) =>
                     alloc.borrow_mut().dealloc(self.ptr, alloc_layout),
             }
@@ -123,7 +123,7 @@ impl AllocBytes for MiriAllocBytes {
         let alloc_fn = |layout, params: &MiriAllocParams| unsafe {
             match params {
                 MiriAllocParams::Global => alloc::alloc(layout),
-                #[cfg(trace)]
+                #[cfg(target_os = "linux")]
                 MiriAllocParams::Isolated(alloc) => alloc.borrow_mut().alloc(layout),
             }
         };
@@ -144,7 +144,7 @@ impl AllocBytes for MiriAllocBytes {
         let alloc_fn = |layout, params: &MiriAllocParams| unsafe {
             match params {
                 MiriAllocParams::Global => alloc::alloc_zeroed(layout),
-                #[cfg(trace)]
+                #[cfg(target_os = "linux")]
                 MiriAllocParams::Isolated(alloc) => alloc.borrow_mut().alloc_zeroed(layout),
             }
         };

src/alloc/alloc_bytes.rs

@@ -304,14 +304,13 @@ impl IsolatedAlloc {
 
     /// Returns a vector of page addresses managed by the allocator.
     pub fn pages(&self) -> Vec<usize> {
-        let mut pages: Vec<usize> =
-            self.page_ptrs.iter().map(|p| p.expose_provenance().get()).collect();
-        for (ptr, size) in self.huge_ptrs.iter() {
-            for i in 0..size / self.page_size {
-                pages.push(ptr.expose_provenance().get().strict_add(i * self.page_size));
-            }
-        }
-        pages
+        let pages = self.page_ptrs.iter().map(|p| p.expose_provenance().get());
+        let pages = pages.chain(self.huge_ptrs.iter().flat_map(|(ptr, size)| {
+            (0..size / self.page_size)
+                .map(|i| ptr.expose_provenance().get().strict_add(i * self.page_size))
+        }));
+
+        pages.collect()
     }
 
     /// Protects all owned memory as `PROT_NONE`, preventing accesses.

src/alloc/isolated_alloc.rs

@@ -1,5 +1,5 @@
 mod alloc_bytes;
-#[cfg(trace)]
+#[cfg(target_os = "linux")]
 pub mod isolated_alloc;
 
 pub use self::alloc_bytes::{MiriAllocBytes, MiriAllocParams};

src/alloc/mod.rs

@@ -233,8 +233,6 @@ 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(trace)]
-                    miri::native_lib::register_retcode_sv(rustc_driver::EXIT_FAILURE);
                     tcx.dcx().abort_if_errors();
                     rustc_driver::EXIT_FAILURE
                 });
@@ -337,6 +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();
+    miri::native_lib::register_retcode_sv(exit_code);
     std::process::exit(exit_code);
 }
 
@@ -750,9 +749,8 @@ fn main() {
 
     debug!("rustc arguments: {:?}", rustc_args);
     debug!("crate arguments: {:?}", miri_config.args);
-    #[cfg(trace)]
     if !miri_config.native_lib.is_empty() && miri_config.native_lib_enable_tracing {
-        // FIXME: This should display a diagnostic / warning on error
+        // FIXME(ptrace): This should display a diagnostic / warning on error
         // SAFETY: If any other threads exist at this point (namely for the ctrlc
         // handler), they will not interact with anything on the main rustc/Miri
         // thread in an async-signal-unsafe way such as by accessing shared

src/bin/miri.rs

@@ -97,7 +97,6 @@ pub use rustc_const_eval::interpret::{self, AllocMap, Provenance as _};
 use rustc_middle::{bug, span_bug};
 use tracing::{info, trace};
 
-#[cfg(trace)]
 pub mod native_lib {
     pub use crate::shims::{init_sv, register_retcode_sv};
 }

src/lib.rs

@@ -530,7 +530,7 @@ pub struct MiriMachine<'tcx> {
     pub(crate) rng: RefCell<StdRng>,
 
     /// The allocator used for the machine's `AllocBytes` in native-libs mode.
-    #[cfg(trace)]
+    #[cfg(target_os = "linux")]
     pub(crate) allocator: Option<Rc<RefCell<crate::alloc::isolated_alloc::IsolatedAlloc>>>,
 
     /// The allocation IDs to report when they are being allocated
@@ -718,7 +718,7 @@ impl<'tcx> MiriMachine<'tcx> {
             local_crates,
             extern_statics: FxHashMap::default(),
             rng: RefCell::new(rng),
-            #[cfg(trace)]
+            #[cfg(target_os = "linux")]
             allocator: if !config.native_lib.is_empty() {
                 Some(Rc::new(RefCell::new(crate::alloc::isolated_alloc::IsolatedAlloc::new())))
             } else { None },
@@ -924,7 +924,7 @@ impl VisitProvenance for MiriMachine<'_> {
             backtrace_style: _,
             local_crates: _,
             rng: _,
-            #[cfg(trace)]
+            #[cfg(target_os = "linux")]
             allocator: _,
             tracked_alloc_ids: _,
             track_alloc_accesses: _,
@@ -1819,12 +1819,12 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
     fn get_default_alloc_params(&self) -> <Self::Bytes as AllocBytes>::AllocParams {
         use crate::alloc::MiriAllocParams;
 
-        #[cfg(trace)]
+        #[cfg(target_os = "linux")]
         match &self.allocator {
             Some(alloc) => MiriAllocParams::Isolated(alloc.clone()),
             None => MiriAllocParams::Global,
         }
-        #[cfg(not(trace))]
+        #[cfg(not(target_os = "linux"))]
         MiriAllocParams::Global
     }
 }

src/machine.rs

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

src/shims/mod.rs

@@ -1,8 +1,5 @@
 //! Implements calling functions from a native library.
 
-#[cfg(trace)]
-pub mod trace;
-
 use std::ops::Deref;
 
 use libffi::high::call as ffi;
@@ -12,15 +9,27 @@ use rustc_middle::mir::interpret::Pointer;
 use rustc_middle::ty::{self as ty, IntTy, UintTy};
 use rustc_span::Symbol;
 
-#[cfg(trace)]
+#[cfg_attr(
+    all(
+        target_os = "linux",
+        target_env = "gnu",
+        any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64")
+    ),
+    path = "trace/mod.rs"
+)]
+#[cfg_attr(
+    not(all(
+        target_os = "linux",
+        target_env = "gnu",
+        any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64")
+    )),
+    path = "trace_stub.rs"
+)]
+pub mod trace;
+
 use self::trace::Supervisor;
 use crate::*;
 
-#[cfg(trace)]
-type CallResult<'tcx> = InterpResult<'tcx, (ImmTy<'tcx>, Option<self::trace::messages::MemEvents>)>;
-#[cfg(not(trace))]
-type CallResult<'tcx> = InterpResult<'tcx, (ImmTy<'tcx>, Option<!>)>;
-
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
 trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
     /// Call native host function and return the output as an immediate.
@@ -30,13 +39,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         dest: &MPlaceTy<'tcx>,
         ptr: CodePtr,
         libffi_args: Vec<libffi::high::Arg<'a>>,
-    ) -> CallResult<'tcx> {
+    ) -> trace::CallResult<'tcx> {
         let this = self.eval_context_mut();
-        #[cfg(trace)]
         let alloc = this.machine.allocator.as_ref().unwrap();
 
         // SAFETY: We don't touch the machine memory past this point.
-        #[cfg(trace)]
         let (guard, stack_ptr) = unsafe { Supervisor::start_ffi(alloc) };
 
         // Call the function (`ptr`) with arguments `libffi_args`, and obtain the return value
@@ -111,10 +118,8 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // SAFETY: We got the guard and stack pointer from start_ffi, and
         // the allocator is the same
-        #[cfg(trace)]
         let events = unsafe { Supervisor::end_ffi(alloc, guard, stack_ptr) };
-        #[cfg(not(trace))]
-        let events = None;
+        //let events = None;
 
         interp_ok((res?, events))
     }
@@ -213,10 +218,7 @@ 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(trace)]
                         tracing: self::trace::Supervisor::is_enabled(),
-                        #[cfg(not(trace))]
-                        tracing: false,
                     });
                 }
 

src/shims/native_lib/mod.rs

@@ -6,3 +6,8 @@ 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.
 const CALLBACK_STACK_SIZE: usize = 1024;
+
+pub(super) type CallResult<'tcx> = rustc_const_eval::interpret::InterpResult<
+    'tcx,
+    (crate::ImmTy<'tcx>, Option<messages::MemEvents>),
+>;