diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index f9cb60c66d916..852ea26ab89e0 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -189,6 +189,8 @@ you can visualize in [Perfetto](https://ui.perfetto.dev/). For example: MIRI_TRACING=1 ./miri run --features=tracing tests/pass/hello.rs ``` +See [doc/tracing.md](./doc/tracing.md) for more information. + ### UI testing We use ui-testing in Miri, meaning we generate `.stderr` and `.stdout` files for the output diff --git a/src/tools/miri/doc/tracing.md b/src/tools/miri/doc/tracing.md index d7114af947dc1..1fbdd945d977e 100644 --- a/src/tools/miri/doc/tracing.md +++ b/src/tools/miri/doc/tracing.md @@ -2,6 +2,9 @@ Miri can be traced to understand how much time is spent in its various components (e.g. borrow tracker, data race checker, etc.). When tracing is enabled, running Miri will create a `.json` trace file that can be opened and analyzed in [Perfetto](https://ui.perfetto.dev/). For any questions regarding this documentation you may contact [Stypox](https://rust-lang.zulipchat.com/#narrow/dm/627563-Stypox) on Zulip. +> [!WARNING] +> Tracing in Miri at the moment is broken due to two bugs in tracing libraries: https://github.com/tokio-rs/tracing/pull/3392 and https://github.com/davidbarsky/tracing-tree/issues/93. Also see https://github.com/rust-lang/miri/issues/4752. + ## Obtaining a trace file ### From the Miri codebase @@ -240,7 +243,7 @@ let _trace = enter_trace_span!(M, "borrow_tracker", borrow_tracker = "on_stack_p ### `tracing_separate_thread` parameter -Miri saves traces using the the `tracing_chrome` `tracing::Layer` so that they can be visualized in Perfetto. To instruct `tracing_chrome` to put some spans on a separate trace thread/line than other spans when viewed in Perfetto, you can pass `tracing_separate_thread = tracing::field::Empty` to the tracing macros. This is useful to separate out spans which just indicate the current step or program frame being processed by the interpreter. As explained in [The timeline](#the-timeline), those spans end up under the "Global Legacy Events" track. You should use a value of `tracing::field::Empty` so that other tracing layers (e.g. the logger) will ignore the `tracing_separate_thread` field. For example: +Miri saves traces using the `tracing_chrome` `tracing::Layer` so that they can be visualized in Perfetto. To instruct `tracing_chrome` to put some spans on a separate trace thread/line than other spans when viewed in Perfetto, you can pass `tracing_separate_thread = tracing::field::Empty` to the tracing macros. This is useful to separate out spans which just indicate the current step or program frame being processed by the interpreter. As explained in [The timeline](#the-timeline), those spans end up under the "Global Legacy Events" track. You should use a value of `tracing::field::Empty` so that other tracing layers (e.g. the logger) will ignore the `tracing_separate_thread` field. For example: ```rust let _trace = enter_trace_span!(M, step::eval_statement, tracing_separate_thread = tracing::field::Empty); ``` @@ -277,9 +280,12 @@ So the solution was to copy-paste [the only file](https://github.com/thoren-d/tr ### Time measurements -tracing-chrome originally used `std::time::Instant` to measure time, however on some x86/x86_64 Linux systems it might be unbearably slow since the underlying system call (`clock_gettime`) would take ≈1.3µs. Read more [here](https://btorpey.github.io/blog/2014/02/18/clock-sources-in-linux/) about how the Linux kernel chooses the clock source. +tracing-chrome uses `std::time::Instant` to measure time. On most modern systems this is ok, since the underlying system call (`clock_gettime`) uses very fast hardware counters (e.g. `tsc`) and has a latency of ≈16ns. + +On some x86/x86_64 Linux systems, however, `tsc` is not "reliable" and the system thus relies on other timers, e.g. `hpet` which takes ≈1.3µs. Read [here](https://btorpey.github.io/blog/2014/02/18/clock-sources-in-linux/) how the Linux kernel chooses the clock source, and how to check if your system is using `tsc`. If it doesn't use `tsc`, then expect most of the trace time being spent in time measurements, which degrades traces' usefulness... See [here](https://github.com/rust-lang/miri/issues/4563) for some discussion. -Therefore, on x86/x86_64 Linux systems with a CPU that has an invariant TSC counter, we read from that instead to measure time, which takes only ≈13ns. There are unfortunately a lot of caveats to this approach though, as explained [in the code](https://github.com/rust-lang/miri/blob/master/src/bin/log/tracing_chrome_instant.rs) and [in the PR](https://github.com/rust-lang/miri/pull/4524). The most impactful one is that: every thread spawned in Miri that wants to trace something (which requires measuring time) needs to pin itself to a single CPU core (using `sched_setaffinity`). +> [!WARNING] +> A (somewhat risky) workaround is to add `tsc=reliable clocksource=tsc hpet=disable` to the kernel boot parameters, which forces it to use `tsc` even if it is unreliable. But this may render the system unstable, so try it at your own risk! ## Other useful stuff diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 28c3e88535f61..90a48de0fcb00 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -7bee525095c0872e87c038c412c781b9bbb3f5dc +d933cf483edf1605142ac6899ff32536c0ad8b22 diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index 32897eb89a83c..91559e76e68c6 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/src/alloc_addresses/mod.rs @@ -168,7 +168,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { if let Some(GlobalAlloc::Function { instance, .. }) = this.tcx.try_get_global_alloc(alloc_id) { - let fn_sig = this.tcx.fn_sig(instance.def_id()).skip_binder().skip_binder(); + let fn_sig = this.tcx.instantiate_bound_regions_with_erased( + this.tcx + .fn_sig(instance.def_id()) + .instantiate(*this.tcx, instance.args), + ); let fn_ptr = crate::shims::native_lib::build_libffi_closure(this, fn_sig)?; #[expect( diff --git a/src/tools/miri/src/bin/log/mod.rs b/src/tools/miri/src/bin/log/mod.rs index 22f74dd46b540..f3b2fdb5348e0 100644 --- a/src/tools/miri/src/bin/log/mod.rs +++ b/src/tools/miri/src/bin/log/mod.rs @@ -1,3 +1,2 @@ pub mod setup; mod tracing_chrome; -mod tracing_chrome_instant; diff --git a/src/tools/miri/src/bin/log/tracing_chrome.rs b/src/tools/miri/src/bin/log/tracing_chrome.rs index 85b4de62a5ec4..2caea73a6e3da 100644 --- a/src/tools/miri/src/bin/log/tracing_chrome.rs +++ b/src/tools/miri/src/bin/log/tracing_chrome.rs @@ -50,9 +50,8 @@ use std::{ thread::JoinHandle, }; -use crate::log::tracing_chrome_instant::TracingChromeInstant; - /// Contains thread-local data for threads that send tracing spans or events. +#[derive(Clone)] struct ThreadData { /// A unique ID for this thread, will populate "tid" field in the output trace file. tid: usize, @@ -61,7 +60,7 @@ struct ThreadData { out: Sender, /// The instant in time this thread was started. All events happening on this thread will be /// saved to the trace file with a timestamp (the "ts" field) measured relative to this instant. - start: TracingChromeInstant, + start: std::time::Instant, } thread_local! { @@ -562,28 +561,46 @@ where #[inline(always)] fn with_elapsed_micros_subtracting_tracing(&self, f: impl Fn(f64, usize, &Sender)) { THREAD_DATA.with(|value| { - let mut thread_data = value.borrow_mut(); - let (ThreadData { tid, out, start }, new_thread) = match thread_data.as_mut() { - Some(thread_data) => (thread_data, false), - None => { - let tid = self.max_tid.fetch_add(1, Ordering::SeqCst); - let out = self.out.lock().unwrap().clone(); - let start = TracingChromeInstant::setup_for_thread_and_start(tid); - *thread_data = Some(ThreadData { tid, out, start }); - (thread_data.as_mut().unwrap(), true) + // Make sure not to keep `value` borrowed when calling `f` below, since the user tracing + // code that `f` might invoke (e.g. fmt::Debug argument formatting) may contain nested + // tracing calls that would cause `value` to be doubly-borrowed mutably. + let (ThreadData { tid, out, start }, new_thread) = { + let mut thread_data = value.borrow_mut(); + match thread_data.as_mut() { + Some(thread_data) => (thread_data.clone(), false), + None => { + let tid = self.max_tid.fetch_add(1, Ordering::SeqCst); + let out = self.out.lock().unwrap().clone(); + let start = std::time::Instant::now(); + *thread_data = Some(ThreadData { tid, out: out.clone(), start }); + (ThreadData { tid, out, start }, true) + } } }; - start.with_elapsed_micros_subtracting_tracing(|ts| { - if new_thread { - let name = match std::thread::current().name() { - Some(name) => name.to_owned(), - None => tid.to_string(), - }; - let _ignored = out.send(Message::NewThread(*tid, name)); - } - f(ts, *tid, out); - }); + // Obtain the current time (before executing `f`). + let instant_before_f = std::time::Instant::now(); + + // Using the current time (`instant_before_f`), calculate the elapsed time (in + // microseconds) since `start` was instantiated, accounting for any time that was + // previously spent executing `f`. The "accounting" part is not computed in this + // line, but is rather done by shifting forward the `start` down below. + let ts = (instant_before_f - start).as_nanos() as f64 / 1000.0; + + // Run the function (supposedly a function internal to the tracing infrastructure). + if new_thread { + let name = match std::thread::current().name() { + Some(name) => name.to_owned(), + None => tid.to_string(), + }; + let _ignored = out.send(Message::NewThread(tid, name)); + } + f(ts, tid, &out); + + // Measure how much time was spent executing `f` and shift `start` forward + // by that amount. This "removes" that time from the trace. + // See comment at the top of this function for why we have to re-borrow here. + value.borrow_mut().as_mut().unwrap().start += std::time::Instant::now() - instant_before_f; }); } } diff --git a/src/tools/miri/src/bin/log/tracing_chrome_instant.rs b/src/tools/miri/src/bin/log/tracing_chrome_instant.rs deleted file mode 100644 index 04705b8846d9c..0000000000000 --- a/src/tools/miri/src/bin/log/tracing_chrome_instant.rs +++ /dev/null @@ -1,183 +0,0 @@ -//! Code in this class was in part inspired by -//! . -//! A useful resource is also -//! , -//! although this file does not implement TSC synchronization but instead pins threads to CPUs, -//! since the former is not reliable (i.e. it might lead to non-monotonic time measurements). -//! Another useful resource for future improvements might be measureme's time measurement utils: -//! . -//! Documentation about how the Linux kernel chooses a clock source can be found here: -//! . -#![cfg(feature = "tracing")] - -/// This alternative `TracingChromeInstant` implementation was made entirely to suit the needs of -/// [crate::log::tracing_chrome], and shouldn't be used for anything else. It features two functions: -/// - [TracingChromeInstant::setup_for_thread_and_start], which sets up the current thread to do -/// proper time tracking and returns a point in time to use as "t=0", and -/// - [TracingChromeInstant::with_elapsed_micros_subtracting_tracing], which allows -/// obtaining how much time elapsed since [TracingChromeInstant::setup_for_thread_and_start] was -/// called while accounting for (and subtracting) the time spent inside tracing-related functions. -/// -/// This measures time using [std::time::Instant], except for x86/x86_64 Linux machines, where -/// [std::time::Instant] is too slow (~1.5us) and thus `rdtsc` is used instead (~5ns). -pub enum TracingChromeInstant { - WallTime { - /// The time at which this instant was created, shifted forward to account - /// for time spent in tracing functions as explained in - /// [TracingChromeInstant::with_elapsed_micros_subtracting_tracing]'s comments. - start_instant: std::time::Instant, - }, - #[cfg(all(target_os = "linux", any(target_arch = "x86", target_arch = "x86_64")))] - Tsc { - /// The value in the TSC counter when this instant was created, shifted forward to account - /// for time spent in tracing functions as explained in - /// [TracingChromeInstant::with_elapsed_micros_subtracting_tracing]'s comments. - start_tsc: u64, - /// The period of the TSC counter in microseconds. - tsc_to_microseconds: f64, - }, -} - -impl TracingChromeInstant { - /// Can be thought of as the same as [std::time::Instant::now()], but also does some setup to - /// make TSC stable in case TSC is available. This is supposed to be called (at most) once per - /// thread since the thread setup takes a few milliseconds. - /// - /// WARNING: If TSC is available, `incremental_thread_id` is used to pick to which CPU to pin - /// the current thread. Thread IDs should be assigned contiguously starting from 0. Be aware - /// that the current thread will be restricted to one CPU for the rest of the execution! - pub fn setup_for_thread_and_start(incremental_thread_id: usize) -> TracingChromeInstant { - #[cfg(all(target_os = "linux", any(target_arch = "x86", target_arch = "x86_64")))] - if *tsc::IS_TSC_AVAILABLE.get_or_init(tsc::is_tsc_available) { - // We need to lock this thread to a specific CPU, because CPUs' TSC timers might be out - // of sync. - tsc::set_cpu_affinity(incremental_thread_id); - - // Can only use tsc_to_microseconds() and rdtsc() after having set the CPU affinity! - // We compute tsc_to_microseconds anew for every new thread just in case some CPU core - // has a different TSC frequency. - let tsc_to_microseconds = tsc::tsc_to_microseconds(); - let start_tsc = tsc::rdtsc(); - return TracingChromeInstant::Tsc { start_tsc, tsc_to_microseconds }; - } - - let _ = incremental_thread_id; // otherwise we get a warning when the TSC branch is disabled - TracingChromeInstant::WallTime { start_instant: std::time::Instant::now() } - } - - /// Calls `f` with the time elapsed in microseconds since this [TracingChromeInstant] was built - /// by [TracingChromeInstant::setup_for_thread_and_start], while subtracting all time previously - /// spent executing other `f`s passed to this function. This behavior allows subtracting time - /// spent in functions that log tracing data (which `f` is supposed to be) from the tracing time - /// measurements. - /// - /// Note: microseconds are used as the time unit since that's what Chrome trace files should - /// contain, see the definition of the "ts" field in - /// . - #[inline(always)] - pub fn with_elapsed_micros_subtracting_tracing(&mut self, f: impl Fn(f64)) { - match self { - TracingChromeInstant::WallTime { start_instant } => { - // Obtain the current time (before executing `f`). - let instant_before_f = std::time::Instant::now(); - - // Using the current time (`instant_before_f`) and the `start_instant` stored in - // `self`, calculate the elapsed time (in microseconds) since this instant was - // instantiated, accounting for any time that was previously spent executing `f`. - // The "accounting" part is not computed in this line, but is rather done by - // shifting forward the `start_instant` down below. - let ts = (instant_before_f - *start_instant).as_nanos() as f64 / 1000.0; - - // Run the function (supposedly a function internal to the tracing infrastructure). - f(ts); - - // Measure how much time was spent executing `f` and shift `start_instant` forward - // by that amount. This "removes" that time from the trace. - *start_instant += std::time::Instant::now() - instant_before_f; - } - - #[cfg(all(target_os = "linux", any(target_arch = "x86", target_arch = "x86_64")))] - TracingChromeInstant::Tsc { start_tsc, tsc_to_microseconds } => { - // the comments above also apply here, since it's the same logic - let tsc_before_f = tsc::rdtsc(); - let ts = ((tsc_before_f - *start_tsc) as f64) * (*tsc_to_microseconds); - f(ts); - *start_tsc += tsc::rdtsc() - tsc_before_f; - } - } - } -} - -#[cfg(all(target_os = "linux", any(target_arch = "x86", target_arch = "x86_64")))] -mod tsc { - - pub static IS_TSC_AVAILABLE: std::sync::OnceLock = std::sync::OnceLock::new(); - - /// Reads the timestamp-counter register. Will give monotonic answers only when called from the - /// same thread, because the TSC of different CPUs might be out of sync. - #[inline(always)] - pub(super) fn rdtsc() -> u64 { - #[cfg(target_arch = "x86")] - use core::arch::x86::_rdtsc; - #[cfg(target_arch = "x86_64")] - use core::arch::x86_64::_rdtsc; - - unsafe { _rdtsc() } - } - - /// Estimates the frequency of the TSC counter by waiting 10ms in a busy loop and - /// looking at how much the TSC increased in the meantime. - pub(super) fn tsc_to_microseconds() -> f64 { - const BUSY_WAIT: std::time::Duration = std::time::Duration::from_millis(10); - let tsc_start = rdtsc(); - let instant_start = std::time::Instant::now(); - while instant_start.elapsed() < BUSY_WAIT { - // `thread::sleep()` is not very precise at waking up the program at the right time, - // so use a busy loop instead. - core::hint::spin_loop(); - } - let tsc_end = rdtsc(); - (BUSY_WAIT.as_nanos() as f64) / 1000.0 / ((tsc_end - tsc_start) as f64) - } - - /// Checks whether the TSC counter is available and runs at a constant rate independently - /// of CPU frequency even across different power states of the CPU (i.e. checks for the - /// `invariant_tsc` CPUID flag). - pub(super) fn is_tsc_available() -> bool { - #[cfg(target_arch = "x86")] - use core::arch::x86::__cpuid; - #[cfg(target_arch = "x86_64")] - use core::arch::x86_64::__cpuid; - - // implemented like https://docs.rs/raw-cpuid/latest/src/raw_cpuid/extended.rs.html#965-967 - const LEAF: u32 = 0x80000007; // this is the leaf for "advanced power management info" - let cpuid = __cpuid(LEAF); - (cpuid.edx & (1 << 8)) != 0 // EDX bit 8 indicates invariant TSC - } - - /// Forces the current thread to run on a single CPU, which ensures the TSC counter is monotonic - /// (since TSCs of different CPUs might be out-of-sync). `incremental_thread_id` is used to pick - /// to which CPU to pin the current thread, and should be an incremental number that starts from - /// 0. - pub(super) fn set_cpu_affinity(incremental_thread_id: usize) { - let cpu_id = match std::thread::available_parallelism() { - Ok(available_parallelism) => incremental_thread_id % available_parallelism, - _ => panic!("Could not determine CPU count to properly set CPU affinity"), - }; - - let mut set = unsafe { std::mem::zeroed::() }; - unsafe { libc::CPU_SET(cpu_id, &mut set) }; - - // Set the current thread's core affinity. - if unsafe { - libc::sched_setaffinity( - 0, // Defaults to current thread - size_of::(), - &set as *const _, - ) - } != 0 - { - panic!("Could not set CPU affinity") - } - } -} diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 14528759472c8..3766debb159d0 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -193,7 +193,11 @@ fn make_miri_codegen_backend(opts: &Options, target: &Target) -> Box( @@ -354,6 +358,9 @@ impl rustc_driver::Callbacks for MiriDepCompilerCalls { ) } }); + + // Register our custom extra symbols. + config.extra_symbols = miri::sym::EXTRA_SYMBOLS.into(); } fn after_analysis<'tcx>( diff --git a/src/tools/miri/src/concurrency/genmc/scheduling.rs b/src/tools/miri/src/concurrency/genmc/scheduling.rs index c760126d787d1..54e87c05818de 100644 --- a/src/tools/miri/src/concurrency/genmc/scheduling.rs +++ b/src/tools/miri/src/concurrency/genmc/scheduling.rs @@ -5,7 +5,8 @@ use rustc_middle::ty::{self, Ty}; use super::GenmcCtx; use crate::{ - InterpCx, InterpResult, MiriMachine, TerminationInfo, ThreadId, interp_ok, throw_machine_stop, + InterpCx, InterpResult, MiriMachine, TerminationInfo, ThreadId, interp_ok, sym, + throw_machine_stop, }; enum NextInstrKind { @@ -76,11 +77,11 @@ fn get_function_kind<'tcx>( // NOTE: Functions intercepted by Miri in `concurrency/genmc/intercep.rs` must also be added here. // Such intercepted functions, like `sys::Mutex::lock`, should be treated as atomics to ensure we call the scheduler when we encounter one of them. // These functions must also be classified whether they may have load semantics. - if ecx.tcx.is_diagnostic_item(rustc_span::sym::sys_mutex_lock, callee_def_id) - || ecx.tcx.is_diagnostic_item(rustc_span::sym::sys_mutex_try_lock, callee_def_id) + if ecx.tcx.is_diagnostic_item(sym::sys_mutex_lock, callee_def_id) + || ecx.tcx.is_diagnostic_item(sym::sys_mutex_try_lock, callee_def_id) { return interp_ok(MaybeAtomic(ActionKind::Load)); - } else if ecx.tcx.is_diagnostic_item(rustc_span::sym::sys_mutex_unlock, callee_def_id) { + } else if ecx.tcx.is_diagnostic_item(sym::sys_mutex_unlock, callee_def_id) { return interp_ok(MaybeAtomic(ActionKind::NonLoad)); } // The next step is a call to a regular Rust function. diff --git a/src/tools/miri/src/concurrency/genmc/shims.rs b/src/tools/miri/src/concurrency/genmc/shims.rs index 4685dfd1b8dd7..ff9d43e70a46d 100644 --- a/src/tools/miri/src/concurrency/genmc/shims.rs +++ b/src/tools/miri/src/concurrency/genmc/shims.rs @@ -24,24 +24,22 @@ impl GenmcCtx { } } +/// Small helper to get the arguments of an intercepted function call. +fn get_fn_args<'tcx, const N: usize>( + instance: ty::Instance<'tcx>, + args: &[FnArg<'tcx>], +) -> InterpResult<'tcx, [OpTy<'tcx>; N]> { + let args = MiriInterpCx::copy_fn_args(args); // FIXME: Should `InPlace` arguments be reset to uninit? + if let Ok(ops) = args.try_into() { + return interp_ok(ops); + } + panic!("{} is a diagnostic item expected to have {} arguments", instance, N); +} + // Handling of code intercepted by Miri in GenMC mode, such as assume statement or `std::sync::Mutex`. impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { - /// Small helper to get the arguments of an intercepted function call. - fn get_fn_args( - &self, - instance: ty::Instance<'tcx>, - args: &[FnArg<'tcx>], - ) -> InterpResult<'tcx, [OpTy<'tcx>; N]> { - let this = self.eval_context_ref(); - let args = this.copy_fn_args(args); // FIXME: Should `InPlace` arguments be reset to uninit? - if let Ok(ops) = args.try_into() { - return interp_ok(ops); - } - panic!("{} is a diagnostic item expected to have {} arguments", instance, N); - } - /**** Blocking functionality ****/ /// Handle a thread getting blocked by a user assume (not an automatically generated assume). @@ -200,17 +198,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); // NOTE: When adding new intercepted functions here, they must also be added to `fn get_function_kind` in `concurrency/genmc/scheduling.rs`. - use rustc_span::sym; if this.tcx.is_diagnostic_item(sym::sys_mutex_lock, instance.def_id()) { - let [mutex] = this.get_fn_args(instance, args)?; + let [mutex] = get_fn_args(instance, args)?; let mutex = this.deref_pointer(&mutex)?; this.intercept_mutex_lock(mutex)?; } else if this.tcx.is_diagnostic_item(sym::sys_mutex_try_lock, instance.def_id()) { - let [mutex] = this.get_fn_args(instance, args)?; + let [mutex] = get_fn_args(instance, args)?; let mutex = this.deref_pointer(&mutex)?; this.intercept_mutex_try_lock(mutex, dest)?; } else if this.tcx.is_diagnostic_item(sym::sys_mutex_unlock, instance.def_id()) { - let [mutex] = this.get_fn_args(instance, args)?; + let [mutex] = get_fn_args(instance, args)?; let mutex = this.deref_pointer(&mutex)?; this.intercept_mutex_unlock(mutex)?; } else { diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 35217da37e5b5..367012e56ad9c 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -149,6 +149,7 @@ pub enum NonHaltingDiagnostic { failure_ordering: AtomicReadOrd, effective_failure_ordering: AtomicReadOrd, }, + FileInProcOpened, } /// Level of Miri specific diagnostics @@ -655,6 +656,7 @@ impl<'tcx> MiriMachine<'tcx> { | ProgressReport { .. } | WeakMemoryOutdatedLoad { .. } => ("tracking was triggered here".to_string(), DiagLevel::Note), + FileInProcOpened => ("open a file in `/proc`".to_string(), DiagLevel::Warning), }; let title = match &e { @@ -702,6 +704,7 @@ impl<'tcx> MiriMachine<'tcx> { }; format!("GenMC currently does not model the failure ordering for `compare_exchange`. {was_upgraded_msg}. Miri with GenMC might miss bugs related to this memory access.") } + FileInProcOpened => format!("files in `/proc` can bypass the Abstract Machine and might not work properly in Miri") }; let notes = match &e { diff --git a/src/tools/miri/src/intrinsics/mod.rs b/src/tools/miri/src/intrinsics/mod.rs index f09fc6c187896..ae7ec1fdcce51 100644 --- a/src/tools/miri/src/intrinsics/mod.rs +++ b/src/tools/miri/src/intrinsics/mod.rs @@ -10,7 +10,7 @@ pub use self::atomic::AtomicRmwOp; use rand::Rng; use rustc_abi::Size; use rustc_middle::{mir, ty}; -use rustc_span::{Symbol, sym}; +use rustc_span::Symbol; use self::atomic::EvalContextExt as _; use self::math::EvalContextExt as _; diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 32cadddc33aa3..5787efdf8cd1a 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -1,4 +1,5 @@ #![cfg_attr(bootstrap, feature(if_let_guard))] +#![cfg_attr(bootstrap, feature(cfg_select))] #![feature(abort_unwind)] #![feature(rustc_private)] #![feature(float_gamma)] @@ -16,7 +17,7 @@ #![feature(derive_coerce_pointee)] #![feature(arbitrary_self_types)] #![feature(iter_advance_by)] -#![cfg_attr(bootstrap, feature(cfg_select))] +#![feature(macro_metavar_expr)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, @@ -39,6 +40,7 @@ clippy::needless_lifetimes, clippy::too_long_first_doc_paragraph, clippy::len_zero, + clippy::collapsible_match, // We are not implementing queries here so it's fine rustc::potential_query_instability, )] @@ -87,6 +89,7 @@ mod math; mod operator; mod provenance_gc; mod shims; +pub mod sym; // Establish a "crate-wide prelude": we often import `crate::*`. // Make all those symbols available in the same place as our own. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 7883673cdd6a2..000f4ac4cecaa 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -300,7 +300,8 @@ pub enum ProvenanceExtra { #[cfg(target_pointer_width = "64")] static_assert_size!(StrictPointer, 24); -// FIXME: this would with in 24bytes but layout optimizations are not smart enough +// Pointer does not fit as the layout algorithm isn't smart enough (but also, we tried using +// pattern types to get a larger niche that makes this fit and it didn't improve performance). // #[cfg(target_pointer_width = "64")] //static_assert_size!(Pointer, 24); #[cfg(target_pointer_width = "64")] diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 3c2e14c181682..460015d4c3ccc 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -196,7 +196,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let [flag] = check_min_vararg_count("fcntl(fd, F_SETFL, ...)", varargs)?; let flag = this.read_scalar(flag)?.to_i32()?; - fd.set_flags(flag, this) + // Ignore flags that never get stored by SETFL. + // "File access mode (O_RDONLY, O_WRONLY, O_RDWR) and file + // creation flags (i.e., O_CREAT, O_EXCL, O_NOCTTY, O_TRUNC) + // in arg are ignored." + let ignored_flags = this.eval_libc_i32("O_RDONLY") + | this.eval_libc_i32("O_WRONLY") + | this.eval_libc_i32("O_RDWR") + | this.eval_libc_i32("O_CREAT") + | this.eval_libc_i32("O_EXCL") + | this.eval_libc_i32("O_NOCTTY") + | this.eval_libc_i32("O_TRUNC"); + + fd.set_flags(flag & !ignored_flags, this) } cmd if this.tcx.sess.target.os == Os::MacOs && cmd == this.eval_libc_i32("F_FULLFSYNC") => diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 48e2ebd0f13ea..130643ec680d2 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -539,6 +539,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } + // Network sockets + "socket" => { + let [domain, type_, protocol] = this.check_shim_sig( + shim_sig!(extern "C" fn(i32, i32, i32) -> i32), + link_name, + abi, + args, + )?; + let result = this.socket(domain, type_, protocol)?; + this.write_scalar(result, dest)?; + } + // Time "gettimeofday" => { let [tv, tz] = this.check_shim_sig( diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index a01755ef95ae7..9873af85b989b 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -7,7 +7,7 @@ use std::fs::{ rename, }; use std::io::{self, ErrorKind, Read, Seek, SeekFrom, Write}; -use std::path::{Path, PathBuf}; +use std::path::{self, Path, PathBuf}; use std::time::SystemTime; use rustc_abi::Size; @@ -354,9 +354,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let path_raw = this.read_pointer(path_raw)?; - let path = this.read_path_from_c_str(path_raw)?; let flag = this.read_scalar(flag)?.to_i32()?; + let path = this.read_path_from_c_str(path_raw)?; + // Files in `/proc` won't work properly. + if matches!(this.tcx.sess.target.os, Os::Linux | Os::Android | Os::Illumos | Os::Solaris) + && path::absolute(&path).is_ok_and(|path| path.starts_with("/proc")) + { + this.machine.emit_diagnostic(NonHaltingDiagnostic::FileInProcOpened); + } + let mut options = OpenOptions::new(); let o_rdonly = this.eval_libc_i32("O_RDONLY"); diff --git a/src/tools/miri/src/shims/unix/mod.rs b/src/tools/miri/src/shims/unix/mod.rs index 660e4ded5cda7..5ea49926fb9fd 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -4,6 +4,7 @@ mod env; mod fd; mod fs; mod mem; +mod socket; mod sync; mod thread; mod unnamed_socket; @@ -21,6 +22,7 @@ pub use self::fd::{EvalContextExt as _, UnixFileDescription}; pub use self::fs::{DirTable, EvalContextExt as _}; pub use self::linux_like::epoll::EpollInterestTable; pub use self::mem::EvalContextExt as _; +pub use self::socket::EvalContextExt as _; pub use self::sync::EvalContextExt as _; pub use self::thread::{EvalContextExt as _, ThreadNameResult}; pub use self::unnamed_socket::EvalContextExt as _; diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs new file mode 100644 index 0000000000000..66fe5d8a44a40 --- /dev/null +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -0,0 +1,157 @@ +use std::cell::Cell; +use std::net::{TcpListener, TcpStream}; + +use rustc_const_eval::interpret::{InterpResult, interp_ok}; +use rustc_middle::throw_unsup_format; +use rustc_target::spec::Os; + +use crate::shims::files::{FdId, FileDescription}; +use crate::{OpTy, Scalar, *}; + +#[derive(Debug, PartialEq)] +enum SocketFamily { + // IPv4 internet protocols + IPv4, + // IPv6 internet protocols + IPv6, +} + +#[derive(Debug)] +enum SocketType { + /// Reliable full-duplex communication, based on connections. + Stream, +} + +#[allow(unused)] +#[derive(Debug)] +enum SocketKind { + TcpListener(TcpListener), + TcpStream(TcpStream), +} + +#[allow(unused)] +#[derive(Debug)] +struct Socket { + /// Family of the socket, used to ensure socket only binds/connects to address of + /// same family. + family: SocketFamily, + /// Type of the socket, either datagram or stream. + /// Only stream is supported at the moment! + socket_type: SocketType, + /// Whether this fd is non-blocking or not. + is_non_block: Cell, +} + +impl FileDescription for Socket { + fn name(&self) -> &'static str { + "socket" + } + + fn destroy<'tcx>( + self, + _self_id: FdId, + _communicate_allowed: bool, + _ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, std::io::Result<()>> + where + Self: Sized, + { + interp_ok(Ok(())) + } + + fn get_flags<'tcx>(&self, ecx: &mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, Scalar> { + let mut flags = ecx.eval_libc_i32("O_RDWR"); + + if self.is_non_block.get() { + flags |= ecx.eval_libc_i32("O_NONBLOCK"); + } + + interp_ok(Scalar::from_i32(flags)) + } + + fn set_flags<'tcx>( + &self, + mut _flag: i32, + _ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, Scalar> { + throw_unsup_format!("fcntl: socket flags aren't supported") + } +} + +impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} +pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + /// For more information on the arguments see the socket manpage: + /// + fn socket( + &mut self, + domain: &OpTy<'tcx>, + type_: &OpTy<'tcx>, + protocol: &OpTy<'tcx>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let domain = this.read_scalar(domain)?.to_i32()?; + let mut flags = this.read_scalar(type_)?.to_i32()?; + let protocol = this.read_scalar(protocol)?.to_i32()?; + + // Reject if isolation is enabled + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`socket`", reject_with)?; + this.set_last_error(LibcError("EACCES"))?; + return interp_ok(Scalar::from_i32(-1)); + } + + let mut is_sock_nonblock = false; + + // Interpret the flag. Every flag we recognize is "subtracted" from `flags`, so + // if there is anything left at the end, that's an unsupported flag. + if matches!(this.tcx.sess.target.os, Os::Linux | Os::Android | Os::FreeBsd) { + // SOCK_NONBLOCK and SOCK_CLOEXEC only exist on Linux, Android and FreeBSD. + let sock_nonblock = this.eval_libc_i32("SOCK_NONBLOCK"); + let sock_cloexec = this.eval_libc_i32("SOCK_CLOEXEC"); + if flags & sock_nonblock == sock_nonblock { + is_sock_nonblock = true; + flags &= !sock_nonblock; + } + if flags & sock_cloexec == sock_cloexec { + // We don't support `exec` so we can ignore this. + flags &= !sock_cloexec; + } + } + + let family = if domain == this.eval_libc_i32("AF_INET") { + SocketFamily::IPv4 + } else if domain == this.eval_libc_i32("AF_INET6") { + SocketFamily::IPv6 + } else { + throw_unsup_format!( + "socket: domain {:#x} is unsupported, only AF_INET and \ + AF_INET6 are allowed.", + domain + ); + }; + + if flags != this.eval_libc_i32("SOCK_STREAM") { + throw_unsup_format!( + "socket: type {:#x} is unsupported, only SOCK_STREAM, \ + SOCK_CLOEXEC and SOCK_NONBLOCK are allowed", + flags + ); + } + if protocol != 0 { + throw_unsup_format!( + "socket: socket protocol {protocol} is unsupported, \ + only 0 is allowed" + ); + } + + let fds = &mut this.machine.fds; + let fd = fds.new_ref(Socket { + family, + is_non_block: Cell::new(is_sock_nonblock), + socket_type: SocketType::Stream, + }); + + interp_ok(Scalar::from_i32(fds.insert(fd))) + } +} diff --git a/src/tools/miri/src/shims/unix/unnamed_socket.rs b/src/tools/miri/src/shims/unix/unnamed_socket.rs index cc371b43a6815..ea34f72feee5b 100644 --- a/src/tools/miri/src/shims/unix/unnamed_socket.rs +++ b/src/tools/miri/src/shims/unix/unnamed_socket.rs @@ -170,12 +170,7 @@ impl FileDescription for AnonSocket { mut flag: i32, ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, Scalar> { - // FIXME: File creation flags should be ignored. - let o_nonblock = ecx.eval_libc_i32("O_NONBLOCK"); - let o_rdonly = ecx.eval_libc_i32("O_RDONLY"); - let o_wronly = ecx.eval_libc_i32("O_WRONLY"); - let o_rdwr = ecx.eval_libc_i32("O_RDWR"); // O_NONBLOCK flag can be set / unset by user. if flag & o_nonblock == o_nonblock { @@ -185,9 +180,6 @@ impl FileDescription for AnonSocket { self.is_nonblock.set(false); } - // Ignore all file access mode flags. - flag &= !(o_rdonly | o_wronly | o_rdwr); - // Throw error if there is any unsupported flag. if flag != 0 { throw_unsup_format!( diff --git a/src/tools/miri/src/sym.rs b/src/tools/miri/src/sym.rs new file mode 100644 index 0000000000000..7c870494f216c --- /dev/null +++ b/src/tools/miri/src/sym.rs @@ -0,0 +1,36 @@ +#![allow(non_upper_case_globals)] + +#[doc(no_inline)] +pub use rustc_span::sym::*; + +macro_rules! val { + ($name:ident) => { + stringify!($name) + }; + ($name:ident $value:literal) => { + $value + }; +} + +macro_rules! generate { + ($($name:ident $(: $value:literal)? ,)*) => { + /// To be supplied to `rustc_interface::Config` + pub const EXTRA_SYMBOLS: &[&str] = &[ + $( + val!($name $($value)?), + )* + ]; + + $( + pub const $name: rustc_span::Symbol = rustc_span::Symbol::new(rustc_span::symbol::PREDEFINED_SYMBOLS_COUNT + ${index()}); + )* + }; +} + +// List of extra symbols to be included in Miri. +// An alternative content can be specified using a colon after the symbol name. +generate! { + sys_mutex_lock, + sys_mutex_try_lock, + sys_mutex_unlock, +} diff --git a/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.rs b/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.rs new file mode 100644 index 0000000000000..73776827aec06 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.rs @@ -0,0 +1,18 @@ +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows + +use std::alloc; +use std::cell::Cell; + +type T = (Cell, i32); + +// Deallocating `x` is UB because not all bytes are in an `UnsafeCell`. +fn foo(x: &T) { + let layout = alloc::Layout::new::(); + unsafe { alloc::dealloc(x as *const _ as *mut T as *mut u8, layout) }; //~ERROR: dealloc +} + +fn main() { + let b: Box = Box::new((Cell::new(0), 0)); + foo(unsafe { std::mem::transmute(Box::into_raw(b)) }); +} diff --git a/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.stack.stderr b/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.stack.stderr new file mode 100644 index 0000000000000..ae99f17fd9c22 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.stack.stderr @@ -0,0 +1,23 @@ +error: Undefined Behavior: attempting deallocation using at ALLOC, but that tag only grants SharedReadOnly permission for this location + --> tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + | +LL | unsafe { alloc::dealloc(x as *const _ as *mut T as *mut u8, layout) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadOnly retag at offsets [0x4..0xc] + --> tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + | +LL | unsafe { alloc::dealloc(x as *const _ as *mut T as *mut u8, layout) }; + | ^ + = note: stack backtrace: + 0: foo + at tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + 1: main + at tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.tree.stderr b/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.tree.stderr new file mode 100644 index 0000000000000..5dc2ffc27c1c7 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/mixed_cell_deallocate.tree.stderr @@ -0,0 +1,24 @@ +error: Undefined Behavior: deallocation through at ALLOC[0x4] is forbidden + --> tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + | +LL | unsafe { alloc::dealloc(x as *const _ as *mut T as *mut u8, layout) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information + = help: the accessed tag has state Frozen which forbids this deallocation (acting as a child write access) +help: the accessed tag was created here, in the initial state Cell + --> tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + | +LL | fn foo(x: &T) { + | ^ + = note: stack backtrace: + 0: foo + at tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + 1: main + at tests/fail/both_borrows/mixed_cell_deallocate.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/native-lib/fail/call_function_ptr.notrace.stderr b/src/tools/miri/tests/native-lib/fail/call_fn_ptr.notrace.stderr similarity index 77% rename from src/tools/miri/tests/native-lib/fail/call_function_ptr.notrace.stderr rename to src/tools/miri/tests/native-lib/fail/call_fn_ptr.notrace.stderr index faabba9ca7257..5e7764652fa74 100644 --- a/src/tools/miri/tests/native-lib/fail/call_function_ptr.notrace.stderr +++ b/src/tools/miri/tests/native-lib/fail/call_fn_ptr.notrace.stderr @@ -1,5 +1,5 @@ warning: sharing memory with a native function called via FFI - --> tests/native-lib/fail/call_function_ptr.rs:LL:CC + --> tests/native-lib/fail/call_fn_ptr.rs:LL:CC | LL | call_fn_ptr(Some(nop)); | ^^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function @@ -10,12 +10,12 @@ LL | call_fn_ptr(Some(nop)); = help: what this means is that Miri will easily miss Undefined Behavior related to incorrect usage of this shared memory, so you should not take a clean Miri run as a signal that your FFI code is UB-free = note: stack backtrace: 0: pass_fn_ptr - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC 1: main - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC error: unsupported operation: calling a function pointer through the FFI boundary - --> tests/native-lib/fail/call_function_ptr.rs:LL:CC + --> tests/native-lib/fail/call_fn_ptr.rs:LL:CC | LL | call_fn_ptr(Some(nop)); | ^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here @@ -23,9 +23,9 @@ LL | call_fn_ptr(Some(nop)); = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support = note: stack backtrace: 0: pass_fn_ptr - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC 1: main - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/native-lib/fail/call_function_ptr.rs b/src/tools/miri/tests/native-lib/fail/call_fn_ptr.rs similarity index 100% rename from src/tools/miri/tests/native-lib/fail/call_function_ptr.rs rename to src/tools/miri/tests/native-lib/fail/call_fn_ptr.rs diff --git a/src/tools/miri/tests/native-lib/fail/call_function_ptr.trace.stderr b/src/tools/miri/tests/native-lib/fail/call_fn_ptr.trace.stderr similarity index 79% rename from src/tools/miri/tests/native-lib/fail/call_function_ptr.trace.stderr rename to src/tools/miri/tests/native-lib/fail/call_fn_ptr.trace.stderr index e56a5ece782b5..7418b08ebe110 100644 --- a/src/tools/miri/tests/native-lib/fail/call_function_ptr.trace.stderr +++ b/src/tools/miri/tests/native-lib/fail/call_fn_ptr.trace.stderr @@ -1,5 +1,5 @@ warning: sharing memory with a native function called via FFI - --> tests/native-lib/fail/call_function_ptr.rs:LL:CC + --> tests/native-lib/fail/call_fn_ptr.rs:LL:CC | LL | call_fn_ptr(Some(nop)); | ^^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function @@ -11,12 +11,12 @@ LL | call_fn_ptr(Some(nop)); = help: tracing memory accesses in native code is not yet fully implemented, so there can be further imprecisions beyond what is documented here = note: stack backtrace: 0: pass_fn_ptr - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC 1: main - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC error: unsupported operation: calling a function pointer through the FFI boundary - --> tests/native-lib/fail/call_function_ptr.rs:LL:CC + --> tests/native-lib/fail/call_fn_ptr.rs:LL:CC | LL | call_fn_ptr(Some(nop)); | ^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here @@ -24,9 +24,9 @@ LL | call_fn_ptr(Some(nop)); = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support = note: stack backtrace: 0: pass_fn_ptr - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC 1: main - at tests/native-lib/fail/call_function_ptr.rs:LL:CC + at tests/native-lib/fail/call_fn_ptr.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.notrace.stderr b/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.notrace.stderr new file mode 100644 index 0000000000000..e0536a4afe583 --- /dev/null +++ b/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.notrace.stderr @@ -0,0 +1,31 @@ +warning: sharing memory with a native function called via FFI + --> tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + | +LL | call_fn_ptr(id::); + | ^^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function + | + = help: when memory is shared with a native function call, Miri stops tracking initialization and provenance for that memory + = help: in particular, Miri assumes that the native call initializes all memory it has access to + = help: Miri also assumes that any part of this memory may be a pointer that is permitted to point to arbitrary exposed memory + = help: what this means is that Miri will easily miss Undefined Behavior related to incorrect usage of this shared memory, so you should not take a clean Miri run as a signal that your FFI code is UB-free + = note: stack backtrace: + 0: pass_fn_ptr + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + 1: main + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + +error: unsupported operation: calling a function pointer through the FFI boundary + --> tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + | +LL | call_fn_ptr(id::); + | ^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support + = note: stack backtrace: + 0: pass_fn_ptr + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + 1: main + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + diff --git a/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.rs b/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.rs new file mode 100644 index 0000000000000..8be29c0c121a2 --- /dev/null +++ b/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.rs @@ -0,0 +1,22 @@ +//@revisions: trace notrace +//@[trace] only-target: x86_64-unknown-linux-gnu i686-unknown-linux-gnu +//@[trace] compile-flags: -Zmiri-native-lib-enable-tracing +//@compile-flags: -Zmiri-permissive-provenance + +fn main() { + pass_fn_ptr() +} + +fn pass_fn_ptr() { + extern "C" { + fn call_fn_ptr(s: extern "C" fn(i32) -> i32); + } + + extern "C" fn id(x: T) -> T { + x + } + + unsafe { + call_fn_ptr(id::); //~ ERROR: unsupported operation: calling a function pointer through the FFI boundary + } +} diff --git a/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.trace.stderr b/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.trace.stderr new file mode 100644 index 0000000000000..24a33e6458685 --- /dev/null +++ b/src/tools/miri/tests/native-lib/fail/call_fn_ptr_with_generic.trace.stderr @@ -0,0 +1,32 @@ +warning: sharing memory with a native function called via FFI + --> tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + | +LL | call_fn_ptr(id::); + | ^^^^^^^^^^^^^^^^^^^^^^ sharing memory with a native function + | + = help: when memory is shared with a native function call, Miri can only track initialisation and provenance on a best-effort basis + = help: in particular, Miri assumes that the native call initializes all memory it has written to + = help: Miri also assumes that any part of this memory may be a pointer that is permitted to point to arbitrary exposed memory + = help: what this means is that Miri will easily miss Undefined Behavior related to incorrect usage of this shared memory, so you should not take a clean Miri run as a signal that your FFI code is UB-free + = help: tracing memory accesses in native code is not yet fully implemented, so there can be further imprecisions beyond what is documented here + = note: stack backtrace: + 0: pass_fn_ptr + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + 1: main + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + +error: unsupported operation: calling a function pointer through the FFI boundary + --> tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + | +LL | call_fn_ptr(id::); + | ^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support + = note: stack backtrace: + 0: pass_fn_ptr + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + 1: main + at tests/native-lib/fail/call_fn_ptr_with_generic.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + diff --git a/src/tools/miri/tests/native-lib/fn_ptr.c b/src/tools/miri/tests/native-lib/fn_ptr.c new file mode 100644 index 0000000000000..752e27b3e8a0a --- /dev/null +++ b/src/tools/miri/tests/native-lib/fn_ptr.c @@ -0,0 +1,17 @@ +#include +#include + +// See comments in build_native_lib() +#define EXPORT __attribute__((visibility("default"))) + +EXPORT void call_fn_ptr(void f(void)) { + if (f != NULL) { + f(); + } +} + +EXPORT void call_fn_ptr_with_arg(int32_t f(int32_t)) { + if (f != NULL) { + f(42); + } +} diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs index 36eff04a03c05..ad4c84d3e83b8 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs @@ -10,7 +10,6 @@ fn main() { test_access_simple(); test_access_nested(); test_access_static(); - pass_fn_ptr(); } /// Test function that dereferences an int pointer and prints its contents from C. @@ -82,16 +81,3 @@ fn test_access_static() { assert_eq!(unsafe { access_static(&STATIC) }, 9001); } - -fn pass_fn_ptr() { - extern "C" { - fn pass_fn_ptr(s: Option); - } - - extern "C" fn nop() {} - - unsafe { - pass_fn_ptr(None); // this one is fine - pass_fn_ptr(Some(nop)); // this one is not - } -} diff --git a/src/tools/miri/tests/native-lib/ptr_read_access.c b/src/tools/miri/tests/native-lib/ptr_read_access.c index 44ba13aa54a62..58017d809af89 100644 --- a/src/tools/miri/tests/native-lib/ptr_read_access.c +++ b/src/tools/miri/tests/native-lib/ptr_read_access.c @@ -62,16 +62,3 @@ EXPORT int32_t access_static(const Static *s_ptr) { EXPORT uintptr_t do_one_deref(const int32_t ***ptr) { return (uintptr_t)*ptr; } - -/* Test: pass_fn_ptr */ - -EXPORT void pass_fn_ptr(void f(void)) { - (void)f; // suppress unused warning -} - -/* Test: function_ptrs */ -EXPORT void call_fn_ptr(void f(void)) { - if (f != NULL) { - f(); - } -} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs b/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs index db68daed53961..8f8d4f85c0109 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs @@ -148,6 +148,24 @@ fn test_pipe_setfl_getfl() { errno_result(unsafe { libc::fcntl(fds[0], libc::F_GETFL) }).unwrap(), libc::O_RDONLY ); + + // Test if ignored flags are indeed ignored. + errno_check(unsafe { + libc::fcntl( + fds[0], + libc::F_SETFL, + libc::O_RDWR + | libc::O_CREAT + | libc::O_EXCL + | libc::O_NOCTTY + | libc::O_TRUNC + | libc::O_NONBLOCK, + ) + }); + assert_eq!( + errno_result(unsafe { libc::fcntl(fds[0], libc::F_GETFL) }).unwrap(), + libc::O_NONBLOCK | libc::O_RDONLY + ); } /// Test the behaviour of F_SETFL/F_GETFL when a fd is blocking. diff --git a/src/tools/miri/tests/pass-dep/libc/libc-socket-with-isolation.rs b/src/tools/miri/tests/pass-dep/libc/libc-socket-with-isolation.rs new file mode 100644 index 0000000000000..ce4df4de72b6d --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-socket-with-isolation.rs @@ -0,0 +1,19 @@ +//@ignore-target: windows # No libc socket on Windows +//@ignore-target: solaris # Socket is a macro for __xnet7_socket which has no shim +//@ignore-target: illumos # Socket is a macro for __xnet7_socket which has no shim +//@compile-flags: -Zmiri-isolation-error=warn-nobacktrace + +use std::io::ErrorKind; + +#[path = "../../utils/libc.rs"] +mod libc_utils; +use libc_utils::*; + +fn main() { + unsafe { + let err = errno_result(libc::socket(libc::AF_INET, libc::SOCK_STREAM, 0)).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PermissionDenied); + // check that it is the right kind of `PermissionDenied` + assert_eq!(err.raw_os_error(), Some(libc::EACCES)); + } +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-socket-with-isolation.stderr b/src/tools/miri/tests/pass-dep/libc/libc-socket-with-isolation.stderr new file mode 100644 index 0000000000000..36fc0a5aac328 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-socket-with-isolation.stderr @@ -0,0 +1,2 @@ +warning: `socket` was made to return an error due to isolation + diff --git a/src/tools/miri/tests/pass-dep/libc/libc-socket.rs b/src/tools/miri/tests/pass-dep/libc/libc-socket.rs new file mode 100644 index 0000000000000..ac9f13367642d --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-socket.rs @@ -0,0 +1,19 @@ +//@ignore-target: windows # No libc socket on Windows +//@ignore-target: solaris # Does socket is a macro for __xnet7_socket which has no shim +//@ignore-target: illumos # Does socket is a macro for __xnet7_socket which has no shim +//@compile-flags: -Zmiri-disable-isolation + +#[path = "../../utils/libc.rs"] +mod libc_utils; +use libc_utils::*; + +fn main() { + test_socket_close(); +} + +fn test_socket_close() { + unsafe { + let sockfd = errno_result(libc::socket(libc::AF_INET, libc::SOCK_STREAM, 0)).unwrap(); + errno_check(libc::close(sockfd)); + } +} diff --git a/src/tools/miri/tests/pass/open_a_file_in_proc.rs b/src/tools/miri/tests/pass/open_a_file_in_proc.rs new file mode 100644 index 0000000000000..8c9887656d384 --- /dev/null +++ b/src/tools/miri/tests/pass/open_a_file_in_proc.rs @@ -0,0 +1,11 @@ +//@compile-flags: -Zmiri-disable-isolation +//@only-target: linux android illumos +//@ignore-host: windows + +fn main() { + let _ = match std::fs::File::open("/proc/doesnotexist ") { + Ok(_f) => {} + Err(_msg) => {} + }; + (); +} diff --git a/src/tools/miri/tests/pass/open_a_file_in_proc.stderr b/src/tools/miri/tests/pass/open_a_file_in_proc.stderr new file mode 100644 index 0000000000000..0ba31ed5adac0 --- /dev/null +++ b/src/tools/miri/tests/pass/open_a_file_in_proc.stderr @@ -0,0 +1,32 @@ +warning: files in `/proc` can bypass the Abstract Machine and might not work properly in Miri + --> RUSTLIB/std/src/sys/fs/PLATFORM.rs:LL:CC + | +LL | let fd = cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ open a file in `/proc` + | + = note: stack backtrace: + 0: std::sys::fs::PLATFORM::File::open_c::{closure#0} + at RUSTLIB/std/src/sys/fs/PLATFORM.rs:LL:CC + 1: std::sys::pal::PLATFORM::cvt_r + at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC + 2: std::sys::fs::PLATFORM::File::open_c + at RUSTLIB/std/src/sys/fs/PLATFORM.rs:LL:CC + 3: std::sys::fs::PLATFORM::File::open::{closure#0} + at RUSTLIB/std/src/sys/fs/PLATFORM.rs:LL:CC + 4: std::sys::helpers::small_c_string::run_with_cstr_stack + at RUSTLIB/std/src/sys/helpers/small_c_string.rs:LL:CC + 5: std::sys::helpers::small_c_string::run_with_cstr + at RUSTLIB/std/src/sys/helpers/small_c_string.rs:LL:CC + 6: std::sys::helpers::small_c_string::run_path_with_cstr + at RUSTLIB/std/src/sys/helpers/small_c_string.rs:LL:CC + 7: std::sys::fs::PLATFORM::File::open + at RUSTLIB/std/src/sys/fs/PLATFORM.rs:LL:CC + 8: std::fs::OpenOptions::_open + at RUSTLIB/std/src/fs.rs:LL:CC + 9: std::fs::OpenOptions::open + at RUSTLIB/std/src/fs.rs:LL:CC + 10: std::fs::File::open + at RUSTLIB/std/src/fs.rs:LL:CC + 11: main + at tests/pass/open_a_file_in_proc.rs:LL:CC + diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 047cdeb357c20..2a6151737d6c0 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -42,7 +42,7 @@ fn build_native_lib(target: &str) -> PathBuf { std::fs::create_dir_all(&so_target_dir) .expect("Failed to create directory for shared object file"); // We use a platform-neutral file extension to avoid having to hard-code alternatives. - let native_lib_path = so_target_dir.join("native-lib.module"); + let native_lib_path = so_target_dir.join("native-lib-tests.so"); let cc_output = Command::new(cc) .args([ "-shared", @@ -58,6 +58,7 @@ fn build_native_lib(target: &str) -> PathBuf { "tests/native-lib/aggregate_arguments.c", "tests/native-lib/ptr_read_access.c", "tests/native-lib/ptr_write_access.c", + "tests/native-lib/fn_ptr.c", // Ensure we notice serious problems in the C code. "-Wall", "-Wextra",