From 81fed2f2005898c3c569fb5b389e7bb3ba874fb1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 16:41:29 +0100 Subject: [PATCH 01/26] remove some flags that haven't had an effect in a while --- src/tools/miri/src/bin/miri.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 281a32b77c5b5..ea11195c945d1 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -406,11 +406,6 @@ fn main() { miri_config.check_alignment = miri::AlignmentCheck::None; } else if arg == "-Zmiri-symbolic-alignment-check" { miri_config.check_alignment = miri::AlignmentCheck::Symbolic; - } else if arg == "-Zmiri-check-number-validity" { - eprintln!( - "WARNING: the flag `-Zmiri-check-number-validity` no longer has any effect \ - since it is now enabled by default" - ); } else if arg == "-Zmiri-disable-abi-check" { eprintln!( "WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed.\n\ @@ -457,8 +452,6 @@ fn main() { miri_config.collect_leak_backtraces = false; } else if arg == "-Zmiri-panic-on-unsupported" { miri_config.panic_on_unsupported = true; - } else if arg == "-Zmiri-tag-raw-pointers" { - eprintln!("WARNING: `-Zmiri-tag-raw-pointers` has no effect; it is enabled by default"); } else if arg == "-Zmiri-strict-provenance" { miri_config.provenance_mode = ProvenanceMode::Strict; } else if arg == "-Zmiri-permissive-provenance" { @@ -474,10 +467,6 @@ fn main() { "scalar" => RetagFields::OnlyScalar, _ => show_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), }; - } else if arg == "-Zmiri-track-raw-pointers" { - eprintln!( - "WARNING: `-Zmiri-track-raw-pointers` has no effect; it is enabled by default" - ); } else if let Some(param) = arg.strip_prefix("-Zmiri-seed=") { if miri_config.seed.is_some() { show_error!("Cannot specify -Zmiri-seed multiple times!"); From c1a5906d54730b156061542f9c6bfc4f187f4105 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 17:41:34 +0100 Subject: [PATCH 02/26] remove the ability to disable ABI checking --- src/tools/miri/README.md | 2 -- src/tools/miri/src/bin/miri.rs | 5 ++-- src/tools/miri/src/eval.rs | 3 -- src/tools/miri/src/helpers.rs | 4 +-- src/tools/miri/src/machine.rs | 9 ++---- .../concurrency/unwind_top_of_stack.rs | 29 ------------------- .../concurrency/unwind_top_of_stack.stderr | 21 -------------- .../fail/function_calls/arg_inplace_mutate.rs | 1 - .../arg_inplace_mutate.stack.stderr | 2 +- .../arg_inplace_mutate.tree.stderr | 2 +- .../exported_symbol_bad_unwind1.rs | 1 - .../exported_symbol_bad_unwind1.stderr | 2 -- .../tests/fail/panic/bad_miri_start_unwind.rs | 12 -------- .../fail/panic/bad_miri_start_unwind.stderr | 17 ----------- src/tools/miri/tests/fail/panic/bad_unwind.rs | 3 ++ .../pass/function_calls/disable_abi_check.rs | 24 --------------- .../function_calls/disable_abi_check.stderr | 2 -- 17 files changed, 11 insertions(+), 128 deletions(-) delete mode 100644 src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.rs delete mode 100644 src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.stderr delete mode 100644 src/tools/miri/tests/fail/panic/bad_miri_start_unwind.rs delete mode 100644 src/tools/miri/tests/fail/panic/bad_miri_start_unwind.stderr delete mode 100644 src/tools/miri/tests/pass/function_calls/disable_abi_check.rs delete mode 100644 src/tools/miri/tests/pass/function_calls/disable_abi_check.stderr diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 2b5ffb1a33d22..dd9c6097ebceb 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -359,8 +359,6 @@ The remaining flags are for advanced use only, and more likely to change or be r Some of these are **unsound**, which means they can lead to Miri failing to detect cases of undefined behavior in a program. -* `-Zmiri-disable-abi-check` disables checking [function ABI]. Using this flag - is **unsound**. This flag is **deprecated**. * `-Zmiri-disable-alignment-check` disables checking pointer alignment, so you can focus on other failures, but it means Miri can miss bugs in your program. Using this flag is **unsound**. diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 2f37a64576e4e..ae9a5f7ff6c49 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -413,10 +413,9 @@ fn main() { ); } else if arg == "-Zmiri-disable-abi-check" { eprintln!( - "WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed.\n\ - If you have a use-case for it, please file an issue." + "WARNING: the flag `-Zmiri-disable-abi-check` no longer has any effect; \ + ABI checks cannot be disabled any more" ); - miri_config.check_abi = false; } else if arg == "-Zmiri-disable-isolation" { if matches!(isolation_enabled, Some(true)) { show_error!( diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index ef50bd43de3b2..4e080f0cc3ba1 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -94,8 +94,6 @@ pub struct MiriConfig { pub unique_is_unique: bool, /// Controls alignment checking. pub check_alignment: AlignmentCheck, - /// Controls function [ABI](Abi) checking. - pub check_abi: bool, /// Action for an op requiring communication with the host. pub isolated_op: IsolatedOp, /// Determines if memory leaks should be ignored. @@ -162,7 +160,6 @@ impl Default for MiriConfig { borrow_tracker: Some(BorrowTrackerMethod::StackedBorrows), unique_is_unique: false, check_alignment: AlignmentCheck::Int, - check_abi: true, isolated_op: IsolatedOp::Reject(RejectOpWith::Abort), ignore_leaks: false, forwarded_env_vars: vec![], diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 65260254ae25f..98068842908d7 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -387,7 +387,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let param_env = ty::ParamEnv::reveal_all(); // in Miri this is always the param_env we use... and this.param_env is private. let callee_abi = f.ty(*this.tcx, param_env).fn_sig(*this.tcx).abi(); - if this.machine.enforce_abi && callee_abi != caller_abi { + if callee_abi != caller_abi { throw_ub_format!( "calling a function with ABI {} using caller ABI {}", callee_abi.name(), @@ -945,7 +945,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Check that the ABI is what we expect. fn check_abi<'a>(&self, abi: Abi, exp_abi: Abi) -> InterpResult<'a, ()> { - if self.eval_context_ref().machine.enforce_abi && abi != exp_abi { + if abi != exp_abi { throw_ub_format!( "calling a function with ABI {} using caller ABI {}", exp_abi.name(), diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index c411962fcf2e2..f6f57dcd59635 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -460,9 +460,6 @@ pub struct MiriMachine<'mir, 'tcx> { /// Whether to enforce the validity invariant. pub(crate) validate: bool, - /// Whether to enforce [ABI](Abi) of function calls. - pub(crate) enforce_abi: bool, - /// The table of file descriptors. pub(crate) file_handler: shims::unix::FileHandler, /// The table of directory descriptors. @@ -641,7 +638,6 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { tls: TlsData::default(), isolated_op: config.isolated_op, validate: config.validate, - enforce_abi: config.check_abi, file_handler: FileHandler::new(config.mute_stdout_stderr), dir_handler: Default::default(), layouts, @@ -784,7 +780,6 @@ impl VisitProvenance for MiriMachine<'_, '_> { tcx: _, isolated_op: _, validate: _, - enforce_abi: _, clock: _, layouts: _, static_roots: _, @@ -932,8 +927,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } #[inline(always)] - fn enforce_abi(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { - ecx.machine.enforce_abi + fn enforce_abi(_ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { + true } #[inline(always)] diff --git a/src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.rs b/src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.rs deleted file mode 100644 index 4704cfed03938..0000000000000 --- a/src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.rs +++ /dev/null @@ -1,29 +0,0 @@ -//@ignore-target-windows: No libc on Windows - -//@compile-flags: -Zmiri-disable-abi-check - -//! Unwinding past the top frame of a stack is Undefined Behavior. - -#![feature(c_unwind)] - -use std::{mem, ptr}; - -extern "C-unwind" fn thread_start(_null: *mut libc::c_void) -> *mut libc::c_void { - //~^ ERROR: unwinding past the topmost frame of the stack - panic!() -} - -fn main() { - unsafe { - let mut native: libc::pthread_t = mem::zeroed(); - let attr: libc::pthread_attr_t = mem::zeroed(); - // assert_eq!(libc::pthread_attr_init(&mut attr), 0); FIXME: this function is not yet implemented. - // Cast to avoid inserting abort-on-unwind. - let thread_start: extern "C-unwind" fn(*mut libc::c_void) -> *mut libc::c_void = - thread_start; - let thread_start: extern "C" fn(*mut libc::c_void) -> *mut libc::c_void = - mem::transmute(thread_start); - assert_eq!(libc::pthread_create(&mut native, &attr, thread_start, ptr::null_mut()), 0); - assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0); - } -} diff --git a/src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.stderr b/src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.stderr deleted file mode 100644 index 0c755215c00ff..0000000000000 --- a/src/tools/miri/tests/fail-dep/concurrency/unwind_top_of_stack.stderr +++ /dev/null @@ -1,21 +0,0 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. -thread '' panicked at $DIR/unwind_top_of_stack.rs:LL:CC: -explicit panic -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -error: Undefined Behavior: unwinding past the topmost frame of the stack - --> $DIR/unwind_top_of_stack.rs:LL:CC - | -LL | / extern "C-unwind" fn thread_start(_null: *mut libc::c_void) -> *mut libc::c_void { -LL | | -LL | | panic!() -LL | | } - | |_^ unwinding past the topmost frame of the stack - | - = 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 on thread `unnamed-ID`: - = note: inside `thread_start` at $DIR/unwind_top_of_stack.rs:LL:CC - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs index 8a5c10913b48a..4f7a12ebd2e8b 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs @@ -19,7 +19,6 @@ fn main() { after_call = { Return() } - } } diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr index 422dc24343612..e0d1bed6332f6 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr @@ -14,7 +14,7 @@ LL | | let _unit: (); LL | | { LL | | let non_copy = S(42); ... | -LL | | +LL | | } LL | | } | |_____^ help: is this argument diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr index 4fe9b7b4728da..21c814e664135 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr @@ -16,7 +16,7 @@ LL | | let _unit: (); LL | | { LL | | let non_copy = S(42); ... | -LL | | +LL | | } LL | | } | |_____^ help: the protected tag was created here, in the initial state Reserved diff --git a/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.rs b/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.rs index 5f4df7c6a1ef3..6d68b9a46d9e3 100644 --- a/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.rs +++ b/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.rs @@ -1,4 +1,3 @@ -//@compile-flags: -Zmiri-disable-abi-check #![feature(c_unwind)] #[no_mangle] diff --git a/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr b/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr index c5ca127457469..ab32883a03562 100644 --- a/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr +++ b/src/tools/miri/tests/fail/function_calls/exported_symbol_bad_unwind1.stderr @@ -1,5 +1,3 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. thread 'main' panicked at $DIR/exported_symbol_bad_unwind1.rs:LL:CC: explicit panic note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/src/tools/miri/tests/fail/panic/bad_miri_start_unwind.rs b/src/tools/miri/tests/fail/panic/bad_miri_start_unwind.rs deleted file mode 100644 index deca836a36c61..0000000000000 --- a/src/tools/miri/tests/fail/panic/bad_miri_start_unwind.rs +++ /dev/null @@ -1,12 +0,0 @@ -//@compile-flags: -Zmiri-disable-abi-check -// This feature is required to trigger the error using the "C" ABI. -#![feature(c_unwind)] - -extern "C" { - fn miri_start_unwind(payload: *mut u8) -> !; -} - -fn main() { - unsafe { miri_start_unwind(&mut 0) } - //~^ ERROR: unwinding past a stack frame that does not allow unwinding -} diff --git a/src/tools/miri/tests/fail/panic/bad_miri_start_unwind.stderr b/src/tools/miri/tests/fail/panic/bad_miri_start_unwind.stderr deleted file mode 100644 index 6c85aac050a20..0000000000000 --- a/src/tools/miri/tests/fail/panic/bad_miri_start_unwind.stderr +++ /dev/null @@ -1,17 +0,0 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. -error: Undefined Behavior: unwinding past a stack frame that does not allow unwinding - --> $DIR/bad_miri_start_unwind.rs:LL:CC - | -LL | unsafe { miri_start_unwind(&mut 0) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^ unwinding past a stack frame that does not allow unwinding - | - = 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/bad_miri_start_unwind.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to 1 previous error - diff --git a/src/tools/miri/tests/fail/panic/bad_unwind.rs b/src/tools/miri/tests/fail/panic/bad_unwind.rs index 370b372a7d373..8c8a9f18cdc63 100644 --- a/src/tools/miri/tests/fail/panic/bad_unwind.rs +++ b/src/tools/miri/tests/fail/panic/bad_unwind.rs @@ -1,6 +1,9 @@ #![feature(c_unwind)] //! Unwinding when the caller ABI is "C" (without "-unwind") is UB. +// The opposite version (callee does not allow unwinding) is impossible to +// even write: MIR validation catches functions that have `UnwindContinue` but +// are not allowed to unwind. extern "C-unwind" fn unwind() { panic!(); diff --git a/src/tools/miri/tests/pass/function_calls/disable_abi_check.rs b/src/tools/miri/tests/pass/function_calls/disable_abi_check.rs deleted file mode 100644 index 0f41047c60330..0000000000000 --- a/src/tools/miri/tests/pass/function_calls/disable_abi_check.rs +++ /dev/null @@ -1,24 +0,0 @@ -//@compile-flags: -Zmiri-disable-abi-check -#![feature(core_intrinsics)] - -fn main() { - fn foo() {} - - extern "C" fn try_fn(ptr: *mut u8) { - assert!(ptr.is_null()); - } - - extern "Rust" { - fn malloc(size: usize) -> *mut std::ffi::c_void; - } - - unsafe { - let _ = malloc(0); - std::mem::transmute::(foo)(); - std::intrinsics::catch_unwind( - std::mem::transmute::(try_fn), - std::ptr::null_mut(), - |_, _| unreachable!(), - ); - } -} diff --git a/src/tools/miri/tests/pass/function_calls/disable_abi_check.stderr b/src/tools/miri/tests/pass/function_calls/disable_abi_check.stderr deleted file mode 100644 index e5b84f6d7090e..0000000000000 --- a/src/tools/miri/tests/pass/function_calls/disable_abi_check.stderr +++ /dev/null @@ -1,2 +0,0 @@ -WARNING: the flag `-Zmiri-disable-abi-check` is deprecated and planned to be removed. -If you have a use-case for it, please file an issue. From 32643dc8536212ef01c8b9c63674cee050c10b8e Mon Sep 17 00:00:00 2001 From: Ross Smyth Date: Mon, 26 Feb 2024 17:19:30 -0500 Subject: [PATCH 03/26] Fix .\miri fmt on Windows --- src/tools/miri/miri-script/src/commands.rs | 46 +++++++++------------ src/tools/miri/miri-script/src/util.rs | 47 +++++++++++++++++++++- 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 88ee62fc18a1a..dc6ef58c52080 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -526,37 +526,27 @@ impl Command { } fn fmt(flags: Vec) -> Result<()> { + use itertools::Itertools; + let e = MiriEnv::new()?; - let toolchain = &e.toolchain; let config_path = path!(e.miri_dir / "rustfmt.toml"); - let mut cmd = cmd!( - e.sh, - "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" - ); - eprintln!("$ {cmd} ..."); - - // Add all the filenames to the command. - // FIXME: `rustfmt` will follow the `mod` statements in these files, so we get a bunch of - // duplicate diffs. - for item in WalkDir::new(&e.miri_dir).into_iter().filter_entry(|entry| { - let name = entry.file_name().to_string_lossy(); - let ty = entry.file_type(); - if ty.is_file() { - name.ends_with(".rs") - } else { - // dir or symlink. skip `target` and `.git`. - &name != "target" && &name != ".git" - } - }) { - let item = item?; - if item.file_type().is_file() { - cmd = cmd.arg(item.into_path()); - } - } + // Collect each rust file in the miri repo. + let files = WalkDir::new(&e.miri_dir) + .into_iter() + .filter_entry(|entry| { + let name = entry.file_name().to_string_lossy(); + let ty = entry.file_type(); + if ty.is_file() { + name.ends_with(".rs") + } else { + // dir or symlink. skip `target` and `.git`. + &name != "target" && &name != ".git" + } + }) + .filter_ok(|item| item.file_type().is_file()) + .map_ok(|item| item.into_path()); - // We want our own error message, repeating the command is too much. - cmd.quiet().run().map_err(|_| anyhow!("`rustfmt` failed"))?; - Ok(()) + e.format_files(files, &e.toolchain[..], &config_path, &flags[..]) } } diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 420ecdab63c9b..5c02024324062 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -1,7 +1,7 @@ use std::ffi::{OsStr, OsString}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use dunce::canonicalize; use path_macro::path; use xshell::{cmd, Shell}; @@ -145,4 +145,47 @@ impl MiriEnv { .run()?; Ok(()) } + + /// Receives an iterator of files. + /// Will format each file with the miri rustfmt config. + /// Does not follow module relationships. + pub fn format_files( + &self, + files: impl Iterator>, + toolchain: &str, + config_path: &Path, + flags: &[OsString], + ) -> anyhow::Result<()> { + use itertools::Itertools; + + let mut first = true; + + // Format in batches as not all out files fit into Windows' command argument limit. + for batch in &files.chunks(256) { + // Build base command. + let mut cmd = cmd!( + self.sh, + "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" + ); + if first { + eprintln!("$ {cmd} ..."); + first = false; + } + // Add files. + for file in batch { + // Make it a relative path so that on platforms with extremely tight argument + // limits (like Windows), we become immune to someone cloning the repo + // 50 directories deep. + let file = file?; + let file = file.strip_prefix(&self.miri_dir)?; + cmd = cmd.arg(file); + } + + // Run commands. + // We want our own error message, repeating the command is too much. + cmd.quiet().run().map_err(|_| anyhow!("`rustfmt` failed"))?; + } + + Ok(()) + } } From cfc10f0aab896fc1b0a1f56ad3f77c462b5bcca0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Mar 2024 12:50:55 +0100 Subject: [PATCH 04/26] fix MIRI_LOG=info not setting the level for miri itself --- src/tools/miri/src/bin/miri.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 2f37a64576e4e..6f82c1876ed21 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -202,7 +202,7 @@ fn rustc_logger_config() -> rustc_log::LoggerConfig { // rustc traced, but you can also do `MIRI_LOG=miri=trace,rustc_const_eval::interpret=debug`. if tracing::Level::from_str(&var).is_ok() { cfg.filter = Ok(format!( - "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var}" + "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var},miri={var}" )); } else { cfg.filter = Ok(var); From 3041c78c9c055988de5d1138a14cc61fb9ea94fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Mar 2024 12:53:32 +0100 Subject: [PATCH 05/26] log when we change the active thread --- src/tools/miri/src/concurrency/thread.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 83ca27a467c8d..8341fd7648b29 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -445,10 +445,13 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { /// Set an active thread and return the id of the thread that was active before. fn set_active_thread_id(&mut self, id: ThreadId) -> ThreadId { - let active_thread_id = self.active_thread; - self.active_thread = id; - assert!(self.active_thread.index() < self.threads.len()); - active_thread_id + assert!(id.index() < self.threads.len()); + info!( + "---------- Now executing on thread `{}` (previous: `{}`) ----------------------------------------", + self.get_thread_display_name(id), + self.get_thread_display_name(self.active_thread) + ); + std::mem::replace(&mut self.active_thread, id) } /// Get the id of the currently active thread. @@ -735,6 +738,11 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { for (id, thread) in threads { debug_assert_ne!(self.active_thread, id); if thread.state == ThreadState::Enabled { + info!( + "---------- Now executing on thread `{}` (previous: `{}`) ----------------------------------------", + self.get_thread_display_name(id), + self.get_thread_display_name(self.active_thread) + ); self.active_thread = id; break; } From 983c2c59c951eaefba46adfd289a17bd109f601e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Mar 2024 13:29:14 +0100 Subject: [PATCH 06/26] do not rely on tracing's scope exit logging, it is wrong --- src/tools/miri/src/bin/miri.rs | 4 ---- src/tools/miri/src/machine.rs | 24 +++++++++++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 6f82c1876ed21..dd08a376e7fc9 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -208,10 +208,6 @@ fn rustc_logger_config() -> rustc_log::LoggerConfig { cfg.filter = Ok(var); } } - // Enable verbose entry/exit logging by default if MIRI_LOG is set. - if matches!(cfg.verbose_entry_exit, Err(VarError::NotPresent)) { - cfg.verbose_entry_exit = Ok(format!("1")); - } } cfg diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index d40f1c4525fc2..f0e851c132f7f 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1454,13 +1454,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { if ecx.machine.borrow_tracker.is_some() { ecx.on_stack_pop(frame)?; } + // tracing-tree can autoamtically annotate scope changes, but it gets very confused by our + // concurrency and what it prints is just plain wrong. So we print our own information + // instead. (Cc https://github.com/rust-lang/miri/issues/2266) + info!("Leaving {}", ecx.frame().instance); Ok(()) } #[inline(always)] fn after_stack_pop( ecx: &mut InterpCx<'mir, 'tcx, Self>, - mut frame: Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>, + frame: Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>, unwinding: bool, ) -> InterpResult<'tcx, StackPopJump> { if frame.extra.is_user_relevant { @@ -1470,10 +1474,20 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { // user-relevant frame and restore that here.) ecx.active_thread_mut().recompute_top_user_relevant_frame(); } - let timing = frame.extra.timing.take(); - let res = ecx.handle_stack_pop_unwind(frame.extra, unwinding); - if let Some(profiler) = ecx.machine.profiler.as_ref() { - profiler.finish_recording_interval_event(timing.unwrap()); + let res = { + // Move `frame`` into a sub-scope so we control when it will be dropped. + let mut frame = frame; + let timing = frame.extra.timing.take(); + let res = ecx.handle_stack_pop_unwind(frame.extra, unwinding); + if let Some(profiler) = ecx.machine.profiler.as_ref() { + profiler.finish_recording_interval_event(timing.unwrap()); + } + res + }; + // Needs to be done after dropping frame to show up on the right nesting level. + // (Cc https://github.com/rust-lang/miri/issues/2266) + if !ecx.active_thread_stack().is_empty() { + info!("Continuing in {}", ecx.frame().instance); } res } From 83e2e2d89dae346d38909f5590d9231cf2a5d218 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Mar 2024 15:00:47 +0100 Subject: [PATCH 07/26] nits and typos --- src/tools/miri/miri-script/src/util.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 5c02024324062..361a4ca0cf769 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -148,7 +148,7 @@ impl MiriEnv { /// Receives an iterator of files. /// Will format each file with the miri rustfmt config. - /// Does not follow module relationships. + /// Does not recursively format modules. pub fn format_files( &self, files: impl Iterator>, @@ -160,7 +160,7 @@ impl MiriEnv { let mut first = true; - // Format in batches as not all out files fit into Windows' command argument limit. + // Format in batches as not all our files fit into Windows' command argument limit. for batch in &files.chunks(256) { // Build base command. let mut cmd = cmd!( @@ -168,6 +168,7 @@ impl MiriEnv { "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" ); if first { + // Log an abbreviating command, and only once. eprintln!("$ {cmd} ..."); first = false; } @@ -181,7 +182,7 @@ impl MiriEnv { cmd = cmd.arg(file); } - // Run commands. + // Run rustfmt. // We want our own error message, repeating the command is too much. cmd.quiet().run().map_err(|_| anyhow!("`rustfmt` failed"))?; } From fbafd36333b76ebb259cd1d657e0ca7daca7ce4e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 3 Mar 2024 14:55:01 +0100 Subject: [PATCH 08/26] disable diagnostic deduplication --- src/tools/miri/src/lib.rs | 3 +++ src/tools/miri/tests/fail/const-ub-checks.stderr | 8 ++++++++ src/tools/miri/tests/fail/erroneous_const2.stderr | 8 ++++++++ 3 files changed, 19 insertions(+) diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index e1d0bc1c18386..c0d1afa8023e2 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -143,4 +143,7 @@ pub const MIRI_DEFAULT_ARGS: &[&str] = &[ "-Zmir-keep-place-mention", "-Zmir-opt-level=0", "-Zmir-enable-passes=-CheckAlignment", + // Deduplicating diagnostics means we miss events when tracking what happens during an + // execution. Let's not do that. + "-Zdeduplicate-diagnostics=no", ]; diff --git a/src/tools/miri/tests/fail/const-ub-checks.stderr b/src/tools/miri/tests/fail/const-ub-checks.stderr index 700a96a9062af..f6ac480f069be 100644 --- a/src/tools/miri/tests/fail/const-ub-checks.stderr +++ b/src/tools/miri/tests/fail/const-ub-checks.stderr @@ -10,6 +10,14 @@ note: erroneous constant encountered LL | let _x = UNALIGNED_READ; | ^^^^^^^^^^^^^^ +note: erroneous constant encountered + --> $DIR/const-ub-checks.rs:LL:CC + | +LL | let _x = UNALIGNED_READ; + | ^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + error: aborting due to 1 previous error For more information about this error, try `rustc --explain E0080`. diff --git a/src/tools/miri/tests/fail/erroneous_const2.stderr b/src/tools/miri/tests/fail/erroneous_const2.stderr index 47b06fa8aaa08..2227436707482 100644 --- a/src/tools/miri/tests/fail/erroneous_const2.stderr +++ b/src/tools/miri/tests/fail/erroneous_const2.stderr @@ -10,6 +10,14 @@ note: erroneous constant encountered LL | println!("{}", FOO); | ^^^ +note: erroneous constant encountered + --> $DIR/erroneous_const2.rs:LL:CC + | +LL | println!("{}", FOO); + | ^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + note: erroneous constant encountered --> $DIR/erroneous_const2.rs:LL:CC | From d5f31bda4fe6223004f5942969c948e438b7ccc0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Mar 2024 09:13:09 +0100 Subject: [PATCH 09/26] implement manual deduplication for isolation-error=warn-nobacktrace --- src/tools/miri/README.md | 4 ++-- src/tools/miri/src/helpers.rs | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 5eb8c0fa7c99e..63756624677ef 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -318,8 +318,8 @@ environment variable. We first document the most relevant and most commonly used and `warn-nobacktrace` are the supported actions. The default is to `abort`, which halts the machine. Some (but not all) operations also support continuing execution with a "permission denied" error being returned to the program. - `warn` prints a full backtrace when that happens; `warn-nobacktrace` is less - verbose. `hide` hides the warning entirely. + `warn` prints a full backtrace each time that happens; `warn-nobacktrace` is less + verbose and shown at most once per operation. `hide` hides the warning entirely. * `-Zmiri-num-cpus` states the number of available CPUs to be reported by miri. By default, the number of available CPUs is `1`. Note that this flag does not affect how miri handles threads in any way. diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 65260254ae25f..c3f4deb7c2eef 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1,6 +1,8 @@ use std::cmp; +use std::collections::BTreeSet; use std::iter; use std::num::NonZero; +use std::sync::Mutex; use std::time::Duration; use rustc_apfloat::ieee::{Double, Single}; @@ -603,9 +605,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { match reject_with { RejectOpWith::Abort => isolation_abort_error(op_name), RejectOpWith::WarningWithoutBacktrace => { - this.tcx - .dcx() - .warn(format!("{op_name} was made to return an error due to isolation")); + // This exists to reduce verbosity; make sure we emit the warning at most once per + // operation. + static EMITTED_WARNINGS: Mutex> = Mutex::new(BTreeSet::new()); + + let mut emitted_warnings = EMITTED_WARNINGS.lock().unwrap(); + if !emitted_warnings.contains(op_name) { + // First time we are seeing this. + emitted_warnings.insert(op_name.to_owned()); + this.tcx + .dcx() + .warn(format!("{op_name} was made to return an error due to isolation")); + } Ok(()) } RejectOpWith::Warning => { From f70feaf604366c4e90e9d17ecd406fa182e54583 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Mar 2024 09:00:42 +0100 Subject: [PATCH 10/26] linux-futex test: add comment --- src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs index 648c004c97cc0..d21f953672dee 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs +++ b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs @@ -219,6 +219,7 @@ fn wait_wake_bitset() { t.join().unwrap(); } +// Crucial test which relies on the SeqCst fences in futex wait/wake. fn concurrent_wait_wake() { const FREE: i32 = 0; const HELD: i32 = 1; From 974446eb92283ed358e513a3613516e1da95ba6a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Mar 2024 15:07:35 +0100 Subject: [PATCH 11/26] we properly rebuild the cache now when MIRI_LIB_SRC contents change --- src/tools/miri/README.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 63756624677ef..372e00f854286 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -465,11 +465,7 @@ Moreover, Miri recognizes some environment variables: * `MIRI_LIB_SRC` defines the directory where Miri expects the sources of the standard library that it will build and use for interpretation. This directory must point to the `library` subdirectory of a `rust-lang/rust` repository - checkout. Note that changing files in that directory does not automatically - trigger a re-build of the standard library; you have to clear the Miri build - cache with `cargo miri clean` or deleting it manually (on Linux, `rm -rf ~/.cache/miri`; - on Windows, `rmdir /S "%LOCALAPPDATA%\rust-lang\miri\cache"`; - and on macOS, `rm -rf ~/Library/Caches/org.rust-lang.miri`). + checkout. * `MIRI_SYSROOT` (recognized by `cargo miri` and the Miri driver) indicates the sysroot to use. When using `cargo miri`, this skips the automatic setup -- only set this if you do not want to use the automatically created sysroot. For directly invoking the Miri driver, this variable (or a From 75f6694ce890dd906c30d2a13bf852b7646fc7dc Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Mon, 4 Mar 2024 10:13:49 -0500 Subject: [PATCH 12/26] Use cargo miri clean in build-all-targets.sh --- src/tools/miri/ci/build-all-targets.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/ci/build-all-targets.sh b/src/tools/miri/ci/build-all-targets.sh index bf3ffd07017c4..fdb645c3ae182 100755 --- a/src/tools/miri/ci/build-all-targets.sh +++ b/src/tools/miri/ci/build-all-targets.sh @@ -12,7 +12,7 @@ PLATFORM_SUPPORT_FILE=$(rustc +miri --print sysroot)/share/doc/rust/html/rustc/p for target in $(python3 ci/scrape-targets.py $PLATFORM_SUPPORT_FILE); do # Wipe the cache before every build to minimize disk usage - rm -rf ~/.cache/miri + cargo +miri miri clean if cargo +miri miri setup --target $target 2>&1 | tee failures/$target; then # If the build succeeds, delete its output. If we have output, a build failed. rm $FAILS_DIR/$target From a477cf45ffafaee7d9523eb49a43b8c9b61c0ff4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Mar 2024 16:22:30 +0100 Subject: [PATCH 13/26] =?UTF-8?q?Conjob=20=E2=86=92=20Cronjob?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/tools/miri/.github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 0954c57a1db1b..49ed7ffce22fa 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -187,7 +187,7 @@ jobs: run: RUSTFLAGS="--cap-lints warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r22.12.06 - name: setup bot git name and email run: | - git config --global user.name 'The Miri Conjob Bot' + git config --global user.name 'The Miri Cronjob Bot' git config --global user.email 'miri@cron.bot' - name: get changes from rustc run: ./miri rustc-pull From 45d955edb0784c59e0b059d7ef5325c334edc3a4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 10:06:50 +0100 Subject: [PATCH 14/26] reorder rustc crate imports a bit --- src/tools/miri/src/bin/miri.rs | 7 +++++-- src/tools/miri/src/lib.rs | 13 +++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index a101b4a29a956..69ea9e5fad95d 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -8,6 +8,11 @@ rustc::untranslatable_diagnostic )] +// Some "regular" crates we want to share with rustc +#[macro_use] +extern crate tracing; + +// The rustc crates we need extern crate rustc_data_structures; extern crate rustc_driver; extern crate rustc_hir; @@ -16,8 +21,6 @@ extern crate rustc_log; extern crate rustc_metadata; extern crate rustc_middle; extern crate rustc_session; -#[macro_use] -extern crate tracing; use std::env::{self, VarError}; use std::num::NonZero; diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index c0d1afa8023e2..c3bd6b912d5ac 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -49,8 +49,12 @@ // Needed for rustdoc from bootstrap (with `-Znormalize-docs`). #![recursion_limit = "256"] -extern crate either; // the one from rustc +// Some "regular" crates we want to share with rustc +extern crate either; +#[macro_use] +extern crate tracing; +// The rustc crates we need extern crate rustc_apfloat; extern crate rustc_ast; extern crate rustc_const_eval; @@ -63,11 +67,8 @@ extern crate rustc_middle; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; -#[macro_use] -extern crate tracing; - -// Necessary to pull in object code as the rest of the rustc crates are shipped only as rmeta -// files. +// Linking `rustc_driver` pulls in the required object code as the rest of the rustc crates are +// shipped only as rmeta files. #[allow(unused_extern_crates)] extern crate rustc_driver; From 2b40181cb3d79ea586d260c3dd647f2aa84473e2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 4 Mar 2024 18:59:52 +0100 Subject: [PATCH 15/26] give macOS even more time to sleep --- src/tools/miri/tests/pass/concurrency/sync.rs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/tests/pass/concurrency/sync.rs b/src/tools/miri/tests/pass/concurrency/sync.rs index a6c181098b74b..91c67b215a1a9 100644 --- a/src/tools/miri/tests/pass/concurrency/sync.rs +++ b/src/tools/miri/tests/pass/concurrency/sync.rs @@ -7,6 +7,10 @@ use std::sync::{Arc, Barrier, Condvar, Mutex, Once, RwLock}; use std::thread; use std::time::{Duration, Instant}; +// We are expecting to sleep for 10ms. How long of a sleep we are accepting? +// Even with 1000ms we still see this test fail on macOS runners. +const MAX_SLEEP_TIME_MS: u64 = 2000; + // Check if Rust barriers are working. /// This test is taken from the Rust documentation. @@ -66,7 +70,7 @@ fn check_conditional_variables_timed_wait_timeout() { let (_guard, timeout) = cvar.wait_timeout(guard, Duration::from_millis(10)).unwrap(); assert!(timeout.timed_out()); let elapsed_time = now.elapsed().as_millis(); - assert!(10 <= elapsed_time && elapsed_time <= 1000); + assert!(10 <= elapsed_time && elapsed_time <= MAX_SLEEP_TIME_MS.into()); } /// Test that signaling a conditional variable when waiting with a timeout works @@ -84,7 +88,8 @@ fn check_conditional_variables_timed_wait_notimeout() { cvar.notify_one(); }); - let (_guard, timeout) = cvar.wait_timeout(guard, Duration::from_millis(1000)).unwrap(); + let (_guard, timeout) = + cvar.wait_timeout(guard, Duration::from_millis(MAX_SLEEP_TIME_MS)).unwrap(); assert!(!timeout.timed_out()); handle.join().unwrap(); } @@ -213,20 +218,21 @@ fn check_once() { fn park_timeout() { let start = Instant::now(); - thread::park_timeout(Duration::from_millis(200)); + thread::park_timeout(Duration::from_millis(10)); // Normally, waiting in park/park_timeout may spuriously wake up early, but we // know Miri's timed synchronization primitives do not do that. - // We allow much longer sleeps as well since the macOS GHA runners seem very oversubscribed - // and sometimes just pause for 1 second or more. let elapsed = start.elapsed(); - assert!((200..2000).contains(&elapsed.as_millis()), "bad sleep time: {elapsed:?}"); + assert!( + (10..MAX_SLEEP_TIME_MS.into()).contains(&elapsed.as_millis()), + "bad sleep time: {elapsed:?}" + ); } fn park_unpark() { let t1 = thread::current(); let t2 = thread::spawn(move || { thread::park(); - thread::sleep(Duration::from_millis(200)); + thread::sleep(Duration::from_millis(10)); t1.unpark(); }); @@ -236,10 +242,11 @@ fn park_unpark() { thread::park(); // Normally, waiting in park/park_timeout may spuriously wake up early, but we // know Miri's timed synchronization primitives do not do that. - // We allow much longer sleeps as well since the macOS GHA runners seem very oversubscribed - // and sometimes just pause for 1 second or more. let elapsed = start.elapsed(); - assert!((200..2000).contains(&elapsed.as_millis()), "bad sleep time: {elapsed:?}"); + assert!( + (10..MAX_SLEEP_TIME_MS.into()).contains(&elapsed.as_millis()), + "bad sleep time: {elapsed:?}" + ); t2.join().unwrap(); } From 5294dd8afd5edb765b2303b36ac70a56fdd32364 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Mar 2024 21:30:41 +0000 Subject: [PATCH 16/26] Bump mio from 0.8.10 to 0.8.11 in /test_dependencies Bumps [mio](https://github.com/tokio-rs/mio) from 0.8.10 to 0.8.11. - [Release notes](https://github.com/tokio-rs/mio/releases) - [Changelog](https://github.com/tokio-rs/mio/blob/master/CHANGELOG.md) - [Commits](https://github.com/tokio-rs/mio/compare/v0.8.10...v0.8.11) --- updated-dependencies: - dependency-name: mio dependency-type: indirect ... Signed-off-by: dependabot[bot] --- src/tools/miri/test_dependencies/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/test_dependencies/Cargo.lock b/src/tools/miri/test_dependencies/Cargo.lock index 9d9db7b049857..11d430bad152c 100644 --- a/src/tools/miri/test_dependencies/Cargo.lock +++ b/src/tools/miri/test_dependencies/Cargo.lock @@ -183,9 +183,9 @@ dependencies = [ [[package]] name = "mio" -version = "0.8.10" +version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f3d0b296e374a4e6f3c7b0a1f5a51d748a0d34c85e7dc48fc3fa9a87657fe09" +checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" dependencies = [ "libc", "wasi 0.11.0+wasi-snapshot-preview1", From 4b955f14db7aedf4778fa6fd261f06d5bf17c54f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Mar 2024 19:40:31 +0100 Subject: [PATCH 17/26] 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 75124dc8ff519..bfce11a730e3e 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -1a1876c9790f168fb51afa335a7ba3e6fc267d75 +bfe762e0ed2e95041cc12c02c5565c4368f2cc9f From 3c2318c0b21e2f4473c2465a4d9481ccd21906dd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Mar 2024 19:48:55 +0100 Subject: [PATCH 18/26] make remaining FloatTy matches exhaustive --- src/tools/miri/src/helpers.rs | 21 +++++++++------------ src/tools/miri/src/shims/intrinsics/mod.rs | 15 ++++++++------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index d69c6fe901282..ddefecca597b2 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1102,20 +1102,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } - let (val, status) = match src.layout.ty.kind() { - // f32 - ty::Float(FloatTy::F32) => + let ty::Float(fty) = src.layout.ty.kind() else { + bug!("float_to_int_checked: non-float input type {}", src.layout.ty) + }; + + let (val, status) = match fty { + FloatTy::F16 => unimplemented!("f16_f128"), + FloatTy::F32 => float_to_int_inner::(this, src.to_scalar().to_f32()?, cast_to, round), - // f64 - ty::Float(FloatTy::F64) => + FloatTy::F64 => float_to_int_inner::(this, src.to_scalar().to_f64()?, cast_to, round), - // Nothing else - _ => - span_bug!( - this.cur_span(), - "attempted float-to-int conversion with non-float input type {}", - src.layout.ty, - ), + FloatTy::F128 => unimplemented!("f16_f128"), }; if status.intersects( diff --git a/src/tools/miri/src/shims/intrinsics/mod.rs b/src/tools/miri/src/shims/intrinsics/mod.rs index b67d588dbc9be..d4905c22a6afc 100644 --- a/src/tools/miri/src/shims/intrinsics/mod.rs +++ b/src/tools/miri/src/shims/intrinsics/mod.rs @@ -274,13 +274,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { _ => bug!(), }; let float_finite = |x: &ImmTy<'tcx, _>| -> InterpResult<'tcx, bool> { - Ok(match x.layout.ty.kind() { - ty::Float(FloatTy::F32) => x.to_scalar().to_f32()?.is_finite(), - ty::Float(FloatTy::F64) => x.to_scalar().to_f64()?.is_finite(), - _ => bug!( - "`{intrinsic_name}` called with non-float input type {ty:?}", - ty = x.layout.ty, - ), + let ty::Float(fty) = x.layout.ty.kind() else { + bug!("float_finite: non-float input type {}", x.layout.ty) + }; + Ok(match fty { + FloatTy::F16 => unimplemented!("f16_f128"), + FloatTy::F32 => x.to_scalar().to_f32()?.is_finite(), + FloatTy::F64 => x.to_scalar().to_f64()?.is_finite(), + FloatTy::F128 => unimplemented!("f16_f128"), }) }; match (float_finite(&a)?, float_finite(&b)?) { From c301bf9629077762fe6bec6ddbf1b293ab258e2a Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 8 Mar 2024 05:09:59 +0000 Subject: [PATCH 19/26] 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 bfce11a730e3e..a54e33ae528e0 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -bfe762e0ed2e95041cc12c02c5565c4368f2cc9f +79d246112dc95bbd67848f7546f3fd1aca516b82 From fcd2efeb1180162091ce8f79087b2cbd56ea0b76 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 8 Mar 2024 08:08:09 +0100 Subject: [PATCH 20/26] fix clippy lints --- src/tools/miri/src/concurrency/thread.rs | 2 +- src/tools/miri/src/eval.rs | 4 ++-- src/tools/miri/src/helpers.rs | 2 +- src/tools/miri/src/shims/windows/sync.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 8341fd7648b29..805c0580b2f15 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -890,7 +890,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { instance, start_abi, &[*func_arg], - Some(&ret_place.into()), + Some(&ret_place), StackPopCleanup::Root { cleanup: true }, )?; diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 4e080f0cc3ba1..f7edbdb9c5e5d 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -391,7 +391,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( argv, Scalar::from_u8(sigpipe).into(), ], - Some(&ret_place.into()), + Some(&ret_place), StackPopCleanup::Root { cleanup: true }, )?; } @@ -400,7 +400,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( entry_instance, Abi::Rust, &[argc.into(), argv], - Some(&ret_place.into()), + Some(&ret_place), StackPopCleanup::Root { cleanup: true }, )?; } diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 9e609833f077c..5e9de3ffb80e7 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -401,7 +401,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let mir = this.load_mir(f.def, None)?; let dest = match dest { Some(dest) => dest.clone(), - None => MPlaceTy::fake_alloc_zst(this.layout_of(mir.return_ty())?).into(), + None => MPlaceTy::fake_alloc_zst(this.layout_of(mir.return_ty())?), }; this.push_stack_frame(f, mir, &dest, stack_pop)?; diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index c0a79e959a6be..5edc18af482ea 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -179,7 +179,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let id = this.init_once_get_id(init_once_op)?; let flags = this.read_scalar(flags_op)?.to_u32()?; - let pending_place = this.deref_pointer(pending_op)?.into(); + let pending_place = this.deref_pointer(pending_op)?; let context = this.read_pointer(context_op)?; if flags != 0 { From 4235ec5baaf5c9594f925b4e58362d8f554bcba9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 00:36:19 +0100 Subject: [PATCH 21/26] compiletest: create fresh tempdir for tests to use --- src/tools/miri/Cargo.lock | 1 + src/tools/miri/Cargo.toml | 1 + src/tools/miri/tests/compiletest.rs | 43 ++++++++++++++++++--------- src/tools/miri/tests/pass/shims/fs.rs | 2 +- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index 1adae2b7a2275..4fb479e1c543b 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -497,6 +497,7 @@ dependencies = [ "regex", "rustc_version", "smallvec", + "tempfile", "ui_test", ] diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 39122c847ce0d..33a485d8939b5 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -45,6 +45,7 @@ ui_test = "0.21.1" rustc_version = "0.4" regex = "1.5.5" lazy_static = "1.4.0" +tempfile = "3" [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)]. diff --git a/src/tools/miri/tests/compiletest.rs b/src/tools/miri/tests/compiletest.rs index db0768848fd37..5e81689a6c241 100644 --- a/src/tools/miri/tests/compiletest.rs +++ b/src/tools/miri/tests/compiletest.rs @@ -79,13 +79,6 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> program.args.push(flag); } - // Add a test env var to do environment communication tests. - program.envs.push(("MIRI_ENV_VAR_TEST".into(), Some("0".into()))); - - // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find). - let miri_temp = env::var_os("MIRI_TEMP").unwrap_or_else(|| env::temp_dir().into()); - program.envs.push(("MIRI_TEMP".into(), Some(miri_temp))); - let mut config = Config { target: Some(target.to_owned()), stderr_filters: STDERR.clone(), @@ -116,9 +109,21 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> config } -fn run_tests(mode: Mode, path: &str, target: &str, with_dependencies: bool) -> Result<()> { +fn run_tests( + mode: Mode, + path: &str, + target: &str, + with_dependencies: bool, + tmpdir: &Path, +) -> Result<()> { let mut config = test_config(target, path, mode, with_dependencies); + // Add a test env var to do environment communication tests. + config.program.envs.push(("MIRI_ENV_VAR_TEST".into(), Some("0".into()))); + + // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find). + config.program.envs.push(("MIRI_TEMP".into(), Some(tmpdir.to_owned().into()))); + // Handle command-line arguments. let args = ui_test::Args::test()?; let default_bless = env::var_os("RUSTC_BLESS").is_some_and(|v| v != "0"); @@ -211,7 +216,13 @@ enum Dependencies { use Dependencies::*; -fn ui(mode: Mode, path: &str, target: &str, with_dependencies: Dependencies) -> Result<()> { +fn ui( + mode: Mode, + path: &str, + target: &str, + with_dependencies: Dependencies, + tmpdir: &Path, +) -> Result<()> { let msg = format!("## Running ui tests in {path} against miri for {target}"); eprintln!("{}", msg.green().bold()); @@ -219,7 +230,7 @@ fn ui(mode: Mode, path: &str, target: &str, with_dependencies: Dependencies) -> WithDependencies => true, WithoutDependencies => false, }; - run_tests(mode, path, target, with_dependencies) + run_tests(mode, path, target, with_dependencies, tmpdir) } fn get_target() -> String { @@ -230,6 +241,7 @@ fn main() -> Result<()> { ui_test::color_eyre::install()?; let target = get_target(); + let tmpdir = tempfile::Builder::new().prefix("miri-compiletest-").tempdir()?; let mut args = std::env::args_os(); @@ -240,28 +252,31 @@ fn main() -> Result<()> { } } - ui(Mode::Pass, "tests/pass", &target, WithoutDependencies)?; - ui(Mode::Pass, "tests/pass-dep", &target, WithDependencies)?; - ui(Mode::Panic, "tests/panic", &target, WithDependencies)?; + ui(Mode::Pass, "tests/pass", &target, WithoutDependencies, tmpdir.path())?; + ui(Mode::Pass, "tests/pass-dep", &target, WithDependencies, tmpdir.path())?; + ui(Mode::Panic, "tests/panic", &target, WithDependencies, tmpdir.path())?; ui( Mode::Fail { require_patterns: true, rustfix: RustfixMode::Disabled }, "tests/fail", &target, WithoutDependencies, + tmpdir.path(), )?; ui( Mode::Fail { require_patterns: true, rustfix: RustfixMode::Disabled }, "tests/fail-dep", &target, WithDependencies, + tmpdir.path(), )?; if cfg!(target_os = "linux") { - ui(Mode::Pass, "tests/extern-so/pass", &target, WithoutDependencies)?; + ui(Mode::Pass, "tests/extern-so/pass", &target, WithoutDependencies, tmpdir.path())?; ui( Mode::Fail { require_patterns: true, rustfix: RustfixMode::Disabled }, "tests/extern-so/fail", &target, WithoutDependencies, + tmpdir.path(), )?; } diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index 304a178dc3404..d10faebac7d73 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -295,7 +295,7 @@ fn test_canonicalize() { drop(File::create(&path).unwrap()); let p = canonicalize(format!("{}/./test_file", dir_path.to_string_lossy())).unwrap(); - assert_eq!(p.to_string_lossy().find('.'), None); + assert_eq!(p.to_string_lossy().find("/./"), None); remove_dir_all(&dir_path).unwrap(); } From 52db5f6cdc8d1e7c6270f9d2ce926d70ac706431 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 9 Mar 2024 04:54:42 +0000 Subject: [PATCH 22/26] 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 a54e33ae528e0..e23293ea558b7 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -79d246112dc95bbd67848f7546f3fd1aca516b82 +4d4bb491b65c300835442f6cb4f34fc9a5685c26 From 91644dd3898c42921f2aac3f76b987c8406cd7a1 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sat, 9 Mar 2024 05:02:53 +0000 Subject: [PATCH 23/26] fmt --- src/tools/miri/src/bin/miri.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index fc54baf1a67be..6955e649b4d17 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -341,7 +341,8 @@ fn main() { // (`install_ice_hook` might change `RUST_BACKTRACE`.) let env_snapshot = env::vars_os().collect::>(); - let args = rustc_driver::args::raw_args(&early_dcx).unwrap_or_else(|_| std::process::exit(rustc_driver::EXIT_FAILURE)); + let args = rustc_driver::args::raw_args(&early_dcx) + .unwrap_or_else(|_| std::process::exit(rustc_driver::EXIT_FAILURE)); // If the environment asks us to actually be rustc, then do that. if let Some(crate_kind) = env::var_os("MIRI_BE_RUSTC") { From 00be3525bf7d78fcc6a554355f6a3f87df9e1d05 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 09:13:50 +0100 Subject: [PATCH 24/26] =?UTF-8?q?rename=20tests/compiletest=20=E2=86=92=20?= =?UTF-8?q?tests/ui?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/tools/miri/Cargo.toml | 2 +- src/tools/miri/miri-script/src/commands.rs | 4 ++-- src/tools/miri/tests/{compiletest.rs => ui.rs} | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/tools/miri/tests/{compiletest.rs => ui.rs} (99%) diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 33a485d8939b5..041254e6f43a4 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -53,7 +53,7 @@ tempfile = "3" rustc_private = true [[test]] -name = "compiletest" +name = "ui" harness = false [features] diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index dc6ef58c52080..7fff6ae80b429 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -510,11 +510,11 @@ impl Command { let miri_flags = flagsplit(&miri_flags); let toolchain = &e.toolchain; let extra_flags = &e.cargo_extra_flags; - let edition_flags = (!have_edition).then_some("--edition=2021"); // keep in sync with `compiletest.rs`.` + let edition_flags = (!have_edition).then_some("--edition=2021"); // keep in sync with `tests/ui.rs`.` if dep { cmd!( e.sh, - "cargo +{toolchain} --quiet test --test compiletest {extra_flags...} --manifest-path {miri_manifest} -- --miri-run-dep-mode {miri_flags...} {edition_flags...} {flags...}" + "cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode {miri_flags...} {edition_flags...} {flags...}" ).quiet().run()?; } else { cmd!( diff --git a/src/tools/miri/tests/compiletest.rs b/src/tools/miri/tests/ui.rs similarity index 99% rename from src/tools/miri/tests/compiletest.rs rename to src/tools/miri/tests/ui.rs index 5e81689a6c241..7f363ccdfe59c 100644 --- a/src/tools/miri/tests/compiletest.rs +++ b/src/tools/miri/tests/ui.rs @@ -241,7 +241,7 @@ fn main() -> Result<()> { ui_test::color_eyre::install()?; let target = get_target(); - let tmpdir = tempfile::Builder::new().prefix("miri-compiletest-").tempdir()?; + let tmpdir = tempfile::Builder::new().prefix("miri-uitest-").tempdir()?; let mut args = std::env::args_os(); From 862f918fa484c680b926463c0cad3236a2eeffed Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 12:33:17 +0100 Subject: [PATCH 25/26] fix clippy lints --- src/tools/miri/src/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index f2dc6c14058b6..f0e3c43a5c5f3 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1333,7 +1333,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { // If we have a borrow tracker, we also have it set up protection so that all reads *and // writes* during this call are insta-UB. let protected_place = if ecx.machine.borrow_tracker.is_some() { - ecx.protect_place(&place)?.into() + ecx.protect_place(place)? } else { // No borrow tracker. place.clone() From 9a308d45cf586750eca6521eaa680fea4f6f4049 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 18:56:52 +0100 Subject: [PATCH 26/26] update lockfile --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 853acd1abd6dd..a87404c89819e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2493,6 +2493,7 @@ dependencies = [ "regex", "rustc_version", "smallvec", + "tempfile", "ui_test 0.21.2", ]