From 9e442c1dc22006536c1c71f5273d72d01d7f4ddd Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Mon, 17 Jun 2024 05:02:59 +0000 Subject: [PATCH 01/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index e49680ba75a5e..c1796cfd82a67 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -f6b4b71ef10307201b52c17b0f9dcf9557cd90ba +e794b0f8557c187b5909d889aa35071f81e0a4cc From b8db9f0785dce07f6ee49eae3c493af5ea161c0a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Jun 2024 11:41:18 +0200 Subject: [PATCH 02/35] show proper UB when making a too large allocation request --- src/tools/miri/src/alloc_bytes.rs | 24 +++++++++++-------- src/tools/miri/src/shims/alloc.rs | 12 ---------- src/tools/miri/src/shims/foreign_items.rs | 24 +++++++++++++++---- src/tools/miri/tests/fail/alloc/too_large.rs | 10 ++++++++ .../miri/tests/fail/alloc/too_large.stderr | 15 ++++++++++++ 5 files changed, 59 insertions(+), 26 deletions(-) create mode 100644 src/tools/miri/tests/fail/alloc/too_large.rs create mode 100644 src/tools/miri/tests/fail/alloc/too_large.stderr diff --git a/src/tools/miri/src/alloc_bytes.rs b/src/tools/miri/src/alloc_bytes.rs index 97841a05cdebf..87579293001de 100644 --- a/src/tools/miri/src/alloc_bytes.rs +++ b/src/tools/miri/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/tools/miri/src/shims/alloc.rs b/src/tools/miri/src/shims/alloc.rs index 7b0c54d763e41..a33657d33a2e1 100644 --- a/src/tools/miri/src/shims/alloc.rs +++ b/src/tools/miri/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/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 898fc111fd43f..b8d85a1950297 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/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/src/tools/miri/tests/fail/alloc/too_large.rs b/src/tools/miri/tests/fail/alloc/too_large.rs new file mode 100644 index 0000000000000..4e28d2401d79f --- /dev/null +++ b/src/tools/miri/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/src/tools/miri/tests/fail/alloc/too_large.stderr b/src/tools/miri/tests/fail/alloc/too_large.stderr new file mode 100644 index 0000000000000..77dcf91d843f9 --- /dev/null +++ b/src/tools/miri/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 54594d64a441fd8750a3be436bc1bdcb30b49d18 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 17 Jun 2024 12:19:12 +0200 Subject: [PATCH 03/35] tell people how to set miri flags --- src/tools/miri/cargo-miri/src/phases.rs | 6 ++++ src/tools/miri/src/diagnostics.rs | 29 +++++++++---------- src/tools/miri/src/eval.rs | 11 ++----- .../libc_pthread_create_main_terminate.stderr | 2 +- .../libc/aligned_alloc_size_zero_leak.stderr | 2 +- .../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 ++--- src/tools/miri/tests/fail/memleak.stderr | 2 +- .../miri/tests/fail/memleak_no_backtrace.rs | 2 +- .../tests/fail/memleak_no_backtrace.stderr | 4 ++- .../miri/tests/fail/memleak_rc.64bit.stderr | 25 ---------------- src/tools/miri/tests/fail/memleak_rc.rs | 2 +- ...leak_rc.32bit.stderr => memleak_rc.stderr} | 4 +-- .../tests/fail/shims/fs/isolated_file.stderr | 4 +-- .../miri/tests/fail/tls_macro_leak.stderr | 2 +- .../miri/tests/fail/tls_static_leak.stderr | 2 +- src/tools/miri/tests/pass/box.stack.stderr | 7 ++--- .../miri/tests/pass/extern_types.stack.stderr | 7 ++--- .../stacked-borrows/issue-miri-2389.stderr | 7 ++--- 21 files changed, 52 insertions(+), 81 deletions(-) delete mode 100644 src/tools/miri/tests/fail/memleak_rc.64bit.stderr rename src/tools/miri/tests/fail/{memleak_rc.32bit.stderr => memleak_rc.stderr} (86%) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 3c36f606d8452..8d48b9c8ad14b 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/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/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 14e29aa423d8d..47f0913accee1 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/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/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 35f7f43f123f1..bd11439971c52 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/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/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr index 078b7d2e0d846..9d6be16b83a4b 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_create_main_terminate.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr b/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr index b0756d572120f..91c67823320a1 100644 --- a/src/tools/miri/tests/fail-dep/libc/aligned_alloc_size_zero_leak.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail-dep/libc/fs/isolated_stdin.stderr b/src/tools/miri/tests/fail-dep/libc/fs/isolated_stdin.stderr index 1d6626dda704f..9abe145ea9ef7 100644 --- a/src/tools/miri/tests/fail-dep/libc/fs/isolated_stdin.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail-dep/libc/malloc_zero_memory_leak.stderr b/src/tools/miri/tests/fail-dep/libc/malloc_zero_memory_leak.stderr index 65ce0dcdcddea..657262b8d4136 100644 --- a/src/tools/miri/tests/fail-dep/libc/malloc_zero_memory_leak.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr b/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr index 7ea0fa3146976..2639031f1cc50 100644 --- a/src/tools/miri/tests/fail-dep/libc/posix_memalign_size_zero_leak.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr index 4e2e721843238..217bc82010df9 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_metadata_uninit_slice_len.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/memleak.stderr b/src/tools/miri/tests/fail/memleak.stderr index 8ba78ef66443e..a9ee76fbe8a78 100644 --- a/src/tools/miri/tests/fail/memleak.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/memleak_no_backtrace.rs b/src/tools/miri/tests/fail/memleak_no_backtrace.rs index a1f8d9957ff8e..1f8d8ba7a7cf4 100644 --- a/src/tools/miri/tests/fail/memleak_no_backtrace.rs +++ b/src/tools/miri/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/src/tools/miri/tests/fail/memleak_no_backtrace.stderr b/src/tools/miri/tests/fail/memleak_no_backtrace.stderr index 22e8c558061ae..6850928e1503c 100644 --- a/src/tools/miri/tests/fail/memleak_no_backtrace.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/memleak_rc.64bit.stderr b/src/tools/miri/tests/fail/memleak_rc.64bit.stderr deleted file mode 100644 index 1c85a0f9d9f69..0000000000000 --- a/src/tools/miri/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/src/tools/miri/tests/fail/memleak_rc.rs b/src/tools/miri/tests/fail/memleak_rc.rs index 0927612d08eac..2d12c1223cf28 100644 --- a/src/tools/miri/tests/fail/memleak_rc.rs +++ b/src/tools/miri/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/src/tools/miri/tests/fail/memleak_rc.32bit.stderr b/src/tools/miri/tests/fail/memleak_rc.stderr similarity index 86% rename from src/tools/miri/tests/fail/memleak_rc.32bit.stderr rename to src/tools/miri/tests/fail/memleak_rc.stderr index 781e1458db9e3..dbf2daf818c6f 100644 --- a/src/tools/miri/tests/fail/memleak_rc.32bit.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr b/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr index 1f08649428a4c..ec956f83348e3 100644 --- a/src/tools/miri/tests/fail/shims/fs/isolated_file.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/tls_macro_leak.stderr b/src/tools/miri/tests/fail/tls_macro_leak.stderr index 40b21f8625a4a..c7c641a30f1bd 100644 --- a/src/tools/miri/tests/fail/tls_macro_leak.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/fail/tls_static_leak.stderr b/src/tools/miri/tests/fail/tls_static_leak.stderr index 580b52c151269..f7b90a1118c85 100644 --- a/src/tools/miri/tests/fail/tls_static_leak.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/pass/box.stack.stderr b/src/tools/miri/tests/pass/box.stack.stderr index 1a4d52ee3146f..341f84c8992df 100644 --- a/src/tools/miri/tests/pass/box.stack.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/pass/extern_types.stack.stderr b/src/tools/miri/tests/pass/extern_types.stack.stderr index 275d718129b4e..03a9167abbc7c 100644 --- a/src/tools/miri/tests/pass/extern_types.stack.stderr +++ b/src/tools/miri/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/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr b/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr index 7cbfad3942b32..b0e1adf27d183 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/issue-miri-2389.stderr +++ b/src/tools/miri/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 028f437abffc4943d085bd2b7be5e7a4ea73aac8 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 18 Jun 2024 04:56:47 +0000 Subject: [PATCH 04/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index c1796cfd82a67..356d4767c7bea 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -e794b0f8557c187b5909d889aa35071f81e0a4cc +c2932aaf9d20acbc9259c762f1a06f8767c6f13f From d9f1d557860138da64d52c0f536db395a99b9a4a Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Wed, 19 Jun 2024 04:54:41 +0000 Subject: [PATCH 05/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 356d4767c7bea..a2da736656d17 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -c2932aaf9d20acbc9259c762f1a06f8767c6f13f +a1ca449981e3b8442e358026437b7bedb9a1458e From 68c84b612b370c292e9ad15acca7e16fa3112ad6 Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Sat, 15 Jun 2024 14:43:29 +0200 Subject: [PATCH 06/35] Implement LLVM x86 bmi intrinsics --- src/tools/miri/src/shims/x86/bmi.rs | 108 +++++++++ src/tools/miri/src/shims/x86/mod.rs | 6 + .../pass/shims/x86/intrinsics-x86-bmi.rs | 216 ++++++++++++++++++ 3 files changed, 330 insertions(+) create mode 100644 src/tools/miri/src/shims/x86/bmi.rs create mode 100644 src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs diff --git a/src/tools/miri/src/shims/x86/bmi.rs b/src/tools/miri/src/shims/x86/bmi.rs new file mode 100644 index 0000000000000..e70757f4397ee --- /dev/null +++ b/src/tools/miri/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/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index b71aec02166be..704c45fdd6d39 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/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/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-bmi.rs new file mode 100644 index 0000000000000..33424117c4542 --- /dev/null +++ b/src/tools/miri/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 c7413154e1bfef6ec75157c589db0303e75bec30 Mon Sep 17 00:00:00 2001 From: Adwin White Date: Thu, 20 Jun 2024 17:21:19 +0800 Subject: [PATCH 07/35] Fix ICE caused by seeking past `i64::MAX` --- src/tools/miri/src/shims/unix/fs.rs | 9 +++++++- .../miri/tests/pass/issues/issue-miri-3680.rs | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 src/tools/miri/tests/pass/issues/issue-miri-3680.rs diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 262e71756c64f..e34aa5c09dfe1 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/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/src/tools/miri/tests/pass/issues/issue-miri-3680.rs b/src/tools/miri/tests/pass/issues/issue-miri-3680.rs new file mode 100644 index 0000000000000..55b896c91adb7 --- /dev/null +++ b/src/tools/miri/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 121b06bd0593369477a70dfee156f10055ca7638 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 21 Jun 2024 05:07:19 +0000 Subject: [PATCH 08/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index a2da736656d17..1502fa120be65 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -a1ca449981e3b8442e358026437b7bedb9a1458e +7a08f84627ff3035de4d66ff3209e5fc93165dcb From 79c9c8067d8e81f058904c3964bb8d71a2e1c849 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 09/35] Use strict ops instead of checked ops --- src/tools/miri/src/alloc_addresses/mod.rs | 2 +- src/tools/miri/src/eval.rs | 2 +- src/tools/miri/src/helpers.rs | 6 +-- src/tools/miri/src/shims/foreign_items.rs | 4 +- src/tools/miri/src/shims/time.rs | 2 +- src/tools/miri/src/shims/unix/env.rs | 6 +-- src/tools/miri/src/shims/unix/fd.rs | 2 +- src/tools/miri/src/shims/unix/socket.rs | 2 +- .../miri/src/shims/windows/foreign_items.rs | 4 +- src/tools/miri/src/shims/windows/handle.rs | 4 +- src/tools/miri/src/shims/x86/avx2.rs | 29 +++++++------ src/tools/miri/src/shims/x86/mod.rs | 41 ++++++++----------- src/tools/miri/src/shims/x86/sse2.rs | 14 +++---- src/tools/miri/src/shims/x86/ssse3.rs | 8 ++-- 14 files changed, 58 insertions(+), 68 deletions(-) diff --git a/src/tools/miri/src/alloc_addresses/mod.rs b/src/tools/miri/src/alloc_addresses/mod.rs index ae95d28d3eb65..d0f977f81433f 100644 --- a/src/tools/miri/src/alloc_addresses/mod.rs +++ b/src/tools/miri/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/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index bd11439971c52..c0827cce26301 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/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/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 843aff024958a..15aff010e0a96 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/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/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 5a293344cc849..f9ccc6ad4d2f6 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/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/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index ae17196f0b78d..e8f906d37e842 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/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/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index 2f78d0f42967a..405431f4327db 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/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/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index b6ac841dc9f45..599f78e712a2a 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/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/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs index c639ea2f84603..6d3d63b4efa02 100644 --- a/src/tools/miri/src/shims/unix/socket.rs +++ b/src/tools/miri/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/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index bfa14bcb5fad4..a8403669774d3 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/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/tools/miri/src/shims/windows/handle.rs b/src/tools/miri/src/shims/windows/handle.rs index 58c8683ff2777..ec461a4cd3681 100644 --- a/src/tools/miri/src/shims/windows/handle.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/avx2.rs b/src/tools/miri/src/shims/x86/avx2.rs index 016c525e57b41..efb0ed38fbc6e 100644 --- a/src/tools/miri/src/shims/x86/avx2.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 704c45fdd6d39..5db6d211a5095 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index e10047fefe6a8..b9561ac070997 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/ssse3.rs b/src/tools/miri/src/shims/x86/ssse3.rs index 6a815e4cea3c6..33bcbc2fa83d6 100644 --- a/src/tools/miri/src/shims/x86/ssse3.rs +++ b/src/tools/miri/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 1ee4a5ae0160bdd41a3aaa665ba482b8126df49c 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 10/35] Fix some missing ones --- src/tools/miri/src/shims/x86/avx2.rs | 6 +++--- src/tools/miri/src/shims/x86/mod.rs | 4 ++-- src/tools/miri/src/shims/x86/sse2.rs | 2 +- src/tools/miri/src/shims/x86/ssse3.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/src/shims/x86/avx2.rs b/src/tools/miri/src/shims/x86/avx2.rs index efb0ed38fbc6e..7f6c9336a9749 100644 --- a/src/tools/miri/src/shims/x86/avx2.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 5db6d211a5095..03c186e629004 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index b9561ac070997..3efdd561d6c60 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/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/tools/miri/src/shims/x86/ssse3.rs b/src/tools/miri/src/shims/x86/ssse3.rs index 33bcbc2fa83d6..ecacaeb9af524 100644 --- a/src/tools/miri/src/shims/x86/ssse3.rs +++ b/src/tools/miri/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 8a657f9ec4f565129fa543ffb0bd85ca58b98188 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2024 09:40:30 +0200 Subject: [PATCH 11/35] don't rely on libc existing on Windows --- src/tools/miri/src/helpers.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 843aff024958a..6fa1c16fec501 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/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 9afd75259155bdfb0ca707bf5ac6ea5faf50812f Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Thu, 20 Jun 2024 20:11:16 +0200 Subject: [PATCH 12/35] Implement LLVM x86 adx intrinsics --- src/tools/miri/src/shims/x86/mod.rs | 91 +++++++++++-------- .../pass/shims/x86/intrinsics-x86-adx.rs | 70 ++++++++++++++ 2 files changed, 123 insertions(+), 38 deletions(-) create mode 100644 src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 704c45fdd6d39..74470fad352f7 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/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/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs b/src/tools/miri/tests/pass/shims/x86/intrinsics-x86-adx.rs new file mode 100644 index 0000000000000..431e7f2c5eb60 --- /dev/null +++ b/src/tools/miri/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 ed83f1acced66941bfc58ce4e884d46d9e96bf64 Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Fri, 21 Jun 2024 17:55:22 +0200 Subject: [PATCH 13/35] Move out addition logic --- src/tools/miri/src/shims/x86/mod.rs | 68 ++++++++++++----------------- 1 file changed, 29 insertions(+), 39 deletions(-) diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 74470fad352f7..7bccf71f04374 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/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 eaacf0003cd3156c5afd4dd9ca35c5a0bdf6fa13 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jun 2024 17:02:31 +0200 Subject: [PATCH 14/35] CI: try to share setup code across actions --- src/tools/miri/.github/workflows/ci.yml | 87 +------------------ .../miri/.github/workflows/setup/action.yml | 52 +++++++++++ 2 files changed, 54 insertions(+), 85 deletions(-) create mode 100644 src/tools/miri/.github/workflows/setup/action.yml diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 3bc4163ab5246..fc4e484fa3898 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.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/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml new file mode 100644 index 0000000000000..8f54b5b8d81a9 --- /dev/null +++ b/src/tools/miri/.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 699b7d43c28b487b33acbc61510bf467e0dc2e6e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 22 Jun 2024 15:04:45 +0200 Subject: [PATCH 15/35] ./miri: nicer error when building miri-script fails --- src/tools/miri/miri | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 5f71fc9443839..07383bb59ebcc 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/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 dd2bd5bb7df300f655b596c287d6ca9eb642ff38 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 22 Jun 2024 17:02:38 +0200 Subject: [PATCH 16/35] evaluate arguments first, not inside the logic --- src/tools/miri/src/shims/x86/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 7bccf71f04374..afaf59eaadbff 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/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 22bbff1c61ee32fd8621557b1cebcc4b39261f5f 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 17/35] nicer batch file error when building miri-script fails --- src/tools/miri/miri.bat | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/miri.bat b/src/tools/miri/miri.bat index 18baa683f6517..98b59a56a0db4 100644 --- a/src/tools/miri/miri.bat +++ b/src/tools/miri/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 f91411b5ff5335601aa5acde43302ddfce8d4b82 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 23 Jun 2024 04:54:11 +0000 Subject: [PATCH 18/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 1502fa120be65..97d37b0ebe596 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -7a08f84627ff3035de4d66ff3209e5fc93165dcb +acb62737aca7045f331e7a05adc38bed213e278d From d73be701390c07ed99648404c08099420c38ca90 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 23 Jun 2024 05:02:48 +0000 Subject: [PATCH 19/35] fmt --- src/tools/miri/src/concurrency/thread.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 6a2b99825adc4..718daf93ea007 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/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 903a424ae2f9d1fc2c36718f71aa665c9507850a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 23 Jun 2024 08:41:18 +0200 Subject: [PATCH 20/35] unix/foreign_items: move getpid to the right part of the file --- src/tools/miri/src/shims/unix/foreign_items.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 2282099fa0d23..53ad40cfd2ccc 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/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 732e6876eac8dda8a54cedfd8376bd8176bc388b Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Mon, 24 Jun 2024 05:03:43 +0000 Subject: [PATCH 21/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 97d37b0ebe596..11a1c43bae9a2 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -acb62737aca7045f331e7a05adc38bed213e278d +d49994b060684af423339b55769439b2f444a7b9 From f071a205c7f3946835c834e252786f02046211f7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2024 08:43:16 +0200 Subject: [PATCH 22/35] tests for when a thread-local gets initialized in a tls dtor --- .../miri/tests/pass/tls/tls_macro_drop.rs | 102 ++++++++++++------ .../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 src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stderr create mode 100644 src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stdout diff --git a/src/tools/miri/tests/pass/tls/tls_macro_drop.rs b/src/tools/miri/tests/pass/tls/tls_macro_drop.rs index bd06eec9cd541..0d8a1cef511d8 100644 --- a/src/tools/miri/tests/pass/tls/tls_macro_drop.rs +++ b/src/tools/miri/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/src/tools/miri/tests/pass/tls/tls_macro_drop.stack.stdout b/src/tools/miri/tests/pass/tls/tls_macro_drop.stack.stdout index b7877820a0ca9..3e17acc832835 100644 --- a/src/tools/miri/tests/pass/tls/tls_macro_drop.stack.stdout +++ b/src/tools/miri/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/src/tools/miri/tests/pass/tls/tls_macro_drop.tree.stdout b/src/tools/miri/tests/pass/tls/tls_macro_drop.tree.stdout index b7877820a0ca9..3e17acc832835 100644 --- a/src/tools/miri/tests/pass/tls/tls_macro_drop.tree.stdout +++ b/src/tools/miri/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/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.rs b/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.rs index f36c460ae5341..082a6f17838d5 100644 --- a/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.rs +++ b/src/tools/miri/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/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stderr b/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stderr deleted file mode 100644 index 09ec1c3c2c511..0000000000000 --- a/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stderr +++ /dev/null @@ -1,3 +0,0 @@ -Continue main. -Dropping: 5 -Dropping: 5 diff --git a/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stdout b/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stdout new file mode 100644 index 0000000000000..6160f2726492d --- /dev/null +++ b/src/tools/miri/tests/pass/tls/tls_macro_drop_single_thread.stdout @@ -0,0 +1,2 @@ +Foo dtor +Bar dtor From 0f114ec41d5f1c5c4d2d77767014f0946c3f3cd5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2024 10:51:49 +0200 Subject: [PATCH 23/35] clarify the warning shown when optimizations are enabled --- src/tools/miri/src/bin/miri.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 829bfa7cd7086..9d8e44ce409e2 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/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 ba61c8f2a6ee3e874c1745d1471a16f4a1616290 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 24 Jun 2024 10:54:26 +0200 Subject: [PATCH 24/35] clarify the status of Tree Borrows --- src/tools/miri/README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 4b4f2f83062f0..87b437a308034 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/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 c6b25234fe92f7b1098017bd3b23eb0c58b7d485 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 25 Jun 2024 18:00:44 -0400 Subject: [PATCH 25/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 11a1c43bae9a2..e5e9f0bbdaf16 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -d49994b060684af423339b55769439b2f444a7b9 +c290e9de32e8ba6a673ef125fde40eadd395d170 From c8a89b05533e7fcec0866e0a25424f94afed93a0 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 27 Jun 2024 04:54:26 +0000 Subject: [PATCH 26/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index e5e9f0bbdaf16..989e9bc6d0242 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -c290e9de32e8ba6a673ef125fde40eadd395d170 +7033f9b14a37f4a00766d6c01326600b31f3a716 From 4cc16a53aeea6b82d05e665cb4bcbcddb1e2d7be Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 27 Jun 2024 09:53:59 +0200 Subject: [PATCH 27/35] tame unexpected_cfgs --- src/tools/miri/build.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/build.rs b/src/tools/miri/build.rs index 0977c0ba016bd..0918c9b13214d 100644 --- a/src/tools/miri/build.rs +++ b/src/tools/miri/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 5ae2b372b4407deb5d6125766a581d3820df2b1d Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Thu, 27 Jun 2024 13:00:37 -0700 Subject: [PATCH 28/35] Fix miri.bat --- src/tools/miri/miri.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/miri.bat b/src/tools/miri/miri.bat index 98b59a56a0db4..92566e009675a 100644 --- a/src/tools/miri/miri.bat +++ b/src/tools/miri/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 9d69154f5621391abad5aa055390545c1867c5ab Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Thu, 27 Jun 2024 13:23:34 -0700 Subject: [PATCH 29/35] Relocate GetCurrentProcessId to Environment Related shims, remove unnecessary std frame restriction --- src/tools/miri/src/shims/windows/foreign_items.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index a8403669774d3..c9db798caad7f 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/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 e7e35d7d78544ae030d043209cfe25315fb4cfbd Mon Sep 17 00:00:00 2001 From: Charlie Gettys Date: Thu, 27 Jun 2024 16:30:33 -0700 Subject: [PATCH 30/35] Switch to the explicit parens version --- src/tools/miri/miri.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/miri.bat b/src/tools/miri/miri.bat index 92566e009675a..6f9a8f38d6559 100644 --- a/src/tools/miri/miri.bat +++ b/src/tools/miri/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 b687053ee76bab4f404de846f09ad4889399f529 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 28 Jun 2024 05:13:00 +0000 Subject: [PATCH 31/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 989e9bc6d0242..a6096c0bf2cbe 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -7033f9b14a37f4a00766d6c01326600b31f3a716 +9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9 From afec0abd599c17566a4fe53e6e31515dfd434f39 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 28 Jun 2024 08:56:30 +0000 Subject: [PATCH 32/35] Bless clippy --- src/tools/miri/miri-script/src/commands.rs | 6 ++---- src/tools/miri/miri-script/src/util.rs | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 57bdfbad9afb9..62a3ab2c34cf5 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/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/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index e9095a45fce76..e1b77be192e31 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/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 db243de8be050ce43e26de86018d5f45a101e863 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 28 Jun 2024 09:43:05 +0200 Subject: [PATCH 33/35] readme: tweak wording around soundness --- src/tools/miri/README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 87b437a308034..b1be596c00679 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/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 64c8366ca2a8c4e9cbf67f3eac6b4ab81e0d63bf Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 29 Jun 2024 05:13:25 +0000 Subject: [PATCH 34/35] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index a6096c0bf2cbe..fd59ad3b8f130 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9 +9ed2ab3790ff41bf741dd690befd6a1c1e2b23ca From 66a885b25dbaf01232834a4c81344422750a5c6b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 29 Jun 2024 12:17:10 +0200 Subject: [PATCH 35/35] iter_exported_symbols: also walk used statics in local crate --- src/tools/miri/src/helpers.rs | 34 ++++++++++++++----- .../miri/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 src/tools/miri/tests/pass/tls/win_tls_callback.rs create mode 100644 src/tools/miri/tests/pass/tls/win_tls_callback.stderr diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 3d2b102b279a7..a7a6f8cfd8729 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/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/src/tools/miri/tests/pass/tls/win_tls_callback.rs b/src/tools/miri/tests/pass/tls/win_tls_callback.rs new file mode 100644 index 0000000000000..99a8de29e9170 --- /dev/null +++ b/src/tools/miri/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/src/tools/miri/tests/pass/tls/win_tls_callback.stderr b/src/tools/miri/tests/pass/tls/win_tls_callback.stderr new file mode 100644 index 0000000000000..8479558954456 --- /dev/null +++ b/src/tools/miri/tests/pass/tls/win_tls_callback.stderr @@ -0,0 +1 @@ +in tls_callback