From 823e31d9fa5ef4aa6076e1f15083fef336bc44fc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 4 May 2024 14:26:11 +0200 Subject: [PATCH 1/2] add helper function to declare an extern static for a weak symbol --- src/tools/miri/src/shims/extern_static.rs | 22 ++++++++++++++----- src/tools/miri/src/shims/foreign_items.rs | 9 ++++++++ .../miri/src/shims/unix/foreign_items.rs | 2 +- .../src/shims/unix/linux/foreign_items.rs | 2 -- .../miri/src/shims/windows/foreign_items.rs | 2 +- 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/src/shims/extern_static.rs b/src/tools/miri/src/shims/extern_static.rs index 442338a6117..5cec51e9604 100644 --- a/src/tools/miri/src/shims/extern_static.rs +++ b/src/tools/miri/src/shims/extern_static.rs @@ -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" @@ -58,12 +73,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" diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 6a12180b836..4f8bb801100 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/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, diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index bd299aaa125..bebf8b4b8a5 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/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, diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 497e8bee70e..58ffd5fc1e0 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -113,9 +113,7 @@ 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() { diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index e8ae80261c6..7b156ee311a 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/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, From 19aa8a021d82d16af95913627055f064ed976b3b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 4 May 2024 14:41:12 +0200 Subject: [PATCH 2/2] make statx a regular function (so we don't need to support the syscall) --- src/tools/miri/src/shims/extern_static.rs | 7 +++--- src/tools/miri/src/shims/unix/fd.rs | 4 +++ .../src/shims/unix/linux/foreign_items.rs | 25 ++++++------------- .../miri/tests/pass/alloc-access-tracking.rs | 5 ++-- .../tests/pass/alloc-access-tracking.stderr | 8 +++--- 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/tools/miri/src/shims/extern_static.rs b/src/tools/miri/src/shims/extern_static.rs index 5cec51e9604..c3c7ef7c1fd 100644 --- a/src/tools/miri/src/shims/extern_static.rs +++ b/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], @@ -59,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); diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 566988cba1f..e536b78d1c0 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/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, diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 58ffd5fc1e0..71cdc3be814 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/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,7 +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() { @@ -134,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)?; diff --git a/src/tools/miri/tests/pass/alloc-access-tracking.rs b/src/tools/miri/tests/pass/alloc-access-tracking.rs index 29c1ee2f7b7..a226783155b 100644 --- a/src/tools/miri/tests/pass/alloc-access-tracking.rs +++ b/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; diff --git a/src/tools/miri/tests/pass/alloc-access-tracking.stderr b/src/tools/miri/tests/pass/alloc-access-tracking.stderr index bef13701ea2..0af6cde833f 100644 --- a/src/tools/miri/tests/pass/alloc-access-tracking.stderr +++ b/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