diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index b71f48e4644bb..da1c2f770ac4f 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -174,7 +174,12 @@ jobs: ~/.local/bin/zulip-send --stream miri --subject "Cron Job Failure (miri, $(date -u +%Y-%m))" \ --message 'Dear @*T-miri*, - It would appear that the Miri cron job build failed. Would you mind investigating this issue? + It would appear that the [Miri cron job build]('"https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"') failed. + + This likely means that rustc changed the miri directory and + we now need to do a [`./miri rustc-pull`](https://github.com/rust-lang/miri/blob/master/CONTRIBUTING.md#importing-changes-from-the-rustc-repo). + + Would you mind investigating this issue? Thanks in advance! Sincerely, diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index d17bb9533b438..46deebf2cdde9 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -282,9 +282,9 @@ checksum = "201de327520df007757c1f0adce6e827fe8562fbc28bfd9c15571c66ca1f5f79" [[package]] name = "libffi" -version = "3.0.1" +version = "3.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e454b3efb16fba3b17810ae5e41df02b649e564ab3c5a34b3b93ed07ad287e6" +checksum = "ce826c243048e3d5cec441799724de52e2d42f820468431fc3fceee2341871e2" dependencies = [ "libc", "libffi-sys", @@ -292,9 +292,9 @@ dependencies = [ [[package]] name = "libffi-sys" -version = "2.0.1" +version = "2.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "84e78d02e5a8eae9c24c38ce6e6026f80e16dff76adcdae4bc5c6c52c2de4a60" +checksum = "dc65067b78c0fc069771e8b9a9e02df71e08858bec92c1f101377c67b9dca7c7" dependencies = [ "cc", ] diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index b70f7e0e55634..4c7351879877e 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -15,7 +15,7 @@ for example: or an invalid enum discriminant) * **Experimental**: Violations of the [Stacked Borrows] rules governing aliasing for reference types -* **Experimental**: Violations of the Tree Borrows aliasing rules, as an optional +* **Experimental**: Violations of the [Tree Borrows] aliasing rules, as an optional alternative to [Stacked Borrows] * **Experimental**: Data races @@ -79,6 +79,7 @@ behavior** in your program, and cannot run all programs: [`unreachable_unchecked`]: https://doc.rust-lang.org/stable/std/hint/fn.unreachable_unchecked.html [`copy_nonoverlapping`]: https://doc.rust-lang.org/stable/std/ptr/fn.copy_nonoverlapping.html [Stacked Borrows]: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md +[Tree Borrows]: https://perso.crans.org/vanille/treebor/ ## Using Miri @@ -359,7 +360,7 @@ to Miri failing to detect cases of undefined behavior in a program. * `-Zmiri-disable-data-race-detector` disables checking for data races. Using this flag is **unsound**. This implies `-Zmiri-disable-weak-memory-emulation`. * `-Zmiri-disable-stacked-borrows` disables checking the experimental - aliasing rules to track borrows ([Stacked Borrows] and Tree Borrows). + aliasing rules to track borrows ([Stacked Borrows] and [Tree Borrows]). This can make Miri run faster, but it also means no aliasing violations will be detected. Using this flag is **unsound** (but the affected soundness rules are experimental). Later flags take precedence: borrow tracking can be reactivated @@ -425,7 +426,7 @@ to Miri failing to detect cases of undefined behavior in a program. * `-Zmiri-track-weak-memory-loads` shows a backtrace when weak memory emulation returns an outdated 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. +* `-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. * `-Zmiri-force-page-size=` overrides the default page size for an architecture, in multiples of 1k. @@ -442,7 +443,7 @@ Some native rustc `-Z` flags are also very relevant for Miri: functions. This is needed so that Miri can execute such functions, so Miri sets this flag per default. * `-Zmir-emit-retag` controls whether `Retag` statements are emitted. Miri - enables this per default because it is needed for [Stacked Borrows] and Tree Borrows. + enables this per default because it is needed for [Stacked Borrows] and [Tree Borrows]. Moreover, Miri recognizes some environment variables: diff --git a/src/tools/miri/ci.sh b/src/tools/miri/ci.sh index ef52a37fe319f..b5b3b211b05f0 100755 --- a/src/tools/miri/ci.sh +++ b/src/tools/miri/ci.sh @@ -43,7 +43,9 @@ function run_tests { # optimizations up all the way, too). # Optimizations change diagnostics (mostly backtraces), so we don't check # them. Also error locations change so we don't run the failing tests. - MIRIFLAGS="${MIRIFLAGS:-} -O -Zmir-opt-level=4" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} + # We explicitly enable debug-assertions here, they are disabled by -O but we have tests + # which exist to check that we panic on debug assertion failures. + MIRIFLAGS="${MIRIFLAGS:-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} # Also run some many-seeds tests. 64 seeds means this takes around a minute per test. for FILE in tests/many-seeds/*.rs; do diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 18c2561242a39..f1ed3be2edd39 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -511364e7874dba9649a264100407e4bffe7b5425 +d4be8efc6296bace5b1e165f1b34d3c6da76aa8e diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 0aea105ccc49d..26a7ead2407cb 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -28,9 +28,11 @@ use rustc_middle::{ middle::exported_symbols::{ ExportedSymbol, SymbolExportInfo, SymbolExportKind, SymbolExportLevel, }, - ty::{query::ExternProviders, TyCtxt}, query::LocalCrate, + ty::{query::ExternProviders, TyCtxt}, }; +use rustc_session::config::OptLevel; + use rustc_session::{config::CrateType, search_paths::PathKind, CtfeBacktrace}; use miri::{BacktraceStyle, BorrowTrackerMethod, ProvenanceMode, RetagFields}; @@ -83,6 +85,21 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { env::set_current_dir(cwd).unwrap(); } + if tcx.sess.opts.optimize != OptLevel::No { + tcx.sess.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."); + } + if tcx.sess.mir_opt_level() > 0 { + tcx.sess.warn("You have explicitly enabled MIR optimizations, overriding Miri's default \ + which is to completely disable them. Any optimizations may hide UB that Miri would \ + otherwise detect, and it is not necessarily possible to predict what kind of UB will \ + be missed. If you are enabling optimizations to make Miri run faster, we advise using \ + cfg(miri) to shrink your workload instead. The performance benefit of enabling MIR \ + optimizations is usually marginal at best."); + } + if let Some(return_code) = miri::eval_entry(tcx, entry_def_id, entry_type, config) { std::process::exit( i32::try_from(return_code).expect("Return value was too large!"), diff --git a/src/tools/miri/src/clock.rs b/src/tools/miri/src/clock.rs index 3f33273e1e541..24bf90f104ffd 100644 --- a/src/tools/miri/src/clock.rs +++ b/src/tools/miri/src/clock.rs @@ -3,7 +3,10 @@ use std::time::{Duration, Instant as StdInstant}; /// When using a virtual clock, this defines how many nanoseconds we pretend are passing for each /// basic block. -const NANOSECONDS_PER_BASIC_BLOCK: u64 = 10; +/// This number is pretty random, but it has been shown to approximately cause +/// some sample programs to run within an order of magnitude of real time on desktop CPUs. +/// (See `tests/pass/shims/time-with-isolation*.rs`.) +const NANOSECONDS_PER_BASIC_BLOCK: u64 = 5000; #[derive(Debug)] pub struct Instant { diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index fb4e59acd001d..5c8aba6d441f4 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -124,11 +124,13 @@ pub use crate::tag_gc::{EvalContextExt as _, VisitTags}; /// Insert rustc arguments at the beginning of the argument list that Miri wants to be /// set per default, for maximal validation power. +/// Also disable the MIR pass that inserts an alignment check on every pointer dereference. Miri +/// does that too, and with a better error message. pub const MIRI_DEFAULT_ARGS: &[&str] = &[ + "--cfg=miri", "-Zalways-encode-mir", + "-Zextra-const-ub-checks", "-Zmir-emit-retag", "-Zmir-opt-level=0", - "--cfg=miri", - "-Cdebug-assertions=on", - "-Zextra-const-ub-checks", + "-Zmir-enable-passes=-CheckAlignment", ]; diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 2cca2f3f3914d..18ae01a19f914 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -157,7 +157,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { &[catch_unwind.data.into(), payload.into()], None, // Directly return to caller of `try`. - StackPopCleanup::Goto { ret: Some(catch_unwind.ret), unwind: mir::UnwindAction::Continue }, + StackPopCleanup::Goto { + ret: Some(catch_unwind.ret), + unwind: mir::UnwindAction::Continue, + }, )?; // We pushed a new stack frame, the engine should not do any jumping now! @@ -211,10 +214,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Abi::Rust, &[index.into(), len.into()], None, - StackPopCleanup::Goto { - ret: None, - unwind, - }, + StackPopCleanup::Goto { ret: None, unwind }, )?; } MisalignedPointerDereference { required, found } => { @@ -235,19 +235,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Abi::Rust, &[required.into(), found.into()], None, - StackPopCleanup::Goto { - ret: None, - unwind, - }, + StackPopCleanup::Goto { ret: None, unwind }, )?; } _ => { // Forward everything else to `panic` lang item. - this.start_panic( - msg.description(), - unwind, - )?; + this.start_panic(msg.description(), unwind)?; } } Ok(()) diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index 1eca389e98429..de271548217af 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -628,13 +628,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let fd = this.read_scalar(&args[0])?.to_i32()?; let cmd = this.read_scalar(&args[1])?.to_i32()?; - // Reject if isolation is enabled. - if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("`fcntl`", reject_with)?; - this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; - return Ok(-1); - } - // We only support getting the flags for a descriptor. if cmd == this.eval_libc_i32("F_GETFD") { // Currently this is the only flag that `F_GETFD` returns. It is OK to just return the @@ -677,6 +670,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { None => this.handle_not_found(), } } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") { + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`fcntl`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } + if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { // FIXME: Support fullfsync for all FDs let FileHandle { file, writable } = diff --git a/src/tools/miri/tests/fail/terminate-terminator.stderr b/src/tools/miri/tests/fail/terminate-terminator.stderr index 3befd83007bfd..c046678f73f5b 100644 --- a/src/tools/miri/tests/fail/terminate-terminator.stderr +++ b/src/tools/miri/tests/fail/terminate-terminator.stderr @@ -1,3 +1,5 @@ +warning: You have explicitly enabled MIR optimizations, overriding Miri's default which is to completely disable them. Any optimizations may hide UB that Miri would otherwise detect, and it is not necessarily possible to predict what kind of UB will be missed. If you are enabling optimizations to make Miri run faster, we advise using cfg(miri) to shrink your workload instead. The performance benefit of enabling MIR optimizations is usually marginal at best. + thread 'main' panicked at 'explicit panic', $DIR/terminate-terminator.rs:LL:CC note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace error: abnormal termination: panic in a function that cannot unwind @@ -23,5 +25,5 @@ LL | panic_abort(); note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to previous error +error: aborting due to previous error; 1 warning emitted diff --git a/src/tools/miri/tests/panic/alignment-assertion.rs b/src/tools/miri/tests/panic/alignment-assertion.rs deleted file mode 100644 index 68aa19a88db09..0000000000000 --- a/src/tools/miri/tests/panic/alignment-assertion.rs +++ /dev/null @@ -1,9 +0,0 @@ -//@compile-flags: -Zmiri-disable-alignment-check -Cdebug-assertions=yes - -fn main() { - let mut x = [0u32; 2]; - let ptr: *mut u8 = x.as_mut_ptr().cast::(); - unsafe { - *(ptr.add(1).cast::()) = 42; - } -} diff --git a/src/tools/miri/tests/panic/alignment-assertion.stderr b/src/tools/miri/tests/panic/alignment-assertion.stderr deleted file mode 100644 index 26cf51b0cd2e3..0000000000000 --- a/src/tools/miri/tests/panic/alignment-assertion.stderr +++ /dev/null @@ -1,2 +0,0 @@ -thread 'main' panicked at 'misaligned pointer dereference: address must be a multiple of 0x4 but is $HEX', $DIR/alignment-assertion.rs:LL:CC -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/src/tools/miri/tests/pass-dep/shims/fcntl_f-fullfsync_apple.rs b/src/tools/miri/tests/pass-dep/shims/fcntl_f-fullfsync_apple.rs new file mode 100644 index 0000000000000..307906f2583b2 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/shims/fcntl_f-fullfsync_apple.rs @@ -0,0 +1,12 @@ +//@only-target-apple: F_FULLFSYNC only on apple systems +//@compile-flags: -Zmiri-isolation-error=warn-nobacktrace + +use std::io::Error; + +fn main() { + // test `fcntl(F_FULLFSYNC)` + unsafe { + assert_eq!(libc::fcntl(1, libc::F_FULLFSYNC, 0), -1); + assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EPERM)); + } +} diff --git a/src/tools/miri/tests/pass-dep/shims/fcntl_f-fullfsync_apple.stderr b/src/tools/miri/tests/pass-dep/shims/fcntl_f-fullfsync_apple.stderr new file mode 100644 index 0000000000000..09a24e1e5d74d --- /dev/null +++ b/src/tools/miri/tests/pass-dep/shims/fcntl_f-fullfsync_apple.stderr @@ -0,0 +1,2 @@ +warning: `fcntl` was made to return an error due to isolation + diff --git a/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.rs b/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.rs index d6d19a3fe8159..5185db0b0e29f 100644 --- a/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.rs +++ b/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.rs @@ -7,10 +7,9 @@ use std::fs; use std::io::{Error, ErrorKind}; fn main() { - // test `fcntl` + // test `fcntl(F_DUPFD): should work even with isolation.` unsafe { - assert_eq!(libc::fcntl(1, libc::F_DUPFD, 0), -1); - assert_eq!(Error::last_os_error().raw_os_error(), Some(libc::EPERM)); + assert!(libc::fcntl(1, libc::F_DUPFD, 0) >= 0); } // test `readlink` diff --git a/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.stderr b/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.stderr index 21fcb65243e26..b0cadfb970bf3 100644 --- a/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.stderr +++ b/src/tools/miri/tests/pass-dep/shims/libc-fs-with-isolation.stderr @@ -1,5 +1,3 @@ -warning: `fcntl` was made to return an error due to isolation - warning: `readlink` was made to return an error due to isolation warning: `$STAT` was made to return an error due to isolation diff --git a/src/tools/miri/tests/pass-dep/tokio/sleep.rs b/src/tools/miri/tests/pass-dep/tokio/sleep.rs index 1341484dda475..00cc68eba3eac 100644 --- a/src/tools/miri/tests/pass-dep/tokio/sleep.rs +++ b/src/tools/miri/tests/pass-dep/tokio/sleep.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmiri-permissive-provenance -Zmiri-backtrace=full +//@compile-flags: -Zmiri-permissive-provenance -Zmiri-backtrace=full //@only-target-x86_64-unknown-linux: support for tokio only on linux and x86 use tokio::time::{sleep, Duration, Instant}; @@ -7,8 +7,6 @@ use tokio::time::{sleep, Duration, Instant}; async fn main() { let start = Instant::now(); sleep(Duration::from_secs(1)).await; - // It takes 96 millisecond to sleep for 1 millisecond - // It takes 1025 millisecond to sleep for 1 second let time_elapsed = &start.elapsed().as_millis(); - assert!(time_elapsed > &1000, "{}", time_elapsed); + assert!((1000..1100).contains(time_elapsed), "{}", time_elapsed); } diff --git a/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs b/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs index 0bca7cc069a78..0ed2a941bc493 100644 --- a/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs +++ b/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs @@ -1,5 +1,5 @@ // Need to disable preemption to stay on the supported MVP codepath in mio. -//@compile-flags: -Zmiri-disable-isolation -Zmiri-permissive-provenance +//@compile-flags: -Zmiri-permissive-provenance //@only-target-x86_64-unknown-linux: support for tokio exists only on linux and x86 #[tokio::main] diff --git a/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs b/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs index bf004012e8489..7852d495e28fa 100644 --- a/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs +++ b/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs @@ -7,6 +7,6 @@ fn main() { thread::park_timeout(Duration::from_millis(200)); - // Thanks to deterministic execution, this will wiat *exactly* 200ms (rounded to 1ms). - assert!((200..201).contains(&start.elapsed().as_millis())); + // Thanks to deterministic execution, this will wait *exactly* 200ms, plus the time for the surrounding code. + assert!((200..210).contains(&start.elapsed().as_millis()), "{}", start.elapsed().as_millis()); } diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation.rs b/src/tools/miri/tests/pass/shims/time-with-isolation.rs index b6444319b59b9..a0c3c6bbaa92e 100644 --- a/src/tools/miri/tests/pass/shims/time-with-isolation.rs +++ b/src/tools/miri/tests/pass/shims/time-with-isolation.rs @@ -22,14 +22,23 @@ fn test_time_passes() { let diff = now2.duration_since(now1); assert_eq!(now1 + diff, now2); assert_eq!(now2 - diff, now1); - // The virtual clock is deterministic and I got 29us on a 64-bit Linux machine. However, this + // The virtual clock is deterministic and I got 15ms on a 64-bit Linux machine. However, this // changes according to the platform so we use an interval to be safe. This should be updated // if `NANOSECONDS_PER_BASIC_BLOCK` changes. - assert!(diff.as_micros() > 10); - assert!(diff.as_micros() < 40); + assert!(diff.as_millis() > 5); + assert!(diff.as_millis() < 20); +} + +fn test_block_for_one_second() { + let end = Instant::now() + Duration::from_secs(1); + // This takes a long time, as we only increment when statements are executed. + // When we sleep on all threads, we fast forward to the sleep duration, which we can't + // do with busy waiting. + while Instant::now() < end {} } fn main() { test_time_passes(); + test_block_for_one_second(); test_sleep(); } diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation2.rs b/src/tools/miri/tests/pass/shims/time-with-isolation2.rs new file mode 100644 index 0000000000000..24e72e22fd895 --- /dev/null +++ b/src/tools/miri/tests/pass/shims/time-with-isolation2.rs @@ -0,0 +1,8 @@ +use std::time::Instant; + +fn main() { + let begin = Instant::now(); + for _ in 0..100_000 {} + let time = begin.elapsed(); + println!("The loop took around {}s", time.as_secs()); +} diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout b/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout new file mode 100644 index 0000000000000..641e469f50c26 --- /dev/null +++ b/src/tools/miri/tests/pass/shims/time-with-isolation2.stdout @@ -0,0 +1 @@ +The loop took around 13s