Auto merge of #3559 - RalfJung:weak-extern-static, r=RalfJung

add helper function to declare an extern static for a weak symbol

and use it to make `statx` a regular function and get rid of the syscall

Author: bors

src/tools/miri/src/shims/extern_static.rs

@@ -16,8 +16,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
 
     /// Zero-initialized pointer-sized extern statics are pretty common.
     /// Most of them are for weak symbols, which we all set to null (indicating that the
-    /// symbol is not supported, and triggering fallback code which ends up calling a
-    /// syscall that we do support).
+    /// symbol is not supported, and triggering fallback code which ends up calling
+    /// some other shim that we do support).
     fn null_ptr_extern_statics(
         this: &mut MiriInterpCx<'mir, 'tcx>,
         names: &[&str],
@@ -29,6 +29,21 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
         Ok(())
     }
 
+    /// Extern statics that are initialized with function pointers to the symbols of the same name.
+    fn weak_symbol_extern_statics(
+        this: &mut MiriInterpCx<'mir, 'tcx>,
+        names: &[&str],
+    ) -> InterpResult<'tcx> {
+        for name in names {
+            assert!(this.is_dyn_sym(name), "{name} is not a dynamic symbol");
+            let layout = this.machine.layouts.const_raw_ptr;
+            let ptr = this.fn_ptr(FnVal::Other(DynSym::from_str(name)));
+            let val = ImmTy::from_scalar(Scalar::from_pointer(ptr, this), layout);
+            Self::alloc_extern_static(this, name, val)?;
+        }
+        Ok(())
+    }
+
     /// Sets up the "extern statics" for this machine.
     pub fn init_extern_statics(this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
         // "__rust_no_alloc_shim_is_unstable"
@@ -44,8 +59,9 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
             "linux" => {
                 Self::null_ptr_extern_statics(
                     this,
-                    &["__cxa_thread_atexit_impl", "getrandom", "statx", "__clock_gettime64"],
+                    &["__cxa_thread_atexit_impl", "__clock_gettime64"],
                 )?;
+                Self::weak_symbol_extern_statics(this, &["getrandom", "statx"])?;
                 // "environ"
                 let environ = this.machine.env_vars.unix().environ();
                 Self::add_extern_static(this, "environ", environ);
@@ -58,12 +74,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
             }
             "android" => {
                 Self::null_ptr_extern_statics(this, &["bsd_signal"])?;
-                // "signal" -- just needs a non-zero pointer value (function does not even get called),
-                // but we arrange for this to call the `signal` function anyway.
-                let layout = this.machine.layouts.const_raw_ptr;
-                let ptr = this.fn_ptr(FnVal::Other(DynSym::from_str("signal")));
-                let val = ImmTy::from_scalar(Scalar::from_pointer(ptr, this), layout);
-                Self::alloc_extern_static(this, "signal", val)?;
+                Self::weak_symbol_extern_statics(this, &["signal"])?;
             }
             "windows" => {
                 // "_tls_used"

src/tools/miri/src/shims/foreign_items.rs

@@ -151,6 +151,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         Ok(None)
     }
 
+    fn is_dyn_sym(&self, name: &str) -> bool {
+        let this = self.eval_context_ref();
+        match this.tcx.sess.target.os.as_ref() {
+            os if this.target_os_is_unix() => shims::unix::foreign_items::is_dyn_sym(name, os),
+            "windows" => shims::windows::foreign_items::is_dyn_sym(name),
+            _ => false,
+        }
+    }
+
     /// Emulates a call to a `DynSym`.
     fn emulate_dyn_sym(
         &mut self,

src/tools/miri/src/shims/unix/fd.rs

@@ -17,6 +17,7 @@ use crate::*;
 pub trait FileDescription: std::fmt::Debug + Any {
     fn name(&self) -> &'static str;
 
+    /// Reads as much as possible into the given buffer, and returns the number of bytes read.
     fn read<'tcx>(
         &mut self,
         _communicate_allowed: bool,
@@ -26,6 +27,7 @@ pub trait FileDescription: std::fmt::Debug + Any {
         throw_unsup_format!("cannot read from {}", self.name());
     }
 
+    /// Writes as much as possible from the given buffer, and returns the number of bytes written.
     fn write<'tcx>(
         &mut self,
         _communicate_allowed: bool,
@@ -35,6 +37,8 @@ pub trait FileDescription: std::fmt::Debug + Any {
         throw_unsup_format!("cannot write to {}", self.name());
     }
 
+    /// Seeks to the given offset (which can be relative to the beginning, end, or current position).
+    /// Returns the new position from the start of the stream.
     fn seek<'tcx>(
         &mut self,
         _communicate_allowed: bool,

src/tools/miri/src/shims/unix/foreign_items.rs

@@ -15,7 +15,7 @@ use shims::unix::freebsd::foreign_items as freebsd;
 use shims::unix::linux::foreign_items as linux;
 use shims::unix::macos::foreign_items as macos;
 
-fn is_dyn_sym(name: &str, target_os: &str) -> bool {
+pub fn is_dyn_sym(name: &str, target_os: &str) -> bool {
     match name {
         // Used for tests.
         "isatty" => true,

src/tools/miri/src/shims/unix/linux/foreign_items.rs

@@ -12,7 +12,7 @@ use shims::unix::linux::mem::EvalContextExt as _;
 use shims::unix::linux::sync::futex;
 
 pub fn is_dyn_sym(name: &str) -> bool {
-    matches!(name, "getrandom")
+    matches!(name, "getrandom" | "statx")
 }
 
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
@@ -29,7 +29,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         // See `fn emulate_foreign_item_inner` in `shims/foreign_items.rs` for the general pattern.
 
         match link_name.as_str() {
-            // File related shims (but also see "syscall" below for statx)
+            // File related shims
             "readdir64" => {
                 let [dirp] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let result = this.linux_readdir64(dirp)?;
@@ -41,6 +41,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let result = this.sync_file_range(fd, offset, nbytes, flags)?;
                 this.write_scalar(result, dest)?;
             }
+            "statx" => {
+                let [dirfd, pathname, flags, mask, statxbuf] =
+                    this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
+                let result = this.linux_statx(dirfd, pathname, flags, mask, statxbuf)?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
 
             // epoll, eventfd
             "epoll_create1" => {
@@ -113,9 +119,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // have the right type.
 
                 let sys_getrandom = this.eval_libc("SYS_getrandom").to_target_usize(this)?;
-
-                let sys_statx = this.eval_libc("SYS_statx").to_target_usize(this)?;
-
                 let sys_futex = this.eval_libc("SYS_futex").to_target_usize(this)?;
 
                 if args.is_empty() {
@@ -136,20 +139,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         }
                         getrandom(this, &args[1], &args[2], &args[3], dest)?;
                     }
-                    // `statx` is used by `libstd` to retrieve metadata information on `linux`
-                    // instead of using `stat`,`lstat` or `fstat` as on `macos`.
-                    id if id == sys_statx => {
-                        // The first argument is the syscall id, so skip over it.
-                        if args.len() < 6 {
-                            throw_ub_format!(
-                                "incorrect number of arguments for `statx` syscall: got {}, expected at least 6",
-                                args.len()
-                            );
-                        }
-                        let result =
-                            this.linux_statx(&args[1], &args[2], &args[3], &args[4], &args[5])?;
-                        this.write_scalar(Scalar::from_target_isize(result.into(), this), dest)?;
-                    }
                     // `futex` is used by some synchronization primitives.
                     id if id == sys_futex => {
                         futex(this, &args[1..], dest)?;

src/tools/miri/src/shims/windows/foreign_items.rs

@@ -15,7 +15,7 @@ use crate::*;
 use shims::foreign_items::EmulateForeignItemResult;
 use shims::windows::handle::{Handle, PseudoHandle};
 
-fn is_dyn_sym(name: &str) -> bool {
+pub fn is_dyn_sym(name: &str) -> bool {
     // std does dynamic detection for these symbols
     matches!(
         name,

src/tools/miri/tests/pass/alloc-access-tracking.rs

@@ -1,7 +1,8 @@
 #![feature(start)]
 #![no_std]
-//@compile-flags: -Zmiri-track-alloc-id=18 -Zmiri-track-alloc-accesses -Cpanic=abort
-//@only-target-linux: alloc IDs differ between OSes for some reason
+//@compile-flags: -Zmiri-track-alloc-id=20 -Zmiri-track-alloc-accesses -Cpanic=abort
+//@normalize-stderr-test: "id 20" -> "id $$ALLOC"
+//@only-target-linux: alloc IDs differ between OSes (due to extern static allocations)
 
 extern "Rust" {
     fn miri_alloc(size: usize, align: usize) -> *mut u8;

src/tools/miri/tests/pass/alloc-access-tracking.stderr

@@ -2,7 +2,7 @@ note: tracking was triggered
   --> $DIR/alloc-access-tracking.rs:LL:CC
    |
 LL |         let ptr = miri_alloc(123, 1);
-   |                   ^^^^^^^^^^^^^^^^^^ created Miri bare-metal heap allocation of 123 bytes (alignment ALIGN bytes) with id 18
+   |                   ^^^^^^^^^^^^^^^^^^ created Miri bare-metal heap allocation of 123 bytes (alignment ALIGN bytes) with id $ALLOC
    |
    = note: BACKTRACE:
    = note: inside `start` at $DIR/alloc-access-tracking.rs:LL:CC
@@ -11,7 +11,7 @@ note: tracking was triggered
   --> $DIR/alloc-access-tracking.rs:LL:CC
    |
 LL |         *ptr = 42; // Crucially, only a write is printed here, no read!
-   |         ^^^^^^^^^ write access to allocation with id 18
+   |         ^^^^^^^^^ write access to allocation with id $ALLOC
    |
    = note: BACKTRACE:
    = note: inside `start` at $DIR/alloc-access-tracking.rs:LL:CC
@@ -20,7 +20,7 @@ note: tracking was triggered
   --> $DIR/alloc-access-tracking.rs:LL:CC
    |
 LL |         assert_eq!(*ptr, 42);
-   |         ^^^^^^^^^^^^^^^^^^^^ read access to allocation with id 18
+   |         ^^^^^^^^^^^^^^^^^^^^ read access to allocation with id $ALLOC
    |
    = note: BACKTRACE:
    = note: inside `start` at RUSTLIB/core/src/macros/mod.rs:LL:CC
@@ -30,7 +30,7 @@ note: tracking was triggered
   --> $DIR/alloc-access-tracking.rs:LL:CC
    |
 LL |         miri_dealloc(ptr, 123, 1);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ freed allocation with id 18
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ freed allocation with id $ALLOC
    |
    = note: BACKTRACE:
    = note: inside `start` at $DIR/alloc-access-tracking.rs:LL:CC