From de909c131474744999e8a1167cfc6e17759300b7 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 15 Jun 2024 17:47:35 +0200 Subject: [PATCH 01/61] std: refactor the TLS implementation As discovered by Mara in #110897, our TLS implementation is a total mess. In the past months, I have simplified the actual macros and their expansions, but the majority of the complexity comes from the platform-specific support code needed to create keys and register destructors. In keeping with #117276, I have therefore moved all of the `thread_local_key`/`thread_local_dtor` modules to the `thread_local` module in `sys` and merged them into a new structure, so that future porters of `std` can simply mix-and-match the existing code instead of having to copy the same (bad) implementation everywhere. The new structure should become obvious when looking at `sys/thread_local/mod.rs`. Unfortunately, the documentation changes associated with the refactoring have made this PR rather large. That said, this contains no functional changes except for two small ones: * the key-based destructor fallback now, by virtue of sharing the implementation used by macOS and others, stores its list in a `#[thread_local]` static instead of in the key, eliminating one indirection layer and drastically simplifying its code. * I've switched over ZKVM (tier 3) to use the same implementation as WebAssembly, as the implementation was just a way worse version of that Please let me know if I can make this easier to review! I know these large PRs aren't optimal, but I couldn't think of any good intermediate steps. @rustbot label +A-thread-locals --- tests/pass-dep/concurrency/tls_pthread_drop_order.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/pass-dep/concurrency/tls_pthread_drop_order.rs b/tests/pass-dep/concurrency/tls_pthread_drop_order.rs index 0eaab96764..52348aad33 100644 --- a/tests/pass-dep/concurrency/tls_pthread_drop_order.rs +++ b/tests/pass-dep/concurrency/tls_pthread_drop_order.rs @@ -1,9 +1,9 @@ //@ignore-target-windows: No pthreads on Windows //! Test that pthread_key destructors are run in the right order. //! Note that these are *not* used by actual `thread_local!` on Linux! Those use -//! `thread_local_dtor::register_dtor` from the stdlib instead. In Miri this hits the fallback path -//! in `register_dtor_fallback`, which uses a *single* pthread_key to manage a thread-local list of -//! dtors to call. +//! `destructors::register` from the stdlib instead. In Miri this ends up hitting +//! the fallback path in `guard::key::enable`, which uses a *single* pthread_key +//! to manage a thread-local list of dtors to call. use std::mem; use std::ptr; From edbc874fd54c86e978a7610a7c4d3ecbf8db69f4 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Mon, 17 Jun 2024 05:02:59 +0000 Subject: [PATCH 02/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index e49680ba75..c1796cfd82 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -f6b4b71ef10307201b52c17b0f9dcf9557cd90ba +e794b0f8557c187b5909d889aa35071f81e0a4cc From 90970e31999466f06d45b4842ddc2392edcf1a32 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Jun 2024 11:41:18 +0200 Subject: [PATCH 03/61] show proper UB when making a too large allocation request --- src/alloc_bytes.rs | 24 ++++++++++++++---------- src/shims/alloc.rs | 12 ------------ src/shims/foreign_items.rs | 24 ++++++++++++++++++++---- tests/fail/alloc/too_large.rs | 10 ++++++++++ tests/fail/alloc/too_large.stderr | 15 +++++++++++++++ 5 files changed, 59 insertions(+), 26 deletions(-) create mode 100644 tests/fail/alloc/too_large.rs create mode 100644 tests/fail/alloc/too_large.stderr diff --git a/src/alloc_bytes.rs b/src/alloc_bytes.rs index 97841a05cd..8757929300 100644 --- a/src/alloc_bytes.rs +++ b/src/alloc_bytes.rs @@ -64,17 +64,19 @@ impl MiriAllocBytes { /// If `size == 0` we allocate using a different `alloc_layout` with `size = 1`, to ensure each allocation has a unique address. /// Returns `Err(alloc_layout)` if the allocation function returns a `ptr` where `ptr.is_null()`. fn alloc_with( - size: usize, - align: usize, + size: u64, + align: u64, alloc_fn: impl FnOnce(Layout) -> *mut u8, - ) -> Result { - let layout = Layout::from_size_align(size, align).unwrap(); + ) -> Result { + let size = usize::try_from(size).map_err(|_| ())?; + let align = usize::try_from(align).map_err(|_| ())?; + let layout = Layout::from_size_align(size, align).map_err(|_| ())?; // When size is 0 we allocate 1 byte anyway, to ensure each allocation has a unique address. let alloc_layout = if size == 0 { Layout::from_size_align(1, align).unwrap() } else { layout }; let ptr = alloc_fn(alloc_layout); if ptr.is_null() { - Err(alloc_layout) + Err(()) } else { // SAFETY: All `MiriAllocBytes` invariants are fulfilled. Ok(Self { ptr, layout }) @@ -86,11 +88,13 @@ impl AllocBytes for MiriAllocBytes { fn from_bytes<'a>(slice: impl Into>, align: Align) -> Self { let slice = slice.into(); let size = slice.len(); - let align = align.bytes_usize(); + let align = align.bytes(); // SAFETY: `alloc_fn` will only be used with `size != 0`. let alloc_fn = |layout| unsafe { alloc::alloc(layout) }; - let alloc_bytes = MiriAllocBytes::alloc_with(size, align, alloc_fn) - .unwrap_or_else(|layout| alloc::handle_alloc_error(layout)); + let alloc_bytes = MiriAllocBytes::alloc_with(size.try_into().unwrap(), align, alloc_fn) + .unwrap_or_else(|()| { + panic!("Miri ran out of memory: cannot create allocation of {size} bytes") + }); // SAFETY: `alloc_bytes.ptr` and `slice.as_ptr()` are non-null, properly aligned // and valid for the `size`-many bytes to be copied. unsafe { alloc_bytes.ptr.copy_from(slice.as_ptr(), size) }; @@ -98,8 +102,8 @@ impl AllocBytes for MiriAllocBytes { } fn zeroed(size: Size, align: Align) -> Option { - let size = size.bytes_usize(); - let align = align.bytes_usize(); + let size = size.bytes(); + let align = align.bytes(); // SAFETY: `alloc_fn` will only be used with `size != 0`. let alloc_fn = |layout| unsafe { alloc::alloc_zeroed(layout) }; MiriAllocBytes::alloc_with(size, align, alloc_fn).ok() diff --git a/src/shims/alloc.rs b/src/shims/alloc.rs index 7b0c54d763..a33657d33a 100644 --- a/src/shims/alloc.rs +++ b/src/shims/alloc.rs @@ -5,18 +5,6 @@ use rustc_target::abi::{Align, Size}; use crate::*; -/// Check some basic requirements for this allocation request: -/// non-zero size, power-of-two alignment. -pub(super) fn check_alloc_request<'tcx>(size: u64, align: u64) -> InterpResult<'tcx> { - if size == 0 { - throw_ub_format!("creating allocation with size 0"); - } - if !align.is_power_of_two() { - throw_ub_format!("creating allocation with non-power-of-two alignment {}", align); - } - Ok(()) -} - impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Returns the alignment that `malloc` would guarantee for requests of the given size. diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 898fc111fd..b8d85a1950 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -12,7 +12,7 @@ use rustc_target::{ spec::abi::Abi, }; -use super::alloc::{check_alloc_request, EvalContextExt as _}; +use super::alloc::EvalContextExt as _; use super::backtrace::EvalContextExt as _; use crate::*; use helpers::{ToHost, ToSoft}; @@ -204,6 +204,22 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { + /// Check some basic requirements for this allocation request: + /// non-zero size, power-of-two alignment. + fn check_rustc_alloc_request(&self, size: u64, align: u64) -> InterpResult<'tcx> { + let this = self.eval_context_ref(); + if size == 0 { + throw_ub_format!("creating allocation with size 0"); + } + if i128::from(size) > this.tcx.data_layout.pointer_size.signed_int_max() { + throw_ub_format!("creating an allocation larger than half the address space"); + } + if !align.is_power_of_two() { + throw_ub_format!("creating allocation with non-power-of-two alignment {}", align); + } + Ok(()) + } + fn emulate_foreign_item_inner( &mut self, link_name: Symbol, @@ -462,7 +478,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let size = this.read_target_usize(size)?; let align = this.read_target_usize(align)?; - check_alloc_request(size, align)?; + this.check_rustc_alloc_request(size, align)?; let memory_kind = match link_name.as_str() { "__rust_alloc" => MiriMemoryKind::Rust, @@ -496,7 +512,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let size = this.read_target_usize(size)?; let align = this.read_target_usize(align)?; - check_alloc_request(size, align)?; + this.check_rustc_alloc_request(size, align)?; let ptr = this.allocate_ptr( Size::from_bytes(size), @@ -560,7 +576,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let new_size = this.read_target_usize(new_size)?; // No need to check old_size; we anyway check that they match the allocation. - check_alloc_request(new_size, align)?; + this.check_rustc_alloc_request(new_size, align)?; let align = Align::from_bytes(align).unwrap(); let new_ptr = this.reallocate_ptr( diff --git a/tests/fail/alloc/too_large.rs b/tests/fail/alloc/too_large.rs new file mode 100644 index 0000000000..4e28d2401d --- /dev/null +++ b/tests/fail/alloc/too_large.rs @@ -0,0 +1,10 @@ +extern "Rust" { + fn __rust_alloc(size: usize, align: usize) -> *mut u8; +} + +fn main() { + let bytes = isize::MAX as usize + 1; + unsafe { + __rust_alloc(bytes, 1); //~ERROR: larger than half the address space + } +} diff --git a/tests/fail/alloc/too_large.stderr b/tests/fail/alloc/too_large.stderr new file mode 100644 index 0000000000..77dcf91d84 --- /dev/null +++ b/tests/fail/alloc/too_large.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: creating an allocation larger than half the address space + --> $DIR/too_large.rs:LL:CC + | +LL | __rust_alloc(bytes, 1); + | ^^^^^^^^^^^^^^^^^^^^^^ creating an allocation larger than half the address space + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at $DIR/too_large.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 + From 79c5027199b0809e7006586726f66c4093a6489b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Jun 2024 12:19:12 +0200 Subject: [PATCH 04/61] tell people how to set miri flags --- cargo-miri/src/phases.rs | 6 ++++ src/diagnostics.rs | 29 +++++++++---------- src/eval.rs | 11 ++----- .../libc_pthread_create_main_terminate.stderr | 2 +- .../libc/aligned_alloc_size_zero_leak.stderr | 2 +- tests/fail-dep/libc/fs/isolated_stdin.stderr | 4 +-- .../libc/malloc_zero_memory_leak.stderr | 2 +- .../libc/posix_memalign_size_zero_leak.stderr | 2 +- .../ptr_metadata_uninit_slice_len.stderr | 7 ++--- tests/fail/memleak.stderr | 2 +- tests/fail/memleak_no_backtrace.rs | 2 +- tests/fail/memleak_no_backtrace.stderr | 4 ++- tests/fail/memleak_rc.64bit.stderr | 25 ---------------- tests/fail/memleak_rc.rs | 2 +- ...leak_rc.32bit.stderr => memleak_rc.stderr} | 4 +-- tests/fail/shims/fs/isolated_file.stderr | 4 +-- tests/fail/tls_macro_leak.stderr | 2 +- tests/fail/tls_static_leak.stderr | 2 +- tests/pass/box.stack.stderr | 7 ++--- tests/pass/extern_types.stack.stderr | 7 ++--- .../stacked-borrows/issue-miri-2389.stderr | 7 ++--- 21 files changed, 52 insertions(+), 81 deletions(-) delete mode 100644 tests/fail/memleak_rc.64bit.stderr rename tests/fail/{memleak_rc.32bit.stderr => memleak_rc.stderr} (86%) diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index 3c36f606d8..8d48b9c8ad 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -23,6 +23,12 @@ Subcommands: clean Clean the Miri cache & target directory The cargo options are exactly the same as for `cargo run` and `cargo test`, respectively. +Furthermore, the following extra flags and environment variables are recognized for `run` and `test`: + + --many-seeds[=from..to] Run the program/tests many times with different seeds in the given range. + The range defaults to `0..64`. + + MIRIFLAGS Extra flags to pass to the Miri driver. Use this to pass `-Zmiri-...` flags. Examples: cargo miri run diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 14e29aa423..47f0913acc 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -227,8 +227,8 @@ pub fn report_error<'tcx>( let helps = match info { UnsupportedInIsolation(_) => vec![ - (None, format!("pass the flag `-Zmiri-disable-isolation` to disable isolation;")), - (None, format!("or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning")), + (None, format!("set `MIRIFLAGS=-Zmiri-disable-isolation` to disable isolation;")), + (None, format!("or set `MIRIFLAGS=-Zmiri-isolation-error=warn` to make Miri return an error code from isolated operations (if supported for that operation) and continue with a warning")), ], UnsupportedForeignItem(_) => { vec![ @@ -463,19 +463,22 @@ pub fn report_leaks<'tcx>( ) { let mut any_pruned = false; for (id, kind, mut alloc) in leaks { + let mut title = format!( + "memory leaked: {id:?} ({}, size: {:?}, align: {:?})", + kind, + alloc.size().bytes(), + alloc.align.bytes() + ); let Some(backtrace) = alloc.extra.backtrace.take() else { + ecx.tcx.dcx().err(title); continue; }; + title.push_str(", allocated here:"); let (backtrace, pruned) = prune_stacktrace(backtrace, &ecx.machine); any_pruned |= pruned; report_msg( DiagLevel::Error, - format!( - "memory leaked: {id:?} ({}, size: {:?}, align: {:?}), allocated here:", - kind, - alloc.size().bytes(), - alloc.align.bytes() - ), + title, vec![], vec![], vec![], @@ -642,13 +645,9 @@ impl<'tcx> MiriMachine<'tcx> { ( None, format!( - "This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`," + "This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program." ), ), - ( - None, - format!("which means that Miri might miss pointer bugs in this program."), - ), ( None, format!( @@ -664,13 +663,13 @@ impl<'tcx> MiriMachine<'tcx> { ( None, format!( - "You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `with_exposed_provenance` semantics." + "You can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics." ), ), ( None, format!( - "Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning." + "Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning." ), ), ], diff --git a/src/eval.rs b/src/eval.rs index 35f7f43f12..bd11439971 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -468,7 +468,7 @@ pub fn eval_entry<'tcx>( // Check for thread leaks. if !ecx.have_all_terminated() { tcx.dcx().err("the main thread terminated without waiting for all remaining threads"); - tcx.dcx().note("pass `-Zmiri-ignore-leaks` to disable this check"); + tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); return None; } // Check for memory leaks. @@ -476,14 +476,7 @@ pub fn eval_entry<'tcx>( let leaks = ecx.find_leaked_allocations(&ecx.machine.static_roots); if !leaks.is_empty() { report_leaks(&ecx, leaks); - let leak_message = "the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check"; - if ecx.machine.collect_leak_backtraces { - // If we are collecting leak backtraces, each leak is a distinct error diagnostic. - tcx.dcx().note(leak_message); - } else { - // If we do not have backtraces, we just report an error without any span. - tcx.dcx().err(leak_message); - }; + tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); // Ignore the provided return code - let the reported error // determine the return code. return None; diff --git a/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr b/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr index 078b7d2e0d..9d6be16b83 100644 --- a/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr +++ b/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr @@ -1,6 +1,6 @@ error: the main thread terminated without waiting for all remaining threads -note: pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr b/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr index b0756d5721..91c6782332 100644 --- a/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr +++ b/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr @@ -9,7 +9,7 @@ LL | aligned_alloc(2, 0); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail-dep/libc/fs/isolated_stdin.stderr b/tests/fail-dep/libc/fs/isolated_stdin.stderr index 1d6626dda7..9abe145ea9 100644 --- a/tests/fail-dep/libc/fs/isolated_stdin.stderr +++ b/tests/fail-dep/libc/fs/isolated_stdin.stderr @@ -4,8 +4,8 @@ error: unsupported operation: `read` from stdin not available when isolation is LL | libc::read(0, bytes.as_mut_ptr() as *mut libc::c_void, 512); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `read` from stdin not available when isolation is enabled | - = help: pass the flag `-Zmiri-disable-isolation` to disable isolation; - = help: or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning + = help: set `MIRIFLAGS=-Zmiri-disable-isolation` to disable isolation; + = help: or set `MIRIFLAGS=-Zmiri-isolation-error=warn` to make Miri return an error code from isolated operations (if supported for that operation) and continue with a warning = note: BACKTRACE: = note: inside `main` at $DIR/isolated_stdin.rs:LL:CC diff --git a/tests/fail-dep/libc/malloc_zero_memory_leak.stderr b/tests/fail-dep/libc/malloc_zero_memory_leak.stderr index 65ce0dcdcd..657262b8d4 100644 --- a/tests/fail-dep/libc/malloc_zero_memory_leak.stderr +++ b/tests/fail-dep/libc/malloc_zero_memory_leak.stderr @@ -9,7 +9,7 @@ LL | let _ptr = libc::malloc(0); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr b/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr index 7ea0fa3146..2639031f1c 100644 --- a/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr +++ b/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr @@ -9,7 +9,7 @@ LL | let _ = unsafe { libc::posix_memalign(&mut ptr, align, size) }; note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr b/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr index 4e2e721843..217bc82010 100644 --- a/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr +++ b/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr @@ -4,12 +4,11 @@ warning: integer-to-pointer cast LL | (*p.as_mut_ptr().cast::<[*const i32; 2]>())[0] = 4 as *const i32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, - = help: which means that Miri might miss pointer bugs in this program. + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation. = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. - = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = help: You can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. + = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. = note: BACKTRACE: = note: inside `main` at $DIR/ptr_metadata_uninit_slice_len.rs:LL:CC diff --git a/tests/fail/memleak.stderr b/tests/fail/memleak.stderr index 8ba78ef664..a9ee76fbe8 100644 --- a/tests/fail/memleak.stderr +++ b/tests/fail/memleak.stderr @@ -18,7 +18,7 @@ LL | std::mem::forget(Box::new(42)); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail/memleak_no_backtrace.rs b/tests/fail/memleak_no_backtrace.rs index a1f8d9957f..1f8d8ba7a7 100644 --- a/tests/fail/memleak_no_backtrace.rs +++ b/tests/fail/memleak_no_backtrace.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-disable-leak-backtraces -//@error-in-other-file: the evaluated program leaked memory +//@error-in-other-file: memory leaked //@normalize-stderr-test: ".*│.*" -> "$$stripped$$" fn main() { diff --git a/tests/fail/memleak_no_backtrace.stderr b/tests/fail/memleak_no_backtrace.stderr index 22e8c55806..6850928e15 100644 --- a/tests/fail/memleak_no_backtrace.stderr +++ b/tests/fail/memleak_no_backtrace.stderr @@ -1,4 +1,6 @@ -error: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +error: memory leaked: ALLOC (Rust heap, size: 4, align: 4) + +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail/memleak_rc.64bit.stderr b/tests/fail/memleak_rc.64bit.stderr deleted file mode 100644 index 1c85a0f9d9..0000000000 --- a/tests/fail/memleak_rc.64bit.stderr +++ /dev/null @@ -1,25 +0,0 @@ -error: memory leaked: ALLOC (Rust heap, size: 32, align: 8), allocated here: - --> RUSTLIB/alloc/src/alloc.rs:LL:CC - | -LL | __rust_alloc(layout.size(), layout.align()) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: BACKTRACE: - = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC - = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC - = note: inside `::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC - = note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC - = note: inside `std::boxed::Box::>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC - = note: inside `std::rc::Rc::>>::new` at RUSTLIB/alloc/src/rc.rs:LL:CC -note: inside `main` - --> $DIR/memleak_rc.rs:LL:CC - | -LL | let x = Dummy(Rc::new(RefCell::new(None))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check - -error: aborting due to 1 previous error - diff --git a/tests/fail/memleak_rc.rs b/tests/fail/memleak_rc.rs index 0927612d08..2d12c1223c 100644 --- a/tests/fail/memleak_rc.rs +++ b/tests/fail/memleak_rc.rs @@ -1,6 +1,6 @@ //@error-in-other-file: memory leaked -//@stderr-per-bitwidth //@normalize-stderr-test: ".*│.*" -> "$$stripped$$" +//@normalize-stderr-test: "Rust heap, size: [0-9]+, align: [0-9]+" -> "Rust heap, SIZE, ALIGN" use std::cell::RefCell; use std::rc::Rc; diff --git a/tests/fail/memleak_rc.32bit.stderr b/tests/fail/memleak_rc.stderr similarity index 86% rename from tests/fail/memleak_rc.32bit.stderr rename to tests/fail/memleak_rc.stderr index 781e1458db..dbf2daf818 100644 --- a/tests/fail/memleak_rc.32bit.stderr +++ b/tests/fail/memleak_rc.stderr @@ -1,4 +1,4 @@ -error: memory leaked: ALLOC (Rust heap, size: 16, align: 4), allocated here: +error: memory leaked: ALLOC (Rust heap, SIZE, ALIGN), allocated here: --> RUSTLIB/alloc/src/alloc.rs:LL:CC | LL | __rust_alloc(layout.size(), layout.align()) @@ -19,7 +19,7 @@ LL | let x = Dummy(Rc::new(RefCell::new(None))); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail/shims/fs/isolated_file.stderr b/tests/fail/shims/fs/isolated_file.stderr index 1f08649428..ec956f8334 100644 --- a/tests/fail/shims/fs/isolated_file.stderr +++ b/tests/fail/shims/fs/isolated_file.stderr @@ -4,8 +4,8 @@ error: unsupported operation: `open` not available when isolation is enabled LL | let fd = cvt_r(|| unsafe { open64(path.as_ptr(), flags, opts.mode as c_int) })?; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `open` not available when isolation is enabled | - = help: pass the flag `-Zmiri-disable-isolation` to disable isolation; - = help: or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning + = help: set `MIRIFLAGS=-Zmiri-disable-isolation` to disable isolation; + = help: or set `MIRIFLAGS=-Zmiri-isolation-error=warn` to make Miri return an error code from isolated operations (if supported for that operation) and continue with a warning = note: BACKTRACE: = note: inside closure at RUSTLIB/std/src/sys/pal/PLATFORM/fs.rs:LL:CC = note: inside `std::sys::pal::PLATFORM::cvt_r::` at RUSTLIB/std/src/sys/pal/PLATFORM/mod.rs:LL:CC diff --git a/tests/fail/tls_macro_leak.stderr b/tests/fail/tls_macro_leak.stderr index 40b21f8625..c7c641a30f 100644 --- a/tests/fail/tls_macro_leak.stderr +++ b/tests/fail/tls_macro_leak.stderr @@ -27,7 +27,7 @@ LL | | }); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/fail/tls_static_leak.stderr b/tests/fail/tls_static_leak.stderr index 580b52c151..f7b90a1118 100644 --- a/tests/fail/tls_static_leak.stderr +++ b/tests/fail/tls_static_leak.stderr @@ -18,7 +18,7 @@ LL | TLS.set(Some(Box::leak(Box::new(123)))); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check +note: set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check error: aborting due to 1 previous error diff --git a/tests/pass/box.stack.stderr b/tests/pass/box.stack.stderr index 1a4d52ee31..341f84c899 100644 --- a/tests/pass/box.stack.stderr +++ b/tests/pass/box.stack.stderr @@ -4,12 +4,11 @@ warning: integer-to-pointer cast LL | let r2 = ((r as usize) + 0) as *mut i32; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, - = help: which means that Miri might miss pointer bugs in this program. + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation. = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. - = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = help: You can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. + = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. = note: BACKTRACE: = note: inside `into_raw` at $DIR/box.rs:LL:CC note: inside `main` diff --git a/tests/pass/extern_types.stack.stderr b/tests/pass/extern_types.stack.stderr index 275d718129..03a9167abb 100644 --- a/tests/pass/extern_types.stack.stderr +++ b/tests/pass/extern_types.stack.stderr @@ -4,12 +4,11 @@ warning: integer-to-pointer cast LL | let x: &Foo = unsafe { &*(16 as *const Foo) }; | ^^^^^^^^^^^^^^^^^^ integer-to-pointer cast | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, - = help: which means that Miri might miss pointer bugs in this program. + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation. = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. - = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = help: You can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. + = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. = note: BACKTRACE: = note: inside `main` at $DIR/extern_types.rs:LL:CC diff --git a/tests/pass/stacked-borrows/issue-miri-2389.stderr b/tests/pass/stacked-borrows/issue-miri-2389.stderr index 7cbfad3942..b0e1adf27d 100644 --- a/tests/pass/stacked-borrows/issue-miri-2389.stderr +++ b/tests/pass/stacked-borrows/issue-miri-2389.stderr @@ -4,12 +4,11 @@ warning: integer-to-pointer cast LL | let wildcard = &root0 as *const Cell as usize as *const Cell; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast | - = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, - = help: which means that Miri might miss pointer bugs in this program. + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`, which means that Miri might miss pointer bugs in this program. = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation. = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. - = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `with_exposed_provenance` semantics. - = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = help: You can then set `MIRIFLAGS=-Zmiri-strict-provenance` to ensure you are not relying on `with_exposed_provenance` semantics. + = help: Alternatively, `MIRIFLAGS=-Zmiri-permissive-provenance` disables this warning. = note: BACKTRACE: = note: inside `main` at $DIR/issue-miri-2389.rs:LL:CC From 3fca11c514a552c582681f6a4897deae238e6267 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 18 Jun 2024 04:56:47 +0000 Subject: [PATCH 05/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index c1796cfd82..356d4767c7 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -e794b0f8557c187b5909d889aa35071f81e0a4cc +c2932aaf9d20acbc9259c762f1a06f8767c6f13f From d865ab3904f4be5ced807e8f8f8d62767ddc0b55 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Wed, 19 Jun 2024 04:54:41 +0000 Subject: [PATCH 06/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 356d4767c7..a2da736656 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -c2932aaf9d20acbc9259c762f1a06f8767c6f13f +a1ca449981e3b8442e358026437b7bedb9a1458e From d388c301c21c41bf578258880c38b20f9ef60c14 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Fri, 25 Aug 2023 13:52:51 +0100 Subject: [PATCH 07/61] Remove c_unwind from tests and fix tests --- tests/fail/function_calls/exported_symbol_bad_unwind1.rs | 2 -- tests/fail/function_calls/exported_symbol_bad_unwind2.rs | 2 +- tests/fail/panic/bad_unwind.rs | 2 -- tests/fail/terminate-terminator.rs | 2 -- tests/fail/unwind-action-terminate.rs | 2 -- tests/panic/function_calls/exported_symbol_good_unwind.rs | 2 +- 6 files changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/fail/function_calls/exported_symbol_bad_unwind1.rs b/tests/fail/function_calls/exported_symbol_bad_unwind1.rs index 6d68b9a46d..dc192d0319 100644 --- a/tests/fail/function_calls/exported_symbol_bad_unwind1.rs +++ b/tests/fail/function_calls/exported_symbol_bad_unwind1.rs @@ -1,5 +1,3 @@ -#![feature(c_unwind)] - #[no_mangle] extern "C-unwind" fn unwind() { panic!(); diff --git a/tests/fail/function_calls/exported_symbol_bad_unwind2.rs b/tests/fail/function_calls/exported_symbol_bad_unwind2.rs index e6aff19b02..1382e9571f 100644 --- a/tests/fail/function_calls/exported_symbol_bad_unwind2.rs +++ b/tests/fail/function_calls/exported_symbol_bad_unwind2.rs @@ -4,7 +4,7 @@ //@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> "" //@normalize-stderr-test: "\n +at [^\n]+" -> "" //@[definition,both]error-in-other-file: aborted execution -#![feature(rustc_attrs, c_unwind)] +#![feature(rustc_attrs)] #[cfg_attr(any(definition, both), rustc_nounwind)] #[no_mangle] diff --git a/tests/fail/panic/bad_unwind.rs b/tests/fail/panic/bad_unwind.rs index 8c8a9f18cd..5370485b2a 100644 --- a/tests/fail/panic/bad_unwind.rs +++ b/tests/fail/panic/bad_unwind.rs @@ -1,5 +1,3 @@ -#![feature(c_unwind)] - //! Unwinding when the caller ABI is "C" (without "-unwind") is UB. // The opposite version (callee does not allow unwinding) is impossible to // even write: MIR validation catches functions that have `UnwindContinue` but diff --git a/tests/fail/terminate-terminator.rs b/tests/fail/terminate-terminator.rs index 7c67282803..465625c757 100644 --- a/tests/fail/terminate-terminator.rs +++ b/tests/fail/terminate-terminator.rs @@ -7,8 +7,6 @@ // Enable MIR inlining to ensure that `TerminatorKind::UnwindTerminate` is generated // instead of just `UnwindAction::Terminate`. -#![feature(c_unwind)] - struct Foo; impl Drop for Foo { diff --git a/tests/fail/unwind-action-terminate.rs b/tests/fail/unwind-action-terminate.rs index 86406872c5..465e07c8db 100644 --- a/tests/fail/unwind-action-terminate.rs +++ b/tests/fail/unwind-action-terminate.rs @@ -3,8 +3,6 @@ //@normalize-stderr-test: "\| +\^+" -> "| ^" //@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> "" //@normalize-stderr-test: "\n +at [^\n]+" -> "" -#![feature(c_unwind)] - extern "C" fn panic_abort() { panic!() } diff --git a/tests/panic/function_calls/exported_symbol_good_unwind.rs b/tests/panic/function_calls/exported_symbol_good_unwind.rs index 71b799a1f1..0e8d45af27 100644 --- a/tests/panic/function_calls/exported_symbol_good_unwind.rs +++ b/tests/panic/function_calls/exported_symbol_good_unwind.rs @@ -2,7 +2,7 @@ // found in this form" errors works without `-C prefer-dynamic` (`panic!` calls foreign function // `__rust_start_panic`). // no-prefer-dynamic -#![feature(c_unwind, unboxed_closures)] +#![feature(unboxed_closures)] use std::panic; From 459eadaf8a1fdb40603410f4935fd0f3e27b7404 Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Sat, 15 Jun 2024 14:43:29 +0200 Subject: [PATCH 08/61] Implement LLVM x86 bmi intrinsics --- src/shims/x86/bmi.rs | 108 +++++++++++ src/shims/x86/mod.rs | 6 + tests/pass/shims/x86/intrinsics-x86-bmi.rs | 216 +++++++++++++++++++++ 3 files changed, 330 insertions(+) create mode 100644 src/shims/x86/bmi.rs create mode 100644 tests/pass/shims/x86/intrinsics-x86-bmi.rs diff --git a/src/shims/x86/bmi.rs b/src/shims/x86/bmi.rs new file mode 100644 index 0000000000..e70757f439 --- /dev/null +++ b/src/shims/x86/bmi.rs @@ -0,0 +1,108 @@ +use rustc_span::Symbol; +use rustc_target::spec::abi::Abi; + +use crate::*; + +impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} +pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + fn emulate_x86_bmi_intrinsic( + &mut self, + link_name: Symbol, + abi: Abi, + args: &[OpTy<'tcx>], + dest: &MPlaceTy<'tcx>, + ) -> InterpResult<'tcx, EmulateItemResult> { + let this = self.eval_context_mut(); + + // Prefix should have already been checked. + let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.bmi.").unwrap(); + + // The intrinsics are suffixed with the bit size of their operands. + let (is_64_bit, unprefixed_name) = if unprefixed_name.ends_with("64") { + (true, unprefixed_name.strip_suffix(".64").unwrap_or("")) + } else { + (false, unprefixed_name.strip_suffix(".32").unwrap_or("")) + }; + + // All intrinsics of the "bmi" namespace belong to the "bmi2" ISA extension. + // The exception is "bextr", which belongs to "bmi1". + let target_feature = if unprefixed_name == "bextr" { "bmi1" } else { "bmi2" }; + this.expect_target_feature_for_intrinsic(link_name, target_feature)?; + + if is_64_bit && this.tcx.sess.target.arch != "x86_64" { + return Ok(EmulateItemResult::NotSupported); + } + + let [left, right] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let left = this.read_scalar(left)?; + let right = this.read_scalar(right)?; + + let left = if is_64_bit { left.to_u64()? } else { u64::from(left.to_u32()?) }; + let right = if is_64_bit { right.to_u64()? } else { u64::from(right.to_u32()?) }; + + let result = match unprefixed_name { + // Extract a contigous range of bits from an unsigned integer. + // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_bextr_u32 + "bextr" => { + let start = u32::try_from(right & 0xff).unwrap(); + let len = u32::try_from((right >> 8) & 0xff).unwrap(); + let shifted = left.checked_shr(start).unwrap_or(0); + // Keep the `len` lowest bits of `shifted`, or all bits if `len` is too big. + if len >= 64 { shifted } else { shifted & 1u64.wrapping_shl(len).wrapping_sub(1) } + } + // Create a copy of an unsigned integer with bits above a certain index cleared. + // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_bzhi_u32 + "bzhi" => { + let index = u32::try_from(right & 0xff).unwrap(); + // Keep the `index` lowest bits of `left`, or all bits if `index` is too big. + if index >= 64 { left } else { left & 1u64.wrapping_shl(index).wrapping_sub(1) } + } + // Extract bit values of an unsigned integer at positions marked by a mask. + // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_pext_u32 + "pext" => { + let mut mask = right; + let mut i = 0u32; + let mut result = 0; + // Iterate over the mask one 1-bit at a time, from + // the least significant bit to the most significant bit. + while mask != 0 { + // Extract the bit marked by the mask's least significant set bit + // and put it at position `i` of the result. + result |= u64::from(left & (1 << mask.trailing_zeros()) != 0) << i; + i = i.wrapping_add(1); + // Clear the least significant set bit. + mask &= mask.wrapping_sub(1); + } + result + } + // Deposit bit values of an unsigned integer to positions marked by a mask. + // https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_pdep_u32 + "pdep" => { + let mut mask = right; + let mut set = left; + let mut result = 0; + // Iterate over the mask one 1-bit at a time, from + // the least significant bit to the most significant bit. + while mask != 0 { + // Put rightmost bit of `set` at the position of the current `mask` bit. + result |= (set & 1) << mask.trailing_zeros(); + // Go to next bit of `set`. + set >>= 1; + // Clear the least significant set bit. + mask &= mask.wrapping_sub(1); + } + result + } + _ => return Ok(EmulateItemResult::NotSupported), + }; + + let result = if is_64_bit { + Scalar::from_u64(result) + } else { + Scalar::from_u32(u32::try_from(result).unwrap()) + }; + this.write_scalar(result, dest)?; + + Ok(EmulateItemResult::NeedsReturn) + } +} diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index b71aec0216..704c45fdd6 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -14,6 +14,7 @@ use helpers::bool_to_simd_element; mod aesni; mod avx; mod avx2; +mod bmi; mod sse; mod sse2; mod sse3; @@ -113,6 +114,11 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { pclmulqdq(this, left, right, imm, dest)?; } + name if name.starts_with("bmi.") => { + return bmi::EvalContextExt::emulate_x86_bmi_intrinsic( + this, link_name, abi, args, dest, + ); + } name if name.starts_with("sse.") => { return sse::EvalContextExt::emulate_x86_sse_intrinsic( this, link_name, abi, args, dest, diff --git a/tests/pass/shims/x86/intrinsics-x86-bmi.rs b/tests/pass/shims/x86/intrinsics-x86-bmi.rs new file mode 100644 index 0000000000..33424117c4 --- /dev/null +++ b/tests/pass/shims/x86/intrinsics-x86-bmi.rs @@ -0,0 +1,216 @@ +// Ignore everything except x86 and x86_64 +// Any new targets that are added to CI should be ignored here. +// (We cannot use `cfg`-based tricks here since the `target-feature` flags below only work on x86.) +//@ignore-target-aarch64 +//@ignore-target-arm +//@ignore-target-avr +//@ignore-target-s390x +//@ignore-target-thumbv7em +//@ignore-target-wasm32 +//@compile-flags: -C target-feature=+bmi1,+bmi2 + +#[cfg(target_arch = "x86")] +use std::arch::x86::*; +#[cfg(target_arch = "x86_64")] +use std::arch::x86_64::*; + +fn main() { + // BMI1 and BMI2 are independent from each other, so both must be checked. + assert!(is_x86_feature_detected!("bmi1")); + assert!(is_x86_feature_detected!("bmi2")); + + unsafe { + test_bmi_32(); + test_bmi_64(); + } +} + +/// Test the 32-bit variants of the intrinsics. +unsafe fn test_bmi_32() { + unsafe fn test_bextr_u32() { + let r = _bextr_u32(0b0101_0000u32, 4, 4); + assert_eq!(r, 0b0000_0101u32); + + for i in 0..16 { + assert_eq!(_bextr_u32(u32::MAX, i, 4), 0b1111); + assert_eq!(_bextr_u32(u32::MAX, 4, i), (1 << i) - 1); + } + + // Ensure that indices larger than the bit count are covered. + // It is important to go above 32 in order to verify the bit selection + // of the instruction. + + for i in 0..256 { + // If the index is out of bounds, the original input won't be changed, thus the `min(32)`. + assert_eq!(_bextr_u32(u32::MAX, 0, i).count_ones(), i.min(32)); + } + + for i in 0..256 { + assert_eq!(_bextr_u32(u32::MAX, i, 0), 0); + } + + // Test cases with completly random values. These cases also test + // that the function works even if upper bits of the control value are set. + assert_eq!(_bextr2_u32(0x7408a392, 0x54ef705), 0x3a0451c); + assert_eq!(_bextr2_u32(0xbc5a3494, 0xdd193203), 0x178b4692); + assert_eq!(_bextr2_u32(0xc0332325, 0xf96e207), 0x1806646); + } + test_bextr_u32(); + + unsafe fn test_pext_u32() { + let n = 0b1011_1110_1001_0011u32; + + let m0 = 0b0110_0011_1000_0101u32; + let s0 = 0b0000_0000_0011_0101u32; + + let m1 = 0b1110_1011_1110_1111u32; + let s1 = 0b0001_0111_0100_0011u32; + + // Testing of random values. + assert_eq!(_pext_u32(n, m0), s0); + assert_eq!(_pext_u32(n, m1), s1); + assert_eq!(_pext_u32(0x12345678, 0xff00fff0), 0x00012567); + + // Testing of various identities. + assert_eq!(_pext_u32(u32::MAX, u32::MAX), u32::MAX); + assert_eq!(_pext_u32(u32::MAX, 0), 0); + assert_eq!(_pext_u32(0, u32::MAX), 0); + } + test_pext_u32(); + + unsafe fn test_pdep_u32() { + let n = 0b1011_1110_1001_0011u32; + + let m0 = 0b0110_0011_1000_0101u32; + let s0 = 0b0000_0010_0000_0101u32; + + let m1 = 0b1110_1011_1110_1111u32; + let s1 = 0b1110_1001_0010_0011u32; + + // Testing of random values. + assert_eq!(_pdep_u32(n, m0), s0); + assert_eq!(_pdep_u32(n, m1), s1); + assert_eq!(_pdep_u32(0x00012567, 0xff00fff0), 0x12005670); + + // Testing of various identities. + assert_eq!(_pdep_u32(u32::MAX, u32::MAX), u32::MAX); + assert_eq!(_pdep_u32(0, u32::MAX), 0); + assert_eq!(_pdep_u32(u32::MAX, 0), 0); + } + test_pdep_u32(); + + unsafe fn test_bzhi_u32() { + let n = 0b1111_0010u32; + let s = 0b0001_0010u32; + assert_eq!(_bzhi_u32(n, 5), s); + + // Ensure that indices larger than the bit count are covered. + // It is important to go above 32 in order to verify the bit selection + // of the instruction. + for i in 0..=512 { + // The instruction only takes the lowest eight bits to generate the index, hence `i & 0xff`. + // If the index is out of bounds, the original input won't be changed, thus the `min(32)`. + let expected = 1u32.checked_shl((i & 0xff).min(32)).unwrap_or(0).wrapping_sub(1); + let actual = _bzhi_u32(u32::MAX, i); + assert_eq!(expected, actual); + } + } + test_bzhi_u32(); +} + +#[cfg(not(target_arch = "x86_64"))] +unsafe fn test_bmi_64() {} + +/// Test the 64-bit variants of the intrinsics. +#[cfg(target_arch = "x86_64")] +unsafe fn test_bmi_64() { + unsafe fn test_bextr_u64() { + let r = _bextr_u64(0b0101_0000u64, 4, 4); + assert_eq!(r, 0b0000_0101u64); + + for i in 0..16 { + assert_eq!(_bextr_u64(u64::MAX, i, 4), 0b1111); + assert_eq!(_bextr_u64(u64::MAX, 32, i), (1 << i) - 1); + } + + // Ensure that indices larger than the bit count are covered. + // It is important to go above 64 in order to verify the bit selection + // of the instruction. + + for i in 0..256 { + // If the index is out of bounds, the original input won't be changed, thus the `min(64)`. + assert_eq!(_bextr_u64(u64::MAX, 0, i).count_ones(), i.min(64)); + } + + for i in 0..256 { + assert_eq!(_bextr_u64(u64::MAX, i, 0), 0); + } + + // Test cases with completly random values. These cases also test + // that the function works even if upper bits of the control value are set. + assert_eq!(_bextr2_u64(0x4ff6cfbcea75f055, 0x216642e228425719), 0x27fb67de75); + assert_eq!(_bextr2_u64(0xb05e991e6f6e1b6, 0xc76dd5d7f67dfc14), 0xb05e991e6f); + assert_eq!(_bextr2_u64(0x5a3a629e323d848f, 0x95ac507d20e7719), 0x2d1d314f19); + } + test_bextr_u64(); + + unsafe fn test_pext_u64() { + let n = 0b1011_1110_1001_0011u64; + + let m0 = 0b0110_0011_1000_0101u64; + let s0 = 0b0000_0000_0011_0101u64; + + let m1 = 0b1110_1011_1110_1111u64; + let s1 = 0b0001_0111_0100_0011u64; + + // Testing of random values. + assert_eq!(_pext_u64(n, m0), s0); + assert_eq!(_pext_u64(n, m1), s1); + assert_eq!(_pext_u64(0x12345678, 0xff00fff0), 0x00012567); + + // Testing of various identities. + assert_eq!(_pext_u64(u64::MAX, u64::MAX), u64::MAX); + assert_eq!(_pext_u64(u64::MAX, 0), 0); + assert_eq!(_pext_u64(0, u64::MAX), 0); + } + test_pext_u64(); + + unsafe fn test_pdep_u64() { + let n = 0b1011_1110_1001_0011u64; + + let m0 = 0b0110_0011_1000_0101u64; + let s0 = 0b0000_0010_0000_0101u64; + + let m1 = 0b1110_1011_1110_1111u64; + let s1 = 0b1110_1001_0010_0011u64; + + // Testing of random values. + assert_eq!(_pdep_u64(n, m0), s0); + assert_eq!(_pdep_u64(n, m1), s1); + assert_eq!(_pdep_u64(0x00012567, 0xff00fff0), 0x12005670); + + // Testing of various identities. + assert_eq!(_pdep_u64(u64::MAX, u64::MAX), u64::MAX); + assert_eq!(_pdep_u64(0, u64::MAX), 0); + assert_eq!(_pdep_u64(u64::MAX, 0), 0); + } + test_pdep_u64(); + + unsafe fn test_bzhi_u64() { + let n = 0b1111_0010u64; + let s = 0b0001_0010u64; + assert_eq!(_bzhi_u64(n, 5), s); + + // Ensure that indices larger than the bit count are covered. + // It is important to go above 255 in order to verify the bit selection + // of the instruction. + for i in 0..=512 { + // The instruction only takes the lowest eight bits to generate the index, hence `i & 0xff`. + // If the index is out of bounds, the original input won't be changed, thus the `min(64)`. + let expected = 1u64.checked_shl((i & 0xff).min(64)).unwrap_or(0).wrapping_sub(1); + let actual = _bzhi_u64(u64::MAX, i); + assert_eq!(expected, actual); + } + } + test_bzhi_u64(); +} From c9c887b7170ef3fdc608ae7f876dac7782e92cf0 Mon Sep 17 00:00:00 2001 From: Adwin White Date: Thu, 20 Jun 2024 17:21:19 +0800 Subject: [PATCH 09/61] Fix ICE caused by seeking past `i64::MAX` --- src/shims/unix/fs.rs | 9 ++++++++- tests/pass/issues/issue-miri-3680.rs | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 tests/pass/issues/issue-miri-3680.rs diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index 262e71756c..e34aa5c09d 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -395,7 +395,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Isolation check is done via `FileDescriptor` trait. let seek_from = if whence == this.eval_libc_i32("SEEK_SET") { - SeekFrom::Start(u64::try_from(offset).unwrap()) + if offset < 0 { + // Negative offsets return `EINVAL`. + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + return Ok(Scalar::from_i64(-1)); + } else { + SeekFrom::Start(u64::try_from(offset).unwrap()) + } } else if whence == this.eval_libc_i32("SEEK_CUR") { SeekFrom::Current(i64::try_from(offset).unwrap()) } else if whence == this.eval_libc_i32("SEEK_END") { diff --git a/tests/pass/issues/issue-miri-3680.rs b/tests/pass/issues/issue-miri-3680.rs new file mode 100644 index 0000000000..55b896c91a --- /dev/null +++ b/tests/pass/issues/issue-miri-3680.rs @@ -0,0 +1,21 @@ +//@ignore-target-windows: File handling is not implemented yet +//@compile-flags: -Zmiri-disable-isolation + +use std::fs::remove_file; +use std::io::{ErrorKind, Seek}; + +#[path = "../../utils/mod.rs"] +mod utils; + +fn main() { + let path = utils::prepare("miri_test_fs_seek_i64_max_plus_1.txt"); + + let mut f = std::fs::File::create(&path).unwrap(); + let error = f.seek(std::io::SeekFrom::Start(i64::MAX as u64 + 1)).unwrap_err(); + + // It should be error due to negative offset. + assert_eq!(error.kind(), ErrorKind::InvalidInput); + + // Cleanup + remove_file(&path).unwrap(); +} From a4b9cea7a8d5be68f61f69b1be12a7880b136622 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 21 Jun 2024 05:07:19 +0000 Subject: [PATCH 10/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index a2da736656..1502fa120b 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -a1ca449981e3b8442e358026437b7bedb9a1458e +7a08f84627ff3035de4d66ff3209e5fc93165dcb From d0a415928a72503f7e81dad0b122eead1de4e3f4 Mon Sep 17 00:00:00 2001 From: Southball <6523469+southball@users.noreply.github.com> Date: Fri, 21 Jun 2024 14:22:51 +0900 Subject: [PATCH 11/61] Use strict ops instead of checked ops --- src/alloc_addresses/mod.rs | 2 +- src/eval.rs | 2 +- src/helpers.rs | 6 ++--- src/shims/foreign_items.rs | 4 +-- src/shims/time.rs | 2 +- src/shims/unix/env.rs | 6 ++--- src/shims/unix/fd.rs | 2 +- src/shims/unix/socket.rs | 2 +- src/shims/windows/foreign_items.rs | 4 +-- src/shims/windows/handle.rs | 4 +-- src/shims/x86/avx2.rs | 29 ++++++++++----------- src/shims/x86/mod.rs | 41 +++++++++++++----------------- src/shims/x86/sse2.rs | 14 +++++----- src/shims/x86/ssse3.rs | 8 +++--- 14 files changed, 58 insertions(+), 68 deletions(-) diff --git a/src/alloc_addresses/mod.rs b/src/alloc_addresses/mod.rs index ae95d28d3e..d0f977f814 100644 --- a/src/alloc_addresses/mod.rs +++ b/src/alloc_addresses/mod.rs @@ -97,7 +97,7 @@ impl GlobalStateInner { fn align_addr(addr: u64, align: u64) -> u64 { match addr % align { 0 => addr, - rem => addr.checked_add(align).unwrap() - rem, + rem => addr.strict_add(align) - rem, } } diff --git a/src/eval.rs b/src/eval.rs index bd11439971..c0827cce26 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -303,7 +303,7 @@ pub fn create_ecx<'tcx>( let mut argvs = Vec::>::with_capacity(config.args.len()); for arg in config.args.iter() { // Make space for `0` terminator. - let size = u64::try_from(arg.len()).unwrap().checked_add(1).unwrap(); + let size = u64::try_from(arg.len()).unwrap().strict_add(1); let arg_type = Ty::new_array(tcx, tcx.types.u8, size); let arg_place = ecx.allocate(ecx.layout_of(arg_type)?, MiriMemoryKind::Machine.into())?; diff --git a/src/helpers.rs b/src/helpers.rs index 843aff0249..15aff010e0 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -963,7 +963,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required null // terminator to memory using the `ptr` pointer would cause an out-of-bounds access. let string_length = u64::try_from(c_str.len()).unwrap(); - let string_length = string_length.checked_add(1).unwrap(); + let string_length = string_length.strict_add(1); if size < string_length { return Ok((false, string_length)); } @@ -1027,7 +1027,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // If `size` is smaller or equal than `bytes.len()`, writing `bytes` plus the required // 0x0000 terminator to memory would cause an out-of-bounds access. let string_length = u64::try_from(wide_str.len()).unwrap(); - let string_length = string_length.checked_add(1).unwrap(); + let string_length = string_length.strict_add(1); if size < string_length { return Ok((false, string_length)); } @@ -1391,7 +1391,7 @@ pub(crate) fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 { if success { // If the function succeeds, the return value is the number of characters stored in the target buffer, // not including the terminating null character. - u32::try_from(len.checked_sub(1).unwrap()).unwrap() + u32::try_from(len.strict_sub(1)).unwrap() } else { // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters, // required to hold the string and its terminating null character. diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 5a293344cc..f9ccc6ad4d 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -402,7 +402,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { }); let (_, addr) = ptr.into_parts(); // we know the offset is absolute // Cannot panic since `align` is a power of 2 and hence non-zero. - if addr.bytes().checked_rem(align.bytes()).unwrap() != 0 { + if addr.bytes().strict_rem(align.bytes()) != 0 { throw_unsup_format!( "`miri_promise_symbolic_alignment`: pointer is not actually aligned" ); @@ -714,7 +714,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // That is probably overly cautious, but there also is no fundamental // reason to have `strcpy` destroy pointer provenance. // This reads at least 1 byte, so we are already enforcing that this is a valid pointer. - let n = this.read_c_str(ptr_src)?.len().checked_add(1).unwrap(); + let n = this.read_c_str(ptr_src)?.len().strict_add(1); this.mem_copy(ptr_src, ptr_dest, Size::from_bytes(n), true)?; this.write_pointer(ptr_dest, dest)?; } diff --git a/src/shims/time.rs b/src/shims/time.rs index ae17196f0b..e8f906d37e 100644 --- a/src/shims/time.rs +++ b/src/shims/time.rs @@ -165,7 +165,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ("tm_hour", dt.hour().into()), ("tm_mday", dt.day().into()), ("tm_mon", dt.month0().into()), - ("tm_year", dt.year().checked_sub(1900).unwrap().into()), + ("tm_year", dt.year().strict_sub(1900).into()), ("tm_wday", dt.weekday().num_days_from_sunday().into()), ("tm_yday", dt.ordinal0().into()), ("tm_isdst", tm_isdst), diff --git a/src/shims/unix/env.rs b/src/shims/unix/env.rs index 2f78d0f429..405431f432 100644 --- a/src/shims/unix/env.rs +++ b/src/shims/unix/env.rs @@ -81,10 +81,8 @@ impl<'tcx> UnixEnvVars<'tcx> { return Ok(None); }; // The offset is used to strip the "{name}=" part of the string. - let var_ptr = var_ptr.offset( - Size::from_bytes(u64::try_from(name.len()).unwrap().checked_add(1).unwrap()), - ecx, - )?; + let var_ptr = var_ptr + .offset(Size::from_bytes(u64::try_from(name.len()).unwrap().strict_add(1)), ecx)?; Ok(Some(var_ptr)) } diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index b6ac841dc9..599f78e712 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -240,7 +240,7 @@ impl FdTable { let new_fd = candidate_new_fd.unwrap_or_else(|| { // find_map ran out of BTreeMap entries before finding a free fd, use one plus the // maximum fd in the map - self.fds.last_key_value().map(|(fd, _)| fd.checked_add(1).unwrap()).unwrap_or(min_fd) + self.fds.last_key_value().map(|(fd, _)| fd.strict_add(1)).unwrap_or(min_fd) }); self.fds.try_insert(new_fd, file_handle).unwrap(); diff --git a/src/shims/unix/socket.rs b/src/shims/unix/socket.rs index c639ea2f84..6d3d63b4ef 100644 --- a/src/shims/unix/socket.rs +++ b/src/shims/unix/socket.rs @@ -116,7 +116,7 @@ impl FileDescription for SocketPair { }; let mut writebuf = writebuf.borrow_mut(); let data_size = writebuf.buf.len(); - let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.checked_sub(data_size).unwrap(); + let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(data_size); if available_space == 0 { if self.is_nonblock { // Non-blocking socketpair with a full buffer. diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index bfa14bcb5f..a840366977 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -647,7 +647,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // If the function succeeds, the return value is the length of the string that // is copied to the buffer, in characters, not including the terminating null // character. - this.write_int(size_needed.checked_sub(1).unwrap(), dest)?; + this.write_int(size_needed.strict_sub(1), dest)?; } else { // If the buffer is too small to hold the module name, the string is truncated // to nSize characters including the terminating null character, the function @@ -689,7 +689,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("FormatMessageW: buffer not big enough"); } // The return value is the number of characters stored *excluding* the null terminator. - this.write_int(length.checked_sub(1).unwrap(), dest)?; + this.write_int(length.strict_sub(1), dest)?; } // Incomplete shims that we "stub out" just to get pre-main initialization code to work. diff --git a/src/shims/windows/handle.rs b/src/shims/windows/handle.rs index 58c8683ff2..ec461a4cd3 100644 --- a/src/shims/windows/handle.rs +++ b/src/shims/windows/handle.rs @@ -74,7 +74,7 @@ impl Handle { /// None of this layout is guaranteed to applications by Windows or Miri. fn to_packed(self) -> u32 { let disc_size = Self::packed_disc_size(); - let data_size = u32::BITS.checked_sub(disc_size).unwrap(); + let data_size = u32::BITS.strict_sub(disc_size); let discriminant = self.discriminant(); let data = self.data(); @@ -103,7 +103,7 @@ impl Handle { /// see docs for `to_packed` fn from_packed(handle: u32) -> Option { let disc_size = Self::packed_disc_size(); - let data_size = u32::BITS.checked_sub(disc_size).unwrap(); + let data_size = u32::BITS.strict_sub(disc_size); // the lower `data_size` bits of this mask are 1 #[allow(clippy::arithmetic_side_effects)] // cannot overflow diff --git a/src/shims/x86/avx2.rs b/src/shims/x86/avx2.rs index 016c525e57..efb0ed38fb 100644 --- a/src/shims/x86/avx2.rs +++ b/src/shims/x86/avx2.rs @@ -75,7 +75,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(dest_len, mask_len); let mask_item_size = mask.layout.field(this, 0).size; - let high_bit_offset = mask_item_size.bits().checked_sub(1).unwrap(); + let high_bit_offset = mask_item_size.bits().strict_sub(1); let scale = this.read_scalar(scale)?.to_i8()?; if !matches!(scale, 1 | 2 | 4 | 8) { @@ -93,8 +93,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let offset = i64::try_from(this.read_scalar(&offset)?.to_int(offset.layout.size)?) .unwrap(); - let ptr = slice - .wrapping_signed_offset(offset.checked_mul(scale).unwrap(), &this.tcx); + let ptr = slice.wrapping_signed_offset(offset.strict_mul(scale), &this.tcx); // Unaligned copy, which is what we want. this.mem_copy( ptr, @@ -127,19 +126,19 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); for i in 0..dest_len { - let j1 = i.checked_mul(2).unwrap(); + let j1 = i.strict_mul(2); let left1 = this.read_scalar(&this.project_index(&left, j1)?)?.to_i16()?; let right1 = this.read_scalar(&this.project_index(&right, j1)?)?.to_i16()?; - let j2 = j1.checked_add(1).unwrap(); + let j2 = j1.strict_add(1); let left2 = this.read_scalar(&this.project_index(&left, j2)?)?.to_i16()?; let right2 = this.read_scalar(&this.project_index(&right, j2)?)?.to_i16()?; let dest = this.project_index(&dest, i)?; // Multiplications are i16*i16->i32, which will not overflow. - let mul1 = i32::from(left1).checked_mul(right1.into()).unwrap(); - let mul2 = i32::from(left2).checked_mul(right2.into()).unwrap(); + let mul1 = i32::from(left1).strict_mul(right1.into()); + let mul2 = i32::from(left2).strict_mul(right2.into()); // However, this addition can overflow in the most extreme case // (-0x8000)*(-0x8000)+(-0x8000)*(-0x8000) = 0x80000000 let res = mul1.wrapping_add(mul2); @@ -164,19 +163,19 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); for i in 0..dest_len { - let j1 = i.checked_mul(2).unwrap(); + let j1 = i.strict_mul(2); let left1 = this.read_scalar(&this.project_index(&left, j1)?)?.to_u8()?; let right1 = this.read_scalar(&this.project_index(&right, j1)?)?.to_i8()?; - let j2 = j1.checked_add(1).unwrap(); + let j2 = j1.strict_add(1); let left2 = this.read_scalar(&this.project_index(&left, j2)?)?.to_u8()?; let right2 = this.read_scalar(&this.project_index(&right, j2)?)?.to_i8()?; let dest = this.project_index(&dest, i)?; // Multiplication of a u8 and an i8 into an i16 cannot overflow. - let mul1 = i16::from(left1).checked_mul(right1.into()).unwrap(); - let mul2 = i16::from(left2).checked_mul(right2.into()).unwrap(); + let mul1 = i16::from(left1).strict_mul(right1.into()); + let mul2 = i16::from(left2).strict_mul(right2.into()); let res = mul1.saturating_add(mul2); this.write_scalar(Scalar::from_i16(res), &dest)?; @@ -309,7 +308,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { for i in 0..2 { let dest = this.project_index(&dest, i)?; - let src = match (imm >> i.checked_mul(4).unwrap()) & 0b11 { + let src = match (imm >> i.strict_mul(4)) & 0b11 { 0 => this.project_index(&left, 0)?, 1 => this.project_index(&left, 1)?, 2 => this.project_index(&right, 0)?, @@ -343,7 +342,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mut acc: u16 = 0; for j in 0..8 { - let src_index = i.checked_mul(8).unwrap().checked_add(j).unwrap(); + let src_index = i.strict_mul(8).strict_add(j); let left = this.project_index(&left, src_index)?; let left = this.read_scalar(&left)?.to_u8()?; @@ -351,7 +350,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let right = this.project_index(&right, src_index)?; let right = this.read_scalar(&right)?.to_u8()?; - acc = acc.checked_add(left.abs_diff(right).into()).unwrap(); + acc = acc.strict_add(left.abs_diff(right).into()); } this.write_scalar(Scalar::from_u64(acc.into()), &dest)?; @@ -377,7 +376,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let res = if right & 0x80 == 0 { // Shuffle each 128-bit (16-byte) block independently. - let j = u64::from(right % 16).checked_add(i & !15).unwrap(); + let j = u64::from(right % 16).strict_add(i & !15); this.read_scalar(&this.project_index(&left, j)?)? } else { // If the highest bit in `right` is 1, write zero. diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 704c45fdd6..5db6d211a5 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -441,8 +441,7 @@ fn apply_random_float_error( ) -> F { let rng = this.machine.rng.get_mut(); // generates rand(0, 2^64) * 2^(scale - 64) = rand(0, 1) * 2^scale - let err = - F::from_u128(rng.gen::().into()).value.scalbn(err_scale.checked_sub(64).unwrap()); + let err = F::from_u128(rng.gen::().into()).value.scalbn(err_scale.strict_sub(64)); // give it a random sign let err = if rng.gen::() { -err } else { err }; // multiple the value with (1+err) @@ -793,7 +792,7 @@ fn split_simd_to_128bit_chunks<'tcx, P: Projectable<'tcx, Provenance>>( assert_eq!(simd_layout.size.bits() % 128, 0); let num_chunks = simd_layout.size.bits() / 128; - let items_per_chunk = simd_len.checked_div(num_chunks).unwrap(); + let items_per_chunk = simd_len.strict_div(num_chunks); // Transmute to `[[T; items_per_chunk]; num_chunks]` let chunked_layout = this @@ -841,13 +840,11 @@ fn horizontal_bin_op<'tcx>( for j in 0..items_per_chunk { // `j` is the index in `dest` // `k` is the index of the 2-item chunk in `src` - let (k, src) = - if j < middle { (j, &left) } else { (j.checked_sub(middle).unwrap(), &right) }; + let (k, src) = if j < middle { (j, &left) } else { (j.strict_sub(middle), &right) }; // `base_i` is the index of the first item of the 2-item chunk in `src` - let base_i = k.checked_mul(2).unwrap(); + let base_i = k.strict_mul(2); let lhs = this.read_immediate(&this.project_index(src, base_i)?)?; - let rhs = - this.read_immediate(&this.project_index(src, base_i.checked_add(1).unwrap())?)?; + let rhs = this.read_immediate(&this.project_index(src, base_i.strict_add(1))?)?; let res = if saturating { Immediate::from(this.saturating_arith(which, &lhs, &rhs)?) @@ -900,7 +897,7 @@ fn conditional_dot_product<'tcx>( // for the initial value because the representation of 0.0 is all zero bits. let mut sum = ImmTy::from_int(0u8, element_layout); for j in 0..items_per_chunk { - if imm & (1 << j.checked_add(4).unwrap()) != 0 { + if imm & (1 << j.strict_add(4)) != 0 { let left = this.read_immediate(&this.project_index(&left, j)?)?; let right = this.read_immediate(&this.project_index(&right, j)?)?; @@ -971,7 +968,7 @@ fn test_high_bits_masked<'tcx>( assert_eq!(op_len, mask_len); - let high_bit_offset = op.layout.field(this, 0).size.bits().checked_sub(1).unwrap(); + let high_bit_offset = op.layout.field(this, 0).size.bits().strict_sub(1); let mut direct = true; let mut negated = true; @@ -1002,7 +999,7 @@ fn mask_load<'tcx>( assert_eq!(dest_len, mask_len); let mask_item_size = mask.layout.field(this, 0).size; - let high_bit_offset = mask_item_size.bits().checked_sub(1).unwrap(); + let high_bit_offset = mask_item_size.bits().strict_sub(1); let ptr = this.read_pointer(ptr)?; for i in 0..dest_len { @@ -1035,7 +1032,7 @@ fn mask_store<'tcx>( assert_eq!(value_len, mask_len); let mask_item_size = mask.layout.field(this, 0).size; - let high_bit_offset = mask_item_size.bits().checked_sub(1).unwrap(); + let high_bit_offset = mask_item_size.bits().strict_sub(1); let ptr = this.read_pointer(ptr)?; for i in 0..value_len { @@ -1082,10 +1079,10 @@ fn mpsadbw<'tcx>( let imm = this.read_scalar(imm)?.to_uint(imm.layout.size)?; // Bit 2 of `imm` specifies the offset for indices of `left`. // The offset is 0 when the bit is 0 or 4 when the bit is 1. - let left_offset = u64::try_from((imm >> 2) & 1).unwrap().checked_mul(4).unwrap(); + let left_offset = u64::try_from((imm >> 2) & 1).unwrap().strict_mul(4); // Bits 0..=1 of `imm` specify the offset for indices of // `right` in blocks of 4 elements. - let right_offset = u64::try_from(imm & 0b11).unwrap().checked_mul(4).unwrap(); + let right_offset = u64::try_from(imm & 0b11).unwrap().strict_mul(4); for i in 0..num_chunks { let left = this.project_index(&left, i)?; @@ -1093,18 +1090,16 @@ fn mpsadbw<'tcx>( let dest = this.project_index(&dest, i)?; for j in 0..dest_items_per_chunk { - let left_offset = left_offset.checked_add(j).unwrap(); + let left_offset = left_offset.strict_add(j); let mut res: u16 = 0; for k in 0..4 { let left = this - .read_scalar(&this.project_index(&left, left_offset.checked_add(k).unwrap())?)? + .read_scalar(&this.project_index(&left, left_offset.strict_add(k))?)? .to_u8()?; let right = this - .read_scalar( - &this.project_index(&right, right_offset.checked_add(k).unwrap())?, - )? + .read_scalar(&this.project_index(&right, right_offset.strict_add(k))?)? .to_u8()?; - res = res.checked_add(left.abs_diff(right).into()).unwrap(); + res = res.strict_add(left.abs_diff(right).into()); } this.write_scalar(Scalar::from_u16(res), &this.project_index(&dest, j)?)?; } @@ -1138,8 +1133,7 @@ fn pmulhrsw<'tcx>( let right = this.read_scalar(&this.project_index(&right, i)?)?.to_i16()?; let dest = this.project_index(&dest, i)?; - let res = - (i32::from(left).checked_mul(right.into()).unwrap() >> 14).checked_add(1).unwrap() >> 1; + let res = (i32::from(left).strict_mul(right.into()) >> 14).strict_add(1) >> 1; // The result of this operation can overflow a signed 16-bit integer. // When `left` and `right` are -0x8000, the result is 0x8000. @@ -1246,8 +1240,7 @@ fn pack_generic<'tcx>( let left = this.read_scalar(&this.project_index(&left, j)?)?; let right = this.read_scalar(&this.project_index(&right, j)?)?; let left_dest = this.project_index(&dest, j)?; - let right_dest = - this.project_index(&dest, j.checked_add(op_items_per_chunk).unwrap())?; + let right_dest = this.project_index(&dest, j.strict_add(op_items_per_chunk))?; let left_res = f(left)?; let right_res = f(right)?; diff --git a/src/shims/x86/sse2.rs b/src/shims/x86/sse2.rs index e10047fefe..b9561ac070 100644 --- a/src/shims/x86/sse2.rs +++ b/src/shims/x86/sse2.rs @@ -50,19 +50,19 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); for i in 0..dest_len { - let j1 = i.checked_mul(2).unwrap(); + let j1 = i.strict_mul(2); let left1 = this.read_scalar(&this.project_index(&left, j1)?)?.to_i16()?; let right1 = this.read_scalar(&this.project_index(&right, j1)?)?.to_i16()?; - let j2 = j1.checked_add(1).unwrap(); + let j2 = j1.strict_add(1); let left2 = this.read_scalar(&this.project_index(&left, j2)?)?.to_i16()?; let right2 = this.read_scalar(&this.project_index(&right, j2)?)?.to_i16()?; let dest = this.project_index(&dest, i)?; // Multiplications are i16*i16->i32, which will not overflow. - let mul1 = i32::from(left1).checked_mul(right1.into()).unwrap(); - let mul2 = i32::from(left2).checked_mul(right2.into()).unwrap(); + let mul1 = i32::from(left1).strict_mul(right1.into()); + let mul2 = i32::from(left2).strict_mul(right2.into()); // However, this addition can overflow in the most extreme case // (-0x8000)*(-0x8000)+(-0x8000)*(-0x8000) = 0x80000000 let res = mul1.wrapping_add(mul2); @@ -94,14 +94,14 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let dest = this.project_index(&dest, i)?; let mut res: u16 = 0; - let n = left_len.checked_div(dest_len).unwrap(); + let n = left_len.strict_div(dest_len); for j in 0..n { - let op_i = j.checked_add(i.checked_mul(n).unwrap()).unwrap(); + let op_i = j.strict_add(i.strict_mul(n)); let left = this.read_scalar(&this.project_index(&left, op_i)?)?.to_u8()?; let right = this.read_scalar(&this.project_index(&right, op_i)?)?.to_u8()?; - res = res.checked_add(left.abs_diff(right).into()).unwrap(); + res = res.strict_add(left.abs_diff(right).into()); } this.write_scalar(Scalar::from_u64(res.into()), &dest)?; diff --git a/src/shims/x86/ssse3.rs b/src/shims/x86/ssse3.rs index 6a815e4cea..33bcbc2fa8 100644 --- a/src/shims/x86/ssse3.rs +++ b/src/shims/x86/ssse3.rs @@ -92,19 +92,19 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); for i in 0..dest_len { - let j1 = i.checked_mul(2).unwrap(); + let j1 = i.strict_mul(2); let left1 = this.read_scalar(&this.project_index(&left, j1)?)?.to_u8()?; let right1 = this.read_scalar(&this.project_index(&right, j1)?)?.to_i8()?; - let j2 = j1.checked_add(1).unwrap(); + let j2 = j1.strict_add(1); let left2 = this.read_scalar(&this.project_index(&left, j2)?)?.to_u8()?; let right2 = this.read_scalar(&this.project_index(&right, j2)?)?.to_i8()?; let dest = this.project_index(&dest, i)?; // Multiplication of a u8 and an i8 into an i16 cannot overflow. - let mul1 = i16::from(left1).checked_mul(right1.into()).unwrap(); - let mul2 = i16::from(left2).checked_mul(right2.into()).unwrap(); + let mul1 = i16::from(left1).strict_mul(right1.into()); + let mul2 = i16::from(left2).strict_mul(right2.into()); let res = mul1.saturating_add(mul2); this.write_scalar(Scalar::from_i16(res), &dest)?; From ef22eb16a974f2572688e2655957438f372556d6 Mon Sep 17 00:00:00 2001 From: Southball <6523469+southball@users.noreply.github.com> Date: Fri, 21 Jun 2024 14:26:47 +0900 Subject: [PATCH 12/61] Fix some missing ones --- src/shims/x86/avx2.rs | 6 +++--- src/shims/x86/mod.rs | 4 ++-- src/shims/x86/sse2.rs | 2 +- src/shims/x86/ssse3.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/shims/x86/avx2.rs b/src/shims/x86/avx2.rs index efb0ed38fb..7f6c9336a9 100644 --- a/src/shims/x86/avx2.rs +++ b/src/shims/x86/avx2.rs @@ -123,7 +123,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (dest, dest_len) = this.mplace_to_simd(dest)?; assert_eq!(left_len, right_len); - assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); + assert_eq!(dest_len.strict_mul(2), left_len); for i in 0..dest_len { let j1 = i.strict_mul(2); @@ -160,7 +160,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (dest, dest_len) = this.mplace_to_simd(dest)?; assert_eq!(left_len, right_len); - assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); + assert_eq!(dest_len.strict_mul(2), left_len); for i in 0..dest_len { let j1 = i.strict_mul(2); @@ -335,7 +335,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (dest, dest_len) = this.mplace_to_simd(dest)?; assert_eq!(left_len, right_len); - assert_eq!(left_len, dest_len.checked_mul(8).unwrap()); + assert_eq!(left_len, dest_len.strict_mul(8)); for i in 0..dest_len { let dest = this.project_index(&dest, i)?; diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 5db6d211a5..03c186e629 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -1074,7 +1074,7 @@ fn mpsadbw<'tcx>( let (_, _, right) = split_simd_to_128bit_chunks(this, right)?; let (_, dest_items_per_chunk, dest) = split_simd_to_128bit_chunks(this, dest)?; - assert_eq!(op_items_per_chunk, dest_items_per_chunk.checked_mul(2).unwrap()); + assert_eq!(op_items_per_chunk, dest_items_per_chunk.strict_mul(2)); let imm = this.read_scalar(imm)?.to_uint(imm.layout.size)?; // Bit 2 of `imm` specifies the offset for indices of `left`. @@ -1229,7 +1229,7 @@ fn pack_generic<'tcx>( let (_, _, right) = split_simd_to_128bit_chunks(this, right)?; let (_, dest_items_per_chunk, dest) = split_simd_to_128bit_chunks(this, dest)?; - assert_eq!(dest_items_per_chunk, op_items_per_chunk.checked_mul(2).unwrap()); + assert_eq!(dest_items_per_chunk, op_items_per_chunk.strict_mul(2)); for i in 0..num_chunks { let left = this.project_index(&left, i)?; diff --git a/src/shims/x86/sse2.rs b/src/shims/x86/sse2.rs index b9561ac070..3efdd561d6 100644 --- a/src/shims/x86/sse2.rs +++ b/src/shims/x86/sse2.rs @@ -47,7 +47,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (dest, dest_len) = this.mplace_to_simd(dest)?; assert_eq!(left_len, right_len); - assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); + assert_eq!(dest_len.strict_mul(2), left_len); for i in 0..dest_len { let j1 = i.strict_mul(2); diff --git a/src/shims/x86/ssse3.rs b/src/shims/x86/ssse3.rs index 33bcbc2fa8..ecacaeb9af 100644 --- a/src/shims/x86/ssse3.rs +++ b/src/shims/x86/ssse3.rs @@ -89,7 +89,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (dest, dest_len) = this.mplace_to_simd(dest)?; assert_eq!(left_len, right_len); - assert_eq!(dest_len.checked_mul(2).unwrap(), left_len); + assert_eq!(dest_len.strict_mul(2), left_len); for i in 0..dest_len { let j1 = i.strict_mul(2); From 760586035eead4e1011b4695c325be2e866b184f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2024 09:40:30 +0200 Subject: [PATCH 13/61] don't rely on libc existing on Windows --- src/helpers.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/helpers.rs b/src/helpers.rs index 843aff0249..6fa1c16fec 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -273,6 +273,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Helper function to get a `libc` constant as a `Scalar`. fn eval_libc(&self, name: &str) -> Scalar { + if self.eval_context_ref().tcx.sess.target.os == "windows" { + panic!( + "`libc` crate is not reliably available on Windows targets; Miri should not use it there" + ); + } self.eval_path_scalar(&["libc", name]) } @@ -316,6 +321,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Helper function to get the `TyAndLayout` of a `libc` type fn libc_ty_layout(&self, name: &str) -> TyAndLayout<'tcx> { let this = self.eval_context_ref(); + if this.tcx.sess.target.os == "windows" { + panic!( + "`libc` crate is not reliably available on Windows targets; Miri should not use it there" + ); + } let ty = this .resolve_path(&["libc", name], Namespace::TypeNS) .ty(*this.tcx, ty::ParamEnv::reveal_all()); @@ -1048,7 +1058,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Always returns a `Vec` no matter the size of `wchar_t`. fn read_wchar_t_str(&self, ptr: Pointer) -> InterpResult<'tcx, Vec> { let this = self.eval_context_ref(); - let wchar_t = this.libc_ty_layout("wchar_t"); + let wchar_t = if this.tcx.sess.target.os == "windows" { + // We don't have libc on Windows so we have to hard-code the type ourselves. + this.machine.layouts.u16 + } else { + this.libc_ty_layout("wchar_t") + }; self.read_c_str_with_char_size(ptr, wchar_t.size, wchar_t.align.abi) } From f677a01642a99903fe7e93266f16c3d447226fd9 Mon Sep 17 00:00:00 2001 From: Strophox Date: Fri, 21 Jun 2024 12:47:04 +0200 Subject: [PATCH 14/61] add as_ptr to trait AllocBytes, fix 2 impls; add pub fn get_bytes_unchecked_raw in allocation.rs; add pub fn get_alloc_bytes_unchecked_raw[_mut] in memory.rs --- src/alloc_bytes.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/alloc_bytes.rs b/src/alloc_bytes.rs index 97841a05cd..8f691456a5 100644 --- a/src/alloc_bytes.rs +++ b/src/alloc_bytes.rs @@ -108,4 +108,8 @@ impl AllocBytes for MiriAllocBytes { fn as_mut_ptr(&mut self) -> *mut u8 { self.ptr } + + fn as_ptr(&self) -> *const u8 { + self.ptr + } } From 073a6c2955545d4ccf5ab5e89efc37506400f161 Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Thu, 20 Jun 2024 20:11:16 +0200 Subject: [PATCH 15/61] Implement LLVM x86 adx intrinsics --- src/shims/x86/mod.rs | 91 +++++++++++++--------- tests/pass/shims/x86/intrinsics-x86-adx.rs | 70 +++++++++++++++++ 2 files changed, 123 insertions(+), 38 deletions(-) create mode 100644 tests/pass/shims/x86/intrinsics-x86-adx.rs diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 704c45fdd6..74470fad35 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -35,63 +35,65 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Prefix should have already been checked. let unprefixed_name = link_name.as_str().strip_prefix("llvm.x86.").unwrap(); match unprefixed_name { - // Used to implement the `_addcarry_u32` and `_addcarry_u64` functions. - // Computes a + b with input and output carry. The input carry is an 8-bit - // value, which is interpreted as 1 if it is non-zero. The output carry is - // an 8-bit value that will be 0 or 1. + // Used to implement the `_addcarry_u{32, 64}` and the `_subborrow_u{32, 64}` functions. + // Computes a + b or a - b with input and output carry/borrow. The input carry/borrow is an 8-bit + // value, which is interpreted as 1 if it is non-zero. The output carry/borrow is an 8-bit value that will be 0 or 1. // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/addcarry-u32-addcarry-u64.html - "addcarry.32" | "addcarry.64" => { - if unprefixed_name == "addcarry.64" && this.tcx.sess.target.arch != "x86_64" { + // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/subborrow-u32-subborrow-u64.html + "addcarry.32" | "addcarry.64" | "subborrow.32" | "subborrow.64" => { + if unprefixed_name.ends_with("64") && this.tcx.sess.target.arch != "x86_64" { return Ok(EmulateItemResult::NotSupported); } - let [c_in, a, b] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; - let c_in = this.read_scalar(c_in)?.to_u8()? != 0; + let op = if unprefixed_name.starts_with("add") { + mir::BinOp::AddWithOverflow + } else { + mir::BinOp::SubWithOverflow + }; + + let [cb_in, a, b] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; + let cb_in = this.read_scalar(cb_in)?.to_u8()? != 0; let a = this.read_immediate(a)?; let b = this.read_immediate(b)?; - let (sum, overflow1) = - this.binary_op(mir::BinOp::AddWithOverflow, &a, &b)?.to_pair(this); - let (sum, overflow2) = this - .binary_op( - mir::BinOp::AddWithOverflow, - &sum, - &ImmTy::from_uint(c_in, a.layout), - )? - .to_pair(this); - let c_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; + let (sum, overflow1) = this.binary_op(op, &a, &b)?.to_pair(this); + let (sum, overflow2) = + this.binary_op(op, &sum, &ImmTy::from_uint(cb_in, a.layout))?.to_pair(this); + let cb_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; - this.write_scalar(Scalar::from_u8(c_out.into()), &this.project_field(dest, 0)?)?; - this.write_immediate(*sum, &this.project_field(dest, 1)?)?; + let d1 = this.project_field(dest, 0)?; + let d2 = this.project_field(dest, 1)?; + write_twice(this, &d1, Scalar::from_u8(cb_out.into()), &d2, sum)?; } - // Used to implement the `_subborrow_u32` and `_subborrow_u64` functions. - // Computes a - b with input and output borrow. The input borrow is an 8-bit - // value, which is interpreted as 1 if it is non-zero. The output borrow is - // an 8-bit value that will be 0 or 1. - // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/subborrow-u32-subborrow-u64.html - "subborrow.32" | "subborrow.64" => { - if unprefixed_name == "subborrow.64" && this.tcx.sess.target.arch != "x86_64" { + + // Used to implement the `_addcarryx_u{32, 64}` functions. They are semantically identical with the `_addcarry_u{32, 64}` functions, + // except for a slightly different type signature and the requirement for the "adx" target feature. + // https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/addcarryx-u32-addcarryx-u64.html + "addcarryx.u32" | "addcarryx.u64" => { + this.expect_target_feature_for_intrinsic(link_name, "adx")?; + + if unprefixed_name.ends_with("64") && this.tcx.sess.target.arch != "x86_64" { return Ok(EmulateItemResult::NotSupported); } - let [b_in, a, b] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; - let b_in = this.read_scalar(b_in)?.to_u8()? != 0; + let [c_in, a, b, out] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; + let c_in = this.read_scalar(c_in)?.to_u8()? != 0; let a = this.read_immediate(a)?; let b = this.read_immediate(b)?; - let (sub, overflow1) = - this.binary_op(mir::BinOp::SubWithOverflow, &a, &b)?.to_pair(this); - let (sub, overflow2) = this + let (sum, overflow1) = + this.binary_op(mir::BinOp::AddWithOverflow, &a, &b)?.to_pair(this); + let (sum, overflow2) = this .binary_op( - mir::BinOp::SubWithOverflow, - &sub, - &ImmTy::from_uint(b_in, a.layout), + mir::BinOp::AddWithOverflow, + &sum, + &ImmTy::from_uint(c_in, a.layout), )? .to_pair(this); - let b_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; + let c_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; - this.write_scalar(Scalar::from_u8(b_out.into()), &this.project_field(dest, 0)?)?; - this.write_immediate(*sub, &this.project_field(dest, 1)?)?; + let out = this.deref_pointer_as(out, sum.layout)?; + write_twice(this, dest, Scalar::from_u8(c_out.into()), &out, sum)?; } // Used to implement the `_mm_pause` function. @@ -1366,3 +1368,16 @@ fn psign<'tcx>( Ok(()) } + +/// Write two values `v1` and `v2` to the places `d1` and `d2`. +fn write_twice<'tcx>( + this: &mut crate::MiriInterpCx<'tcx>, + d1: &MPlaceTy<'tcx>, + v1: Scalar, + d2: &MPlaceTy<'tcx>, + v2: ImmTy<'tcx>, +) -> InterpResult<'tcx, ()> { + this.write_scalar(v1, d1)?; + this.write_immediate(*v2, d2)?; + Ok(()) +} diff --git a/tests/pass/shims/x86/intrinsics-x86-adx.rs b/tests/pass/shims/x86/intrinsics-x86-adx.rs new file mode 100644 index 0000000000..431e7f2c5e --- /dev/null +++ b/tests/pass/shims/x86/intrinsics-x86-adx.rs @@ -0,0 +1,70 @@ +// Ignore everything except x86 and x86_64 +// Any new targets that are added to CI should be ignored here. +// (We cannot use `cfg`-based tricks here since the `target-feature` flags below only work on x86.) +//@ignore-target-aarch64 +//@ignore-target-arm +//@ignore-target-avr +//@ignore-target-s390x +//@ignore-target-thumbv7em +//@ignore-target-wasm32 +//@compile-flags: -C target-feature=+adx + +#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +mod x86 { + #[cfg(target_arch = "x86")] + use core::arch::x86 as arch; + #[cfg(target_arch = "x86_64")] + use core::arch::x86_64 as arch; + + fn adc(c_in: u8, a: u32, b: u32) -> (u8, u32) { + let mut sum = 0; + // SAFETY: There are no safety requirements for calling `_addcarry_u32`. + // It's just unsafe for API consistency with other intrinsics. + let c_out = unsafe { arch::_addcarryx_u32(c_in, a, b, &mut sum) }; + (c_out, sum) + } + + pub fn main() { + assert_eq!(adc(0, 1, 1), (0, 2)); + assert_eq!(adc(1, 1, 1), (0, 3)); + assert_eq!(adc(2, 1, 1), (0, 3)); // any non-zero carry acts as 1! + assert_eq!(adc(u8::MAX, 1, 1), (0, 3)); + assert_eq!(adc(0, u32::MAX, u32::MAX), (1, u32::MAX - 1)); + assert_eq!(adc(1, u32::MAX, u32::MAX), (1, u32::MAX)); + assert_eq!(adc(2, u32::MAX, u32::MAX), (1, u32::MAX)); + assert_eq!(adc(u8::MAX, u32::MAX, u32::MAX), (1, u32::MAX)); + } +} + +#[cfg(target_arch = "x86_64")] +mod x86_64 { + use core::arch::x86_64 as arch; + + fn adc(c_in: u8, a: u64, b: u64) -> (u8, u64) { + let mut sum = 0; + // SAFETY: There are no safety requirements for calling `_addcarry_u64`. + // It's just unsafe for API consistency with other intrinsics. + let c_out = unsafe { arch::_addcarryx_u64(c_in, a, b, &mut sum) }; + (c_out, sum) + } + + pub fn main() { + assert_eq!(adc(0, 1, 1), (0, 2)); + assert_eq!(adc(1, 1, 1), (0, 3)); + assert_eq!(adc(2, 1, 1), (0, 3)); // any non-zero carry acts as 1! + assert_eq!(adc(u8::MAX, 1, 1), (0, 3)); + assert_eq!(adc(0, u64::MAX, u64::MAX), (1, u64::MAX - 1)); + assert_eq!(adc(1, u64::MAX, u64::MAX), (1, u64::MAX)); + assert_eq!(adc(2, u64::MAX, u64::MAX), (1, u64::MAX)); + assert_eq!(adc(u8::MAX, u64::MAX, u64::MAX), (1, u64::MAX)); + } +} + +fn main() { + assert!(is_x86_feature_detected!("adx")); + + #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] + x86::main(); + #[cfg(target_arch = "x86_64")] + x86_64::main(); +} From 6f3afe9613002a3ac9afac697bf56a28176254f4 Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Fri, 21 Jun 2024 17:55:22 +0200 Subject: [PATCH 16/61] Move out addition logic --- src/shims/x86/mod.rs | 68 +++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 39 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 74470fad35..7bccf71f04 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -45,25 +45,17 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(EmulateItemResult::NotSupported); } + let [cb_in, a, b] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; + let op = if unprefixed_name.starts_with("add") { mir::BinOp::AddWithOverflow } else { mir::BinOp::SubWithOverflow }; - let [cb_in, a, b] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; - let cb_in = this.read_scalar(cb_in)?.to_u8()? != 0; - let a = this.read_immediate(a)?; - let b = this.read_immediate(b)?; - - let (sum, overflow1) = this.binary_op(op, &a, &b)?.to_pair(this); - let (sum, overflow2) = - this.binary_op(op, &sum, &ImmTy::from_uint(cb_in, a.layout))?.to_pair(this); - let cb_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; - - let d1 = this.project_field(dest, 0)?; - let d2 = this.project_field(dest, 1)?; - write_twice(this, &d1, Scalar::from_u8(cb_out.into()), &d2, sum)?; + let (sum, cb_out) = carrying_add(this, cb_in, a, b, op)?; + this.write_scalar(cb_out, &this.project_field(dest, 0)?)?; + this.write_immediate(*sum, &this.project_field(dest, 1)?)?; } // Used to implement the `_addcarryx_u{32, 64}` functions. They are semantically identical with the `_addcarry_u{32, 64}` functions, @@ -77,23 +69,10 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } let [c_in, a, b, out] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; - let c_in = this.read_scalar(c_in)?.to_u8()? != 0; - let a = this.read_immediate(a)?; - let b = this.read_immediate(b)?; - - let (sum, overflow1) = - this.binary_op(mir::BinOp::AddWithOverflow, &a, &b)?.to_pair(this); - let (sum, overflow2) = this - .binary_op( - mir::BinOp::AddWithOverflow, - &sum, - &ImmTy::from_uint(c_in, a.layout), - )? - .to_pair(this); - let c_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; - let out = this.deref_pointer_as(out, sum.layout)?; - write_twice(this, dest, Scalar::from_u8(c_out.into()), &out, sum)?; + let (sum, c_out) = carrying_add(this, c_in, a, b, mir::BinOp::AddWithOverflow)?; + this.write_scalar(c_out, dest)?; + this.write_immediate(*sum, &this.deref_pointer_as(out, sum.layout)?)?; } // Used to implement the `_mm_pause` function. @@ -1369,15 +1348,26 @@ fn psign<'tcx>( Ok(()) } -/// Write two values `v1` and `v2` to the places `d1` and `d2`. -fn write_twice<'tcx>( +/// Calcultates either `a + b + cb_in` or `a - b - cb_in` depending on the value +/// of `op` and returns both the sum and the overflow bit. `op` is expected to be +/// either one of `mir::BinOp::AddWithOverflow` and `mir::BinOp::SubWithOverflow`. +fn carrying_add<'tcx>( this: &mut crate::MiriInterpCx<'tcx>, - d1: &MPlaceTy<'tcx>, - v1: Scalar, - d2: &MPlaceTy<'tcx>, - v2: ImmTy<'tcx>, -) -> InterpResult<'tcx, ()> { - this.write_scalar(v1, d1)?; - this.write_immediate(*v2, d2)?; - Ok(()) + cb_in: &OpTy<'tcx>, + a: &OpTy<'tcx>, + b: &OpTy<'tcx>, + op: mir::BinOp, +) -> InterpResult<'tcx, (ImmTy<'tcx>, Scalar)> { + assert!(op == mir::BinOp::AddWithOverflow || op == mir::BinOp::SubWithOverflow); + + let cb_in = this.read_scalar(cb_in)?.to_u8()? != 0; + let a = this.read_immediate(a)?; + let b = this.read_immediate(b)?; + + let (sum, overflow1) = this.binary_op(op, &a, &b)?.to_pair(this); + let (sum, overflow2) = + this.binary_op(op, &sum, &ImmTy::from_uint(cb_in, a.layout))?.to_pair(this); + let cb_out = overflow1.to_scalar().to_bool()? | overflow2.to_scalar().to_bool()?; + + Ok((sum, Scalar::from_u8(cb_out.into()))) } From 4d9ce31468b00b6bae5997e47a33118648e47d56 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2024 17:02:31 +0200 Subject: [PATCH 17/61] CI: try to share setup code across actions --- .github/workflows/ci.yml | 87 +----------------------------- .github/workflows/setup/action.yml | 52 ++++++++++++++++++ 2 files changed, 54 insertions(+), 85 deletions(-) create mode 100644 .github/workflows/setup/action.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3bc4163ab5..fc4e484fa3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,50 +33,7 @@ jobs: HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v4 - - - name: Show Rust version (stable toolchain) - run: | - rustup show - rustc -Vv - cargo -V - - # Cache the global cargo directory, but NOT the local `target` directory which - # we cannot reuse anyway when the nightly changes (and it grows quite large - # over time). - - name: Add cache for cargo - id: cache - uses: actions/cache@v4 - with: - path: | - # Taken from . - # Cache package/registry information - ~/.cargo/registry/index - ~/.cargo/registry/cache - ~/.cargo/git/db - # Cache installed binaries - ~/.cargo/bin - ~/.cargo/.crates.toml - ~/.cargo/.crates2.json - key: cargo-${{ runner.os }}-reset20240425-${{ hashFiles('**/Cargo.lock') }} - restore-keys: cargo-${{ runner.os }}-reset20240425 - - - name: Install tools - if: steps.cache.outputs.cache-hit != 'true' - run: cargo install -f rustup-toolchain-install-master hyperfine - - - name: Install miri toolchain - run: | - if [[ ${{ github.event_name }} == 'schedule' ]]; then - echo "Building against latest rustc git version" - git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1 > rust-version - fi - ./miri toolchain --host ${{ matrix.host_target }} - - - name: Show Rust version (miri toolchain) - run: | - rustup show - rustc -Vv - cargo -V + - uses: ./.github/workflows/setup # The `style` job only runs on Linux; this makes sure the Windows-host-specific # code is also covered by clippy. @@ -92,47 +49,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - # This is exactly duplicated from above. GHA is pretty terrible when it comes - # to avoiding code duplication. - - # Cache the global cargo directory, but NOT the local `target` directory which - # we cannot reuse anyway when the nightly changes (and it grows quite large - # over time). - - name: Add cache for cargo - id: cache - uses: actions/cache@v4 - with: - path: | - # Taken from . - # Cache package/registry information - ~/.cargo/registry/index - ~/.cargo/registry/cache - ~/.cargo/git/db - # Cache installed binaries - ~/.cargo/bin - ~/.cargo/.crates.toml - ~/.cargo/.crates2.json - key: cargo-${{ runner.os }}-reset20240331-${{ hashFiles('**/Cargo.lock') }} - restore-keys: cargo-${{ runner.os }}-reset20240331 - - - name: Install rustup-toolchain-install-master - if: steps.cache.outputs.cache-hit != 'true' - run: cargo install -f rustup-toolchain-install-master - - - name: Install "master" toolchain - run: | - if [[ ${{ github.event_name }} == 'schedule' ]]; then - echo "Building against latest rustc git version" - git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1 > rust-version - fi - ./miri toolchain - - - name: Show Rust version - run: | - rustup show - rustc -Vv - cargo -V + - uses: ./.github/workflows/setup - name: rustfmt run: ./miri fmt --check diff --git a/.github/workflows/setup/action.yml b/.github/workflows/setup/action.yml new file mode 100644 index 0000000000..8f54b5b8d8 --- /dev/null +++ b/.github/workflows/setup/action.yml @@ -0,0 +1,52 @@ +name: "Miri CI setup" +description: "Sets up Miri CI" +runs: + using: "composite" + steps: + - name: Show Rust version (stable toolchain) + run: | + rustup show + rustc -Vv + cargo -V + shell: bash + + # Cache the global cargo directory, but NOT the local `target` directory which + # we cannot reuse anyway when the nightly changes (and it grows quite large + # over time). + - name: Add cache for cargo + id: cache + uses: actions/cache@v4 + with: + path: | + # Taken from . + # Cache package/registry information + ~/.cargo/registry/index + ~/.cargo/registry/cache + ~/.cargo/git/db + # Cache installed binaries + ~/.cargo/bin + ~/.cargo/.crates.toml + ~/.cargo/.crates2.json + key: cargo-${{ runner.os }}-${{ hashFiles('**/Cargo.lock', '.github/workflows/**/*.yml') }} + restore-keys: cargo-${{ runner.os }} + + - name: Install rustup-toolchain-install-master + if: steps.cache.outputs.cache-hit != 'true' + run: cargo install -f rustup-toolchain-install-master hyperfine + shell: bash + + - name: Install "master" toolchain + run: | + if [[ ${{ github.event_name }} == 'schedule' ]]; then + echo "Building against latest rustc git version" + git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1 > rust-version + fi + ./miri toolchain + shell: bash + + - name: Show Rust version (miri toolchain) + run: | + rustup show + rustc -Vv + cargo -V + shell: bash From 50b9c514fba2ce2ebc753d8686505c1a8ce1d6f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 22 Jun 2024 15:04:45 +0200 Subject: [PATCH 18/61] ./miri: nicer error when building miri-script fails --- miri | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/miri b/miri index 5f71fc9443..07383bb59e 100755 --- a/miri +++ b/miri @@ -3,5 +3,6 @@ set -e # Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through # rustup (that sets it's own environmental variables), which is undesirable. MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target -cargo +stable build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml +cargo +stable build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml || \ + ( echo "Failed to build miri-script. Is the 'stable' toolchain installed?"; exit 1 ) "$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" From 47e66d50b8a578a4269e6c91dac739e72301b1a6 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Fri, 21 Jun 2024 12:22:29 +0000 Subject: [PATCH 19/61] Make `effects` an incomplete feature --- tests/fail/intrinsic_fallback_is_spec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fail/intrinsic_fallback_is_spec.rs b/tests/fail/intrinsic_fallback_is_spec.rs index 888c548e49..fa7c0bf5c0 100644 --- a/tests/fail/intrinsic_fallback_is_spec.rs +++ b/tests/fail/intrinsic_fallback_is_spec.rs @@ -1,4 +1,4 @@ -#![feature(rustc_attrs, effects)] +#![feature(rustc_attrs)] #[rustc_intrinsic] #[rustc_nounwind] From be9eece2afea54aad02dfa061b0fe628e4cc9270 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 22 Jun 2024 17:02:38 +0200 Subject: [PATCH 20/61] evaluate arguments first, not inside the logic --- src/shims/x86/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 7bccf71f04..afaf59eaad 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -64,15 +64,20 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { "addcarryx.u32" | "addcarryx.u64" => { this.expect_target_feature_for_intrinsic(link_name, "adx")?; - if unprefixed_name.ends_with("64") && this.tcx.sess.target.arch != "x86_64" { + let is_u64 = unprefixed_name.ends_with("64"); + if is_u64 && this.tcx.sess.target.arch != "x86_64" { return Ok(EmulateItemResult::NotSupported); } let [c_in, a, b, out] = this.check_shim(abi, Abi::Unadjusted, link_name, args)?; + let out = this.deref_pointer_as( + out, + if is_u64 { this.machine.layouts.u64 } else { this.machine.layouts.u32 }, + )?; let (sum, c_out) = carrying_add(this, c_in, a, b, mir::BinOp::AddWithOverflow)?; this.write_scalar(c_out, dest)?; - this.write_immediate(*sum, &this.deref_pointer_as(out, sum.layout)?)?; + this.write_immediate(*sum, &out)?; } // Used to implement the `_mm_pause` function. From 6b3267ffa93918663ff95400a31d09df2c59e322 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 22 Jun 2024 16:26:30 +0200 Subject: [PATCH 21/61] don't ICE when encountering an extern type field during validation --- src/diagnostics.rs | 4 +++- tests/fail/extern-type-field-offset.stderr | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 12fb76f397..1b70a1a1cf 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -311,7 +311,9 @@ pub fn report_error<'tcx>( ResourceExhaustion(_) => "resource exhaustion", Unsupported( // We list only the ones that can actually happen. - UnsupportedOpInfo::Unsupported(_) | UnsupportedOpInfo::UnsizedLocal, + UnsupportedOpInfo::Unsupported(_) + | UnsupportedOpInfo::UnsizedLocal + | UnsupportedOpInfo::ExternTypeField, ) => "unsupported operation", InvalidProgram( // We list only the ones that can actually happen. diff --git a/tests/fail/extern-type-field-offset.stderr b/tests/fail/extern-type-field-offset.stderr index e0d6e9ebf1..3ed5732b4e 100644 --- a/tests/fail/extern-type-field-offset.stderr +++ b/tests/fail/extern-type-field-offset.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern type` does not have a known offset +error: unsupported operation: `extern type` field does not have a known offset --> $DIR/extern-type-field-offset.rs:LL:CC | LL | let _field = &x.a; - | ^^^^ `extern type` does not have a known offset + | ^^^^ `extern type` field does not have a known offset | = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support = note: BACKTRACE: From b74ba4317fae0e9c8c71b1f5ce98bfa127a6fb9b Mon Sep 17 00:00:00 2001 From: Ross Smyth <18294397+RossSmyth@users.noreply.github.com> Date: Sat, 22 Jun 2024 23:28:05 -0400 Subject: [PATCH 22/61] nicer batch file error when building miri-script fails --- miri.bat | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/miri.bat b/miri.bat index 18baa683f6..98b59a56a0 100644 --- a/miri.bat +++ b/miri.bat @@ -5,7 +5,8 @@ set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target :: If any other steps are added, the "|| exit /b" must be appended to early :: return from the script. If not, it will continue execution. -cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml || exit /b +cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ + || echo Failed to build miri-script. Is the 'stable' toolchain installed? & exit /b :: Forwards all arguments to this file to the executable. :: We invoke the binary directly to avoid going through rustup, which would set some extra From 59b951cbb9d2993ebeb22b5983902a85c0aed0f6 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 23 Jun 2024 04:54:11 +0000 Subject: [PATCH 23/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 1502fa120b..97d37b0ebe 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -7a08f84627ff3035de4d66ff3209e5fc93165dcb +acb62737aca7045f331e7a05adc38bed213e278d From 2071ac20e0e22d689c0e485d8ac116c90f41ccc1 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 23 Jun 2024 05:02:48 +0000 Subject: [PATCH 24/61] fmt --- src/concurrency/thread.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/concurrency/thread.rs b/src/concurrency/thread.rs index 6a2b99825a..718daf93ea 100644 --- a/src/concurrency/thread.rs +++ b/src/concurrency/thread.rs @@ -643,8 +643,7 @@ impl<'tcx> ThreadManager<'tcx> { if !self.threads[joined_thread_id].state.is_terminated() { trace!( "{:?} blocked on {:?} when trying to join", - self.active_thread, - joined_thread_id + self.active_thread, joined_thread_id ); // The joined thread is still running, we need to wait for it. // Unce we get unblocked, perform the appropriate synchronization. From 68d591ca0e69b7ffc43885e8214f5bb60a7dfbbf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 23 Jun 2024 08:41:18 +0200 Subject: [PATCH 25/61] unix/foreign_items: move getpid to the right part of the file --- src/shims/unix/foreign_items.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index 2282099fa0..53ad40cfd2 100644 --- a/src/shims/unix/foreign_items.rs +++ b/src/shims/unix/foreign_items.rs @@ -51,7 +51,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // See `fn emulate_foreign_item_inner` in `shims/foreign_items.rs` for the general pattern. #[rustfmt::skip] match link_name.as_str() { - // Environment variables + // Environment related shims "getenv" => { let [name] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.getenv(name)?; @@ -78,6 +78,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.chdir(path)?; this.write_scalar(Scalar::from_i32(result), dest)?; } + "getpid" => { + let [] = this.check_shim(abi, Abi::C { unwind: false}, link_name, args)?; + let result = this.getpid()?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } // File descriptors "read" => { @@ -583,11 +588,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let ret = if complete { 0 } else { this.eval_libc_i32("ERANGE") }; this.write_int(ret, dest)?; } - "getpid" => { - let [] = this.check_shim(abi, Abi::C { unwind: false}, link_name, args)?; - let result = this.getpid()?; - this.write_scalar(Scalar::from_i32(result), dest)?; - } "getentropy" => { // This function is non-standard but exists with the same signature and behavior on // Linux, macOS, FreeBSD and Solaris/Illumos. From 5d014f49d70a6f02e2eb782027093cee740e0498 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Mon, 24 Jun 2024 05:03:43 +0000 Subject: [PATCH 26/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 97d37b0ebe..11a1c43bae 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -acb62737aca7045f331e7a05adc38bed213e278d +d49994b060684af423339b55769439b2f444a7b9 From 9f839ab3f8d5f98d33876fa376ee6f28d6097535 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2024 08:43:16 +0200 Subject: [PATCH 27/61] tests for when a thread-local gets initialized in a tls dtor --- tests/pass/tls/tls_macro_drop.rs | 102 ++++++++++++------ tests/pass/tls/tls_macro_drop.stack.stdout | 3 + tests/pass/tls/tls_macro_drop.tree.stdout | 3 + .../pass/tls/tls_macro_drop_single_thread.rs | 36 +++---- .../tls/tls_macro_drop_single_thread.stderr | 3 - .../tls/tls_macro_drop_single_thread.stdout | 2 + 6 files changed, 91 insertions(+), 58 deletions(-) delete mode 100644 tests/pass/tls/tls_macro_drop_single_thread.stderr create mode 100644 tests/pass/tls/tls_macro_drop_single_thread.stdout diff --git a/tests/pass/tls/tls_macro_drop.rs b/tests/pass/tls/tls_macro_drop.rs index bd06eec9cd..0d8a1cef51 100644 --- a/tests/pass/tls/tls_macro_drop.rs +++ b/tests/pass/tls/tls_macro_drop.rs @@ -4,27 +4,28 @@ use std::cell::RefCell; use std::thread; -struct TestCell { - value: RefCell, -} +/// Check that destructors of the library thread locals are executed immediately +/// after a thread terminates. +fn check_destructors() { + struct TestCell { + value: RefCell, + } -impl Drop for TestCell { - fn drop(&mut self) { - for _ in 0..10 { - thread::yield_now(); + impl Drop for TestCell { + fn drop(&mut self) { + for _ in 0..10 { + thread::yield_now(); + } + println!("Dropping: {} (should be before 'Continue main 1').", *self.value.borrow()) } - println!("Dropping: {} (should be before 'Continue main 1').", *self.value.borrow()) } -} -thread_local! { - static A: TestCell = TestCell { value: RefCell::new(0) }; - static A_CONST: TestCell = const { TestCell { value: RefCell::new(10) } }; -} + // Test both regular and `const` thread-locals. + thread_local! { + static A: TestCell = TestCell { value: RefCell::new(0) }; + static A_CONST: TestCell = const { TestCell { value: RefCell::new(10) } }; + } -/// Check that destructors of the library thread locals are executed immediately -/// after a thread terminates. -fn check_destructors() { // We use the same value for both of them, since destructor order differs between Miri on Linux // (which uses `register_dtor_fallback`, in the end using a single pthread_key to manage a // thread-local linked list of dtors to call), real Linux rustc (which uses @@ -44,26 +45,29 @@ fn check_destructors() { println!("Continue main 1.") } -struct JoinCell { - value: RefCell>>, -} +/// Check that the destructor can be blocked joining another thread. +fn check_blocking() { + struct JoinCell { + value: RefCell>>, + } -impl Drop for JoinCell { - fn drop(&mut self) { - for _ in 0..10 { - thread::yield_now(); + impl Drop for JoinCell { + fn drop(&mut self) { + for _ in 0..10 { + thread::yield_now(); + } + let join_handle = self.value.borrow_mut().take().unwrap(); + println!( + "Joining: {} (should be before 'Continue main 2').", + join_handle.join().unwrap() + ); } - let join_handle = self.value.borrow_mut().take().unwrap(); - println!("Joining: {} (should be before 'Continue main 2').", join_handle.join().unwrap()); } -} -thread_local! { - static B: JoinCell = JoinCell { value: RefCell::new(None) }; -} + thread_local! { + static B: JoinCell = JoinCell { value: RefCell::new(None) }; + } -/// Check that the destructor can be blocked joining another thread. -fn check_blocking() { thread::spawn(|| { B.with(|f| { assert!(f.value.borrow().is_none()); @@ -74,10 +78,36 @@ fn check_blocking() { .join() .unwrap(); println!("Continue main 2."); - // Preempt the main thread so that the destructor gets executed and can join - // the thread. - thread::yield_now(); - thread::yield_now(); +} + +fn check_tls_init_in_dtor() { + struct Bar; + + impl Drop for Bar { + fn drop(&mut self) { + println!("Bar dtor (should be before `Continue main 3`)."); + } + } + + struct Foo; + + impl Drop for Foo { + fn drop(&mut self) { + println!("Foo dtor (should be before `Bar dtor`)."); + // We initialize another thread-local inside the dtor, which is an interesting corner case. + thread_local!(static BAR: Bar = Bar); + BAR.with(|_| {}); + } + } + + thread_local!(static FOO: Foo = Foo); + + thread::spawn(|| { + FOO.with(|_| {}); + }) + .join() + .unwrap(); + println!("Continue main 3."); } // This test tests that TLS destructors have run before the thread joins. The @@ -248,6 +278,8 @@ fn dtors_in_dtors_in_dtors() { fn main() { check_destructors(); check_blocking(); + check_tls_init_in_dtor(); + join_orders_after_tls_destructors(); dtors_in_dtors_in_dtors(); } diff --git a/tests/pass/tls/tls_macro_drop.stack.stdout b/tests/pass/tls/tls_macro_drop.stack.stdout index b7877820a0..3e17acc832 100644 --- a/tests/pass/tls/tls_macro_drop.stack.stdout +++ b/tests/pass/tls/tls_macro_drop.stack.stdout @@ -3,3 +3,6 @@ Dropping: 8 (should be before 'Continue main 1'). Continue main 1. Joining: 7 (should be before 'Continue main 2'). Continue main 2. +Foo dtor (should be before `Bar dtor`). +Bar dtor (should be before `Continue main 3`). +Continue main 3. diff --git a/tests/pass/tls/tls_macro_drop.tree.stdout b/tests/pass/tls/tls_macro_drop.tree.stdout index b7877820a0..3e17acc832 100644 --- a/tests/pass/tls/tls_macro_drop.tree.stdout +++ b/tests/pass/tls/tls_macro_drop.tree.stdout @@ -3,3 +3,6 @@ Dropping: 8 (should be before 'Continue main 1'). Continue main 1. Joining: 7 (should be before 'Continue main 2'). Continue main 2. +Foo dtor (should be before `Bar dtor`). +Bar dtor (should be before `Continue main 3`). +Continue main 3. diff --git a/tests/pass/tls/tls_macro_drop_single_thread.rs b/tests/pass/tls/tls_macro_drop_single_thread.rs index f36c460ae5..082a6f1783 100644 --- a/tests/pass/tls/tls_macro_drop_single_thread.rs +++ b/tests/pass/tls/tls_macro_drop_single_thread.rs @@ -1,31 +1,27 @@ -//! Check that destructors of the thread locals are executed on all OSes -//! (even when we do not support concurrency, and cannot run the other test). +//! Check that destructors of main thread thread locals are executed. -use std::cell::RefCell; +struct Bar; -struct TestCell { - value: RefCell, +impl Drop for Bar { + fn drop(&mut self) { + println!("Bar dtor"); + } } -impl Drop for TestCell { +struct Foo; + +impl Drop for Foo { fn drop(&mut self) { - eprintln!("Dropping: {}", *self.value.borrow()) + println!("Foo dtor"); + // We initialize another thread-local inside the dtor, which is an interesting corner case. + // Also we use a `const` thread-local here, just to also have that code path covered. + thread_local!(static BAR: Bar = const { Bar }); + BAR.with(|_| {}); } } -thread_local! { - static A: TestCell = TestCell { value: RefCell::new(0) }; - static A_CONST: TestCell = const { TestCell { value: RefCell::new(10) } }; -} +thread_local!(static FOO: Foo = Foo); fn main() { - A.with(|f| { - assert_eq!(*f.value.borrow(), 0); - *f.value.borrow_mut() = 5; - }); - A_CONST.with(|f| { - assert_eq!(*f.value.borrow(), 10); - *f.value.borrow_mut() = 5; // Same value as above since the drop order is different on different platforms - }); - eprintln!("Continue main.") + FOO.with(|_| {}); } diff --git a/tests/pass/tls/tls_macro_drop_single_thread.stderr b/tests/pass/tls/tls_macro_drop_single_thread.stderr deleted file mode 100644 index 09ec1c3c2c..0000000000 --- a/tests/pass/tls/tls_macro_drop_single_thread.stderr +++ /dev/null @@ -1,3 +0,0 @@ -Continue main. -Dropping: 5 -Dropping: 5 diff --git a/tests/pass/tls/tls_macro_drop_single_thread.stdout b/tests/pass/tls/tls_macro_drop_single_thread.stdout new file mode 100644 index 0000000000..6160f27264 --- /dev/null +++ b/tests/pass/tls/tls_macro_drop_single_thread.stdout @@ -0,0 +1,2 @@ +Foo dtor +Bar dtor From 25fdd416993b490d569ce3bf96f11057857daeff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2024 10:51:49 +0200 Subject: [PATCH 28/61] clarify the warning shown when optimizations are enabled --- src/bin/miri.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 829bfa7cd7..9d8e44ce40 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -98,10 +98,9 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { } if tcx.sess.opts.optimize != OptLevel::No { - tcx.dcx().warn("Miri does not support optimizations. If you have enabled optimizations \ - by selecting a Cargo profile (such as --release) which changes other profile settings \ - such as whether debug assertions and overflow checks are enabled, those settings are \ - still applied."); + tcx.dcx().warn("Miri does not support optimizations: the opt-level is ignored. The only effect \ + of selecting a Cargo profile that enables optimizations (such as --release) is to apply \ + its remaining settings, such as whether debug assertions and overflow checks are enabled."); } if tcx.sess.mir_opt_level() > 0 { tcx.dcx().warn("You have explicitly enabled MIR optimizations, overriding Miri's default \ From 4f7bc6ad8698d581a4ec25c44a06586feacd0964 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2024 10:54:26 +0200 Subject: [PATCH 29/61] clarify the status of Tree Borrows --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4b4f2f8306..87b437a308 100644 --- a/README.md +++ b/README.md @@ -425,8 +425,12 @@ to Miri failing to detect cases of undefined behavior in a program. value from a load. This can help diagnose problems that disappear under `-Zmiri-disable-weak-memory-emulation`. * `-Zmiri-tree-borrows` replaces [Stacked Borrows] with the [Tree Borrows] rules. - The soundness rules are already experimental without this flag, but even more - so with this flag. + Tree Borrows is even more experimental than Stacked Borrows. While Tree Borrows + is still sound in the sense of catching all aliasing violations that current versions + of the compiler might exploit, it is likely that the eventual final aliasing model + of Rust will be stricter than Tree Borrows. In other words, if you use Tree Borrows, + even if your code is accepted today, it might be declared UB in the future. + This is much less likely with Stacked Borrows. * `-Zmiri-force-page-size=` overrides the default page size for an architecture, in multiples of 1k. `4` is default for most targets. This value should always be a power of 2 and nonzero. * `-Zmiri-unique-is-unique` performs additional aliasing checks for `core::ptr::Unique` to ensure From 79b4eac42da47164df80f8685a4dbce8834a32b2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 25 Jun 2024 12:02:55 +0200 Subject: [PATCH 30/61] miri: make sure we can find link_section statics even for the local crate --- tests/pass/tls/win_tls_callback.rs | 16 ++++++++++++++++ tests/pass/tls/win_tls_callback.stderr | 1 + 2 files changed, 17 insertions(+) create mode 100644 tests/pass/tls/win_tls_callback.rs create mode 100644 tests/pass/tls/win_tls_callback.stderr diff --git a/tests/pass/tls/win_tls_callback.rs b/tests/pass/tls/win_tls_callback.rs new file mode 100644 index 0000000000..99a8de29e9 --- /dev/null +++ b/tests/pass/tls/win_tls_callback.rs @@ -0,0 +1,16 @@ +//! Ensure that we call Windows TLS callbacks in the local crate. +//@only-target-windows +// Calling eprintln in the callback seems to (re-)initialize some thread-local storage +// and then leak the memory allocated for that. Let's just ignore these leaks, +// that's not what this test is about. +//@compile-flags: -Zmiri-ignore-leaks + +#[link_section = ".CRT$XLB"] +#[used] // Miri only considers explicitly `#[used]` statics for `lookup_link_section` +pub static CALLBACK: unsafe extern "system" fn(*const (), u32, *const ()) = tls_callback; + +unsafe extern "system" fn tls_callback(_h: *const (), _dw_reason: u32, _pv: *const ()) { + eprintln!("in tls_callback"); +} + +fn main() {} diff --git a/tests/pass/tls/win_tls_callback.stderr b/tests/pass/tls/win_tls_callback.stderr new file mode 100644 index 0000000000..8479558954 --- /dev/null +++ b/tests/pass/tls/win_tls_callback.stderr @@ -0,0 +1 @@ +in tls_callback From 398c3759bc3e2b6c754ccf7b387fef1d1018ac6a Mon Sep 17 00:00:00 2001 From: xFrednet Date: Sat, 10 Feb 2024 22:33:25 +0000 Subject: [PATCH 31/61] RFC 2383: Stabilize `lint_reasons` in Miri --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index f8410db4dd..8da00861f9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,7 +10,7 @@ #![feature(yeet_expr)] #![feature(nonzero_ops)] #![feature(let_chains)] -#![feature(lint_reasons)] +#![cfg_attr(bootstrap, feature(lint_reasons))] #![feature(trait_upcasting)] #![feature(strict_overflow_ops)] #![feature(is_none_or)] From 14230ff4d9784b316c0b201cd9f8eeb46608a8e5 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 25 Jun 2024 18:00:44 -0400 Subject: [PATCH 32/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 11a1c43bae..e5e9f0bbda 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -d49994b060684af423339b55769439b2f444a7b9 +c290e9de32e8ba6a673ef125fde40eadd395d170 From 84b10fae282de4471207954dc73f5ef15c859052 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 27 Jun 2024 04:54:26 +0000 Subject: [PATCH 33/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index e5e9f0bbda..989e9bc6d0 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -c290e9de32e8ba6a673ef125fde40eadd395d170 +7033f9b14a37f4a00766d6c01326600b31f3a716 From fee0430f089f4ab78a8de9f530f942a0ff673e8f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 27 Jun 2024 09:53:59 +0200 Subject: [PATCH 34/61] tame unexpected_cfgs --- build.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/build.rs b/build.rs index 0977c0ba01..0918c9b132 100644 --- a/build.rs +++ b/build.rs @@ -1,8 +1,10 @@ fn main() { // Don't rebuild miri when nothing changed. println!("cargo:rerun-if-changed=build.rs"); - // Re-export the TARGET environment variable so it can - // be accessed by miri. + // Re-export the TARGET environment variable so it can be accessed by miri. Needed to know the + // "host" triple inside Miri. let target = std::env::var("TARGET").unwrap(); println!("cargo:rustc-env=TARGET={target}"); + // Allow some cfgs. + println!("cargo::rustc-check-cfg=cfg(bootstrap)"); } From f846fc37c94059f7268c672b760540ffaf4cd26d Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Thu, 27 Jun 2024 13:00:37 -0700 Subject: [PATCH 35/61] Fix miri.bat --- miri.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miri.bat b/miri.bat index 98b59a56a0..92566e0096 100644 --- a/miri.bat +++ b/miri.bat @@ -6,7 +6,7 @@ set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target :: If any other steps are added, the "|| exit /b" must be appended to early :: return from the script. If not, it will continue execution. cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ - || echo Failed to build miri-script. Is the 'stable' toolchain installed? & exit /b + || echo Failed to build miri-script. Is the 'stable' toolchain installed? && exit /b :: Forwards all arguments to this file to the executable. :: We invoke the binary directly to avoid going through rustup, which would set some extra From 3fc1560c76de36129cf68abfe7ed4cc5efd2091b Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Thu, 27 Jun 2024 13:23:34 -0700 Subject: [PATCH 36/61] Relocate GetCurrentProcessId to Environment Related shims, remove unnecessary std frame restriction --- src/shims/windows/foreign_items.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index a840366977..c9db798caa 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -138,6 +138,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.GetUserProfileDirectoryW(token, buf, size)?; this.write_scalar(result, dest)?; } + "GetCurrentProcessId" => { + let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; + let result = this.GetCurrentProcessId()?; + this.write_int(result, dest)?; + } // File related shims "NtWriteFile" => { @@ -743,11 +748,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Any non zero value works for the stdlib. This is just used for stack overflows anyway. this.write_int(1, dest)?; } - "GetCurrentProcessId" if this.frame_in_std() => { - let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - let result = this.GetCurrentProcessId()?; - this.write_int(result, dest)?; - } // this is only callable from std because we know that std ignores the return value "SwitchToThread" if this.frame_in_std() => { let [] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; From b551931fad2d78efbb8d0f3b467faae740be9482 Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Thu, 27 Jun 2024 16:30:33 -0700 Subject: [PATCH 37/61] Switch to the explicit parens version --- miri.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miri.bat b/miri.bat index 92566e0096..6f9a8f38d6 100644 --- a/miri.bat +++ b/miri.bat @@ -6,7 +6,7 @@ set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target :: If any other steps are added, the "|| exit /b" must be appended to early :: return from the script. If not, it will continue execution. cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ - || echo Failed to build miri-script. Is the 'stable' toolchain installed? && exit /b + || (echo Failed to build miri-script. Is the 'stable' toolchain installed? & exit /b) :: Forwards all arguments to this file to the executable. :: We invoke the binary directly to avoid going through rustup, which would set some extra From c2f705c2ab6afad122e1d20259832f887fd4f0df Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 28 Jun 2024 05:13:00 +0000 Subject: [PATCH 38/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 989e9bc6d0..a6096c0bf2 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -7033f9b14a37f4a00766d6c01326600b31f3a716 +9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9 From ea401b8bc2616ae4d8c37e240ccae8c5ef3ff7f0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 28 Jun 2024 08:56:30 +0000 Subject: [PATCH 39/61] Bless clippy --- miri-script/src/commands.rs | 6 ++---- miri-script/src/util.rs | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/miri-script/src/commands.rs b/miri-script/src/commands.rs index 57bdfbad9a..62a3ab2c34 100644 --- a/miri-script/src/commands.rs +++ b/miri-script/src/commands.rs @@ -252,12 +252,11 @@ impl Command { // Fetch given rustc commit. cmd!(sh, "git fetch http://localhost:{JOSH_PORT}/rust-lang/rust.git@{commit}{JOSH_FILTER}.git") .run() - .map_err(|e| { + .inspect_err(|_| { // Try to un-do the previous `git commit`, to leave the repo in the state we found it. cmd!(sh, "git reset --hard HEAD^") .run() .expect("FAILED to clean up again after failed `git fetch`, sorry for that"); - e }) .context("FAILED to fetch new commits, something went wrong (committing the rust-version file has been undone)")?; @@ -545,9 +544,8 @@ impl Command { if let Some(seed_range) = many_seeds { e.run_many_times(seed_range, |sh, seed| { eprintln!("Trying seed: {seed}"); - run_miri(sh, Some(format!("-Zmiri-seed={seed}"))).map_err(|err| { + run_miri(sh, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| { eprintln!("FAILING SEED: {seed}"); - err }) })?; } else { diff --git a/miri-script/src/util.rs b/miri-script/src/util.rs index e9095a45fc..e1b77be192 100644 --- a/miri-script/src/util.rs +++ b/miri-script/src/util.rs @@ -219,10 +219,9 @@ impl MiriEnv { break; } // Run the command with this seed. - run(&local_shell, cur).map_err(|err| { + run(&local_shell, cur).inspect_err(|_| { // If we failed, tell everyone about this. failed.store(true, Ordering::Relaxed); - err })?; // Check if some other command failed (in which case we'll stop as well). if failed.load(Ordering::Relaxed) { From 4573efbd6b112532a9a66f259a2d55d79be5f648 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 28 Jun 2024 09:43:05 +0200 Subject: [PATCH 40/61] readme: tweak wording around soundness --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 87b437a308..b1be596c00 100644 --- a/README.md +++ b/README.md @@ -72,11 +72,13 @@ Further caveats that Miri users should be aware of: when `SeqCst` fences are used that are not actually permitted by the Rust memory model, and it cannot produce all behaviors possibly observable on real hardware. -Moreover, Miri fundamentally cannot tell you whether your code is *sound*. [Soundness] is the property -of never causing undefined behavior when invoked from arbitrary safe code, even in combination with +Moreover, Miri fundamentally cannot ensure that your code is *sound*. [Soundness] is the property of +never causing undefined behavior when invoked from arbitrary safe code, even in combination with other sound code. In contrast, Miri can just tell you if *a particular way of interacting with your -code* (e.g., a test suite) causes any undefined behavior. It is up to you to ensure sufficient -coverage. +code* (e.g., a test suite) causes any undefined behavior *in a particular execution* (of which there +may be many, e.g. when concurrency or other forms of non-determinism are involved). When Miri finds +UB, your code is definitely unsound, but when Miri does not find UB, then you may just have to test +more inputs or more possible non-deterministic choices. [rust]: https://www.rust-lang.org/ [mir]: https://github.com/rust-lang/rfcs/blob/master/text/1211-mir.md From 2f8b09913b3d22a76139463c4d8d4b822812e659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Fri, 28 Jun 2024 20:59:01 +0000 Subject: [PATCH 41/61] Revert "Rollup merge of #126938 - RalfJung:link_section, r=compiler-errors" This reverts commit 5c4ede88c61e746ed5c852d7a7e38ab1a824ae52, reversing changes made to 95332b89187bb6a0c910574cfeff1933b619565a. --- tests/pass/tls/win_tls_callback.rs | 16 ---------------- tests/pass/tls/win_tls_callback.stderr | 1 - 2 files changed, 17 deletions(-) delete mode 100644 tests/pass/tls/win_tls_callback.rs delete mode 100644 tests/pass/tls/win_tls_callback.stderr diff --git a/tests/pass/tls/win_tls_callback.rs b/tests/pass/tls/win_tls_callback.rs deleted file mode 100644 index 99a8de29e9..0000000000 --- a/tests/pass/tls/win_tls_callback.rs +++ /dev/null @@ -1,16 +0,0 @@ -//! Ensure that we call Windows TLS callbacks in the local crate. -//@only-target-windows -// Calling eprintln in the callback seems to (re-)initialize some thread-local storage -// and then leak the memory allocated for that. Let's just ignore these leaks, -// that's not what this test is about. -//@compile-flags: -Zmiri-ignore-leaks - -#[link_section = ".CRT$XLB"] -#[used] // Miri only considers explicitly `#[used]` statics for `lookup_link_section` -pub static CALLBACK: unsafe extern "system" fn(*const (), u32, *const ()) = tls_callback; - -unsafe extern "system" fn tls_callback(_h: *const (), _dw_reason: u32, _pv: *const ()) { - eprintln!("in tls_callback"); -} - -fn main() {} diff --git a/tests/pass/tls/win_tls_callback.stderr b/tests/pass/tls/win_tls_callback.stderr deleted file mode 100644 index 8479558954..0000000000 --- a/tests/pass/tls/win_tls_callback.stderr +++ /dev/null @@ -1 +0,0 @@ -in tls_callback From 9cd99af84047138e3a1a065716f1a93dfec41944 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 29 Jun 2024 05:13:25 +0000 Subject: [PATCH 42/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index a6096c0bf2..fd59ad3b8f 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9 +9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca From 6720f186c941674faf12d7ac0bab6880320dcab9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Jun 2024 12:17:10 +0200 Subject: [PATCH 43/61] iter_exported_symbols: also walk used statics in local crate --- src/helpers.rs | 34 ++++++++++++++++++++------ tests/pass/tls/win_tls_callback.rs | 16 ++++++++++++ tests/pass/tls/win_tls_callback.stderr | 1 + 3 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 tests/pass/tls/win_tls_callback.rs create mode 100644 tests/pass/tls/win_tls_callback.stderr diff --git a/src/helpers.rs b/src/helpers.rs index 3d2b102b27..a7a6f8cfd8 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -14,6 +14,7 @@ use rustc_hir::{ def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}, }; use rustc_index::IndexVec; +use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::dependency_format::Linkage; use rustc_middle::middle::exported_symbols::ExportedSymbol; use rustc_middle::mir; @@ -163,22 +164,39 @@ pub fn iter_exported_symbols<'tcx>( tcx: TyCtxt<'tcx>, mut f: impl FnMut(CrateNum, DefId) -> InterpResult<'tcx>, ) -> InterpResult<'tcx> { + // First, the symbols in the local crate. We can't use `exported_symbols` here as that + // skips `#[used]` statics (since `reachable_set` skips them in binary crates). + // So we walk all HIR items ourselves instead. + let crate_items = tcx.hir_crate_items(()); + for def_id in crate_items.definitions() { + let exported = tcx.def_kind(def_id).has_codegen_attrs() && { + let codegen_attrs = tcx.codegen_fn_attrs(def_id); + codegen_attrs.contains_extern_indicator() + || codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) + || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED) + || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) + }; + if exported { + f(LOCAL_CRATE, def_id.into())?; + } + } + + // Next, all our dependencies. // `dependency_formats` includes all the transitive informations needed to link a crate, // which is what we need here since we need to dig out `exported_symbols` from all transitive // dependencies. let dependency_formats = tcx.dependency_formats(()); + // Find the dependencies of the executable we are running. let dependency_format = dependency_formats .iter() .find(|(crate_type, _)| *crate_type == CrateType::Executable) .expect("interpreting a non-executable crate"); - for cnum in iter::once(LOCAL_CRATE).chain(dependency_format.1.iter().enumerate().filter_map( - |(num, &linkage)| { - // We add 1 to the number because that's what rustc also does everywhere it - // calls `CrateNum::new`... - #[allow(clippy::arithmetic_side_effects)] - (linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1)) - }, - )) { + for cnum in dependency_format.1.iter().enumerate().filter_map(|(num, &linkage)| { + // We add 1 to the number because that's what rustc also does everywhere it + // calls `CrateNum::new`... + #[allow(clippy::arithmetic_side_effects)] + (linkage != Linkage::NotLinked).then_some(CrateNum::new(num + 1)) + }) { // We can ignore `_export_info` here: we are a Rust crate, and everything is exported // from a Rust crate. for &(symbol, _export_info) in tcx.exported_symbols(cnum) { diff --git a/tests/pass/tls/win_tls_callback.rs b/tests/pass/tls/win_tls_callback.rs new file mode 100644 index 0000000000..99a8de29e9 --- /dev/null +++ b/tests/pass/tls/win_tls_callback.rs @@ -0,0 +1,16 @@ +//! Ensure that we call Windows TLS callbacks in the local crate. +//@only-target-windows +// Calling eprintln in the callback seems to (re-)initialize some thread-local storage +// and then leak the memory allocated for that. Let's just ignore these leaks, +// that's not what this test is about. +//@compile-flags: -Zmiri-ignore-leaks + +#[link_section = ".CRT$XLB"] +#[used] // Miri only considers explicitly `#[used]` statics for `lookup_link_section` +pub static CALLBACK: unsafe extern "system" fn(*const (), u32, *const ()) = tls_callback; + +unsafe extern "system" fn tls_callback(_h: *const (), _dw_reason: u32, _pv: *const ()) { + eprintln!("in tls_callback"); +} + +fn main() {} diff --git a/tests/pass/tls/win_tls_callback.stderr b/tests/pass/tls/win_tls_callback.stderr new file mode 100644 index 0000000000..8479558954 --- /dev/null +++ b/tests/pass/tls/win_tls_callback.stderr @@ -0,0 +1 @@ +in tls_callback From 6c977b2215c41e8bb197eb2b4f29fc63d048d272 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 29 Jun 2024 17:25:44 -0400 Subject: [PATCH 44/61] Fix FnMut/Fn shim for coroutine-closures that capture references --- tests/pass/async-closure.rs | 21 ++++++++++++++------- tests/pass/async-closure.stdout | 6 ++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/pass/async-closure.rs b/tests/pass/async-closure.rs index 2f7ec2b9e6..721af57888 100644 --- a/tests/pass/async-closure.rs +++ b/tests/pass/async-closure.rs @@ -1,7 +1,8 @@ #![feature(async_closure, noop_waker, async_fn_traits)] +#![allow(unused)] use std::future::Future; -use std::ops::{AsyncFnMut, AsyncFnOnce}; +use std::ops::{AsyncFn, AsyncFnMut, AsyncFnOnce}; use std::pin::pin; use std::task::*; @@ -17,6 +18,10 @@ pub fn block_on(fut: impl Future) -> T { } } +async fn call(f: &mut impl AsyncFn(i32)) { + f(0).await; +} + async fn call_mut(f: &mut impl AsyncFnMut(i32)) { f(0).await; } @@ -26,10 +31,10 @@ async fn call_once(f: impl AsyncFnOnce(i32)) { } async fn call_normal>(f: &impl Fn(i32) -> F) { - f(0).await; + f(1).await; } -async fn call_normal_once>(f: impl FnOnce(i32) -> F) { +async fn call_normal_mut>(f: &mut impl FnMut(i32) -> F) { f(1).await; } @@ -39,14 +44,16 @@ pub fn main() { let mut async_closure = async move |a: i32| { println!("{a} {b}"); }; + call(&mut async_closure).await; call_mut(&mut async_closure).await; call_once(async_closure).await; - // No-capture closures implement `Fn`. - let async_closure = async move |a: i32| { - println!("{a}"); + let b = 2i32; + let mut async_closure = async |a: i32| { + println!("{a} {b}"); }; call_normal(&async_closure).await; - call_normal_once(async_closure).await; + call_normal_mut(&mut async_closure).await; + call_once(async_closure).await; }); } diff --git a/tests/pass/async-closure.stdout b/tests/pass/async-closure.stdout index 7baae1aa94..217944c84a 100644 --- a/tests/pass/async-closure.stdout +++ b/tests/pass/async-closure.stdout @@ -1,4 +1,6 @@ 0 2 +0 2 +1 2 +1 2 +1 2 1 2 -0 -1 From 050a7cdff9dd19e9e1287dd5792cb8ebefaaa263 Mon Sep 17 00:00:00 2001 From: Adwin White Date: Mon, 24 Jun 2024 14:40:04 +0800 Subject: [PATCH 45/61] add syscall `dup()` --- src/shims/unix/fd.rs | 28 ++++++++++++++++++++++++++++ src/shims/unix/foreign_items.rs | 13 +++++++++++++ tests/pass-dep/libc/libc-fs.rs | 26 ++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index 599f78e712..87e20954a7 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -273,6 +273,34 @@ impl FdTable { impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + + match this.machine.fds.dup(old_fd) { + Some(dup_fd) => Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0)), + None => this.fd_not_found(), + } + } + + fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + + match this.machine.fds.dup(old_fd) { + Some(dup_fd) => { + if new_fd != old_fd { + // Close new_fd if it is previously opened. + // If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive. + if let Some(file_descriptor) = this.machine.fds.fds.insert(new_fd, dup_fd) { + // Ignore close error (not interpreter's) according to dup2() doc. + file_descriptor.close(this.machine.communicate())?.ok(); + } + } + Ok(new_fd) + } + None => this.fd_not_found(), + } + } + fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index 53ad40cfd2..2421f9244f 100644 --- a/src/shims/unix/foreign_items.rs +++ b/src/shims/unix/foreign_items.rs @@ -115,6 +115,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.fcntl(args)?; this.write_scalar(Scalar::from_i32(result), dest)?; } + "dup" => { + let [old_fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let old_fd = this.read_scalar(old_fd)?.to_i32()?; + let new_fd = this.dup(old_fd)?; + this.write_scalar(Scalar::from_i32(new_fd), dest)?; + } + "dup2" => { + let [old_fd, new_fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let old_fd = this.read_scalar(old_fd)?.to_i32()?; + let new_fd = this.read_scalar(new_fd)?.to_i32()?; + let result = this.dup2(old_fd, new_fd)?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } // File and file system access "open" | "open64" => { diff --git a/tests/pass-dep/libc/libc-fs.rs b/tests/pass-dep/libc/libc-fs.rs index 80c9757e9c..da685e5c6b 100644 --- a/tests/pass-dep/libc/libc-fs.rs +++ b/tests/pass-dep/libc/libc-fs.rs @@ -15,6 +15,7 @@ use std::path::PathBuf; mod utils; fn main() { + test_dup(); test_dup_stdout_stderr(); test_canonicalize_too_long(); test_rename(); @@ -74,6 +75,31 @@ fn test_dup_stdout_stderr() { } } +fn test_dup() { + let bytes = b"dup and dup2"; + let path = utils::prepare_with_content("miri_test_libc_dup.txt", bytes); + + let mut name = path.into_os_string(); + name.push("\0"); + let name_ptr = name.as_bytes().as_ptr().cast::(); + unsafe { + let fd = libc::open(name_ptr, libc::O_RDONLY); + let mut first_buf = [0u8; 4]; + libc::read(fd, first_buf.as_mut_ptr() as *mut libc::c_void, 4); + assert_eq!(&first_buf, b"dup "); + + let new_fd = libc::dup(fd); + let mut second_buf = [0u8; 4]; + libc::read(new_fd, second_buf.as_mut_ptr() as *mut libc::c_void, 4); + assert_eq!(&second_buf, b"and "); + + let new_fd2 = libc::dup2(fd, 8); + let mut third_buf = [0u8; 4]; + libc::read(new_fd2, third_buf.as_mut_ptr() as *mut libc::c_void, 4); + assert_eq!(&third_buf, b"dup2"); + } +} + fn test_canonicalize_too_long() { // Make sure we get an error for long paths. let too_long = "x/".repeat(libc::PATH_MAX.try_into().unwrap()); From 332db9a710f258f8a16464a02c91d40d239e3f11 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 1 Jul 2024 17:40:00 +0000 Subject: [PATCH 46/61] Use the symbol_name query instead of trying to infer from the link_name attribute This prevents the calculated name from going out of sync with exported_symbols. It also avoids having to special case the panic_impl lang item. --- src/helpers.rs | 10 +--------- src/machine.rs | 4 ++-- src/shims/foreign_items.rs | 15 --------------- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index a7a6f8cfd8..590e8984e9 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -24,7 +24,7 @@ use rustc_middle::ty::{ FloatTy, IntTy, Ty, TyCtxt, UintTy, }; use rustc_session::config::CrateType; -use rustc_span::{sym, Span, Symbol}; +use rustc_span::{Span, Symbol}; use rustc_target::abi::{Align, FieldIdx, FieldsShape, Size, Variants}; use rustc_target::spec::abi::Abi; @@ -1182,14 +1182,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.alloc_mark_immutable(provenance.get_alloc_id().unwrap()).unwrap(); } - fn item_link_name(&self, def_id: DefId) -> Symbol { - let tcx = self.eval_context_ref().tcx; - match tcx.get_attrs(def_id, sym::link_name).filter_map(|a| a.value_str()).next() { - Some(name) => name, - None => tcx.item_name(def_id), - } - } - /// Converts `src` from floating point to integer type `dest_ty` /// after rounding with mode `round`. /// Returns `None` if `f` is NaN or out of range. diff --git a/src/machine.rs b/src/machine.rs index 0d91279f9f..e321237bb4 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -954,7 +954,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { // foreign function // Any needed call to `goto_block` will be performed by `emulate_foreign_item`. let args = ecx.copy_fn_args(args); // FIXME: Should `InPlace` arguments be reset to uninit? - let link_name = ecx.item_link_name(instance.def_id()); + let link_name = Symbol::intern(ecx.tcx.symbol_name(instance).name); return ecx.emulate_foreign_item(link_name, abi, &args, dest, ret, unwind); } @@ -1050,7 +1050,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { ecx: &MiriInterpCx<'tcx>, def_id: DefId, ) -> InterpResult<'tcx, StrictPointer> { - let link_name = ecx.item_link_name(def_id); + let link_name = Symbol::intern(ecx.tcx.symbol_name(Instance::mono(*ecx.tcx, def_id)).name); if let Some(&ptr) = ecx.machine.extern_statics.get(&link_name) { // Various parts of the engine rely on `get_alloc_info` for size and alignment // information. That uses the type information of this static. diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index f9ccc6ad4d..9004f7efc8 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -46,24 +46,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { unwind: mir::UnwindAction, ) -> InterpResult<'tcx, Option<(&'tcx mir::Body<'tcx>, ty::Instance<'tcx>)>> { let this = self.eval_context_mut(); - let tcx = this.tcx.tcx; // Some shims forward to other MIR bodies. match link_name.as_str() { - // This matches calls to the foreign item `panic_impl`. - // The implementation is provided by the function with the `#[panic_handler]` attribute. - "panic_impl" => { - // We don't use `check_shim` here because we are just forwarding to the lang - // item. Argument count checking will be performed when the returned `Body` is - // called. - this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?; - let panic_impl_id = tcx.lang_items().panic_impl().unwrap(); - let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id); - return Ok(Some(( - this.load_mir(panic_impl_instance.def, None)?, - panic_impl_instance, - ))); - } "__rust_alloc_error_handler" => { // Forward to the right symbol that implements this function. let Some(handler_kind) = this.tcx.alloc_error_handler_kind(()) else { From 634879942ea039fdc07b865a3cfcd983db1ec497 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2024 08:21:44 +0200 Subject: [PATCH 47/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index fd59ad3b8f..912aa11ded 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca +7d97c59438e933e86f557ed999da3b8dfc6855a7 From 07d1c773b8f090e66cf3a65b670e20e2e4ec8720 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 1 Jul 2024 19:09:25 +0000 Subject: [PATCH 48/61] Allow _Unwind_RaiseException with MinGW --- src/shims/windows/foreign_items.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index c9db798caa..71f6a2bc03 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -758,6 +758,22 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_null(dest)?; } + "_Unwind_RaiseException" => { + // This is not formally part of POSIX, but it is very wide-spread on POSIX systems. + // It was originally specified as part of the Itanium C++ ABI: + // https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html#base-throw. + // MinGW implements _Unwind_RaiseException on top of SEH exceptions. + if this.tcx.sess.target.env != "gnu" { + throw_unsup_format!( + "`_Unwind_RaiseException` is not supported on non-MinGW Windows", + ); + } + // This function looks and behaves excatly like miri_start_unwind. + let [payload] = this.check_shim(abi, Abi::C { unwind: true }, link_name, args)?; + this.handle_miri_start_unwind(payload)?; + return Ok(EmulateItemResult::NeedsUnwind); + } + _ => return Ok(EmulateItemResult::NotSupported), } From 71f864d7da773b5d3f5cbdcdc6011522e861a8ae Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2024 21:03:13 +0200 Subject: [PATCH 49/61] use let-else to avoid rightwards drift --- src/shims/unix/fd.rs | 45 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs index 87e20954a7..7f6a097810 100644 --- a/src/shims/unix/fd.rs +++ b/src/shims/unix/fd.rs @@ -276,29 +276,27 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - match this.machine.fds.dup(old_fd) { - Some(dup_fd) => Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0)), - None => this.fd_not_found(), - } + let Some(dup_fd) = this.machine.fds.dup(old_fd) else { + return this.fd_not_found(); + }; + Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, 0)) } fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); - match this.machine.fds.dup(old_fd) { - Some(dup_fd) => { - if new_fd != old_fd { - // Close new_fd if it is previously opened. - // If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive. - if let Some(file_descriptor) = this.machine.fds.fds.insert(new_fd, dup_fd) { - // Ignore close error (not interpreter's) according to dup2() doc. - file_descriptor.close(this.machine.communicate())?.ok(); - } - } - Ok(new_fd) + let Some(dup_fd) = this.machine.fds.dup(old_fd) else { + return this.fd_not_found(); + }; + if new_fd != old_fd { + // Close new_fd if it is previously opened. + // If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive. + if let Some(file_descriptor) = this.machine.fds.fds.insert(new_fd, dup_fd) { + // Ignore close error (not interpreter's) according to dup2() doc. + file_descriptor.close(this.machine.communicate())?.ok(); } - None => this.fd_not_found(), } + Ok(new_fd) } fn fcntl(&mut self, args: &[OpTy<'tcx>]) -> InterpResult<'tcx, i32> { @@ -362,14 +360,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let fd = this.read_scalar(fd_op)?.to_i32()?; - Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.remove(fd) { - let result = file_descriptor.close(this.machine.communicate())?; - // return `0` if close is successful - let result = result.map(|()| 0i32); - this.try_unwrap_io_result(result)? - } else { - this.fd_not_found()? - })) + let Some(file_descriptor) = this.machine.fds.remove(fd) else { + return Ok(Scalar::from_i32(this.fd_not_found()?)); + }; + let result = file_descriptor.close(this.machine.communicate())?; + // return `0` if close is successful + let result = result.map(|()| 0i32); + Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) } /// Function used when a file descriptor does not exist. It returns `Ok(-1)`and sets From 94b832f755f00050d77f3d5bb351540339d6f898 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2024 21:05:22 +0200 Subject: [PATCH 50/61] Miri function identity hack: account for possible inlining --- src/shims/backtrace.rs | 2 +- tests/pass/function_pointers.rs | 3 ++- tests/pass/issues/issue-91636.rs | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shims/backtrace.rs b/src/shims/backtrace.rs index 06be9c1e63..24a4b5f26a 100644 --- a/src/shims/backtrace.rs +++ b/src/shims/backtrace.rs @@ -119,7 +119,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let (alloc_id, offset, _prov) = this.ptr_get_alloc_id(ptr)?; // This has to be an actual global fn ptr, not a dlsym function. - let fn_instance = if let Some(GlobalAlloc::Function(instance)) = + let fn_instance = if let Some(GlobalAlloc::Function { instance, .. }) = this.tcx.try_get_global_alloc(alloc_id) { instance diff --git a/tests/pass/function_pointers.rs b/tests/pass/function_pointers.rs index 36679b7180..2aa3ebf2dd 100644 --- a/tests/pass/function_pointers.rs +++ b/tests/pass/function_pointers.rs @@ -23,6 +23,7 @@ fn h(i: i32, j: i32) -> i32 { j * i * 7 } +#[inline(never)] fn i() -> i32 { 73 } @@ -77,7 +78,7 @@ fn main() { assert_eq!(indirect_mut3(h), 210); assert_eq!(indirect_once3(h), 210); // Check that `i` always has the same address. This is not guaranteed - // but Miri currently uses a fixed address for monomorphic functions. + // but Miri currently uses a fixed address for non-inlineable monomorphic functions. assert!(return_fn_ptr(i) == i); assert!(return_fn_ptr(i) as unsafe fn() -> i32 == i as fn() -> i32 as unsafe fn() -> i32); // Miri gives different addresses to different reifications of a generic function. diff --git a/tests/pass/issues/issue-91636.rs b/tests/pass/issues/issue-91636.rs index 21000bb68d..0037016581 100644 --- a/tests/pass/issues/issue-91636.rs +++ b/tests/pass/issues/issue-91636.rs @@ -10,6 +10,7 @@ impl Function { } } +#[inline(never)] fn dummy(_: &str) {} fn main() { From 1baf4bab8dd1743b2df4063e01ee8bc7f999b40b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 2 Jul 2024 15:55:17 -0400 Subject: [PATCH 51/61] Instance::resolve -> Instance::try_resolve, and other nits --- src/eval.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/eval.rs b/src/eval.rs index c0827cce26..9142b8b5fd 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -375,7 +375,7 @@ pub fn create_ecx<'tcx>( }); let main_ret_ty = tcx.fn_sig(entry_id).no_bound_vars().unwrap().output(); let main_ret_ty = main_ret_ty.no_bound_vars().unwrap(); - let start_instance = ty::Instance::resolve( + let start_instance = ty::Instance::try_resolve( tcx, ty::ParamEnv::reveal_all(), start_id, From c84a28a03c9dc663afbb0af3f478defb3564cc0a Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Mon, 1 Jul 2024 21:01:49 +0200 Subject: [PATCH 52/61] Implement the `_mm256_zeroupper` and `_mm256_zeroall` intrinsics --- src/shims/x86/avx.rs | 11 +++++++++++ tests/pass/shims/x86/intrinsics-x86-avx.rs | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/src/shims/x86/avx.rs b/src/shims/x86/avx.rs index 0d2977b7b6..f36bb4826e 100644 --- a/src/shims/x86/avx.rs +++ b/src/shims/x86/avx.rs @@ -338,6 +338,17 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(Scalar::from_i32(res.into()), dest)?; } + // Used to implement the `_mm256_zeroupper` and `_mm256_zeroall` functions. + // These function clear out the upper 128 bits of all avx registers or + // zero out all avx registers respectively. + "vzeroupper" | "vzeroall" => { + // These functions are purely a performance hint for the CPU. + // Any registers currently in use will be saved beforehand by the + // compiler, making these functions no-ops. + + // The only thing that needs to be ensured is the correct calling convention. + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + } _ => return Ok(EmulateItemResult::NotSupported), } Ok(EmulateItemResult::NeedsReturn) diff --git a/tests/pass/shims/x86/intrinsics-x86-avx.rs b/tests/pass/shims/x86/intrinsics-x86-avx.rs index 7d43cc596a..728f57d48f 100644 --- a/tests/pass/shims/x86/intrinsics-x86-avx.rs +++ b/tests/pass/shims/x86/intrinsics-x86-avx.rs @@ -1342,6 +1342,11 @@ unsafe fn test_avx() { assert_eq!(r, 1); } test_mm_testnzc_ps(); + + // These intrinsics are functionally no-ops. The only thing + // that needs to be tested is that they can be executed. + _mm256_zeroupper(); + _mm256_zeroall(); } #[target_feature(enable = "sse2")] From 86b25beb7936a43f5c78b78e2e79d93a4b3c1f79 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 4 Jul 2024 04:54:26 +0000 Subject: [PATCH 53/61] Preparing for merge from rustc --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 912aa11ded..5a35166769 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -7d97c59438e933e86f557ed999da3b8dfc6855a7 +66b4f0021bfb11a8c20d084c99a40f4a78ce1d38 From 06153a8d99b8e77c65b8f32071bc11f7112e2a2c Mon Sep 17 00:00:00 2001 From: Johannes Hostert Date: Wed, 3 Jul 2024 22:56:31 +0200 Subject: [PATCH 54/61] TB: Make FnEntry access on protected locations be a write under certain circumstances --- .../tree_borrows/diagnostics.rs | 7 ++-- src/borrow_tracker/tree_borrows/mod.rs | 19 ++------- src/borrow_tracker/tree_borrows/perms.rs | 4 ++ src/borrow_tracker/tree_borrows/tree.rs | 41 +++++++++++-------- 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/borrow_tracker/tree_borrows/diagnostics.rs index 8abc8530f7..498b7dc3e4 100644 --- a/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -19,7 +19,7 @@ pub enum AccessCause { Explicit(AccessKind), Reborrow, Dealloc, - FnExit, + FnExit(AccessKind), } impl fmt::Display for AccessCause { @@ -28,7 +28,8 @@ impl fmt::Display for AccessCause { Self::Explicit(kind) => write!(f, "{kind}"), Self::Reborrow => write!(f, "reborrow"), Self::Dealloc => write!(f, "deallocation"), - Self::FnExit => write!(f, "protector release"), + Self::FnExit(AccessKind::Read) => write!(f, "protector release read"), + Self::FnExit(AccessKind::Write) => write!(f, "protector release write"), } } } @@ -40,7 +41,7 @@ impl AccessCause { Self::Explicit(kind) => format!("{rel} {kind}"), Self::Reborrow => format!("reborrow (acting as a {rel} read access)"), Self::Dealloc => format!("deallocation (acting as a {rel} write access)"), - Self::FnExit => format!("protector release (acting as a {rel} read access)"), + Self::FnExit(kind) => format!("protector release (acting as a {rel} {kind})"), } } } diff --git a/src/borrow_tracker/tree_borrows/mod.rs b/src/borrow_tracker/tree_borrows/mod.rs index 77e003ab8a..8607438408 100644 --- a/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/borrow_tracker/tree_borrows/mod.rs @@ -68,13 +68,11 @@ impl<'tcx> Tree { let global = machine.borrow_tracker.as_ref().unwrap(); let span = machine.current_span(); self.perform_access( - access_kind, tag, - Some(range), + Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))), global, alloc_id, span, - diagnostics::AccessCause::Explicit(access_kind), ) } @@ -115,15 +113,8 @@ impl<'tcx> Tree { alloc_id: AllocId, // diagnostics ) -> InterpResult<'tcx> { let span = machine.current_span(); - self.perform_access( - AccessKind::Read, - tag, - None, // no specified range because it occurs on the entire allocation - global, - alloc_id, - span, - diagnostics::AccessCause::FnExit, - ) + // `None` makes it the magic on-protector-end operation + self.perform_access(tag, None, global, alloc_id, span) } } @@ -297,13 +288,11 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // All reborrows incur a (possibly zero-sized) read access to the parent tree_borrows.perform_access( - AccessKind::Read, orig_tag, - Some(range), + Some((range, AccessKind::Read, diagnostics::AccessCause::Reborrow)), this.machine.borrow_tracker.as_ref().unwrap(), alloc_id, this.machine.current_span(), - diagnostics::AccessCause::Reborrow, )?; // Record the parent-child pair in the tree. tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; diff --git a/src/borrow_tracker/tree_borrows/perms.rs b/src/borrow_tracker/tree_borrows/perms.rs index fb3a4c8dad..7aa9c3e862 100644 --- a/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/borrow_tracker/tree_borrows/perms.rs @@ -186,6 +186,10 @@ impl Permission { pub fn is_disabled(&self) -> bool { self.inner == Disabled } + /// Check if `self` is the post-child-write state of a pointer (is `Active`). + pub fn is_active(&self) -> bool { + self.inner == Active + } /// Default initial permission of the root of a new tree at inbounds positions. /// Must *only* be used for the root, this is not in general an "initial" permission! diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index ff4589657a..90bd110321 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -530,13 +530,11 @@ impl<'tcx> Tree { span: Span, // diagnostics ) -> InterpResult<'tcx> { self.perform_access( - AccessKind::Write, tag, - Some(access_range), + Some((access_range, AccessKind::Write, diagnostics::AccessCause::Dealloc)), global, alloc_id, span, - diagnostics::AccessCause::Dealloc, )?; for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size) { TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms } @@ -570,12 +568,16 @@ impl<'tcx> Tree { } /// Map the per-node and per-location `LocationState::perform_access` - /// to each location of `access_range`, on every tag of the allocation. + /// to each location of the first component of `access_range_and_kind`, + /// on every tag of the allocation. /// - /// If `access_range` is `None`, this is interpreted as the special + /// If `access_range_and_kind` is `None`, this is interpreted as the special /// access that is applied on protector release: /// - the access will be applied only to initialized locations of the allocation, - /// - and it will not be visible to children. + /// - it will not be visible to children, + /// - it will be recorded as a `FnExit` diagnostic access + /// - and it will be a read except if the location is `Active`, i.e. has been written to, + /// in which case it will be a write. /// /// `LocationState::perform_access` will take care of raising transition /// errors and updating the `initialized` status of each location, @@ -585,13 +587,11 @@ impl<'tcx> Tree { /// - recording the history. pub fn perform_access( &mut self, - access_kind: AccessKind, tag: BorTag, - access_range: Option, + access_range_and_kind: Option<(AllocRange, AccessKind, diagnostics::AccessCause)>, global: &GlobalState, - alloc_id: AllocId, // diagnostics - span: Span, // diagnostics - access_cause: diagnostics::AccessCause, // diagnostics + alloc_id: AllocId, // diagnostics + span: Span, // diagnostics ) -> InterpResult<'tcx> { use std::ops::Range; // Performs the per-node work: @@ -605,6 +605,8 @@ impl<'tcx> Tree { // `perms_range` is only for diagnostics (it is the range of // the `RangeMap` on which we are currently working). let node_app = |perms_range: Range, + access_kind: AccessKind, + access_cause: diagnostics::AccessCause, args: NodeAppArgs<'_>| -> Result { let NodeAppArgs { node, mut perm, rel_pos } = args; @@ -618,14 +620,13 @@ impl<'tcx> Tree { let protected = global.borrow().protected_tags.contains_key(&node.tag); let transition = old_state.perform_access(access_kind, rel_pos, protected)?; - // Record the event as part of the history if !transition.is_noop() { node.debug_info.history.push(diagnostics::Event { transition, is_foreign: rel_pos.is_foreign(), access_cause, - access_range, + access_range: access_range_and_kind.map(|x| x.0), transition_range: perms_range, span, }); @@ -636,6 +637,7 @@ impl<'tcx> Tree { // Error handler in case `node_app` goes wrong. // Wraps the faulty transition in more context for diagnostics. let err_handler = |perms_range: Range, + access_cause: diagnostics::AccessCause, args: ErrHandlerArgs<'_, TransitionError>| -> InterpError<'tcx> { let ErrHandlerArgs { error_kind, conflicting_info, accessed_info } = args; @@ -650,7 +652,7 @@ impl<'tcx> Tree { .build() }; - if let Some(access_range) = access_range { + if let Some((access_range, access_kind, access_cause)) = access_range_and_kind { // Default branch: this is a "normal" access through a known range. // We iterate over affected locations and traverse the tree for each of them. for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size) @@ -658,8 +660,8 @@ impl<'tcx> Tree { TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms } .traverse_parents_this_children_others( tag, - |args| node_app(perms_range.clone(), args), - |args| err_handler(perms_range.clone(), args), + |args| node_app(perms_range.clone(), access_kind, access_cause, args), + |args| err_handler(perms_range.clone(), access_cause, args), )?; } } else { @@ -678,11 +680,14 @@ impl<'tcx> Tree { if let Some(p) = perms.get(idx) && p.initialized { + let access_kind = + if p.permission.is_active() { AccessKind::Write } else { AccessKind::Read }; + let access_cause = diagnostics::AccessCause::FnExit(access_kind); TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms } .traverse_nonchildren( tag, - |args| node_app(perms_range.clone(), args), - |args| err_handler(perms_range.clone(), args), + |args| node_app(perms_range.clone(), access_kind, access_cause, args), + |args| err_handler(perms_range.clone(), access_cause, args), )?; } } From 11742b99808e5d9a9c1cda74a00e59749c410372 Mon Sep 17 00:00:00 2001 From: Johannes Hostert Date: Thu, 4 Jul 2024 10:59:30 +0200 Subject: [PATCH 55/61] Add UI test for protector end write semantics --- .../fail/tree_borrows/protector-write-lazy.rs | 35 +++++++++++++++++++ .../tree_borrows/protector-write-lazy.stderr | 27 ++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 tests/fail/tree_borrows/protector-write-lazy.rs create mode 100644 tests/fail/tree_borrows/protector-write-lazy.stderr diff --git a/tests/fail/tree_borrows/protector-write-lazy.rs b/tests/fail/tree_borrows/protector-write-lazy.rs new file mode 100644 index 0000000000..238f6dba9d --- /dev/null +++ b/tests/fail/tree_borrows/protector-write-lazy.rs @@ -0,0 +1,35 @@ +//@compile-flags: -Zmiri-tree-borrows +// This test tests that TB's protector end semantics correctly ensure +// that protected activated writes can be reordered. +fn the_other_function(ref_to_fst_elem: &mut i32, ptr_to_vec: *mut i32) -> *mut i32 { + // Activate the reference. Afterwards, we should be able to reorder arbitrary writes. + *ref_to_fst_elem = 0; + // Here is such an arbitrary write. + // It could be moved down after the retag, in which case the `funky_ref` would be invalidated. + // We need to ensure that the `funky_ptr` is unusable even if the write to `ref_to_fst_elem` + // happens before the retag. + *ref_to_fst_elem = 42; + // this creates a reference that is Reserved Lazy on the first element (offset 0). + // It does so by doing a proper retag on the second element (offset 1), which is fine + // since nothing else happens at that offset, but the lazy init mechanism means it's + // also reserved at offset 0, but not initialized. + let funky_ptr_lazy_on_fst_elem = + unsafe { (&mut *(ptr_to_vec.wrapping_add(1))) as *mut i32 }.wrapping_sub(1); + // If we write to `ref_to_fst_elem` here, then any further access to `funky_ptr_lazy_on_fst_elem` would + // definitely be UB. Since the compiler ought to be able to reorder the write of `42` above down to + // here, that means we want this program to also be UB. + return funky_ptr_lazy_on_fst_elem; +} + +fn main() { + let mut v = vec![0, 1]; + // get a pointer to the root of the allocation + // note that it's not important it's the actual root, what matters is that it's a parent + // of both references that will be created + let ptr_to_vec = v.as_mut_ptr(); + let ref_to_fst_elem = unsafe { &mut *ptr_to_vec }; + let funky_ptr_lazy_on_fst_elem = the_other_function(ref_to_fst_elem, ptr_to_vec); + // now we try to use the funky lazy pointer. + // It should be UB, since the write-on-protector-end should disable it. + unsafe { println!("Value of funky: {}", *funky_ptr_lazy_on_fst_elem) } //~ ERROR: /reborrow through .* is forbidden/ +} diff --git a/tests/fail/tree_borrows/protector-write-lazy.stderr b/tests/fail/tree_borrows/protector-write-lazy.stderr new file mode 100644 index 0000000000..955abd144c --- /dev/null +++ b/tests/fail/tree_borrows/protector-write-lazy.stderr @@ -0,0 +1,27 @@ +error: Undefined Behavior: reborrow through at ALLOC[0x0] is forbidden + --> $DIR/protector-write-lazy.rs:LL:CC + | +LL | unsafe { println!("Value of funky: {}", *funky_ptr_lazy_on_fst_elem) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ reborrow through at ALLOC[0x0] is forbidden + | + = 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: the accessed tag has state Disabled which forbids this reborrow (acting as a child read access) +help: the accessed tag was created here, in the initial state Reserved + --> $DIR/protector-write-lazy.rs:LL:CC + | +LL | unsafe { (&mut *(ptr_to_vec.wrapping_add(1))) as *mut i32 }.wrapping_sub(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the accessed tag later transitioned to Disabled due to a protector release (acting as a foreign write access) on every location previously accessed by this tag + --> $DIR/protector-write-lazy.rs:LL:CC + | +LL | } + | ^ + = help: this transition corresponds to a loss of read and write permissions + = note: BACKTRACE (of the first span): + = note: inside `main` at $DIR/protector-write-lazy.rs:LL:CC + = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + From 5d110375e7da52a707a94eac642daa4af5bcb0b4 Mon Sep 17 00:00:00 2001 From: Johannes Hostert Date: Thu, 4 Jul 2024 12:01:49 +0200 Subject: [PATCH 56/61] TB: protector end semantics never causes immediate UB --- src/borrow_tracker/tree_borrows/diagnostics.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/borrow_tracker/tree_borrows/diagnostics.rs index 498b7dc3e4..a753de28a0 100644 --- a/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -28,8 +28,11 @@ impl fmt::Display for AccessCause { Self::Explicit(kind) => write!(f, "{kind}"), Self::Reborrow => write!(f, "reborrow"), Self::Dealloc => write!(f, "deallocation"), - Self::FnExit(AccessKind::Read) => write!(f, "protector release read"), - Self::FnExit(AccessKind::Write) => write!(f, "protector release write"), + // This is dead code, since the protector release access itself can never + // cause UB (while the protector is active, if some other access invalidates + // further use of the protected tag, that is immediate UB). + // Describing the cause of UB is the only time this function is called. + Self::FnExit(_) => unreachable!("protector accesses can never be the source of UB"), } } } From 83fae11a12bbe25b998937ef3e9bfcec558e01de Mon Sep 17 00:00:00 2001 From: lukas Date: Thu, 4 Jul 2024 16:47:20 +0200 Subject: [PATCH 57/61] Mark format! with must_use hint --- tests/pass/intptrcast.rs | 4 ++-- tests/pass/packed_struct.rs | 2 +- tests/pass/shims/fs.rs | 4 ++-- tests/pass/shims/io.rs | 2 +- tests/pass/vecdeque.rs | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/pass/intptrcast.rs b/tests/pass/intptrcast.rs index 4e9fa12c18..fb1a1dfae5 100644 --- a/tests/pass/intptrcast.rs +++ b/tests/pass/intptrcast.rs @@ -35,7 +35,7 @@ fn cast_dangling() { fn format() { // Pointer string formatting! We can't check the output as it changes when libstd changes, // but we can make sure Miri does not error. - format!("{:?}", &mut 13 as *mut _); + let _ = format!("{:?}", &mut 13 as *mut _); } fn transmute() { @@ -52,7 +52,7 @@ fn ptr_bitops1() { let one = bytes.as_ptr().wrapping_offset(1); let three = bytes.as_ptr().wrapping_offset(3); let res = (one as usize) | (three as usize); - format!("{}", res); + let _ = format!("{}", res); } fn ptr_bitops2() { diff --git a/tests/pass/packed_struct.rs b/tests/pass/packed_struct.rs index b86235e0c6..039eb5adef 100644 --- a/tests/pass/packed_struct.rs +++ b/tests/pass/packed_struct.rs @@ -138,7 +138,7 @@ fn test_derive() { assert_eq!(x.partial_cmp(&y).unwrap(), x.cmp(&y)); x.hash(&mut DefaultHasher::new()); P::default(); - format!("{:?}", x); + let _ = format!("{:?}", x); } fn main() { diff --git a/tests/pass/shims/fs.rs b/tests/pass/shims/fs.rs index 35980fad15..16d3e8cab3 100644 --- a/tests/pass/shims/fs.rs +++ b/tests/pass/shims/fs.rs @@ -202,7 +202,7 @@ fn test_errors() { // Opening a non-existing file should fail with a "not found" error. assert_eq!(ErrorKind::NotFound, File::open(&path).unwrap_err().kind()); // Make sure we can also format this. - format!("{0}: {0:?}", File::open(&path).unwrap_err()); + let _ = format!("{0}: {0:?}", File::open(&path).unwrap_err()); // Removing a non-existing file should fail with a "not found" error. assert_eq!(ErrorKind::NotFound, remove_file(&path).unwrap_err().kind()); // Reading the metadata of a non-existing file should fail with a "not found" error. @@ -301,5 +301,5 @@ fn test_from_raw_os_error() { let error = Error::from_raw_os_error(code); assert!(matches!(error.kind(), ErrorKind::Uncategorized)); // Make sure we can also format this. - format!("{error:?}"); + let _ = format!("{error:?}"); } diff --git a/tests/pass/shims/io.rs b/tests/pass/shims/io.rs index d20fc75b79..420ef95a0c 100644 --- a/tests/pass/shims/io.rs +++ b/tests/pass/shims/io.rs @@ -15,5 +15,5 @@ fn main() { panic!("unsupported OS") }; let err = io::Error::from_raw_os_error(raw_os_error); - format!("{err}: {err:?}"); + let _ = format!("{err}: {err:?}"); } diff --git a/tests/pass/vecdeque.rs b/tests/pass/vecdeque.rs index 77c4ca5a04..9153c428e1 100644 --- a/tests/pass/vecdeque.rs +++ b/tests/pass/vecdeque.rs @@ -31,8 +31,8 @@ fn main() { } // Regression test for Debug impl's - format!("{:?} {:?}", dst, dst.iter()); - format!("{:?}", VecDeque::::new().iter()); + let _ = format!("{:?} {:?}", dst, dst.iter()); + let _ = format!("{:?}", VecDeque::::new().iter()); for a in dst { assert_eq!(*a, 2); From d0a815d85676110f561d509253ceddc1cb9a53df Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 17 Jun 2024 15:37:33 +0000 Subject: [PATCH 58/61] add miri tests and a fixme --- tests/fail/tail_calls/cc-mismatch.rs | 10 +++++ tests/fail/tail_calls/cc-mismatch.stderr | 25 ++++++++++++ .../fail/tail_calls/signature-mismatch-arg.rs | 17 ++++++++ .../tail_calls/signature-mismatch-arg.stderr | 17 ++++++++ tests/pass/tail_call.rs | 39 +++++++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 tests/fail/tail_calls/cc-mismatch.rs create mode 100644 tests/fail/tail_calls/cc-mismatch.stderr create mode 100644 tests/fail/tail_calls/signature-mismatch-arg.rs create mode 100644 tests/fail/tail_calls/signature-mismatch-arg.stderr create mode 100644 tests/pass/tail_call.rs diff --git a/tests/fail/tail_calls/cc-mismatch.rs b/tests/fail/tail_calls/cc-mismatch.rs new file mode 100644 index 0000000000..5f00dbf257 --- /dev/null +++ b/tests/fail/tail_calls/cc-mismatch.rs @@ -0,0 +1,10 @@ +//@error-in-other-file: Undefined Behavior: calling a function with calling convention C using calling convention Rust +#![feature(explicit_tail_calls)] +#![allow(incomplete_features)] + +fn main() { + let f = unsafe { std::mem::transmute::(f) }; + become f(); +} + +extern "C" fn f() {} diff --git a/tests/fail/tail_calls/cc-mismatch.stderr b/tests/fail/tail_calls/cc-mismatch.stderr new file mode 100644 index 0000000000..708972e6ef --- /dev/null +++ b/tests/fail/tail_calls/cc-mismatch.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: calling a function with calling convention C using calling convention Rust + --> RUSTLIB/core/src/ops/function.rs:LL:CC + | +LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ calling a function with calling convention C using calling convention Rust + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `>::call_once - shim(fn())` at RUSTLIB/core/src/ops/function.rs:LL:CC + = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::` at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC + = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC + = note: inside `std::ops::function::impls:: for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at RUSTLIB/core/src/ops/function.rs:LL:CC + = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panicking::r#try:: i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panic.rs:LL:CC + = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC + = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#2}}, isize>` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panicking::r#try::` at RUSTLIB/std/src/panicking.rs:LL:CC + = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#2}}, isize>` at RUSTLIB/std/src/panic.rs:LL:CC + = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC + = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC + +error: aborting due to 1 previous error + diff --git a/tests/fail/tail_calls/signature-mismatch-arg.rs b/tests/fail/tail_calls/signature-mismatch-arg.rs new file mode 100644 index 0000000000..3264a74d15 --- /dev/null +++ b/tests/fail/tail_calls/signature-mismatch-arg.rs @@ -0,0 +1,17 @@ +#![feature(explicit_tail_calls)] +#![allow(incomplete_features)] + +fn main() { + // FIXME(explicit_tail_calls): + // the error should point to `become f(x)`, + // but tail calls mess up the backtrace it seems like... + f(0); + //~^ error: Undefined Behavior: calling a function with argument of type i32 passing data of type u32 +} + +fn f(x: u32) { + let g = unsafe { std::mem::transmute::(g) }; + become g(x); +} + +fn g(_: i32) {} diff --git a/tests/fail/tail_calls/signature-mismatch-arg.stderr b/tests/fail/tail_calls/signature-mismatch-arg.stderr new file mode 100644 index 0000000000..2ecc5674c6 --- /dev/null +++ b/tests/fail/tail_calls/signature-mismatch-arg.stderr @@ -0,0 +1,17 @@ +error: Undefined Behavior: calling a function with argument of type i32 passing data of type u32 + --> $DIR/signature-mismatch-arg.rs:LL:CC + | +LL | f(0); + | ^^^^ calling a function with argument of type i32 passing data of type u32 + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue + = note: BACKTRACE: + = note: inside `main` at $DIR/signature-mismatch-arg.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/tests/pass/tail_call.rs b/tests/pass/tail_call.rs new file mode 100644 index 0000000000..f620070639 --- /dev/null +++ b/tests/pass/tail_call.rs @@ -0,0 +1,39 @@ +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] + +fn main() { + assert_eq!(factorial(10), 3_628_800); + assert_eq!(mutually_recursive_identity(1000), 1000); +} + +fn factorial(n: u32) -> u32 { + fn factorial_acc(n: u32, acc: u32) -> u32 { + match n { + 0 => acc, + _ => become factorial_acc(n - 1, acc * n), + } + } + + factorial_acc(n, 1) +} + +// this is of course very silly, but we need to demonstrate mutual recursion somehow so... +fn mutually_recursive_identity(x: u32) -> u32 { + fn switch(src: u32, tgt: u32) -> u32 { + match src { + 0 => tgt, + _ if src % 7 == 0 => become advance_with_extra_steps(src, tgt), + _ => become advance(src, tgt), + } + } + + fn advance(src: u32, tgt: u32) -> u32 { + become switch(src - 1, tgt + 1) + } + + fn advance_with_extra_steps(src: u32, tgt: u32) -> u32 { + become advance(src, tgt) + } + + switch(x, 0) +} From 93a1c5f214e0a9107a50ee1101d66d9348a8691b Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 17 Jun 2024 17:25:14 +0000 Subject: [PATCH 59/61] make `StackPop` field names less confusing --- src/machine.rs | 2 +- src/shims/panic.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/machine.rs b/src/machine.rs index 0d91279f9f..d4d50ebdd1 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -1434,7 +1434,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { ecx: &mut InterpCx<'tcx, Self>, frame: Frame<'tcx, Provenance, FrameExtra<'tcx>>, unwinding: bool, - ) -> InterpResult<'tcx, StackPopJump> { + ) -> InterpResult<'tcx, ReturnAction> { if frame.extra.is_user_relevant { // All that we store is whether or not the frame we just removed is local, so now we // have no idea where the next topmost local frame is. So we recompute it. diff --git a/src/shims/panic.rs b/src/shims/panic.rs index ef832f5bbb..306dce5edc 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -113,7 +113,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { &mut self, mut extra: FrameExtra<'tcx>, unwinding: bool, - ) -> InterpResult<'tcx, StackPopJump> { + ) -> InterpResult<'tcx, ReturnAction> { let this = self.eval_context_mut(); trace!("handle_stack_pop_unwind(extra = {:?}, unwinding = {})", extra, unwinding); @@ -150,9 +150,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { )?; // We pushed a new stack frame, the engine should not do any jumping now! - Ok(StackPopJump::NoJump) + Ok(ReturnAction::NoJump) } else { - Ok(StackPopJump::Normal) + Ok(ReturnAction::Normal) } } From 3c3854f627f6aef53f4d3cdee2888c0b51078b5a Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Sun, 7 Jul 2024 18:04:29 +0200 Subject: [PATCH 60/61] Fix conflicts after rebase - r-l/r 126784 - r-l/r 127113 - r-l/miri 3562 --- tests/fail/tail_calls/cc-mismatch.stderr | 2 +- tests/fail/tail_calls/signature-mismatch-arg.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fail/tail_calls/cc-mismatch.stderr b/tests/fail/tail_calls/cc-mismatch.stderr index 708972e6ef..b157e9f0b2 100644 --- a/tests/fail/tail_calls/cc-mismatch.stderr +++ b/tests/fail/tail_calls/cc-mismatch.stderr @@ -8,7 +8,7 @@ LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information = note: BACKTRACE: = note: inside `>::call_once - shim(fn())` at RUSTLIB/core/src/ops/function.rs:LL:CC - = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::` at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC + = note: inside `std::sys::backtrace::__rust_begin_short_backtrace::` at RUSTLIB/std/src/sys/backtrace.rs:LL:CC = note: inside closure at RUSTLIB/std/src/rt.rs:LL:CC = note: inside `std::ops::function::impls:: for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at RUSTLIB/core/src/ops/function.rs:LL:CC = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at RUSTLIB/std/src/panicking.rs:LL:CC diff --git a/tests/fail/tail_calls/signature-mismatch-arg.stderr b/tests/fail/tail_calls/signature-mismatch-arg.stderr index 2ecc5674c6..8823ab9b97 100644 --- a/tests/fail/tail_calls/signature-mismatch-arg.stderr +++ b/tests/fail/tail_calls/signature-mismatch-arg.stderr @@ -7,7 +7,7 @@ LL | f(0); = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets - = help: if you think this code should be accepted anyway, please report an issue + = help: if you think this code should be accepted anyway, please report an issue with Miri = note: BACKTRACE: = note: inside `main` at $DIR/signature-mismatch-arg.rs:LL:CC From 0ed53e10df5d5474f713bf159ae5b6a31caa5402 Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Sun, 7 Jul 2024 20:18:42 +0200 Subject: [PATCH 61/61] Fixup a typo in a comment in a test --- tests/fail/tail_calls/signature-mismatch-arg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fail/tail_calls/signature-mismatch-arg.rs b/tests/fail/tail_calls/signature-mismatch-arg.rs index 3264a74d15..6df132d325 100644 --- a/tests/fail/tail_calls/signature-mismatch-arg.rs +++ b/tests/fail/tail_calls/signature-mismatch-arg.rs @@ -3,7 +3,7 @@ fn main() { // FIXME(explicit_tail_calls): - // the error should point to `become f(x)`, + // the error should point to `become g(x)`, // but tail calls mess up the backtrace it seems like... f(0); //~^ error: Undefined Behavior: calling a function with argument of type i32 passing data of type u32