diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1097fc6db7..b0dab9f509 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,17 +24,22 @@ jobs: include: - os: ubuntu-latest host_target: x86_64-unknown-linux-gnu - - os: macos-latest - host_target: x86_64-apple-darwin + - os: macos-14 + host_target: aarch64-apple-darwin - os: windows-latest host_target: i686-pc-windows-msvc runs-on: ${{ matrix.os }} env: - RUST_BACKTRACE: 1 HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v3 + - 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). @@ -44,21 +49,22 @@ jobs: with: path: | # Taken from . - ~/.cargo/bin + # Cache package/registry information ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db - # contains package information of crates installed via `cargo install`. + # Cache installed binaries + ~/.cargo/bin ~/.cargo/.crates.toml ~/.cargo/.crates2.json - key: ${{ runner.os }}-cargo-reset20230315-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-cargo-reset20230315 + 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 + - name: Install miri toolchain run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then echo "Building against latest rustc git version" @@ -66,13 +72,13 @@ jobs: fi ./miri toolchain --host ${{ matrix.host_target }} - - name: Show Rust version + - name: Show Rust version (miri toolchain) run: | rustup show rustc -Vv cargo -V - - name: Test + - name: Test Miri run: ./ci/ci.sh style: @@ -93,15 +99,16 @@ jobs: with: path: | # Taken from . - ~/.cargo/bin + # Cache package/registry information ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db - # contains package information of crates installed via `cargo install`. + # Cache installed binaries + ~/.cargo/bin ~/.cargo/.crates.toml ~/.cargo/.crates2.json - key: ${{ runner.os }}-cargo-reset20230315-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-cargo-reset20230315 + 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' }} @@ -125,6 +132,10 @@ jobs: run: ./miri fmt --check - name: clippy run: ./miri clippy -- -D warnings + - name: clippy (no features) + run: ./miri clippy --no-default-features -- -D warnings + - name: clippy (all features) + run: ./miri clippy --all-features -- -D warnings - name: rustdoc run: RUSTDOCFLAGS="-Dwarnings" ./miri cargo doc --document-private-items @@ -184,7 +195,7 @@ jobs: with: fetch-depth: 256 # get a bit more of the history - name: install josh-proxy - run: RUSTFLAGS="--cap-lints warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r22.12.06 + run: RUSTFLAGS="--cap-lints warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r23.12.04 - name: setup bot git name and email run: | git config --global user.name 'The Miri Cronjob Bot' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f2f3a642e0..60bc1d5282 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,19 +64,22 @@ For example, you can (cross-)run the driver on a particular file by doing ./miri run tests/pass/hello.rs --target i686-unknown-linux-gnu ``` -and you can (cross-)run the entire test suite using: +Tests in ``pass-dep`` need to be run using ``./miri run --dep ``. +For example: +```sh +./miri run --dep tests/pass-dep/shims/libc-fs.rs +``` + +You can (cross-)run the entire test suite using: ``` ./miri test MIRI_TEST_TARGET=i686-unknown-linux-gnu ./miri test ``` -If your target doesn't support libstd that should usually just work. However, if you are using a -custom target file, you might have to set `MIRI_NO_STD=1`. - `./miri test FILTER` only runs those tests that contain `FILTER` in their filename (including the base directory, e.g. `./miri test fail` will run all compile-fail tests). These filters are passed -to `cargo test`, so for multiple filers you need to use `./miri test -- FILTER1 FILTER2`. +to `cargo test`, so for multiple filters you need to use `./miri test -- FILTER1 FILTER2`. #### Fine grained logging @@ -178,6 +181,7 @@ to `.vscode/settings.json` in your local Miri clone: "cargo", "clippy", // make this `check` when working with a locally built rustc "--message-format=json", + "--all-targets", ], // Contrary to what the name suggests, this also affects proc macros. "rust-analyzer.cargo.buildScripts.overrideCommand": [ @@ -187,6 +191,7 @@ to `.vscode/settings.json` in your local Miri clone: "cargo", "check", "--message-format=json", + "--all-targets", ], } ``` @@ -236,18 +241,20 @@ We use the [`josh` proxy](https://github.com/josh-project/josh) to transmit chan rustc and Miri repositories. You can install it as follows: ```sh -cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r22.12.06 +RUSTFLAGS="--cap-lints=warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r23.12.04 ``` Josh will automatically be started and stopped by `./miri`. ### Importing changes from the rustc repo +*Note: this usually happens automatically, so these steps rarely have to be done by hand.* + We assume we start on an up-to-date master branch in the Miri repo. ```sh # Fetch and merge rustc side of the history. Takes ca 5 min the first time. -# This will also update the 'rustc-version' file. +# This will also update the `rustc-version` file. ./miri rustc-pull # Update local toolchain and apply formatting. ./miri toolchain && ./miri fmt @@ -261,12 +268,6 @@ needed. ### Exporting changes to the rustc repo -Keep in mind that pushing is the most complicated job that josh has to do -- pulling just filters -the rustc history, but pushing needs to construct a new rustc history that would filter to the given -Miri history! To avoid problems, it is a good idea to always pull immediately before you push. If -you are getting strange errors, chances are you are running into [this josh -bug](https://github.com/josh-project/josh/issues/998). In that case, please get in touch on Zulip. - We will use the josh proxy to push to your fork of rustc. Run the following in the Miri repo, assuming we are on an up-to-date master branch: @@ -275,9 +276,9 @@ assuming we are on an up-to-date master branch: ./miri rustc-push YOUR_NAME miri ``` -This will create a new branch called 'miri' in your fork, and the output should -include a link to create a rustc PR that will integrate those changes into the -main repository. +This will create a new branch called `miri` in your fork, and the output should include a link that +creates a rustc PR to integrate those changes into the main repository. If that PR has conflicts, +you need to pull rustc changes into Miri first, and then re-do the rustc push. If this fails due to authentication problems, it can help to make josh push via ssh instead of https. Add the following to your `.gitconfig`: diff --git a/Cargo.toml b/Cargo.toml index eb75ec2a17..dce0c0c56a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,7 @@ harness = false [features] default = ["stack-cache"] stack-cache = [] +stack-cache-consistency-check = ["stack-cache"] # Be aware that this file is inside a workspace when used via the # submodule in the rustc repo. That means there are many cargo features diff --git a/README.md b/README.md index b043805291..60a1c1fa1d 100644 --- a/README.md +++ b/README.md @@ -324,7 +324,7 @@ environment variable. We first document the most relevant and most commonly used number of available CPUs is `1`. Note that this flag does not affect how miri handles threads in any way. * `-Zmiri-permissive-provenance` disables the warning for integer-to-pointer casts and - [`ptr::from_exposed_addr`](https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html). + [`ptr::with_exposed_provenance`](https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html). This will necessarily miss some bugs as those operations are not efficiently and accurately implementable in a sanitizer, but it will only miss bugs that concern memory/pointers which is subject to these operations. @@ -505,6 +505,10 @@ binaries, and as such worth documenting: * `MIRI_LOCAL_CRATES` is set by `cargo-miri` to tell the Miri driver which crates should be given special treatment in diagnostics, in addition to the crate currently being compiled. +* `MIRI_ORIG_RUSTDOC` is set and read by different phases of `cargo-miri` to remember the + value of `RUSTDOC` from before it was overwritten. +* `MIRI_REPLACE_LIBRS_IF_NOT_TEST` when set to any value enables a hack that helps bootstrap + run the standard library tests in Miri. * `MIRI_VERBOSE` when set to any value tells the various `cargo-miri` phases to perform verbose logging. * `MIRI_HOST_SYSROOT` is set by bootstrap to tell `cargo-miri` which sysroot to use for *host* diff --git a/cargo-miri/src/main.rs b/cargo-miri/src/main.rs index c5fada6fe5..9fdd4c3e47 100644 --- a/cargo-miri/src/main.rs +++ b/cargo-miri/src/main.rs @@ -11,10 +11,18 @@ use std::{env, iter}; use crate::phases::*; +/// Returns `true` if our flags look like they may be for rustdoc, i.e., this is cargo calling us to +/// be rustdoc. It's hard to be sure as cargo does not have a RUSTDOC_WRAPPER or an env var that +/// would let us get a clear signal. +fn looks_like_rustdoc() -> bool { + // The `--test-run-directory` flag only exists for rustdoc and cargo always passes it. Perfect! + env::args().any(|arg| arg == "--test-run-directory") +} + fn main() { // Rustc does not support non-UTF-8 arguments so we make no attempt either. // (We do support non-UTF-8 environment variables though.) - let mut args = std::env::args(); + let mut args = env::args(); // Skip binary name. args.next().unwrap(); @@ -91,10 +99,16 @@ fn main() { // (see https://github.com/rust-lang/cargo/issues/10886). phase_rustc(args, RustcPhase::Build) } - _ => { - // Everything else must be rustdoc. But we need to get `first` "back onto the iterator", + _ if looks_like_rustdoc() => { + // This is probably rustdoc. But we need to get `first` "back onto the iterator", // it is some part of the rustdoc invocation. phase_rustdoc(iter::once(first).chain(args)); } + _ => { + show_error!( + "`cargo-miri` failed to recognize which phase of the build process this is, please report a bug.\nThe command-line arguments were: {:#?}", + Vec::from_iter(env::args()), + ); + } } } diff --git a/cargo-miri/src/phases.rs b/cargo-miri/src/phases.rs index 81ff68545c..3f6c484a05 100644 --- a/cargo-miri/src/phases.rs +++ b/cargo-miri/src/phases.rs @@ -3,7 +3,7 @@ use std::env; use std::fs::{self, File}; use std::io::BufReader; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; use rustc_version::VersionMeta; @@ -89,8 +89,12 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { let verbose = num_arg_flag("-v"); // Determine the involved architectures. - let rustc_version = VersionMeta::for_command(miri_for_host()) - .expect("failed to determine underlying rustc version of Miri"); + let rustc_version = VersionMeta::for_command(miri_for_host()).unwrap_or_else(|err| { + panic!( + "failed to determine underlying rustc version of Miri ({:?}):\n{err:?}", + miri_for_host() + ) + }); let host = &rustc_version.host; let target = get_arg_flag_value("--target"); let target = target.as_ref().unwrap_or(host); @@ -179,18 +183,27 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { ); } cmd.env("RUSTC_WRAPPER", &cargo_miri_path); + // There's also RUSTC_WORKSPACE_WRAPPER, which gets in the way of our own wrapping. + if env::var_os("RUSTC_WORKSPACE_WRAPPER").is_some() { + println!( + "WARNING: Ignoring `RUSTC_WORKSPACE_WRAPPER` environment variable, Miri does not support wrapping." + ); + } + cmd.env_remove("RUSTC_WORKSPACE_WRAPPER"); // We are going to invoke `MIRI` for everything, not `RUSTC`. if env::var_os("RUSTC").is_some() && env::var_os("MIRI").is_none() { println!( "WARNING: Ignoring `RUSTC` environment variable; set `MIRI` if you want to control the binary used as the driver." ); } - // Build scripts (and also cargo: https://github.com/rust-lang/cargo/issues/10885) will invoke - // `rustc` even when `RUSTC_WRAPPER` is set. To make sure everything is coherent, we want that - // to be the Miri driver, but acting as rustc, on the target level. (Target, rather than host, - // is needed for cross-interpretation situations.) This is not a perfect emulation of real rustc - // (it might be unable to produce binaries since the sysroot is check-only), but it's as close - // as we can get, and it's good enough for autocfg. + // Ideally we would set RUSTC to some non-existent path, so we can be sure our wrapping is + // always applied. However, buggy build scripts (https://github.com/eyre-rs/eyre/issues/84) and + // also cargo (https://github.com/rust-lang/cargo/issues/10885) will invoke `rustc` even when + // `RUSTC_WRAPPER` is set, bypassing the wrapper. To make sure everything is coherent, we want + // that to be the Miri driver, but acting as rustc, on the target level. (Target, rather than + // host, is needed for cross-interpretation situations.) This is not a perfect emulation of real + // rustc (it might be unable to produce binaries since the sysroot is check-only), but it's as + // close as we can get, and it's good enough for autocfg. // // In `main`, we need the value of `RUSTC` to distinguish RUSTC_WRAPPER invocations from rustdoc // or TARGET_RUNNER invocations, so we canonicalize it here to make it exceedingly unlikely that @@ -202,6 +215,9 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { cmd.env("MIRI_BE_RUSTC", "target"); // we better remember to *unset* this in the other phases! // Set rustdoc to us as well, so we can run doctests. + if let Some(orig_rustdoc) = env::var_os("RUSTDOC") { + cmd.env("MIRI_ORIG_RUSTDOC", orig_rustdoc); + } cmd.env("RUSTDOC", &cargo_miri_path); cmd.env("MIRI_LOCAL_CRATES", local_crates(&metadata)); @@ -210,7 +226,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { } // Run cargo. - debug_cmd("[cargo-miri miri]", verbose, &cmd); + debug_cmd("[cargo-miri cargo]", verbose, &cmd); exec(cmd) } @@ -244,6 +260,16 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { /// Cargo does not give us this information directly, so we need to check /// various command-line flags. fn is_runnable_crate() -> bool { + // Determine whether this is cargo invoking rustc to get some infos. Ideally we'd check "is + // there a filename passed to rustc", but that's very hard as we would have to know whether + // e.g. `--print foo` is a booolean flag `--print` followed by filename `foo` or equivalent + // to `--print=foo`. So instead we use this more fragile approach of detecting the presence + // of a "query" flag rather than the absence of a filename. + let info_query = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV"); + if info_query { + // Nothing to run. + return false; + } let is_bin = get_arg_flag_value("--crate-type").as_deref().unwrap_or("bin") == "bin"; let is_test = has_arg_flag("--test"); is_bin || is_test @@ -282,16 +308,9 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } } - // phase_cargo_miri set `MIRI_BE_RUSTC` for when build scripts directly invoke the driver; - // however, if we get called back by cargo here, we'll carefully compute the right flags - // ourselves, so we first un-do what the earlier phase did. - env::remove_var("MIRI_BE_RUSTC"); - let verbose = std::env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); let target_crate = is_target_crate(); - // Determine whether this is cargo invoking rustc to get some infos. - let info_query = get_arg_flag_value("--print").is_some() || has_arg_flag("-vV"); let store_json = |info: CrateRunInfo| { if get_arg_flag_value("--emit").unwrap_or_default().split(',').any(|e| e == "dep-info") { @@ -318,7 +337,7 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } }; - let runnable_crate = !info_query && is_runnable_crate(); + let runnable_crate = is_runnable_crate(); if runnable_crate && target_crate { assert!( @@ -392,10 +411,26 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { let mut emit_link_hack = false; // Arguments are treated very differently depending on whether this crate is // for interpretation by Miri, or for use by a build script / proc macro. - if !info_query && target_crate { - // Forward arguments, but remove "link" from "--emit" to make this a check-only build. + if target_crate { + // Forward arguments, but patched. let emit_flag = "--emit"; + // This hack helps bootstrap run standard library tests in Miri. The issue is as follows: + // when running `cargo miri test` on libcore, cargo builds a local copy of core and makes it + // a dependency of the integration test crate. This copy duplicates all the lang items, so + // the build fails. (Regular testing avoids this because the sysroot is a literal copy of + // what `cargo build` produces, but since Miri builds its own sysroot this does not work for + // us.) So we need to make it so that the locally built libcore contains all the items from + // `core`, but does not re-define them -- we want to replace the entire crate but a + // re-export of the sysroot crate. We do this by swapping out the source file: if + // `MIRI_REPLACE_LIBRS_IF_NOT_TEST` is set and we are building a `lib.rs` file, and a + // `lib.miri.rs` file exists in the same folder, we build that instead. But crucially we + // only do that for the library, not the unit test crate (which would be runnable) or + // rustdoc (which would have a different `phase`). + let replace_librs = env::var_os("MIRI_REPLACE_LIBRS_IF_NOT_TEST").is_some() + && !runnable_crate + && phase == RustcPhase::Build; while let Some(arg) = args.next() { + // Patch `--emit`: remove "link" from "--emit" to make this a check-only build. if let Some(val) = arg.strip_prefix(emit_flag) { // Patch this argument. First, extract its value. let val = @@ -410,13 +445,36 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { } } cmd.arg(format!("{emit_flag}={}", val.join(","))); - } else if arg == "--extern" { - // Patch `--extern` filenames, since Cargo sometimes passes stub `.rlib` files: - // https://github.com/rust-lang/miri/issues/1705 + continue; + } + // Patch `--extern` filenames, since Cargo sometimes passes stub `.rlib` files: + // https://github.com/rust-lang/miri/issues/1705 + if arg == "--extern" { forward_patched_extern_arg(&mut args, &mut cmd); - } else { - cmd.arg(arg); + continue; } + // If the REPLACE_LIBRS hack is enabled and we are building a `lib.rs` file, and a + // `lib.miri.rs` file exists, then build that instead. We only consider relative paths + // as cargo uses those for files in the workspace; dependencies from crates.io get + // absolute paths. + if replace_librs { + let path = Path::new(&arg); + if path.is_relative() + && path.file_name().is_some_and(|f| f == "lib.rs") + && path.is_file() + { + let miri_rs = Path::new(&arg).with_extension("miri.rs"); + if miri_rs.is_file() { + if verbose > 0 { + eprintln!("Performing REPLACE_LIBRS hack: {arg:?} -> {miri_rs:?}"); + } + cmd.arg(miri_rs); + continue; + } + } + } + // Fallback: just propagate the argument. + cmd.arg(arg); } // During setup, patch the panic runtime for `libpanic_abort` (mirroring what bootstrap usually does). @@ -426,17 +484,14 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { cmd.arg("-C").arg("panic=abort"); } } else { - // For host crates (but not when we are just printing some info), - // we might still have to set the sysroot. - if !info_query { - // When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly - // due to bootstrap complications. - if let Some(sysroot) = std::env::var_os("MIRI_HOST_SYSROOT") { - cmd.arg("--sysroot").arg(sysroot); - } + // This is a host crate. + // When we're running `cargo-miri` from `x.py` we need to pass the sysroot explicitly + // due to bootstrap complications. + if let Some(sysroot) = std::env::var_os("MIRI_HOST_SYSROOT") { + cmd.arg("--sysroot").arg(sysroot); } - // For host crates or when we are printing, just forward everything. + // Forward everything. cmd.args(args); } @@ -448,9 +503,7 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { // Run it. if verbose > 0 { - eprintln!( - "[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate} info_query={info_query}" - ); + eprintln!("[cargo-miri rustc] target_crate={target_crate} runnable_crate={runnable_crate}"); } // Create a stub .rlib file if "link" was requested by cargo. @@ -477,11 +530,6 @@ pub enum RunnerPhase { } pub fn phase_runner(mut binary_args: impl Iterator, phase: RunnerPhase) { - // phase_cargo_miri set `MIRI_BE_RUSTC` for when build scripts directly invoke the driver; - // however, if we get called back by cargo here, we'll carefully compute the right flags - // ourselves, so we first un-do what the earlier phase did. - env::remove_var("MIRI_BE_RUSTC"); - let verbose = std::env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); @@ -539,15 +587,13 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner // but when we run here, cargo does not interpret the JSON any more. `--json` // then also needs to be dropped. let mut args = info.args.into_iter(); - let error_format_flag = "--error-format"; - let json_flag = "--json"; while let Some(arg) = args.next() { if arg == "--extern" { forward_patched_extern_arg(&mut args, &mut cmd); - } else if let Some(suffix) = arg.strip_prefix(error_format_flag) { + } else if let Some(suffix) = arg.strip_prefix("--error-format") { assert!(suffix.starts_with('=')); // Drop this argument. - } else if let Some(suffix) = arg.strip_prefix(json_flag) { + } else if let Some(suffix) = arg.strip_prefix("--json") { assert!(suffix.starts_with('=')); // Drop this argument. } else { @@ -581,17 +627,16 @@ pub fn phase_rustdoc(mut args: impl Iterator) { let verbose = std::env::var("MIRI_VERBOSE") .map_or(0, |verbose| verbose.parse().expect("verbosity flag must be an integer")); - // phase_cargo_miri sets the RUSTDOC env var to ourselves, so we can't use that here; - // just default to a straight-forward invocation for now: - let mut cmd = Command::new("rustdoc"); + // phase_cargo_miri sets the RUSTDOC env var to ourselves, and puts a backup + // of the old value into MIRI_ORIG_RUSTDOC. So that's what we have to invoke now. + let rustdoc = env::var("MIRI_ORIG_RUSTDOC").unwrap_or("rustdoc".to_string()); + let mut cmd = Command::new(rustdoc); - let extern_flag = "--extern"; - let runtool_flag = "--runtool"; while let Some(arg) = args.next() { - if arg == extern_flag { + if arg == "--extern" { // Patch --extern arguments to use *.rmeta files, since phase_cargo_rustc only creates stub *.rlib files. forward_patched_extern_arg(&mut args, &mut cmd); - } else if arg == runtool_flag { + } else if arg == "--runtool" { // An existing --runtool flag indicates cargo is running in cross-target mode, which we don't support. // Note that this is only passed when cargo is run with the unstable -Zdoctest-xcompile flag; // otherwise, we won't be called as rustdoc at all. @@ -620,7 +665,7 @@ pub fn phase_rustdoc(mut args: impl Iterator) { // The `--test-builder` and `--runtool` arguments are unstable rustdoc features, // which are disabled by default. We first need to enable them explicitly: - cmd.arg("-Z").arg("unstable-options"); + cmd.arg("-Zunstable-options"); // rustdoc needs to know the right sysroot. cmd.arg("--sysroot").arg(env::var_os("MIRI_SYSROOT").unwrap()); diff --git a/cargo-miri/src/setup.rs b/cargo-miri/src/setup.rs index a98e1fcd48..401e9158fa 100644 --- a/cargo-miri/src/setup.rs +++ b/cargo-miri/src/setup.rs @@ -90,13 +90,13 @@ pub fn setup( let cargo_cmd = { let mut command = cargo(); // Use Miri as rustc to build a libstd compatible with us (and use the right flags). + // We set ourselves (`cargo-miri`) instead of Miri directly to be able to patch the flags + // for `libpanic_abort` (usually this is done by bootstrap but we have to do it ourselves). + // The `MIRI_CALLED_FROM_SETUP` will mean we dispatch to `phase_setup_rustc`. // However, when we are running in bootstrap, we cannot just overwrite `RUSTC`, // because we still need bootstrap to distinguish between host and target crates. // In that case we overwrite `RUSTC_REAL` instead which determines the rustc used // for target crates. - // We set ourselves (`cargo-miri`) instead of Miri directly to be able to patch the flags - // for `libpanic_abort` (usually this is done by bootstrap but we have to do it ourselves). - // The `MIRI_CALLED_FROM_SETUP` will mean we dispatch to `phase_setup_rustc`. let cargo_miri_path = std::env::current_exe().expect("current executable path invalid"); if env::var_os("RUSTC_STAGE").is_some() { assert!(env::var_os("RUSTC").is_some()); diff --git a/cargo-miri/src/util.rs b/cargo-miri/src/util.rs index 6c1a074cd8..d99957d9c2 100644 --- a/cargo-miri/src/util.rs +++ b/cargo-miri/src/util.rs @@ -101,7 +101,12 @@ pub fn find_miri() -> PathBuf { } pub fn miri() -> Command { - Command::new(find_miri()) + let mut cmd = Command::new(find_miri()); + // We never want to inherit this from the environment. + // However, this is sometimes set in the environment to work around build scripts that don't + // honor RUSTC_WRAPPER. So remove it again in case it is set. + cmd.env_remove("MIRI_BE_RUSTC"); + cmd } pub fn miri_for_host() -> Command { diff --git a/ci/ci.sh b/ci/ci.sh index 9d2c3f362e..f8ba612750 100755 --- a/ci/ci.sh +++ b/ci/ci.sh @@ -13,57 +13,73 @@ function endgroup { begingroup "Building Miri" -# Determine configuration for installed build -echo "Installing release version of Miri" +# Special Windows hacks +if [ "$HOST_TARGET" = i686-pc-windows-msvc ]; then + # The $BASH variable is `/bin/bash` here, but that path does not actually work. There are some + # hacks in place somewhere to try to paper over this, but the hacks dont work either (see + # ). So we hard-code the correct location for Github + # CI instead. + BASH="C:/Program Files/Git/usr/bin/bash" +fi + +# Global configuration export RUSTFLAGS="-D warnings" export CARGO_INCREMENTAL=0 export CARGO_EXTRA_FLAGS="--locked" -./miri install -# Prepare debug build for direct `./miri` invocations +# Determine configuration for installed build (used by test-cargo-miri). +echo "Installing release version of Miri" +time ./miri install + +# Prepare debug build for direct `./miri` invocations. +# We enable all features to make sure the Stacked Borrows consistency check runs. echo "Building debug version of Miri" -./miri check --no-default-features # make sure this can be built -./miri check --all-features # and this, too -./miri build --all-targets # the build that all the `./miri test` below will use +export CARGO_EXTRA_FLAGS="$CARGO_EXTRA_FLAGS --all-features" +time ./miri build --all-targets # the build that all the `./miri test` below will use endgroup -# Test +# Run tests. Recognizes these variables: +# - MIRI_TEST_TARGET: the target to test. Empty for host target. +# - GC_STRESS: if non-empty, run the GC stress test for the main test suite. +# - MIR_OPT: if non-empty, re-run test `pass` tests with mir-opt-level=4 +# - MANY_SEEDS: if set to N, run the "many-seeds" tests N times +# - TEST_BENCH: if non-empty, check that the benchmarks all build +# - CARGO_MIRI_ENV: if non-empty, set some env vars and config to potentially confuse cargo-miri function run_tests { - if [ -n "${MIRI_TEST_TARGET:-}" ]; then + if [ -n "${MIRI_TEST_TARGET-}" ]; then begingroup "Testing foreign architecture $MIRI_TEST_TARGET" else begingroup "Testing host architecture" fi ## ui test suite - # On the host and on Linux, also stress-test the GC. - if [ -z "${MIRI_TEST_TARGET:-}" ] || [ "$HOST_TARGET" = x86_64-unknown-linux-gnu ]; then - MIRIFLAGS="${MIRIFLAGS:-} -Zmiri-provenance-gc=1" ./miri test + if [ -n "${GC_STRESS-}" ]; then + time MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test else - ./miri test + time ./miri test fi - # Host-only tests - if [ -z "${MIRI_TEST_TARGET:-}" ]; then - # Running these on all targets is unlikely to catch more problems and would - # cost a lot of CI time. - + ## advanced tests + if [ -n "${MIR_OPT-}" ]; then # Tests with optimizations (`-O` is what cargo passes, but crank MIR 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. # 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} - + time MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} + fi + if [ -n "${MANY_SEEDS-}" ]; then # Also run some many-seeds tests. 64 seeds means this takes around a minute per test. - for FILE in tests/many-seeds/*.rs; do - MIRI_SEEDS=64 ./miri many-seeds ./miri run "$FILE" + # (Need to invoke via explicit `bash -c` for Windows.) + time for FILE in tests/many-seeds/*.rs; do + MIRI_SEEDS=$MANY_SEEDS ./miri many-seeds "$BASH" -c "./miri run '$FILE'" done - + fi + if [ -n "${TEST_BENCH-}" ]; then # Check that the benchmarks build and run, but without actually benchmarking. - HYPERFINE="bash -c" ./miri bench + time HYPERFINE="'$BASH' -c" ./miri bench fi ## test-cargo-miri @@ -74,16 +90,18 @@ function run_tests { PYTHON=python fi # Some environment setup that attempts to confuse the heck out of cargo-miri. - if [ "$HOST_TARGET" = x86_64-unknown-linux-gnu ]; then - # These act up on Windows (`which miri` produces a filename that does not exist?!?), - # so let's do this only on Linux. Also makes sure things work without these set. - export RUSTC=$(which rustc) # Produces a warning unless we also set MIRI + if [ -n "${CARGO_MIRI_ENV-}" ]; then + # These act up on Windows (`which miri` produces a filename that does not exist?!?). + # RUSTC is the main thing to set (it changes the first argument our wrapper will see). + # Unless MIRI is also set, that produces a warning. + export RUSTC=$(which rustc) export MIRI=$(rustc +miri --print sysroot)/bin/miri + # We entirely ignore other wrappers. + mkdir -p .cargo + echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml fi - mkdir -p .cargo - echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml # Run the actual test - ${PYTHON} test-cargo-miri/run-test.py + time ${PYTHON} test-cargo-miri/run-test.py # Clean up unset RUSTC MIRI rm -rf .cargo @@ -92,7 +110,7 @@ function run_tests { } function run_tests_minimal { - if [ -n "${MIRI_TEST_TARGET:-}" ]; then + if [ -n "${MIRI_TEST_TARGET-}" ]; then begingroup "Testing MINIMAL foreign architecture $MIRI_TEST_TARGET: only testing $@" else echo "run_tests_minimal requires MIRI_TEST_TARGET to be set" @@ -109,38 +127,49 @@ function run_tests_minimal { ## Main Testing Logic ## -# Host target. -run_tests - -# Extra targets. # In particular, fully cover all tier 1 targets. case $HOST_TARGET in x86_64-unknown-linux-gnu) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests + # Extra tier 1 MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests - MIRI_TEST_TARGET=aarch64-apple-darwin run_tests + MIRI_TEST_TARGET=x86_64-apple-darwin run_tests MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests - # Some targets are only partially supported. + MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests + # Extra tier 2 + MIRI_TEST_TARGET=aarch64-apple-darwin run_tests + MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests + # Partially supported targets (tier 2) MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align num_cpus MIRI_TEST_TARGET=i686-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align num_cpus - MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal hello integer vec panic/panic MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal no_std integer strings wasm MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std integer strings wasm - MIRI_TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std # no_std embedded architecture - MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std # JSON target file + MIRI_TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std + # Custom target JSON file + MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std ;; - x86_64-apple-darwin) + aarch64-apple-darwin) + # Host (tier 2) + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests + # Extra tier 1 + MIRI_TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests + # Extra tier 2 MIRI_TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture - MIRI_TEST_TARGET=x86_64-pc-windows-msvc run_tests ;; i686-pc-windows-msvc) - MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests + # Host + # Only smoke-test `many-seeds`; 64 runs take 15min here! + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=1 TEST_BENCH=1 run_tests + # Extra tier 1 + # We really want to ensure a Linux target works on a Windows host, + # and a 64bit target works on a 32bit host. MIRI_TEST_TARGET=x86_64-unknown-linux-gnu run_tests - MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests ;; *) - echo "FATAL: unknown OS" + echo "FATAL: unknown host target: $HOST_TARGET" exit 1 ;; esac diff --git a/miri b/miri index 169f4521f2..5f71fc9443 100755 --- a/miri +++ b/miri @@ -3,5 +3,5 @@ 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 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 "$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" diff --git a/miri-script/src/commands.rs b/miri-script/src/commands.rs index 7fff6ae80b..55b3b62819 100644 --- a/miri-script/src/commands.rs +++ b/miri-script/src/commands.rs @@ -178,7 +178,7 @@ impl Command { .context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; let sh = Shell::new()?; sh.change_dir(miri_dir()?); - let new_commit = Some(sh.read_file("rust-version")?.trim().to_owned()); + let new_commit = sh.read_file("rust-version")?.trim().to_owned(); let current_commit = { let rustc_info = cmd!(sh, "rustc +miri --version -v").read(); if rustc_info.is_err() { @@ -193,7 +193,7 @@ impl Command { } }; // Check if we already are at that commit. - if current_commit == new_commit { + if current_commit.as_ref() == Some(&new_commit) { if active_toolchain()? != "miri" { cmd!(sh, "rustup override set miri").run()?; } @@ -202,7 +202,7 @@ impl Command { // Install and setup new toolchain. cmd!(sh, "rustup toolchain uninstall miri").run()?; - cmd!(sh, "rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy {flags...} -- {new_commit...}").run()?; + cmd!(sh, "rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy {flags...} -- {new_commit}").run()?; cmd!(sh, "rustup override set miri").run()?; // Cleanup. cmd!(sh, "cargo clean").run()?; @@ -297,7 +297,7 @@ impl Command { }; // Prepare the branch. Pushing works much better if we use as base exactly // the commit that we pulled from last time, so we use the `rust-version` - // file as a good approximation of that. + // file to find out which commit that would be. println!("Preparing {github_user}/rust (base: {base})..."); if cmd!(sh, "git fetch https://github.com/{github_user}/rust {branch}") .ignore_stderr() @@ -380,9 +380,9 @@ impl Command { .env("MIRIFLAGS", miriflags) .quiet() .run(); - if status.is_err() { + if let Err(err) = status { println!("Failing seed: {seed}"); - break; + return Err(err.into()); } } Ok(()) @@ -479,10 +479,11 @@ impl Command { Ok(()) } - fn run(dep: bool, flags: Vec) -> Result<()> { + fn run(dep: bool, mut flags: Vec) -> Result<()> { let mut e = MiriEnv::new()?; // Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so - // that we set the MIRI_SYSROOT up the right way. + // that we set the MIRI_SYSROOT up the right way. We must make sure that + // MIRI_TEST_TARGET and `--target` are in sync. use itertools::Itertools; let target = flags .iter() @@ -493,33 +494,35 @@ impl Command { // Found it! e.sh.set_var("MIRI_TEST_TARGET", target); } else if let Ok(target) = std::env::var("MIRI_TEST_TARGET") { - // Make sure miri actually uses this target. - let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default(); - e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}")); + // Convert `MIRI_TEST_TARGET` into `--target`. + flags.push("--target".into()); + flags.push(target.into()); } - // Scan for "--edition" (we'll set one ourselves if that flag is not present). + // Scan for "--edition", set one ourselves if that flag is not present. let have_edition = flags.iter().take_while(|arg| *arg != "--").any(|arg| *arg == "--edition"); + if !have_edition { + flags.push("--edition=2021".into()); // keep in sync with `tests/ui.rs`.` + } // Prepare a sysroot. e.build_miri_sysroot(/* quiet */ true)?; - // Then run the actual command. + // Then run the actual command. Also add MIRIFLAGS. let miri_manifest = path!(e.miri_dir / "Cargo.toml"); let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); 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 `tests/ui.rs`.` if dep { cmd!( e.sh, - "cargo +{toolchain} --quiet test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --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...} {flags...}" ).quiet().run()?; } else { cmd!( e.sh, - "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {edition_flags...} {flags...}" + "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {flags...}" ).quiet().run()?; } Ok(()) diff --git a/miri.bat b/miri.bat index 959e54d884..18baa683f6 100644 --- a/miri.bat +++ b/miri.bat @@ -5,7 +5,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 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 || 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 diff --git a/rust-version b/rust-version index 0cdb0de033..6ad8fba723 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -148a41c6b5687f941c5256d9ef8145eb03b72094 +23d47dba319331d4418827cfbb8c1af283497d3c diff --git a/src/alloc_addresses/mod.rs b/src/alloc_addresses/mod.rs index e1714aa9e4..fec39ec2b8 100644 --- a/src/alloc_addresses/mod.rs +++ b/src/alloc_addresses/mod.rs @@ -18,12 +18,12 @@ use reuse_pool::ReusePool; #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum ProvenanceMode { - /// We support `expose_addr`/`from_exposed_addr` via "wildcard" provenance. - /// However, we want on `from_exposed_addr` to alert the user of the precision loss. + /// We support `expose_provenance`/`with_exposed_provenance` via "wildcard" provenance. + /// However, we warn on `with_exposed_provenance` to alert the user of the precision loss. Default, /// Like `Default`, but without the warning. Permissive, - /// We error on `from_exposed_addr`, ensuring no precision loss. + /// We error on `with_exposed_provenance`, ensuring no precision loss. Strict, } diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 6955e649b4..8fffb91542 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -179,6 +179,26 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { }); } } + + fn after_analysis<'tcx>( + &mut self, + _: &rustc_interface::interface::Compiler, + queries: &'tcx rustc_interface::Queries<'tcx>, + ) -> Compilation { + queries.global_ctxt().unwrap().enter(|tcx| { + if self.target_crate { + // cargo-miri has patched the compiler flags to make these into check-only builds, + // but we are still emulating regular rustc builds, which would perform post-mono + // const-eval during collection. So let's also do that here, even if we might be + // running with `--emit=metadata`. In particular this is needed to make + // `compile_fail` doc tests trigger post-mono errors. + // In general `collect_and_partition_mono_items` is not safe to call in check-only + // builds, but we are setting `-Zalways-encode-mir` which avoids those issues. + let _ = tcx.collect_and_partition_mono_items(()); + } + }); + Compilation::Continue + } } fn show_error(msg: &impl std::fmt::Display) -> ! { @@ -256,7 +276,7 @@ fn run_compiler( // If no `--sysroot` is given, the `MIRI_SYSROOT` env var is consulted to find where // that sysroot lives, and that is passed to rustc. let sysroot_flag = "--sysroot"; - if !args.iter().any(|e| e == sysroot_flag) { + if !args.iter().any(|e| e.starts_with(sysroot_flag)) { // Using the built-in default here would be plain wrong, so we *require* // the env var to make sure things make sense. let miri_sysroot = env::var("MIRI_SYSROOT").unwrap_or_else(|_| { @@ -344,6 +364,10 @@ fn main() { let args = rustc_driver::args::raw_args(&early_dcx) .unwrap_or_else(|_| std::process::exit(rustc_driver::EXIT_FAILURE)); + // Install the ctrlc handler that sets `rustc_const_eval::CTRL_C_RECEIVED`, even if + // MIRI_BE_RUSTC is set. + rustc_driver::install_ctrlc_handler(); + // If the environment asks us to actually be rustc, then do that. if let Some(crate_kind) = env::var_os("MIRI_BE_RUSTC") { // Earliest rustc setup. diff --git a/src/borrow_tracker/stacked_borrows/mod.rs b/src/borrow_tracker/stacked_borrows/mod.rs index 7a6a85a2f7..96ff298402 100644 --- a/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/borrow_tracker/stacked_borrows/mod.rs @@ -18,6 +18,7 @@ use crate::borrow_tracker::{ stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder}, GlobalStateInner, ProtectorKind, }; +use crate::concurrency::data_race::{NaReadType, NaWriteType}; use crate::*; use diagnostics::{RetagCause, RetagInfo}; @@ -90,7 +91,7 @@ impl NewPermission { } } } - ty::RawPtr(ty::TypeAndMut { mutbl: Mutability::Mut, .. }) => { + ty::RawPtr(_, Mutability::Mut) => { assert!(protector.is_none()); // RetagKind can't be both FnEntry and Raw. // Mutable raw pointer. No access, not protected. NewPermission::Uniform { @@ -114,7 +115,7 @@ impl NewPermission { // This fixes https://github.com/rust-lang/rust/issues/55005. } } - ty::RawPtr(ty::TypeAndMut { mutbl: Mutability::Not, .. }) => { + ty::RawPtr(_, Mutability::Not) => { assert!(protector.is_none()); // RetagKind can't be both FnEntry and Raw. // `*const T`, when freshly created, are read-only in the frozen part. NewPermission::FreezeSensitive { @@ -751,7 +752,13 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' assert_eq!(access, AccessKind::Write); // Make sure the data race model also knows about this. if let Some(data_race) = alloc_extra.data_race.as_mut() { - data_race.write(alloc_id, range, machine)?; + data_race.write( + alloc_id, + range, + NaWriteType::Retag, + Some(place.layout.ty), + machine, + )?; } } } @@ -794,7 +801,13 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' assert_eq!(access, AccessKind::Read); // Make sure the data race model also knows about this. if let Some(data_race) = alloc_extra.data_race.as_ref() { - data_race.read(alloc_id, range, &this.machine)?; + data_race.read( + alloc_id, + range, + NaReadType::Retag, + Some(place.layout.ty), + &this.machine, + )?; } } Ok(()) diff --git a/src/borrow_tracker/stacked_borrows/stack.rs b/src/borrow_tracker/stacked_borrows/stack.rs index 712c26a9af..76430498e2 100644 --- a/src/borrow_tracker/stacked_borrows/stack.rs +++ b/src/borrow_tracker/stacked_borrows/stack.rs @@ -146,7 +146,7 @@ impl<'tcx> Stack { /// Panics if any of the caching mechanisms have broken, /// - The StackCache indices don't refer to the parallel items, /// - There are no Unique items outside of first_unique..last_unique - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] fn verify_cache_consistency(&self) { // Only a full cache needs to be valid. Also see the comments in find_granting_cache // and set_unknown_bottom. @@ -190,7 +190,7 @@ impl<'tcx> Stack { tag: ProvenanceExtra, exposed_tags: &FxHashSet, ) -> Result, ()> { - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); let ProvenanceExtra::Concrete(tag) = tag else { @@ -327,7 +327,7 @@ impl<'tcx> Stack { // This primes the cache for the next access, which is almost always the just-added tag. self.cache.add(new_idx, new); - #[cfg(debug_assertions)] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); } @@ -410,7 +410,7 @@ impl<'tcx> Stack { self.unique_range.end = self.unique_range.end.min(disable_start); } - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); Ok(()) @@ -465,7 +465,7 @@ impl<'tcx> Stack { self.unique_range = 0..0; } - #[cfg(all(feature = "stack-cache", debug_assertions))] + #[cfg(feature = "stack-cache-consistency-check")] self.verify_cache_consistency(); Ok(()) } diff --git a/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/borrow_tracker/tree_borrows/diagnostics.rs index 4394e3c2c8..b9f0b5bc17 100644 --- a/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -48,7 +48,7 @@ impl AccessCause { /// Complete data for an event: #[derive(Clone, Debug)] pub struct Event { - /// Transformation of permissions that occured because of this event. + /// Transformation of permissions that occurred because of this event. pub transition: PermTransition, /// Kind of the access that triggered this event. pub access_cause: AccessCause, @@ -58,7 +58,7 @@ pub struct Event { /// `None` means that this is an implicit access to the entire allocation /// (used for the implicit read on protector release). pub access_range: Option, - /// The transition recorded by this event only occured on a subrange of + /// The transition recorded by this event only occurred on a subrange of /// `access_range`: a single access on `access_range` triggers several events, /// each with their own mutually disjoint `transition_range`. No-op transitions /// should not be recorded as events, so the union of all `transition_range` is not @@ -365,7 +365,7 @@ type S = &'static str; /// Pretty-printing details /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtWrapper { /// top: '>', /// bot: '<', @@ -393,7 +393,7 @@ struct DisplayFmtWrapper { /// Formating of the permissions on each range. /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtPermission { /// open: "[", /// sep: "|", @@ -425,7 +425,7 @@ struct DisplayFmtPermission { /// Formating of the tree structure. /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtPadding { /// join_middle: "|-", /// join_last: "'-", @@ -463,7 +463,7 @@ struct DisplayFmtPadding { /// How to show whether a location has been accessed /// /// Example: -/// ``` +/// ```rust,ignore (private type) /// DisplayFmtAccess { /// yes: " ", /// no: "?", diff --git a/src/borrow_tracker/tree_borrows/mod.rs b/src/borrow_tracker/tree_borrows/mod.rs index 80bdcbb755..a3d49756e4 100644 --- a/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/borrow_tracker/tree_borrows/mod.rs @@ -9,8 +9,11 @@ use rustc_middle::{ use rustc_span::def_id::DefId; use rustc_target::abi::{Abi, Size}; -use crate::borrow_tracker::{GlobalState, GlobalStateInner, ProtectorKind}; use crate::*; +use crate::{ + borrow_tracker::{GlobalState, GlobalStateInner, ProtectorKind}, + concurrency::data_race::NaReadType, +}; pub mod diagnostics; mod perms; @@ -312,7 +315,13 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Also inform the data race model (but only if any bytes are actually affected). if range.size.bytes() > 0 { if let Some(data_race) = alloc_extra.data_race.as_ref() { - data_race.read(alloc_id, range, &this.machine)?; + data_race.read( + alloc_id, + range, + NaReadType::Retag, + Some(place.layout.ty), + &this.machine, + )?; } } @@ -475,7 +484,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { NewPermission::from_ref_ty(pointee, mutability, self.kind, self.ecx); self.retag_ptr_inplace(place, new_perm)?; } - ty::RawPtr(_) => { + ty::RawPtr(_, _) => { // We definitely do *not* want to recurse into raw pointers -- wide raw // pointers have fields, and for dyn Trait pointees those can have reference // type! diff --git a/src/borrow_tracker/tree_borrows/perms.rs b/src/borrow_tracker/tree_borrows/perms.rs index 5c7f5ea46b..bec51c7cdf 100644 --- a/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/borrow_tracker/tree_borrows/perms.rs @@ -181,8 +181,12 @@ impl Permission { pub fn is_initial(&self) -> bool { self.inner.is_initial() } + /// Check if `self` is the terminal state of a pointer (is `Disabled`). + pub fn is_disabled(&self) -> bool { + self.inner == Disabled + } - /// Default initial permission of the root of a new tree. + /// Default initial permission of the root of a new tree at inbounds positions. /// Must *only* be used for the root, this is not in general an "initial" permission! pub fn new_active() -> Self { Self { inner: Active } @@ -193,11 +197,17 @@ impl Permission { Self { inner: Reserved { ty_is_freeze, conflicted: false } } } - /// Default initial permission of a reborrowed shared reference + /// Default initial permission of a reborrowed shared reference. pub fn new_frozen() -> Self { Self { inner: Frozen } } + /// Default initial permission of the root of a new tre at out-of-bounds positions. + /// Must *only* be used for the root, this is not in general an "initial" permission! + pub fn new_disabled() -> Self { + Self { inner: Disabled } + } + /// Apply the transition to the inner PermissionPriv. pub fn perform_access( kind: AccessKind, diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index dda1c7cca1..2470624181 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -42,6 +42,7 @@ pub(super) struct LocationState { /// protected, that is *not* the case for uninitialized locations. Instead we just have a latent /// "future initial permission" of `Disabled`, causing UB only if an access is ever actually /// performed. + /// Note that the tree root is also always initialized, as if the allocation was a write access. initialized: bool, /// This pointer's current permission / future initial permission. permission: Permission, @@ -55,17 +56,18 @@ pub(super) struct LocationState { } impl LocationState { - /// Default initial state has never been accessed and has been subjected to no - /// foreign access. - fn new(permission: Permission) -> Self { + /// Constructs a new initial state. It has neither been accessed, nor been subjected + /// to any foreign access yet. + /// The permission is not allowed to be `Active`. + fn new_uninit(permission: Permission) -> Self { + assert!(permission.is_initial() || permission.is_disabled()); Self { permission, initialized: false, latest_foreign_access: None } } - /// Record that this location was accessed through a child pointer by - /// marking it as initialized - fn with_access(mut self) -> Self { - self.initialized = true; - self + /// Constructs a new initial state. It has not yet been subjected + /// to any foreign access. However, it is already marked as having been accessed. + fn new_init(permission: Permission) -> Self { + Self { permission, initialized: true, latest_foreign_access: None } } /// Check if the location has been initialized, i.e. if it has @@ -238,8 +240,10 @@ pub(super) struct Node { /// If the pointer was reborrowed, it has children. // FIXME: bench to compare this to FxHashSet and to other SmallVec sizes pub children: SmallVec<[UniIndex; 4]>, - /// Either `Reserved` or `Frozen`, the permission this tag will be lazily initialized - /// to on the first access. + /// Either `Reserved`, `Frozen`, or `Disabled`, it is the permission this tag will + /// lazily be initialized to on the first access. + /// It is only ever `Disabled` for a tree root, since the root is initialized to `Active` by + /// its own separate mechanism. default_initial_perm: Permission, /// Some extra information useful only for debugging purposes pub debug_info: NodeDebugInfo, @@ -444,12 +448,14 @@ impl<'tree> TreeVisitor<'tree> { impl Tree { /// Create a new tree, with only a root pointer. pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { - let root_perm = Permission::new_active(); + // The root has `Disabled` as the default permission, + // so that any access out of bounds is invalid. + let root_default_perm = Permission::new_disabled(); let mut tag_mapping = UniKeyMap::default(); let root_idx = tag_mapping.insert(root_tag); let nodes = { let mut nodes = UniValMap::::default(); - let mut debug_info = NodeDebugInfo::new(root_tag, root_perm, span); + let mut debug_info = NodeDebugInfo::new(root_tag, root_default_perm, span); // name the root so that all allocations contain one named pointer debug_info.add_name("root of the allocation"); nodes.insert( @@ -458,7 +464,7 @@ impl Tree { tag: root_tag, parent: None, children: SmallVec::default(), - default_initial_perm: root_perm, + default_initial_perm: root_default_perm, debug_info, }, ); @@ -466,7 +472,11 @@ impl Tree { }; let rperms = { let mut perms = UniValMap::default(); - perms.insert(root_idx, LocationState::new(root_perm).with_access()); + // We manually set it to `Active` on all in-bounds positions. + // We also ensure that it is initialized, so that no `Active` but + // not yet initialized nodes exist. Essentially, we pretend there + // was a write that initialized these to `Active`. + perms.insert(root_idx, LocationState::new_init(Permission::new_active())); RangeMap::new(size, perms) }; Self { root: root_idx, nodes, rperms, tag_mapping } @@ -500,7 +510,7 @@ impl<'tcx> Tree { // Register new_tag as a child of parent_tag self.nodes.get_mut(parent_idx).unwrap().children.push(idx); // Initialize perms - let perm = LocationState::new(default_initial_perm).with_access(); + let perm = LocationState::new_init(default_initial_perm); for (_perms_range, perms) in self.rperms.iter_mut(reborrow_range.start, reborrow_range.size) { perms.insert(idx, perm); @@ -599,7 +609,7 @@ impl<'tcx> Tree { -> Result { let NodeAppArgs { node, mut perm, rel_pos } = args; - let old_state = perm.or_insert(LocationState::new(node.default_initial_perm)); + let old_state = perm.or_insert(LocationState::new_uninit(node.default_initial_perm)); match old_state.skip_if_known_noop(access_kind, rel_pos) { ContinueTraversal::SkipChildren => return Ok(ContinueTraversal::SkipChildren), diff --git a/src/borrow_tracker/tree_borrows/tree/tests.rs b/src/borrow_tracker/tree_borrows/tree/tests.rs index 35f3b53afd..6777f41ac2 100644 --- a/src/borrow_tracker/tree_borrows/tree/tests.rs +++ b/src/borrow_tracker/tree_borrows/tree/tests.rs @@ -75,7 +75,7 @@ fn protected_enforces_noalias() { } } -/// We are going to exhaustively test the possibily of inserting +/// We are going to exhaustively test the possibility of inserting /// a spurious read in some code. /// /// We choose some pointer `x` through which we want a spurious read to be inserted. @@ -270,7 +270,7 @@ mod spurious_read { match self { TestEvent::Access(acc) => write!(f, "{acc}"), // The fields of the `Ret` variants just serve to make them - // impossible to instanciate via the `RetX = NoRet` type; we can + // impossible to instantiate via the `RetX = NoRet` type; we can // always ignore their value. TestEvent::RetX(_) => write!(f, "ret x"), TestEvent::RetY(_) => write!(f, "ret y"), @@ -395,7 +395,7 @@ mod spurious_read { match evt { TestEvent::Access(acc) => self.perform_test_access(acc), // The fields of the `Ret` variants just serve to make them - // impossible to instanciate via the `RetX = NoRet` type; we can + // impossible to instantiate via the `RetX = NoRet` type; we can // always ignore their value. TestEvent::RetX(_) => self.end_protector_x(), TestEvent::RetY(_) => self.end_protector_y(), @@ -516,11 +516,11 @@ mod spurious_read { let source = LocStateProtPair { xy_rel: RelPosXY::MutuallyForeign, x: LocStateProt { - state: LocationState::new(Permission::new_frozen()).with_access(), + state: LocationState::new_init(Permission::new_frozen()), prot: true, }, y: LocStateProt { - state: LocationState::new(Permission::new_reserved(false)), + state: LocationState::new_uninit(Permission::new_reserved(false)), prot: true, }, }; diff --git a/src/borrow_tracker/tree_borrows/unimap.rs b/src/borrow_tracker/tree_borrows/unimap.rs index d9cad9c8e0..f45b2d9e00 100644 --- a/src/borrow_tracker/tree_borrows/unimap.rs +++ b/src/borrow_tracker/tree_borrows/unimap.rs @@ -132,7 +132,7 @@ where /// the associated `UniIndex` from ALL `UniValMap`s. /// /// Example of such behavior: - /// ``` + /// ```rust,ignore (private type can't be doctested) /// let mut keymap = UniKeyMap::::default(); /// let mut valmap = UniValMap::::default(); /// // Insert 'a' -> _ -> 'A' diff --git a/src/concurrency/data_race.rs b/src/concurrency/data_race.rs index 127d97bd5a..d51160b283 100644 --- a/src/concurrency/data_race.rs +++ b/src/concurrency/data_race.rs @@ -49,7 +49,7 @@ use std::{ use rustc_ast::Mutability; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_index::{Idx, IndexVec}; -use rustc_middle::mir; +use rustc_middle::{mir, ty::Ty}; use rustc_span::Span; use rustc_target::abi::{Align, HasDataLayout, Size}; @@ -200,18 +200,38 @@ enum AtomicAccessType { Rmw, } -/// Type of write operation: allocating memory -/// non-atomic writes and deallocating memory -/// are all treated as writes for the purpose -/// of the data-race detector. +/// Type of a non-atomic read operation. #[derive(Copy, Clone, PartialEq, Eq, Debug)] -enum NaWriteType { +pub enum NaReadType { + /// Standard unsynchronized write. + Read, + + // An implicit read generated by a retag. + Retag, +} + +impl NaReadType { + fn description(self) -> &'static str { + match self { + NaReadType::Read => "non-atomic read", + NaReadType::Retag => "retag read", + } + } +} + +/// Type of a non-atomic write operation: allocating memory, non-atomic writes, and +/// deallocating memory are all treated as writes for the purpose of the data-race detector. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub enum NaWriteType { /// Allocate memory. Allocate, /// Standard unsynchronized write. Write, + // An implicit write generated by a retag. + Retag, + /// Deallocate memory. /// Note that when memory is deallocated first, later non-atomic accesses /// will be reported as use-after-free, not as data races. @@ -224,6 +244,7 @@ impl NaWriteType { match self { NaWriteType::Allocate => "creating a new allocation", NaWriteType::Write => "non-atomic write", + NaWriteType::Retag => "retag write", NaWriteType::Deallocate => "deallocation", } } @@ -231,7 +252,7 @@ impl NaWriteType { #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum AccessType { - NaRead, + NaRead(NaReadType), NaWrite(NaWriteType), AtomicLoad, AtomicStore, @@ -239,29 +260,48 @@ enum AccessType { } impl AccessType { - fn description(self) -> &'static str { - match self { - AccessType::NaRead => "non-atomic read", + fn description(self, ty: Option>, size: Option) -> String { + let mut msg = String::new(); + + if let Some(size) = size { + msg.push_str(&format!("{}-byte {}", size.bytes(), msg)) + } + + msg.push_str(match self { + AccessType::NaRead(w) => w.description(), AccessType::NaWrite(w) => w.description(), AccessType::AtomicLoad => "atomic load", AccessType::AtomicStore => "atomic store", AccessType::AtomicRmw => "atomic read-modify-write", + }); + + if let Some(ty) = ty { + msg.push_str(&format!(" of type `{}`", ty)); } + + msg } fn is_atomic(self) -> bool { match self { AccessType::AtomicLoad | AccessType::AtomicStore | AccessType::AtomicRmw => true, - AccessType::NaRead | AccessType::NaWrite(_) => false, + AccessType::NaRead(_) | AccessType::NaWrite(_) => false, } } fn is_read(self) -> bool { match self { - AccessType::AtomicLoad | AccessType::NaRead => true, + AccessType::AtomicLoad | AccessType::NaRead(_) => true, AccessType::NaWrite(_) | AccessType::AtomicStore | AccessType::AtomicRmw => false, } } + + fn is_retag(self) -> bool { + matches!( + self, + AccessType::NaRead(NaReadType::Retag) | AccessType::NaWrite(NaWriteType::Retag) + ) + } } /// Memory Cell vector clock metadata @@ -502,12 +542,14 @@ impl MemoryCellClocks { &mut self, thread_clocks: &mut ThreadClockSet, index: VectorIdx, + read_type: NaReadType, current_span: Span, ) -> Result<(), DataRace> { trace!("Unsynchronized read with vectors: {:#?} :: {:#?}", self, thread_clocks); if !current_span.is_dummy() { thread_clocks.clock[index].span = current_span; } + thread_clocks.clock[index].set_read_type(read_type); if self.write_was_before(&thread_clocks.clock) { let race_free = if let Some(atomic) = self.atomic() { // We must be ordered-after all atomic accesses, reads and writes. @@ -875,7 +917,8 @@ impl VClockAlloc { /// This finds the two racing threads and the type /// of data-race that occurred. This will also /// return info about the memory location the data-race - /// occurred in. + /// occurred in. The `ty` parameter is used for diagnostics, letting + /// the user know which type was involved in the access. #[cold] #[inline(never)] fn report_data_race<'tcx>( @@ -885,6 +928,7 @@ impl VClockAlloc { access: AccessType, access_size: Size, ptr_dbg: Pointer, + ty: Option>, ) -> InterpResult<'tcx> { let (current_index, current_clocks) = global.current_thread_state(thread_mgr); let mut other_size = None; // if `Some`, this was a size-mismatch race @@ -908,7 +952,7 @@ impl VClockAlloc { write_clock = mem_clocks.write(); (AccessType::NaWrite(mem_clocks.write_type), mem_clocks.write.0, &write_clock) } else if let Some(idx) = Self::find_gt_index(&mem_clocks.read, ¤t_clocks.clock) { - (AccessType::NaRead, idx, &mem_clocks.read) + (AccessType::NaRead(mem_clocks.read[idx].read_type()), idx, &mem_clocks.read) // Finally, mixed-size races. } else if access.is_atomic() && let Some(atomic) = mem_clocks.atomic() && atomic.size != access_size { // This is only a race if we are not synchronized with all atomic accesses, so find @@ -950,37 +994,33 @@ impl VClockAlloc { Err(err_machine_stop!(TerminationInfo::DataRace { involves_non_atomic, extra, + retag_explain: access.is_retag() || other_access.is_retag(), ptr: ptr_dbg, op1: RacingOp { - action: if let Some(other_size) = other_size { - format!("{}-byte {}", other_size.bytes(), other_access.description()) - } else { - other_access.description().to_owned() - }, + action: other_access.description(None, other_size), thread_info: other_thread_info, span: other_clock.as_slice()[other_thread.index()].span_data(), }, op2: RacingOp { - action: if other_size.is_some() { - format!("{}-byte {}", access_size.bytes(), access.description()) - } else { - access.description().to_owned() - }, + action: access.description(ty, other_size.map(|_| access_size)), thread_info: current_thread_info, span: current_clocks.clock.as_slice()[current_index.index()].span_data(), }, }))? } - /// Detect data-races for an unsynchronized read operation, will not perform + /// Detect data-races for an unsynchronized read operation. It will not perform /// data-race detection if `race_detecting()` is false, either due to no threads /// being created or if it is temporarily disabled during a racy read or write /// operation for which data-race detection is handled separately, for example - /// atomic read operations. + /// atomic read operations. The `ty` parameter is used for diagnostics, letting + /// the user know which type was read. pub fn read<'tcx>( &self, alloc_id: AllocId, access_range: AllocRange, + read_type: NaReadType, + ty: Option>, machine: &MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { let current_span = machine.current_span(); @@ -992,7 +1032,7 @@ impl VClockAlloc { alloc_ranges.iter_mut(access_range.start, access_range.size) { if let Err(DataRace) = - mem_clocks.read_race_detect(&mut thread_clocks, index, current_span) + mem_clocks.read_race_detect(&mut thread_clocks, index, read_type, current_span) { drop(thread_clocks); // Report data-race. @@ -1000,9 +1040,10 @@ impl VClockAlloc { global, &machine.threads, mem_clocks, - AccessType::NaRead, + AccessType::NaRead(read_type), access_range.size, Pointer::new(alloc_id, Size::from_bytes(mem_clocks_range.start)), + ty, ); } } @@ -1012,12 +1053,17 @@ impl VClockAlloc { } } - // Shared code for detecting data-races on unique access to a section of memory - fn unique_access<'tcx>( + /// Detect data-races for an unsynchronized write operation. It will not perform + /// data-race detection if `race_detecting()` is false, either due to no threads + /// being created or if it is temporarily disabled during a racy read or write + /// operation. The `ty` parameter is used for diagnostics, letting + /// the user know which type was written. + pub fn write<'tcx>( &mut self, alloc_id: AllocId, access_range: AllocRange, write_type: NaWriteType, + ty: Option>, machine: &mut MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { let current_span = machine.current_span(); @@ -1042,6 +1088,7 @@ impl VClockAlloc { AccessType::NaWrite(write_type), access_range.size, Pointer::new(alloc_id, Size::from_bytes(mem_clocks_range.start)), + ty, ); } } @@ -1050,37 +1097,6 @@ impl VClockAlloc { Ok(()) } } - - /// Detect data-races for an unsynchronized write operation, will not perform - /// data-race threads if `race_detecting()` is false, either due to no threads - /// being created or if it is temporarily disabled during a racy read or write - /// operation - pub fn write<'tcx>( - &mut self, - alloc_id: AllocId, - range: AllocRange, - machine: &mut MiriMachine<'_, '_>, - ) -> InterpResult<'tcx> { - self.unique_access(alloc_id, range, NaWriteType::Write, machine) - } - - /// Detect data-races for an unsynchronized deallocate operation, will not perform - /// data-race threads if `race_detecting()` is false, either due to no threads - /// being created or if it is temporarily disabled during a racy read or write - /// operation - pub fn deallocate<'tcx>( - &mut self, - alloc_id: AllocId, - size: Size, - machine: &mut MiriMachine<'_, '_>, - ) -> InterpResult<'tcx> { - self.unique_access( - alloc_id, - alloc_range(Size::ZERO, size), - NaWriteType::Deallocate, - machine, - ) - } } impl<'mir, 'tcx: 'mir> EvalContextPrivExt<'mir, 'tcx> for MiriInterpCx<'mir, 'tcx> {} @@ -1279,7 +1295,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { let alloc_meta = this.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap(); trace!( "Atomic op({}) with ordering {:?} on {:?} (size={})", - access.description(), + access.description(None, None), &atomic, place.ptr(), size.bytes() @@ -1307,6 +1323,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> { alloc_id, Size::from_bytes(mem_clocks_range.start), ), + None, ) .map(|_| true); } diff --git a/src/concurrency/thread.rs b/src/concurrency/thread.rs index 805c0580b2..d0d73bb1b3 100644 --- a/src/concurrency/thread.rs +++ b/src/concurrency/thread.rs @@ -3,12 +3,13 @@ use std::cell::RefCell; use std::collections::hash_map::Entry; use std::num::TryFromIntError; -use std::sync::atomic::{AtomicBool, Ordering::Relaxed}; +use std::sync::atomic::Ordering::Relaxed; use std::task::Poll; use std::time::{Duration, SystemTime}; use either::Either; +use rustc_const_eval::CTRL_C_RECEIVED; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def_id::DefId; use rustc_index::{Idx, IndexVec}; @@ -1045,21 +1046,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Run the core interpreter loop. Returns only when an interrupt occurs (an error or program /// termination). fn run_threads(&mut self) -> InterpResult<'tcx, !> { - static SIGNALED: AtomicBool = AtomicBool::new(false); - ctrlc::set_handler(move || { - // Indicate that we have ben signaled to stop. If we were already signaled, exit - // immediately. In our interpreter loop we try to consult this value often, but if for - // whatever reason we don't get to that check or the cleanup we do upon finding that - // this bool has become true takes a long time, the exit here will promptly exit the - // process on the second Ctrl-C. - if SIGNALED.swap(true, Relaxed) { - std::process::exit(1); - } - }) - .unwrap(); let this = self.eval_context_mut(); loop { - if SIGNALED.load(Relaxed) { + if CTRL_C_RECEIVED.load(Relaxed) { this.machine.handle_abnormal_termination(); std::process::exit(1); } diff --git a/src/concurrency/vector_clock.rs b/src/concurrency/vector_clock.rs index fa93c9e00b..fe719943dc 100644 --- a/src/concurrency/vector_clock.rs +++ b/src/concurrency/vector_clock.rs @@ -4,9 +4,11 @@ use smallvec::SmallVec; use std::{ cmp::Ordering, fmt::Debug, - ops::{Index, IndexMut}, + ops::{Index, IndexMut, Shr}, }; +use super::data_race::NaReadType; + /// A vector clock index, this is associated with a thread id /// but in some cases one vector index may be shared with /// multiple thread ids if it's safe to do so. @@ -50,13 +52,51 @@ const SMALL_VECTOR: usize = 4; /// so that diagnostics can report what code was responsible for an operation. #[derive(Clone, Copy, Debug)] pub struct VTimestamp { - time: u32, + /// The lowest bit indicates read type, the rest is the time. + /// `1` indicates a retag read, `0` a regular read. + time_and_read_type: u32, pub span: Span, } impl VTimestamp { - pub const ZERO: VTimestamp = VTimestamp { time: 0, span: DUMMY_SP }; + pub const ZERO: VTimestamp = VTimestamp::new(0, NaReadType::Read, DUMMY_SP); + + #[inline] + const fn encode_time_and_read_type(time: u32, read_type: NaReadType) -> u32 { + let read_type_bit = match read_type { + NaReadType::Read => 0, + NaReadType::Retag => 1, + }; + // Put the `read_type` in the lowest bit and `time` in the rest + read_type_bit | time.checked_mul(2).expect("Vector clock overflow") + } + + #[inline] + const fn new(time: u32, read_type: NaReadType, span: Span) -> Self { + Self { time_and_read_type: Self::encode_time_and_read_type(time, read_type), span } + } + + #[inline] + fn time(&self) -> u32 { + self.time_and_read_type.shr(1) + } + #[inline] + fn set_time(&mut self, time: u32) { + self.time_and_read_type = Self::encode_time_and_read_type(time, self.read_type()); + } + + #[inline] + pub fn read_type(&self) -> NaReadType { + if self.time_and_read_type & 1 == 0 { NaReadType::Read } else { NaReadType::Retag } + } + + #[inline] + pub fn set_read_type(&mut self, read_type: NaReadType) { + self.time_and_read_type = Self::encode_time_and_read_type(self.time(), read_type); + } + + #[inline] pub fn span_data(&self) -> SpanData { self.span.data() } @@ -64,7 +104,7 @@ impl VTimestamp { impl PartialEq for VTimestamp { fn eq(&self, other: &Self) -> bool { - self.time == other.time + self.time() == other.time() } } @@ -78,7 +118,7 @@ impl PartialOrd for VTimestamp { impl Ord for VTimestamp { fn cmp(&self, other: &Self) -> Ordering { - self.time.cmp(&other.time) + self.time().cmp(&other.time()) } } @@ -130,7 +170,7 @@ impl VClock { let idx = idx.index(); let mut_slice = self.get_mut_with_min_len(idx + 1); let idx_ref = &mut mut_slice[idx]; - idx_ref.time = idx_ref.time.checked_add(1).expect("Vector clock overflow"); + idx_ref.set_time(idx_ref.time().checked_add(1).expect("Vector clock overflow")); if !current_span.is_dummy() { idx_ref.span = current_span; } @@ -379,8 +419,8 @@ impl IndexMut for VClock { /// test suite #[cfg(test)] mod tests { - use super::{VClock, VTimestamp, VectorIdx}; + use crate::concurrency::data_race::NaReadType; use rustc_span::DUMMY_SP; use std::cmp::Ordering; @@ -448,7 +488,13 @@ mod tests { while let Some(0) = slice.last() { slice = &slice[..slice.len() - 1] } - VClock(slice.iter().copied().map(|time| VTimestamp { time, span: DUMMY_SP }).collect()) + VClock( + slice + .iter() + .copied() + .map(|time| VTimestamp::new(time, NaReadType::Read, DUMMY_SP)) + .collect(), + ) } fn assert_order(l: &[u32], r: &[u32], o: Option) { diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 03428b081c..30349c003a 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -46,6 +46,7 @@ pub enum TerminationInfo { op1: RacingOp, op2: RacingOp, extra: Option<&'static str>, + retag_explain: bool, }, } @@ -65,7 +66,7 @@ impl fmt::Display for TerminationInfo { Int2PtrWithStrictProvenance => write!( f, - "integer-to-pointer casts and `ptr::from_exposed_addr` are not supported with `-Zmiri-strict-provenance`" + "integer-to-pointer casts and `ptr::with_exposed_provenance` are not supported with `-Zmiri-strict-provenance`" ), StackedBorrowsUb { msg, .. } => write!(f, "{msg}"), TreeBorrowsUb { title, .. } => write!(f, "{title}"), @@ -263,12 +264,17 @@ pub fn report_error<'tcx, 'mir>( vec![(Some(*span), format!("the `{link_name}` symbol is defined here"))], Int2PtrWithStrictProvenance => vec![(None, format!("use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead"))], - DataRace { op1, extra, .. } => { + DataRace { op1, extra, retag_explain, .. } => { let mut helps = vec![(Some(op1.span), format!("and (1) occurred earlier here"))]; if let Some(extra) = extra { helps.push((None, format!("{extra}"))); helps.push((None, format!("see https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#memory-model-for-atomic-accesses for more information about the Rust memory model"))); } + if *retag_explain { + helps.push((None, "retags occur on all (re)borrows and as well as when references are copied or moved".to_owned())); + helps.push((None, "retags permit optimizations that insert speculative reads or writes".to_owned())); + helps.push((None, "therefore from the perspective of data races, a retag has the same implications as a read or write".to_owned())); + } helps.push((None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior"))); helps.push((None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information"))); helps @@ -587,7 +593,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { ( None, format!( - "This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`," + "This program is using integer-to-pointer casts or (equivalently) `ptr::with_exposed_provenance`," ), ), ( @@ -597,7 +603,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { ( None, format!( - "See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation." + "See https://doc.rust-lang.org/nightly/std/ptr/fn.with_exposed_provenance.html for more details on that operation." ), ), ( @@ -609,7 +615,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { ( None, format!( - "You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics." + "You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `with_exposed_provenance` semantics." ), ), ( diff --git a/src/helpers.rs b/src/helpers.rs index c12fe0e086..6e320b60ee 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -105,24 +105,41 @@ fn try_resolve_did(tcx: TyCtxt<'_>, path: &[&str], namespace: Option) (path, None) }; - // First find the crate. - let krate = - tcx.crates(()).iter().find(|&&krate| tcx.crate_name(krate).as_str() == crate_name)?; - let mut cur_item = DefId { krate: *krate, index: CRATE_DEF_INDEX }; - // Then go over the modules. - for &segment in modules { - cur_item = find_children(tcx, cur_item, segment) - .find(|item| tcx.def_kind(item) == DefKind::Mod)?; - } - // Finally, look up the desired item in this module, if any. - match item { - Some((item_name, namespace)) => - Some( - find_children(tcx, cur_item, item_name) - .find(|item| tcx.def_kind(item).ns() == Some(namespace))?, - ), - None => Some(cur_item), + // There may be more than one crate with this name. We try them all. + // (This is particularly relevant when running `std` tests as then there are two `std` crates: + // the one in the sysroot and the one locally built by `cargo test`.) + // FIXME: can we prefer the one from the sysroot? + 'crates: for krate in + tcx.crates(()).iter().filter(|&&krate| tcx.crate_name(krate).as_str() == crate_name) + { + let mut cur_item = DefId { krate: *krate, index: CRATE_DEF_INDEX }; + // Go over the modules. + for &segment in modules { + let Some(next_item) = find_children(tcx, cur_item, segment) + .find(|item| tcx.def_kind(item) == DefKind::Mod) + else { + continue 'crates; + }; + cur_item = next_item; + } + // Finally, look up the desired item in this module, if any. + match item { + Some((item_name, namespace)) => { + let Some(item) = find_children(tcx, cur_item, item_name) + .find(|item| tcx.def_kind(item).ns() == Some(namespace)) + else { + continue 'crates; + }; + return Some(item); + } + None => { + // Just return the module. + return Some(cur_item); + } + } } + // Item not found in any of the crates with the right name. + None } /// Convert a softfloat type to its corresponding hostfloat type. @@ -968,10 +985,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn frame_in_std(&self) -> bool { let this = self.eval_context_ref(); - let Some(start_fn) = this.tcx.lang_items().start_fn() else { - // no_std situations - return false; - }; let frame = this.frame(); // Make an attempt to get at the instance of the function this is inlined from. let instance: Option<_> = try { @@ -982,13 +995,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; // Fall back to the instance of the function itself. let instance = instance.unwrap_or(frame.instance); - // Now check if this is in the same crate as start_fn. - // As a special exception we also allow unit tests from - // to call these - // shims. + // Now check the crate it is in. We could try to be clever here and e.g. check if this is + // the same crate as `start_fn`, but that would not work for running std tests in Miri, so + // we'd need some more hacks anyway. So we just check the name of the crate. If someone + // calls their crate `std` then we'll just let them keep the pieces. let frame_crate = this.tcx.def_path(instance.def_id()).krate; - frame_crate == this.tcx.def_path(start_fn).krate - || this.tcx.crate_name(frame_crate).as_str() == "std_miri_test" + let crate_name = this.tcx.crate_name(frame_crate); + let crate_name = crate_name.as_str(); + // On miri-test-libstd, the name of the crate is different. + crate_name == "std" || crate_name == "std_miri_test" } /// Handler that should be called when unsupported functionality is encountered. diff --git a/src/lib.rs b/src/lib.rs index 416d0cda8f..7821aa9efd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ #![feature(rustc_private)] #![feature(cell_update)] +#![feature(const_option)] #![feature(float_gamma)] #![feature(generic_nonzero)] #![feature(map_try_insert)] diff --git a/src/machine.rs b/src/machine.rs index 3a4ab32e4a..ff081328a7 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -12,7 +12,6 @@ use rand::rngs::StdRng; use rand::Rng; use rand::SeedableRng; -use rustc_ast::ast::Mutability; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; #[allow(unused)] use rustc_data_structures::static_assert_size; @@ -22,7 +21,7 @@ use rustc_middle::{ ty::{ self, layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout}, - Instance, Ty, TyCtxt, TypeAndMut, + Instance, Ty, TyCtxt, }, }; use rustc_span::def_id::{CrateNum, DefId}; @@ -32,10 +31,13 @@ use rustc_target::spec::abi::Abi; use crate::{ concurrency::{data_race, weak_memory}, - shims::unix::FileHandler, + shims::unix::FdTable, *, }; +use self::concurrency::data_race::NaReadType; +use self::concurrency::data_race::NaWriteType; + /// First real-time signal. /// `signal(7)` says this must be between 32 and 64 and specifies 34 or 35 /// as typical values. @@ -373,10 +375,8 @@ pub struct PrimitiveLayouts<'tcx> { impl<'mir, 'tcx: 'mir> PrimitiveLayouts<'tcx> { fn new(layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) -> Result> { let tcx = layout_cx.tcx; - let mut_raw_ptr = - Ty::new_ptr(tcx, TypeAndMut { ty: tcx.types.unit, mutbl: Mutability::Mut }); - let const_raw_ptr = - Ty::new_ptr(tcx, TypeAndMut { ty: tcx.types.unit, mutbl: Mutability::Not }); + let mut_raw_ptr = Ty::new_mut_ptr(tcx, tcx.types.unit); + let const_raw_ptr = Ty::new_imm_ptr(tcx, tcx.types.unit); Ok(Self { unit: layout_cx.layout_of(Ty::new_unit(tcx))?, i8: layout_cx.layout_of(tcx.types.i8)?, @@ -463,9 +463,9 @@ pub struct MiriMachine<'mir, 'tcx> { pub(crate) validate: bool, /// The table of file descriptors. - pub(crate) file_handler: shims::unix::FileHandler, + pub(crate) fds: shims::unix::FdTable, /// The table of directory descriptors. - pub(crate) dir_handler: shims::unix::DirHandler, + pub(crate) dirs: shims::unix::DirTable, /// This machine's monotone clock. pub(crate) clock: Clock, @@ -640,8 +640,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { tls: TlsData::default(), isolated_op: config.isolated_op, validate: config.validate, - file_handler: FileHandler::new(config.mute_stdout_stderr), - dir_handler: Default::default(), + fds: FdTable::new(config.mute_stdout_stderr), + dirs: Default::default(), layouts, threads: ThreadManager::default(), static_roots: Vec::new(), @@ -774,11 +774,11 @@ impl VisitProvenance for MiriMachine<'_, '_> { argv, cmd_line, extern_statics, - dir_handler, + dirs, borrow_tracker, data_race, alloc_addresses, - file_handler, + fds, tcx: _, isolated_op: _, validate: _, @@ -817,8 +817,8 @@ impl VisitProvenance for MiriMachine<'_, '_> { threads.visit_provenance(visit); tls.visit_provenance(visit); env_vars.visit_provenance(visit); - dir_handler.visit_provenance(visit); - file_handler.visit_provenance(visit); + dirs.visit_provenance(visit); + fds.visit_provenance(visit); data_race.visit_provenance(visit); borrow_tracker.visit_provenance(visit); alloc_addresses.visit_provenance(visit); @@ -1066,7 +1066,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let extern_decl_layout = ecx.tcx.layout_of(ty::ParamEnv::empty().and(def_ty)).unwrap(); if extern_decl_layout.size != shim_size || extern_decl_layout.align.abi != shim_align { throw_unsup_format!( - "`extern` static `{name}` from crate `{krate}` has been declared \ + "extern static `{link_name}` has been declared as `{krate}::{name}` \ with a size of {decl_size} bytes and alignment of {decl_align} bytes, \ but Miri emulates it via an extern static shim \ with a size of {shim_size} bytes and alignment of {shim_align} bytes", @@ -1080,11 +1080,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } Ok(ptr) } else { - throw_unsup_format!( - "`extern` static `{name}` from crate `{krate}` is not supported by Miri", - name = ecx.tcx.def_path_str(def_id), - krate = ecx.tcx.crate_name(def_id.krate), - ) + throw_unsup_format!("extern static `{link_name}` is not supported by Miri",) } } @@ -1241,7 +1237,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { .emit_diagnostic(NonHaltingDiagnostic::AccessedAlloc(alloc_id, AccessKind::Read)); } if let Some(data_race) = &alloc_extra.data_race { - data_race.read(alloc_id, range, machine)?; + data_race.read(alloc_id, range, NaReadType::Read, None, machine)?; } if let Some(borrow_tracker) = &alloc_extra.borrow_tracker { borrow_tracker.before_memory_read(alloc_id, prov_extra, range, machine)?; @@ -1265,7 +1261,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { .emit_diagnostic(NonHaltingDiagnostic::AccessedAlloc(alloc_id, AccessKind::Write)); } if let Some(data_race) = &mut alloc_extra.data_race { - data_race.write(alloc_id, range, machine)?; + data_race.write(alloc_id, range, NaWriteType::Write, None, machine)?; } if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker { borrow_tracker.before_memory_write(alloc_id, prov_extra, range, machine)?; @@ -1289,7 +1285,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { machine.emit_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id)); } if let Some(data_race) = &mut alloc_extra.data_race { - data_race.deallocate(alloc_id, size, machine)?; + data_race.write( + alloc_id, + alloc_range(Size::ZERO, size), + NaWriteType::Deallocate, + None, + machine, + )?; } if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker { borrow_tracker.before_memory_deallocation(alloc_id, prove_extra, size, machine)?; diff --git a/src/shims/intrinsics/simd.rs b/src/shims/intrinsics/simd.rs index c97a052f51..a2fc4f0f76 100644 --- a/src/shims/intrinsics/simd.rs +++ b/src/shims/intrinsics/simd.rs @@ -33,6 +33,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { | "round" | "trunc" | "fsqrt" + | "fsin" + | "fcos" + | "fexp" + | "fexp2" + | "flog" + | "flog2" + | "flog10" | "ctlz" | "cttz" | "bswap" @@ -45,17 +52,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { assert_eq!(dest_len, op_len); #[derive(Copy, Clone)] - enum Op { + enum Op<'a> { MirOp(mir::UnOp), Abs, - Sqrt, Round(rustc_apfloat::Round), Numeric(Symbol), + HostOp(&'a str), } let which = match intrinsic_name { "neg" => Op::MirOp(mir::UnOp::Neg), "fabs" => Op::Abs, - "fsqrt" => Op::Sqrt, "ceil" => Op::Round(rustc_apfloat::Round::TowardPositive), "floor" => Op::Round(rustc_apfloat::Round::TowardNegative), "round" => Op::Round(rustc_apfloat::Round::NearestTiesToAway), @@ -64,7 +70,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { "cttz" => Op::Numeric(sym::cttz), "bswap" => Op::Numeric(sym::bswap), "bitreverse" => Op::Numeric(sym::bitreverse), - _ => unreachable!(), + _ => Op::HostOp(intrinsic_name), }; for i in 0..dest_len { @@ -89,7 +95,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { FloatTy::F128 => unimplemented!("f16_f128"), } } - Op::Sqrt => { + Op::HostOp(host_op) => { let ty::Float(float_ty) = op.layout.ty.kind() else { span_bug!(this.cur_span(), "{} operand is not a float", intrinsic_name) }; @@ -98,13 +104,37 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { FloatTy::F16 => unimplemented!("f16_f128"), FloatTy::F32 => { let f = op.to_scalar().to_f32()?; - let res = f.to_host().sqrt().to_soft(); + let f_host = f.to_host(); + let res = match host_op { + "fsqrt" => f_host.sqrt(), + "fsin" => f_host.sin(), + "fcos" => f_host.cos(), + "fexp" => f_host.exp(), + "fexp2" => f_host.exp2(), + "flog" => f_host.ln(), + "flog2" => f_host.log2(), + "flog10" => f_host.log10(), + _ => bug!(), + }; + let res = res.to_soft(); let res = this.adjust_nan(res, &[f]); Scalar::from(res) } FloatTy::F64 => { let f = op.to_scalar().to_f64()?; - let res = f.to_host().sqrt().to_soft(); + let f_host = f.to_host(); + let res = match host_op { + "fsqrt" => f_host.sqrt(), + "fsin" => f_host.sin(), + "fcos" => f_host.cos(), + "fexp" => f_host.exp(), + "fexp2" => f_host.exp2(), + "flog" => f_host.ln(), + "flog2" => f_host.log2(), + "flog10" => f_host.log10(), + _ => bug!(), + }; + let res = res.to_soft(); let res = this.adjust_nan(res, &[f]); Scalar::from(res) } @@ -427,7 +457,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let bitmask_len = u32::try_from(bitmask_len).unwrap(); // To read the mask, we transmute it to an integer. - // That does the right thing wrt endianess. + // That does the right thing wrt endianness. let mask_ty = this.machine.layouts.uint(mask.layout.size).unwrap(); let mask = mask.transmute(mask_ty, this)?; let mask: u64 = this.read_scalar(&mask)?.to_bits(mask_ty.size)?.try_into().unwrap(); @@ -479,12 +509,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } // We have to change the type of the place to be able to write `res` into it. This - // transmutes the integer to an array, which does the right thing wrt endianess. + // transmutes the integer to an array, which does the right thing wrt endianness. let dest = dest.transmute(this.machine.layouts.uint(dest.layout.size).unwrap(), this)?; this.write_int(res, &dest)?; } - "cast" | "as" | "cast_ptr" | "expose_addr" | "from_exposed_addr" => { + "cast" | "as" | "cast_ptr" | "expose_provenance" | "with_exposed_provenance" => { let [op] = check_arg_count(args)?; let (op, op_len) = this.operand_to_simd(op)?; let (dest, dest_len) = this.mplace_to_simd(dest)?; @@ -494,8 +524,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let unsafe_cast = intrinsic_name == "cast"; let safe_cast = intrinsic_name == "as"; let ptr_cast = intrinsic_name == "cast_ptr"; - let expose_cast = intrinsic_name == "expose_addr"; - let from_exposed_cast = intrinsic_name == "from_exposed_addr"; + let expose_cast = intrinsic_name == "expose_provenance"; + let from_exposed_cast = intrinsic_name == "with_exposed_provenance"; for i in 0..dest_len { let op = this.read_immediate(&this.project_index(&op, i)?)?; @@ -527,9 +557,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.ptr_to_ptr(&op, dest.layout)?, // Ptr/Int casts (ty::RawPtr(..), ty::Int(_) | ty::Uint(_)) if expose_cast => - this.pointer_expose_address_cast(&op, dest.layout)?, + this.pointer_expose_provenance_cast(&op, dest.layout)?, (ty::Int(_) | ty::Uint(_), ty::RawPtr(..)) if from_exposed_cast => - this.pointer_from_exposed_address_cast(&op, dest.layout)?, + this.pointer_with_exposed_provenance_cast(&op, dest.layout)?, // Error otherwise _ => throw_unsup_format!( diff --git a/src/shims/panic.rs b/src/shims/panic.rs index 34b6481dd3..bb31ef733c 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -256,8 +256,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } _ => { - // Forward everything else to `panic` lang item. - this.start_panic(msg.description(), unwind)?; + // Call the lang item associated with this message. + let fn_item = this.tcx.require_lang_item(msg.panic_function(), None); + let instance = ty::Instance::mono(this.tcx.tcx, fn_item); + this.call_function( + instance, + Abi::Rust, + &[], + None, + StackPopCleanup::Goto { ret: None, unwind }, + )?; } } Ok(()) diff --git a/src/shims/unix/fd.rs b/src/shims/unix/fd.rs new file mode 100644 index 0000000000..a5fe38b902 --- /dev/null +++ b/src/shims/unix/fd.rs @@ -0,0 +1,431 @@ +//! General management of file descriptors, and support for +//! standard file descriptors (stdin/stdout/stderr). + +use std::any::Any; +use std::collections::BTreeMap; +use std::io::{self, ErrorKind, IsTerminal, Read, SeekFrom, Write}; + +use rustc_middle::ty::TyCtxt; +use rustc_target::abi::Size; + +use crate::shims::unix::*; +use crate::*; + +/// Represents an open file descriptor. +pub trait FileDescriptor: std::fmt::Debug + Any { + fn name(&self) -> &'static str; + + fn read<'tcx>( + &mut self, + _communicate_allowed: bool, + _bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot read from {}", self.name()); + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + _bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot write to {}", self.name()); + } + + fn seek<'tcx>( + &mut self, + _communicate_allowed: bool, + _offset: SeekFrom, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot seek on {}", self.name()); + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot close {}", self.name()); + } + + /// Return a new file descriptor *that refers to the same underlying object*. + fn dup(&mut self) -> io::Result>; + + fn is_tty(&self, _communicate_allowed: bool) -> bool { + // Most FDs are not tty's and the consequence of a wrong `false` are minor, + // so we use a default impl here. + false + } +} + +impl dyn FileDescriptor { + #[inline(always)] + pub fn downcast_ref(&self) -> Option<&T> { + (self as &dyn Any).downcast_ref() + } + + #[inline(always)] + pub fn downcast_mut(&mut self) -> Option<&mut T> { + (self as &mut dyn Any).downcast_mut() + } +} + +impl FileDescriptor for io::Stdin { + fn name(&self) -> &'static str { + "stdin" + } + + fn read<'tcx>( + &mut self, + communicate_allowed: bool, + bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + if !communicate_allowed { + // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. + helpers::isolation_abort_error("`read` from stdin")?; + } + Ok(Read::read(self, bytes)) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(io::stdin())) + } + + fn is_tty(&self, communicate_allowed: bool) -> bool { + communicate_allowed && self.is_terminal() + } +} + +impl FileDescriptor for io::Stdout { + fn name(&self) -> &'static str { + "stdout" + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + // We allow writing to stderr even with isolation enabled. + let result = Write::write(&mut { self }, bytes); + // Stdout is buffered, flush to make sure it appears on the + // screen. This is the write() syscall of the interpreted + // program, we want it to correspond to a write() syscall on + // the host -- there is no good in adding extra buffering + // here. + io::stdout().flush().unwrap(); + + Ok(result) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(io::stdout())) + } + + fn is_tty(&self, communicate_allowed: bool) -> bool { + communicate_allowed && self.is_terminal() + } +} + +impl FileDescriptor for io::Stderr { + fn name(&self) -> &'static str { + "stderr" + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + // We allow writing to stderr even with isolation enabled. + // No need to flush, stderr is not buffered. + Ok(Write::write(&mut { self }, bytes)) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(io::stderr())) + } + + fn is_tty(&self, communicate_allowed: bool) -> bool { + communicate_allowed && self.is_terminal() + } +} + +/// Like /dev/null +#[derive(Debug)] +pub struct NullOutput; + +impl FileDescriptor for NullOutput { + fn name(&self) -> &'static str { + "stderr and stdout" + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + // We just don't write anything, but report to the user that we did. + Ok(Ok(bytes.len())) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(NullOutput)) + } +} + +/// The file descriptor table +#[derive(Debug)] +pub struct FdTable { + pub fds: BTreeMap>, +} + +impl VisitProvenance for FdTable { + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { + // All our FileDescriptor do not have any tags. + } +} + +impl FdTable { + pub(crate) fn new(mute_stdout_stderr: bool) -> FdTable { + let mut fds: BTreeMap<_, Box> = BTreeMap::new(); + fds.insert(0i32, Box::new(io::stdin())); + if mute_stdout_stderr { + fds.insert(1i32, Box::new(NullOutput)); + fds.insert(2i32, Box::new(NullOutput)); + } else { + fds.insert(1i32, Box::new(io::stdout())); + fds.insert(2i32, Box::new(io::stderr())); + } + FdTable { fds } + } + + pub fn insert_fd(&mut self, file_handle: Box) -> i32 { + self.insert_fd_with_min_fd(file_handle, 0) + } + + /// Insert a new FD that is at least `min_fd`. + pub fn insert_fd_with_min_fd( + &mut self, + file_handle: Box, + min_fd: i32, + ) -> i32 { + // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in + // between used FDs, the find_map combinator will return it. If the first such unused FD + // is after all other used FDs, the find_map combinator will return None, and we will use + // the FD following the greatest FD thus far. + let candidate_new_fd = + self.fds.range(min_fd..).zip(min_fd..).find_map(|((fd, _fh), counter)| { + if *fd != counter { + // There was a gap in the fds stored, return the first unused one + // (note that this relies on BTreeMap iterating in key order) + Some(counter) + } else { + // This fd is used, keep going + None + } + }); + 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.try_insert(new_fd, file_handle).unwrap(); + new_fd + } + + pub fn get(&self, fd: i32) -> Option<&dyn FileDescriptor> { + Some(&**self.fds.get(&fd)?) + } + + pub fn get_mut(&mut self, fd: i32) -> Option<&mut dyn FileDescriptor> { + Some(&mut **self.fds.get_mut(&fd)?) + } + + pub fn remove(&mut self, fd: i32) -> Option> { + self.fds.remove(&fd) + } + + pub fn is_fd(&self, fd: i32) -> bool { + self.fds.contains_key(&fd) + } +} + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + fn fcntl(&mut self, args: &[OpTy<'tcx, Provenance>]) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + + if args.len() < 2 { + throw_ub_format!( + "incorrect number of arguments for fcntl: got {}, expected at least 2", + args.len() + ); + } + let fd = this.read_scalar(&args[0])?.to_i32()?; + let cmd = this.read_scalar(&args[1])?.to_i32()?; + + // 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 + // `FD_CLOEXEC` value without checking if the flag is set for the file because `std` + // always sets this flag when opening a file. However we still need to check that the + // file itself is open. + if this.machine.fds.is_fd(fd) { + Ok(this.eval_libc_i32("FD_CLOEXEC")) + } else { + this.fd_not_found() + } + } else if cmd == this.eval_libc_i32("F_DUPFD") + || cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC") + { + // Note that we always assume the FD_CLOEXEC flag is set for every open file, in part + // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only + // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor, + // thus they can share the same implementation here. + if args.len() < 3 { + throw_ub_format!( + "incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3", + args.len() + ); + } + let start = this.read_scalar(&args[2])?.to_i32()?; + + match this.machine.fds.get_mut(fd) { + Some(file_descriptor) => { + let dup_result = file_descriptor.dup(); + match dup_result { + Ok(dup_fd) => Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, start)), + Err(e) => { + this.set_last_error_from_io_error(e.kind())?; + Ok(-1) + } + } + } + None => this.fd_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); + } + + this.ffullsync_fd(fd) + } else { + throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd); + } + } + + fn close(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let fd = this.read_scalar(fd_op)?.to_i32()?; + + Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.remove(fd) { + let result = file_descriptor.close(this.machine.communicate())?; + this.try_unwrap_io_result(result)? + } else { + this.fd_not_found()? + })) + } + + /// Function used when a file descriptor does not exist. It returns `Ok(-1)`and sets + /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses + /// `T: From` instead of `i32` directly because some fs functions return different integer + /// types (like `read`, that returns an `i64`). + fn fd_not_found>(&mut self) -> InterpResult<'tcx, T> { + let this = self.eval_context_mut(); + let ebadf = this.eval_libc("EBADF"); + this.set_last_error(ebadf)?; + Ok((-1).into()) + } + + fn read( + &mut self, + fd: i32, + buf: Pointer>, + count: u64, + ) -> InterpResult<'tcx, i64> { + let this = self.eval_context_mut(); + + // Isolation check is done via `FileDescriptor` trait. + + trace!("Reading from FD {}, size {}", fd, count); + + // Check that the *entire* buffer is actually valid memory. + this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; + + // We cap the number of read bytes to the largest value that we are able to fit in both the + // host's and target's `isize`. This saves us from having to handle overflows later. + let count = count + .min(u64::try_from(this.target_isize_max()).unwrap()) + .min(u64::try_from(isize::MAX).unwrap()); + let communicate = this.machine.communicate(); + + if let Some(file_descriptor) = this.machine.fds.get_mut(fd) { + trace!("read: FD mapped to {:?}", file_descriptor); + // We want to read at most `count` bytes. We are sure that `count` is not negative + // because it was a target's `usize`. Also we are sure that its smaller than + // `usize::MAX` because it is bounded by the host's `isize`. + let mut bytes = vec![0; usize::try_from(count).unwrap()]; + // `File::read` never returns a value larger than `count`, + // so this cannot fail. + let result = file_descriptor + .read(communicate, &mut bytes, *this.tcx)? + .map(|c| i64::try_from(c).unwrap()); + + match result { + Ok(read_bytes) => { + // If reading to `bytes` did not fail, we write those bytes to the buffer. + this.write_bytes_ptr(buf, bytes)?; + Ok(read_bytes) + } + Err(e) => { + this.set_last_error_from_io_error(e.kind())?; + Ok(-1) + } + } + } else { + trace!("read: FD not found"); + this.fd_not_found() + } + } + + fn write( + &mut self, + fd: i32, + buf: Pointer>, + count: u64, + ) -> InterpResult<'tcx, i64> { + let this = self.eval_context_mut(); + + // Isolation check is done via `FileDescriptor` trait. + + // Check that the *entire* buffer is actually valid memory. + this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; + + // We cap the number of written bytes to the largest value that we are able to fit in both the + // host's and target's `isize`. This saves us from having to handle overflows later. + let count = count + .min(u64::try_from(this.target_isize_max()).unwrap()) + .min(u64::try_from(isize::MAX).unwrap()); + let communicate = this.machine.communicate(); + + if let Some(file_descriptor) = this.machine.fds.get(fd) { + let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?; + let result = file_descriptor + .write(communicate, bytes, *this.tcx)? + .map(|c| i64::try_from(c).unwrap()); + this.try_unwrap_io_result(result) + } else { + this.fd_not_found() + } + } +} diff --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index bfa4b5bdff..96dd8b11da 100644 --- a/src/shims/unix/foreign_items.rs +++ b/src/shims/unix/foreign_items.rs @@ -6,12 +6,9 @@ use rustc_span::Symbol; use rustc_target::abi::{Align, Size}; use rustc_target::spec::abi::Abi; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::mem::EvalContextExt as _; -use shims::unix::sync::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; use shims::unix::freebsd::foreign_items as freebsd; use shims::unix::linux::foreign_items as linux; @@ -51,7 +48,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // See `fn emulate_foreign_item_inner` in `shims/foreign_items.rs` for the general pattern. #[rustfmt::skip] match link_name.as_str() { - // Environment related shims + // Environment variables "getenv" => { let [name] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.getenv(name)?; @@ -79,25 +76,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_scalar(Scalar::from_i32(result), dest)?; } - // File related shims - "open" | "open64" => { - // `open` is variadic, the third argument is only present when the second argument has O_CREAT (or on linux O_TMPFILE, but miri doesn't support that) set - this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; - let result = this.open(args)?; - this.write_scalar(Scalar::from_i32(result), dest)?; - } - "close" => { - let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let result = this.close(fd)?; - this.write_scalar(result, dest)?; - } - "fcntl" => { - // `fcntl` is variadic. The argument count is checked based on the first argument - // in `this.fcntl()`, so we do not use `check_shim` here. - this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; - let result = this.fcntl(args)?; - this.write_scalar(Scalar::from_i32(result), dest)?; - } + // File descriptors "read" => { let [fd, buf, count] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let fd = this.read_scalar(fd)?.to_i32()?; @@ -116,6 +95,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Now, `result` is the value we return back to the program. this.write_scalar(Scalar::from_target_isize(result, this), dest)?; } + "close" => { + let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.close(fd)?; + this.write_scalar(result, dest)?; + } + "fcntl" => { + // `fcntl` is variadic. The argument count is checked based on the first argument + // in `this.fcntl()`, so we do not use `check_shim` here. + this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; + let result = this.fcntl(args)?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } + + // File and file system access + "open" | "open64" => { + // `open` is variadic, the third argument is only present when the second argument has O_CREAT (or on linux O_TMPFILE, but miri doesn't support that) set + this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; + let result = this.open(args)?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } "unlink" => { let [path] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.unlink(path)?; @@ -219,7 +218,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_scalar(Scalar::from_i32(result), dest)?; } - // Time related shims + // Sockets + "socketpair" => { + let [domain, type_, protocol, sv] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + let result = this.socketpair(domain, type_, protocol, sv)?; + this.write_scalar(result, dest)?; + } + + // Time "gettimeofday" => { let [tv, tz] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.gettimeofday(tv, tz)?; @@ -603,7 +611,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if bufsize > 256 { let err = this.eval_libc("EIO"); this.set_last_error(err)?; - this.write_scalar(Scalar::from_i32(-1), dest)? + this.write_scalar(Scalar::from_i32(-1), dest)?; } else { this.gen_random(buf, bufsize)?; this.write_scalar(Scalar::from_i32(0), dest)?; diff --git a/src/shims/unix/freebsd/foreign_items.rs b/src/shims/unix/freebsd/foreign_items.rs index 6814e0d428..ffb583123d 100644 --- a/src/shims/unix/freebsd/foreign_items.rs +++ b/src/shims/unix/freebsd/foreign_items.rs @@ -1,10 +1,9 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; pub fn is_dyn_sym(_name: &str) -> bool { false diff --git a/src/shims/unix/fs.rs b/src/shims/unix/fs.rs index b141ca4a01..31076fdfaf 100644 --- a/src/shims/unix/fs.rs +++ b/src/shims/unix/fs.rs @@ -1,6 +1,6 @@ -use std::any::Any; +//! File and file system access + use std::borrow::Cow; -use std::collections::BTreeMap; use std::fs::{ read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir, }; @@ -13,70 +13,16 @@ use rustc_middle::ty::TyCtxt; use rustc_target::abi::Size; use crate::shims::os_str::bytes_to_os_str; +use crate::shims::unix::*; use crate::*; use shims::time::system_time_to_duration; #[derive(Debug)] -pub struct FileHandle { +struct FileHandle { file: File, writable: bool, } -pub trait FileDescriptor: std::fmt::Debug + Any { - fn name(&self) -> &'static str; - - fn read<'tcx>( - &mut self, - _communicate_allowed: bool, - _bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot read from {}", self.name()); - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - _bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot write to {}", self.name()); - } - - fn seek<'tcx>( - &mut self, - _communicate_allowed: bool, - _offset: SeekFrom, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot seek on {}", self.name()); - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot close {}", self.name()); - } - - fn dup(&mut self) -> io::Result>; - - fn is_tty(&self, _communicate_allowed: bool) -> bool { - false - } -} - -impl dyn FileDescriptor { - #[inline(always)] - pub fn downcast_ref(&self) -> Option<&T> { - (self as &dyn Any).downcast_ref() - } - - #[inline(always)] - pub fn downcast_mut(&mut self) -> Option<&mut T> { - (self as &mut dyn Any).downcast_mut() - } -} - impl FileDescriptor for FileHandle { fn name(&self) -> &'static str { "FILE" @@ -147,172 +93,6 @@ impl FileDescriptor for FileHandle { } } -impl FileDescriptor for io::Stdin { - fn name(&self) -> &'static str { - "stdin" - } - - fn read<'tcx>( - &mut self, - communicate_allowed: bool, - bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - if !communicate_allowed { - // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. - helpers::isolation_abort_error("`read` from stdin")?; - } - Ok(Read::read(self, bytes)) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(io::stdin())) - } - - fn is_tty(&self, communicate_allowed: bool) -> bool { - communicate_allowed && self.is_terminal() - } -} - -impl FileDescriptor for io::Stdout { - fn name(&self) -> &'static str { - "stdout" - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - // We allow writing to stderr even with isolation enabled. - let result = Write::write(&mut { self }, bytes); - // Stdout is buffered, flush to make sure it appears on the - // screen. This is the write() syscall of the interpreted - // program, we want it to correspond to a write() syscall on - // the host -- there is no good in adding extra buffering - // here. - io::stdout().flush().unwrap(); - - Ok(result) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(io::stdout())) - } - - fn is_tty(&self, communicate_allowed: bool) -> bool { - communicate_allowed && self.is_terminal() - } -} - -impl FileDescriptor for io::Stderr { - fn name(&self) -> &'static str { - "stderr" - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - // We allow writing to stderr even with isolation enabled. - // No need to flush, stderr is not buffered. - Ok(Write::write(&mut { self }, bytes)) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(io::stderr())) - } - - fn is_tty(&self, communicate_allowed: bool) -> bool { - communicate_allowed && self.is_terminal() - } -} - -#[derive(Debug)] -struct NullOutput; - -impl FileDescriptor for NullOutput { - fn name(&self) -> &'static str { - "stderr and stdout" - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - // We just don't write anything, but report to the user that we did. - Ok(Ok(bytes.len())) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(NullOutput)) - } -} - -#[derive(Debug)] -pub struct FileHandler { - pub handles: BTreeMap>, -} - -impl VisitProvenance for FileHandler { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // All our FileDescriptor do not have any tags. - } -} - -impl FileHandler { - pub(crate) fn new(mute_stdout_stderr: bool) -> FileHandler { - let mut handles: BTreeMap<_, Box> = BTreeMap::new(); - handles.insert(0i32, Box::new(io::stdin())); - if mute_stdout_stderr { - handles.insert(1i32, Box::new(NullOutput)); - handles.insert(2i32, Box::new(NullOutput)); - } else { - handles.insert(1i32, Box::new(io::stdout())); - handles.insert(2i32, Box::new(io::stderr())); - } - FileHandler { handles } - } - - pub fn insert_fd(&mut self, file_handle: Box) -> i32 { - self.insert_fd_with_min_fd(file_handle, 0) - } - - fn insert_fd_with_min_fd(&mut self, file_handle: Box, min_fd: i32) -> i32 { - // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in - // between used FDs, the find_map combinator will return it. If the first such unused FD - // is after all other used FDs, the find_map combinator will return None, and we will use - // the FD following the greatest FD thus far. - let candidate_new_fd = - self.handles.range(min_fd..).zip(min_fd..).find_map(|((fd, _fh), counter)| { - if *fd != counter { - // There was a gap in the fds stored, return the first unused one - // (note that this relies on BTreeMap iterating in key order) - Some(counter) - } else { - // This fd is used, keep going - None - } - }); - 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.handles - .last_key_value() - .map(|(fd, _)| fd.checked_add(1).unwrap()) - .unwrap_or(min_fd) - }); - - self.handles.try_insert(new_fd, file_handle).unwrap(); - new_fd - } -} - impl<'mir, 'tcx: 'mir> EvalContextExtPrivate<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn macos_stat_write_buf( @@ -411,10 +191,11 @@ trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx /// An open directory, tracked by DirHandler. #[derive(Debug)] -pub struct OpenDir { +struct OpenDir { /// The directory reader on the host. read_dir: ReadDir, - /// The most recent entry returned by readdir() + /// The most recent entry returned by readdir(). + /// Will be freed by the next call. entry: Pointer>, } @@ -425,8 +206,11 @@ impl OpenDir { } } +/// The table of open directories. +/// Curiously, Unix/POSIX does not unify this into the "file descriptor" concept... everything +/// is a file, except a directory is not? #[derive(Debug)] -pub struct DirHandler { +pub struct DirTable { /// Directory iterators used to emulate libc "directory streams", as used in opendir, readdir, /// and closedir. /// @@ -441,7 +225,7 @@ pub struct DirHandler { next_id: u64, } -impl DirHandler { +impl DirTable { #[allow(clippy::arithmetic_side_effects)] fn insert_new(&mut self, read_dir: ReadDir) -> u64 { let id = self.next_id; @@ -451,9 +235,9 @@ impl DirHandler { } } -impl Default for DirHandler { - fn default() -> DirHandler { - DirHandler { +impl Default for DirTable { + fn default() -> DirTable { + DirTable { streams: FxHashMap::default(), // Skip 0 as an ID, because it looks like a null pointer to libc next_id: 1, @@ -461,9 +245,9 @@ impl Default for DirHandler { } } -impl VisitProvenance for DirHandler { +impl VisitProvenance for DirTable { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let DirHandler { streams, next_id: _ } = self; + let DirTable { streams, next_id: _ } = self; for dir in streams.values() { dir.entry.visit_provenance(visit); @@ -615,200 +399,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } let fd = options.open(path).map(|file| { - let fh = &mut this.machine.file_handler; + let fh = &mut this.machine.fds; fh.insert_fd(Box::new(FileHandle { file, writable })) }); this.try_unwrap_io_result(fd) } - fn fcntl(&mut self, args: &[OpTy<'tcx, Provenance>]) -> InterpResult<'tcx, i32> { - let this = self.eval_context_mut(); - - if args.len() < 2 { - throw_ub_format!( - "incorrect number of arguments for fcntl: got {}, expected at least 2", - args.len() - ); - } - let fd = this.read_scalar(&args[0])?.to_i32()?; - let cmd = this.read_scalar(&args[1])?.to_i32()?; - - // 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 - // `FD_CLOEXEC` value without checking if the flag is set for the file because `std` - // always sets this flag when opening a file. However we still need to check that the - // file itself is open. - if this.machine.file_handler.handles.contains_key(&fd) { - Ok(this.eval_libc_i32("FD_CLOEXEC")) - } else { - this.handle_not_found() - } - } else if cmd == this.eval_libc_i32("F_DUPFD") - || cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC") - { - // Note that we always assume the FD_CLOEXEC flag is set for every open file, in part - // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only - // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor, - // thus they can share the same implementation here. - if args.len() < 3 { - throw_ub_format!( - "incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3", - args.len() - ); - } - let start = this.read_scalar(&args[2])?.to_i32()?; - - let fh = &mut this.machine.file_handler; - - match fh.handles.get_mut(&fd) { - Some(file_descriptor) => { - let dup_result = file_descriptor.dup(); - match dup_result { - Ok(dup_fd) => Ok(fh.insert_fd_with_min_fd(dup_fd, start)), - Err(e) => { - this.set_last_error_from_io_error(e.kind())?; - Ok(-1) - } - } - } - 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 } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`F_FULLFSYNC` is only supported on file-backed file descriptors" - ) - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_all); - this.try_unwrap_io_result(io_result) - } else { - this.handle_not_found() - } - } else { - throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd); - } - } - - fn close(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let fd = this.read_scalar(fd_op)?.to_i32()?; - - Ok(Scalar::from_i32( - if let Some(file_descriptor) = this.machine.file_handler.handles.remove(&fd) { - let result = file_descriptor.close(this.machine.communicate())?; - this.try_unwrap_io_result(result)? - } else { - this.handle_not_found()? - }, - )) - } - - /// Function used when a handle is not found inside `FileHandler`. It returns `Ok(-1)`and sets - /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses - /// `T: From` instead of `i32` directly because some fs functions return different integer - /// types (like `read`, that returns an `i64`). - fn handle_not_found>(&mut self) -> InterpResult<'tcx, T> { - let this = self.eval_context_mut(); - let ebadf = this.eval_libc("EBADF"); - this.set_last_error(ebadf)?; - Ok((-1).into()) - } - - fn read( - &mut self, - fd: i32, - buf: Pointer>, - count: u64, - ) -> InterpResult<'tcx, i64> { - let this = self.eval_context_mut(); - - // Isolation check is done via `FileDescriptor` trait. - - trace!("Reading from FD {}, size {}", fd, count); - - // Check that the *entire* buffer is actually valid memory. - this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; - - // We cap the number of read bytes to the largest value that we are able to fit in both the - // host's and target's `isize`. This saves us from having to handle overflows later. - let count = count - .min(u64::try_from(this.target_isize_max()).unwrap()) - .min(u64::try_from(isize::MAX).unwrap()); - let communicate = this.machine.communicate(); - - if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) { - trace!("read: FD mapped to {:?}", file_descriptor); - // We want to read at most `count` bytes. We are sure that `count` is not negative - // because it was a target's `usize`. Also we are sure that its smaller than - // `usize::MAX` because it is bounded by the host's `isize`. - let mut bytes = vec![0; usize::try_from(count).unwrap()]; - // `File::read` never returns a value larger than `count`, - // so this cannot fail. - let result = file_descriptor - .read(communicate, &mut bytes, *this.tcx)? - .map(|c| i64::try_from(c).unwrap()); - - match result { - Ok(read_bytes) => { - // If reading to `bytes` did not fail, we write those bytes to the buffer. - this.write_bytes_ptr(buf, bytes)?; - Ok(read_bytes) - } - Err(e) => { - this.set_last_error_from_io_error(e.kind())?; - Ok(-1) - } - } - } else { - trace!("read: FD not found"); - this.handle_not_found() - } - } - - fn write( - &mut self, - fd: i32, - buf: Pointer>, - count: u64, - ) -> InterpResult<'tcx, i64> { - let this = self.eval_context_mut(); - - // Isolation check is done via `FileDescriptor` trait. - - // Check that the *entire* buffer is actually valid memory. - this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; - - // We cap the number of written bytes to the largest value that we are able to fit in both the - // host's and target's `isize`. This saves us from having to handle overflows later. - let count = count - .min(u64::try_from(this.target_isize_max()).unwrap()) - .min(u64::try_from(isize::MAX).unwrap()); - let communicate = this.machine.communicate(); - - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?; - let result = file_descriptor - .write(communicate, bytes, *this.tcx)? - .map(|c| i64::try_from(c).unwrap()); - this.try_unwrap_io_result(result) - } else { - this.handle_not_found() - } - } - fn lseek64( &mut self, fd: i32, @@ -832,16 +429,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; let communicate = this.machine.communicate(); - Ok(Scalar::from_i64( - if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) { - let result = file_descriptor - .seek(communicate, seek_from)? - .map(|offset| i64::try_from(offset).unwrap()); - this.try_unwrap_io_result(result)? - } else { - this.handle_not_found()? - }, - )) + Ok(Scalar::from_i64(if let Some(file_descriptor) = this.machine.fds.get_mut(fd) { + let result = file_descriptor + .seek(communicate, seek_from)? + .map(|offset| i64::try_from(offset).unwrap()); + this.try_unwrap_io_result(result)? + } else { + this.fd_not_found()? + })) } fn unlink(&mut self, path_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { @@ -970,7 +565,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fstat`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } let metadata = match FileMetadata::from_fd(this, fd)? { @@ -1269,7 +864,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { match result { Ok(dir_iter) => { - let id = this.machine.dir_handler.insert_new(dir_iter); + let id = this.machine.dirs.insert_new(dir_iter); // The libc API for opendir says that this method returns a pointer to an opaque // structure, but we are returning an ID number. Thus, pass it as a scalar of @@ -1301,7 +896,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(Scalar::null_ptr(this)); } - let open_dir = this.machine.dir_handler.streams.get_mut(&dirp).ok_or_else(|| { + let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { err_unsup_format!("the DIR pointer passed to readdir64 did not come from opendir") })?; @@ -1366,7 +961,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } }; - let open_dir = this.machine.dir_handler.streams.get_mut(&dirp).unwrap(); + let open_dir = this.machine.dirs.streams.get_mut(&dirp).unwrap(); let old_entry = std::mem::replace(&mut open_dir.entry, entry); this.free(old_entry, MiriMemoryKind::Runtime)?; @@ -1391,10 +986,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`readdir_r`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let open_dir = this.machine.dir_handler.streams.get_mut(&dirp).ok_or_else(|| { + let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { err_unsup_format!("the DIR pointer passed to readdir_r did not come from opendir") })?; Ok(Scalar::from_i32(match open_dir.read_dir.next() { @@ -1507,15 +1102,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`closedir`", reject_with)?; // Set error code as "EBADF" (bad fd) - return this.handle_not_found(); + return this.fd_not_found(); } - if let Some(open_dir) = this.machine.dir_handler.streams.remove(&dirp) { + if let Some(open_dir) = this.machine.dirs.streams.remove(&dirp) { this.free(open_dir.entry, MiriMemoryKind::Runtime)?; drop(open_dir); Ok(0) } else { - this.handle_not_found() + this.fd_not_found() } } @@ -1526,37 +1121,35 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`ftruncate64`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } - Ok(Scalar::from_i32( - if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) { - // FIXME: Support ftruncate64 for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`ftruncate64` is only supported on file-backed file descriptors" - ) - })?; - if *writable { - if let Ok(length) = length.try_into() { - let result = file.set_len(length); - this.try_unwrap_io_result(result.map(|_| 0i32))? - } else { - let einval = this.eval_libc("EINVAL"); - this.set_last_error(einval)?; - -1 - } + Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.get_mut(fd) { + // FIXME: Support ftruncate64 for all FDs + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!( + "`ftruncate64` is only supported on file-backed file descriptors" + ) + })?; + if *writable { + if let Ok(length) = length.try_into() { + let result = file.set_len(length); + this.try_unwrap_io_result(result.map(|_| 0i32))? } else { - // The file is not writable let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; -1 } } else { - this.handle_not_found()? - }, - )) + // The file is not writable + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + -1 + } + } else { + this.fd_not_found()? + })) } fn fsync(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { @@ -1573,20 +1166,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fsync`", reject_with)?; // Set error code as "EBADF" (bad fd) - return this.handle_not_found(); + return this.fd_not_found(); } - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support fsync for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!("`fsync` is only supported on file-backed file descriptors") - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_all); - this.try_unwrap_io_result(io_result) - } else { - this.handle_not_found() - } + return self.ffullsync_fd(fd); + } + + fn ffullsync_fd(&mut self, fd: i32) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(this.fd_not_found()?); + }; + // Only regular files support synchronization. + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!("`fsync` is only supported on file-backed file descriptors") + })?; + let io_result = maybe_sync_file(file, *writable, File::sync_all); + this.try_unwrap_io_result(io_result) } fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { @@ -1598,22 +1195,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fdatasync`", reject_with)?; // Set error code as "EBADF" (bad fd) - return this.handle_not_found(); + return this.fd_not_found(); } - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support fdatasync for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`fdatasync` is only supported on file-backed file descriptors" - ) - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_data); - this.try_unwrap_io_result(io_result) - } else { - this.handle_not_found() - } + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(this.fd_not_found()?); + }; + // Only regular files support synchronization. + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors") + })?; + let io_result = maybe_sync_file(file, *writable, File::sync_data); + this.try_unwrap_io_result(io_result) } fn sync_file_range( @@ -1648,22 +1242,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`sync_file_range`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support sync_data_range for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`sync_data_range` is only supported on file-backed file descriptors" - ) - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_data); - Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) - } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) - } + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(Scalar::from_i32(this.fd_not_found()?)); + }; + // Only regular files support synchronization. + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!( + "`sync_data_range` is only supported on file-backed file descriptors" + ) + })?; + let io_result = maybe_sync_file(file, *writable, File::sync_data); + Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) } fn readlink( @@ -1720,7 +1313,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // "returns 1 if fd is an open file descriptor referring to a terminal; // otherwise 0 is returned, and errno is set to indicate the error" let fd = this.read_scalar(miri_fd)?.to_i32()?; - let error = if let Some(fd) = this.machine.file_handler.handles.get(&fd) { + let error = if let Some(fd) = this.machine.fds.get(fd) { if fd.is_tty(this.machine.communicate()) { return Ok(Scalar::from_i32(1)); } else { @@ -1897,7 +1490,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { match file { Ok(f) => { - let fh = &mut this.machine.file_handler; + let fh = &mut this.machine.fds; let fd = fh.insert_fd(Box::new(FileHandle { file: f, writable: true })); return Ok(fd); } @@ -1963,7 +1556,7 @@ impl FileMetadata { ecx: &mut MiriInterpCx<'_, 'tcx>, fd: i32, ) -> InterpResult<'tcx, Option> { - let option = ecx.machine.file_handler.handles.get(&fd); + let option = ecx.machine.fds.get(fd); let file = match option { Some(file_descriptor) => &file_descriptor @@ -1974,7 +1567,7 @@ impl FileMetadata { ) })? .file, - None => return ecx.handle_not_found().map(|_: i32| None), + None => return ecx.fd_not_found().map(|_: i32| None), }; let metadata = file.metadata(); diff --git a/src/shims/unix/linux/fd.rs b/src/shims/unix/linux/epoll.rs similarity index 55% rename from src/shims/unix/linux/fd.rs rename to src/shims/unix/linux/epoll.rs index 22fbb6da95..5161d91ca3 100644 --- a/src/shims/unix/linux/fd.rs +++ b/src/shims/unix/linux/epoll.rs @@ -1,17 +1,52 @@ -use std::cell::Cell; +use std::io; -use rustc_middle::ty::ScalarInt; +use rustc_data_structures::fx::FxHashMap; +use crate::shims::unix::*; use crate::*; -use epoll::{Epoll, EpollEvent}; -use event::Event; -use socketpair::SocketPair; -use shims::unix::fs::EvalContextExt as _; +/// An `Epoll` file descriptor connects file handles and epoll events +#[derive(Clone, Debug, Default)] +struct Epoll { + /// The file descriptors we are watching, and what we are watching for. + file_descriptors: FxHashMap, +} + +/// Epoll Events associate events with data. +/// These fields are currently unused by miri. +/// This matches the `epoll_event` struct defined +/// by the epoll_ctl man page. For more information +/// see the man page: +/// +/// +#[derive(Clone, Debug)] +struct EpollEvent { + #[allow(dead_code)] + events: u32, + /// `Scalar` is used to represent the + /// `epoll_data` type union. + #[allow(dead_code)] + data: Scalar, +} + +impl FileDescriptor for Epoll { + fn name(&self) -> &'static str { + "epoll" + } -pub mod epoll; -pub mod event; -pub mod socketpair; + fn dup(&mut self) -> io::Result> { + // FIXME: this is probably wrong -- check if the `dup`ed descriptor truly uses an + // independent event set. + Ok(Box::new(self.clone())) + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + Ok(Ok(0)) + } +} impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { @@ -35,7 +70,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { throw_unsup_format!("epoll_create1 flags {flags} are not implemented"); } - let fd = this.machine.file_handler.insert_fd(Box::new(Epoll::default())); + let fd = this.machine.fds.insert_fd(Box::new(Epoll::default())); Ok(Scalar::from_i32(fd)) } @@ -79,7 +114,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let data = this.read_scalar(&data)?; let event = EpollEvent { events, data }; - if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { + if let Some(epfd) = this.machine.fds.get_mut(epfd) { let epfd = epfd .downcast_mut::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; @@ -87,10 +122,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { epfd.file_descriptors.insert(fd, event); Ok(Scalar::from_i32(0)) } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) + Ok(Scalar::from_i32(this.fd_not_found()?)) } } else if op == epoll_ctl_del { - if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { + if let Some(epfd) = this.machine.fds.get_mut(epfd) { let epfd = epfd .downcast_mut::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; @@ -98,7 +133,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { epfd.file_descriptors.remove(&fd); Ok(Scalar::from_i32(0)) } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) + Ok(Scalar::from_i32(this.fd_not_found()?)) } } else { let einval = this.eval_libc("EINVAL"); @@ -150,7 +185,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let _maxevents = this.read_scalar(maxevents)?.to_i32()?; let _timeout = this.read_scalar(timeout)?.to_i32()?; - if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { + if let Some(epfd) = this.machine.fds.get_mut(epfd) { let _epfd = epfd .downcast_mut::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; @@ -158,96 +193,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // FIXME return number of events ready when scheme for marking events ready exists throw_unsup_format!("returning ready events from epoll_wait is not yet implemented"); } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) + Ok(Scalar::from_i32(this.fd_not_found()?)) } } - - /// This function creates an `Event` that is used as an event wait/notify mechanism by - /// user-space applications, and by the kernel to notify user-space applications of events. - /// The `Event` contains an `u64` counter maintained by the kernel. The counter is initialized - /// with the value specified in the `initval` argument. - /// - /// A new file descriptor referring to the `Event` is returned. The `read`, `write`, `poll`, - /// `select`, and `close` operations can be performed on the file descriptor. For more - /// information on these operations, see the man page linked below. - /// - /// The `flags` are not currently implemented for eventfd. - /// The `flags` may be bitwise ORed to change the behavior of `eventfd`: - /// `EFD_CLOEXEC` - Set the close-on-exec (`FD_CLOEXEC`) flag on the new file descriptor. - /// `EFD_NONBLOCK` - Set the `O_NONBLOCK` file status flag on the new open file description. - /// `EFD_SEMAPHORE` - miri does not support semaphore-like semantics. - /// - /// - #[expect(clippy::needless_if)] - fn eventfd( - &mut self, - val: &OpTy<'tcx, Provenance>, - flags: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let val = this.read_scalar(val)?.to_u32()?; - let flags = this.read_scalar(flags)?.to_i32()?; - - let efd_cloexec = this.eval_libc_i32("EFD_CLOEXEC"); - let efd_nonblock = this.eval_libc_i32("EFD_NONBLOCK"); - let efd_semaphore = this.eval_libc_i32("EFD_SEMAPHORE"); - - if flags & (efd_cloexec | efd_nonblock | efd_semaphore) == 0 { - throw_unsup_format!("{flags} is unsupported"); - } - // FIXME handle the cloexec and nonblock flags - if flags & efd_cloexec == efd_cloexec {} - if flags & efd_nonblock == efd_nonblock {} - if flags & efd_semaphore == efd_semaphore { - throw_unsup_format!("EFD_SEMAPHORE is unsupported"); - } - - let fh = &mut this.machine.file_handler; - let fd = fh.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); - Ok(Scalar::from_i32(fd)) - } - - /// Currently this function creates new `SocketPair`s without specifying the domain, type, or - /// protocol of the new socket and these are stored in the socket values `sv` argument. - /// - /// This function creates an unnamed pair of connected sockets in the specified domain, of the - /// specified type, and using the optionally specified protocol. - /// - /// The `domain` argument specified a communication domain; this selects the protocol family - /// used for communication. The socket `type` specifies the communication semantics. - /// The `protocol` specifies a particular protocol to use with the socket. Normally there's - /// only a single protocol supported for a particular socket type within a given protocol - /// family, in which case `protocol` can be specified as 0. It is possible that many protocols - /// exist and in that case, a particular protocol must be specified. - /// - /// For more information on the arguments see the socket manpage: - /// - /// - /// - fn socketpair( - &mut self, - domain: &OpTy<'tcx, Provenance>, - type_: &OpTy<'tcx, Provenance>, - protocol: &OpTy<'tcx, Provenance>, - sv: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let _domain = this.read_scalar(domain)?.to_i32()?; - let _type_ = this.read_scalar(type_)?.to_i32()?; - let _protocol = this.read_scalar(protocol)?.to_i32()?; - let sv = this.deref_pointer(sv)?; - - let fh = &mut this.machine.file_handler; - let sv0 = fh.insert_fd(Box::new(SocketPair)); - let sv0 = ScalarInt::try_from_int(sv0, sv.layout.size).unwrap(); - let sv1 = fh.insert_fd(Box::new(SocketPair)); - let sv1 = ScalarInt::try_from_int(sv1, sv.layout.size).unwrap(); - - this.write_scalar(sv0, &sv)?; - this.write_scalar(sv1, &sv.offset(sv.layout.size, sv.layout, this)?)?; - - Ok(Scalar::from_i32(0)) - } } diff --git a/src/shims/unix/linux/eventfd.rs b/src/shims/unix/linux/eventfd.rs new file mode 100644 index 0000000000..0f28b69ac4 --- /dev/null +++ b/src/shims/unix/linux/eventfd.rs @@ -0,0 +1,125 @@ +//! Linux `eventfd` implementation. +//! Currently just a stub. +use std::cell::Cell; +use std::io; + +use rustc_middle::ty::TyCtxt; +use rustc_target::abi::Endian; + +use crate::shims::unix::*; +use crate::*; + +/// A kind of file descriptor created by `eventfd`. +/// The `Event` type isn't currently written to by `eventfd`. +/// The interface is meant to keep track of objects associated +/// with a file descriptor. For more information see the man +/// page below: +/// +/// +#[derive(Debug)] +struct Event { + /// The object contains an unsigned 64-bit integer (uint64_t) counter that is maintained by the + /// kernel. This counter is initialized with the value specified in the argument initval. + val: Cell, +} + +impl FileDescriptor for Event { + fn name(&self) -> &'static str { + "event" + } + + fn dup(&mut self) -> io::Result> { + // FIXME: this is wrong, the new and old FD should refer to the same event object! + Ok(Box::new(Event { val: self.val.clone() })) + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + Ok(Ok(0)) + } + + /// A write call adds the 8-byte integer value supplied in + /// its buffer (in native endianness) to the counter. The maximum value that may be + /// stored in the counter is the largest unsigned 64-bit value + /// minus 1 (i.e., 0xfffffffffffffffe). If the addition would + /// cause the counter's value to exceed the maximum, then the + /// write either blocks until a read is performed on the + /// file descriptor, or fails with the error EAGAIN if the + /// file descriptor has been made nonblocking. + + /// A write fails with the error EINVAL if the size of the + /// supplied buffer is less than 8 bytes, or if an attempt is + /// made to write the value 0xffffffffffffffff. + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + let v1 = self.val.get(); + let bytes: [u8; 8] = bytes.try_into().unwrap(); // FIXME fail gracefully when this has the wrong size + // Convert from target endianness to host endianness. + let num = match tcx.sess.target.endian { + Endian::Little => u64::from_le_bytes(bytes), + Endian::Big => u64::from_be_bytes(bytes), + }; + // FIXME handle blocking when addition results in exceeding the max u64 value + // or fail with EAGAIN if the file descriptor is nonblocking. + let v2 = v1.checked_add(num).unwrap(); + self.val.set(v2); + assert_eq!(8, bytes.len()); + Ok(Ok(8)) + } +} + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// This function creates an `Event` that is used as an event wait/notify mechanism by + /// user-space applications, and by the kernel to notify user-space applications of events. + /// The `Event` contains an `u64` counter maintained by the kernel. The counter is initialized + /// with the value specified in the `initval` argument. + /// + /// A new file descriptor referring to the `Event` is returned. The `read`, `write`, `poll`, + /// `select`, and `close` operations can be performed on the file descriptor. For more + /// information on these operations, see the man page linked below. + /// + /// The `flags` are not currently implemented for eventfd. + /// The `flags` may be bitwise ORed to change the behavior of `eventfd`: + /// `EFD_CLOEXEC` - Set the close-on-exec (`FD_CLOEXEC`) flag on the new file descriptor. + /// `EFD_NONBLOCK` - Set the `O_NONBLOCK` file status flag on the new open file description. + /// `EFD_SEMAPHORE` - miri does not support semaphore-like semantics. + /// + /// + fn eventfd( + &mut self, + val: &OpTy<'tcx, Provenance>, + flags: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let val = this.read_scalar(val)?.to_u32()?; + let flags = this.read_scalar(flags)?.to_i32()?; + + let efd_cloexec = this.eval_libc_i32("EFD_CLOEXEC"); + let efd_nonblock = this.eval_libc_i32("EFD_NONBLOCK"); + let efd_semaphore = this.eval_libc_i32("EFD_SEMAPHORE"); + + if flags & (efd_cloexec | efd_nonblock | efd_semaphore) != flags { + throw_unsup_format!("eventfd: flag {flags:#x} is unsupported"); + } + if flags & efd_cloexec == efd_cloexec { + // cloexec does nothing as we don't support `exec` + } + if flags & efd_nonblock == efd_nonblock { + // FIXME remember the nonblock flag + } + if flags & efd_semaphore == efd_semaphore { + throw_unsup_format!("eventfd: EFD_SEMAPHORE is unsupported"); + } + + let fd = this.machine.fds.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); + Ok(Scalar::from_i32(fd)) + } +} diff --git a/src/shims/unix/linux/fd/epoll.rs b/src/shims/unix/linux/fd/epoll.rs deleted file mode 100644 index a429caaf8f..0000000000 --- a/src/shims/unix/linux/fd/epoll.rs +++ /dev/null @@ -1,45 +0,0 @@ -use crate::*; - -use crate::shims::unix::fs::FileDescriptor; - -use rustc_data_structures::fx::FxHashMap; -use std::io; - -/// An `Epoll` file descriptor connects file handles and epoll events -#[derive(Clone, Debug, Default)] -pub struct Epoll { - /// The file descriptors we are watching, and what we are watching for. - pub file_descriptors: FxHashMap, -} - -/// Epoll Events associate events with data. -/// These fields are currently unused by miri. -/// This matches the `epoll_event` struct defined -/// by the epoll_ctl man page. For more information -/// see the man page: -/// -/// -#[derive(Clone, Debug)] -pub struct EpollEvent { - pub events: u32, - /// `Scalar` is used to represent the - /// `epoll_data` type union. - pub data: Scalar, -} - -impl FileDescriptor for Epoll { - fn name(&self) -> &'static str { - "epoll" - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(self.clone())) - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - Ok(Ok(0)) - } -} diff --git a/src/shims/unix/linux/fd/event.rs b/src/shims/unix/linux/fd/event.rs deleted file mode 100644 index 1f17ffb88c..0000000000 --- a/src/shims/unix/linux/fd/event.rs +++ /dev/null @@ -1,72 +0,0 @@ -use crate::shims::unix::fs::FileDescriptor; - -use rustc_const_eval::interpret::InterpResult; -use rustc_middle::ty::TyCtxt; -use rustc_target::abi::Endian; - -use std::cell::Cell; -use std::io; - -/// A kind of file descriptor created by `eventfd`. -/// The `Event` type isn't currently written to by `eventfd`. -/// The interface is meant to keep track of objects associated -/// with a file descriptor. For more information see the man -/// page below: -/// -/// -#[derive(Debug)] -pub struct Event { - /// The object contains an unsigned 64-bit integer (uint64_t) counter that is maintained by the - /// kernel. This counter is initialized with the value specified in the argument initval. - pub val: Cell, -} - -impl FileDescriptor for Event { - fn name(&self) -> &'static str { - "event" - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(Event { val: self.val.clone() })) - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - Ok(Ok(0)) - } - - /// A write call adds the 8-byte integer value supplied in - /// its buffer (in native endianess) to the counter. The maximum value that may be - /// stored in the counter is the largest unsigned 64-bit value - /// minus 1 (i.e., 0xfffffffffffffffe). If the addition would - /// cause the counter's value to exceed the maximum, then the - /// write either blocks until a read is performed on the - /// file descriptor, or fails with the error EAGAIN if the - /// file descriptor has been made nonblocking. - - /// A write fails with the error EINVAL if the size of the - /// supplied buffer is less than 8 bytes, or if an attempt is - /// made to write the value 0xffffffffffffffff. - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - let v1 = self.val.get(); - let bytes: [u8; 8] = bytes.try_into().unwrap(); // FIXME fail gracefully when this has the wrong size - // Convert from target endianess to host endianess. - let num = match tcx.sess.target.endian { - Endian::Little => u64::from_le_bytes(bytes), - Endian::Big => u64::from_be_bytes(bytes), - }; - // FIXME handle blocking when addition results in exceeding the max u64 value - // or fail with EAGAIN if the file descriptor is nonblocking. - let v2 = v1.checked_add(num).unwrap(); - self.val.set(v2); - assert_eq!(8, bytes.len()); - Ok(Ok(8)) - } -} diff --git a/src/shims/unix/linux/fd/socketpair.rs b/src/shims/unix/linux/fd/socketpair.rs deleted file mode 100644 index 6adae88235..0000000000 --- a/src/shims/unix/linux/fd/socketpair.rs +++ /dev/null @@ -1,28 +0,0 @@ -use crate::*; - -use crate::shims::unix::fs::FileDescriptor; - -use std::io; - -/// Pair of connected sockets. -/// -/// We currently don't allow sending any data through this pair, so this can be just a dummy. -#[derive(Debug)] -pub struct SocketPair; - -impl FileDescriptor for SocketPair { - fn name(&self) -> &'static str { - "socketpair" - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(SocketPair)) - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - Ok(Ok(0)) - } -} diff --git a/src/shims/unix/linux/foreign_items.rs b/src/shims/unix/linux/foreign_items.rs index d13ada0f4c..497e8bee70 100644 --- a/src/shims/unix/linux/foreign_items.rs +++ b/src/shims/unix/linux/foreign_items.rs @@ -3,15 +3,13 @@ use rustc_target::spec::abi::Abi; use crate::machine::SIGRTMAX; use crate::machine::SIGRTMIN; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::linux::fd::EvalContextExt as _; +use shims::unix::linux::epoll::EvalContextExt as _; +use shims::unix::linux::eventfd::EvalContextExt as _; use shims::unix::linux::mem::EvalContextExt as _; use shims::unix::linux::sync::futex; -use shims::unix::mem::EvalContextExt as _; -use shims::unix::sync::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; pub fn is_dyn_sym(name: &str) -> bool { matches!(name, "getrandom") @@ -31,34 +29,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // See `fn emulate_foreign_item_inner` in `shims/foreign_items.rs` for the general pattern. match link_name.as_str() { - // errno - "__errno_location" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let errno_place = this.last_error_place()?; - this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?; - } - // File related shims (but also see "syscall" below for statx) "readdir64" => { let [dirp] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.linux_readdir64(dirp)?; this.write_scalar(result, dest)?; } - "mmap64" => { - let [addr, length, prot, flags, fd, offset] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let offset = this.read_scalar(offset)?.to_i64()?; - let ptr = this.mmap(addr, length, prot, flags, fd, offset.into())?; - this.write_scalar(ptr, dest)?; - } - - // Linux-only "sync_file_range" => { let [fd, offset, nbytes, flags] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.sync_file_range(fd, offset, nbytes, flags)?; this.write_scalar(result, dest)?; } + + // epoll, eventfd "epoll_create1" => { let [flag] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.epoll_create1(flag)?; @@ -82,29 +66,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.eventfd(val, flag)?; this.write_scalar(result, dest)?; } - "mremap" => { - let [old_address, old_size, new_size, flags] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let ptr = this.mremap(old_address, old_size, new_size, flags)?; - this.write_scalar(ptr, dest)?; - } - "socketpair" => { - let [domain, type_, protocol, sv] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - let result = this.socketpair(domain, type_, protocol, sv)?; - this.write_scalar(result, dest)?; - } - "__libc_current_sigrtmin" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - this.write_scalar(Scalar::from_i32(SIGRTMIN), dest)?; - } - "__libc_current_sigrtmax" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - this.write_scalar(Scalar::from_i32(SIGRTMAX), dest)?; - } // Threading "pthread_condattr_setclock" => { @@ -206,6 +167,34 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; getrandom(this, ptr, len, flags, dest)?; } + "mmap64" => { + let [addr, length, prot, flags, fd, offset] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let offset = this.read_scalar(offset)?.to_i64()?; + let ptr = this.mmap(addr, length, prot, flags, fd, offset.into())?; + this.write_scalar(ptr, dest)?; + } + "mremap" => { + let [old_address, old_size, new_size, flags] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let ptr = this.mremap(old_address, old_size, new_size, flags)?; + this.write_scalar(ptr, dest)?; + } + "__errno_location" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let errno_place = this.last_error_place()?; + this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?; + } + "__libc_current_sigrtmin" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + this.write_scalar(Scalar::from_i32(SIGRTMIN), dest)?; + } + "__libc_current_sigrtmax" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + this.write_scalar(Scalar::from_i32(SIGRTMAX), dest)?; + } // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. diff --git a/src/shims/unix/linux/mod.rs b/src/shims/unix/linux/mod.rs index fe18f1a32f..84b604eb9b 100644 --- a/src/shims/unix/linux/mod.rs +++ b/src/shims/unix/linux/mod.rs @@ -1,4 +1,5 @@ -pub mod fd; +pub mod epoll; +pub mod eventfd; pub mod foreign_items; pub mod mem; pub mod sync; diff --git a/src/shims/unix/macos/foreign_items.rs b/src/shims/unix/macos/foreign_items.rs index 3af01eb44d..53a02bf5e0 100644 --- a/src/shims/unix/macos/foreign_items.rs +++ b/src/shims/unix/macos/foreign_items.rs @@ -1,10 +1,9 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; pub fn is_dyn_sym(_name: &str) -> bool { false diff --git a/src/shims/unix/mod.rs b/src/shims/unix/mod.rs index 638473da02..2bc41e1a62 100644 --- a/src/shims/unix/mod.rs +++ b/src/shims/unix/mod.rs @@ -1,7 +1,9 @@ pub mod foreign_items; +mod fd; mod fs; mod mem; +mod socket; mod sync; mod thread; @@ -9,7 +11,15 @@ mod freebsd; mod linux; mod macos; -pub use fs::{DirHandler, FileHandler}; +pub use fd::{FdTable, FileDescriptor}; +pub use fs::DirTable; +// All the unix-specific extension traits +pub use fd::EvalContextExt as _; +pub use fs::EvalContextExt as _; +pub use mem::EvalContextExt as _; +pub use socket::EvalContextExt as _; +pub use sync::EvalContextExt as _; +pub use thread::EvalContextExt as _; // Make up some constants. const UID: u32 = 1000; diff --git a/src/shims/unix/socket.rs b/src/shims/unix/socket.rs new file mode 100644 index 0000000000..84ddd746fb --- /dev/null +++ b/src/shims/unix/socket.rs @@ -0,0 +1,65 @@ +use std::io; + +use crate::shims::unix::*; +use crate::*; + +/// Pair of connected sockets. +/// +/// We currently don't allow sending any data through this pair, so this can be just a dummy. +#[derive(Debug)] +struct SocketPair; + +impl FileDescriptor for SocketPair { + fn name(&self) -> &'static str { + "socketpair" + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(SocketPair)) + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + Ok(Ok(0)) + } +} + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// Currently this function this function is a stub. Eventually we need to + /// properly implement an FD type for sockets and have this function create + /// two sockets and associated FDs such that writing to one will produce + /// data that can be read from the other. + /// + /// For more information on the arguments see the socketpair manpage: + /// + fn socketpair( + &mut self, + domain: &OpTy<'tcx, Provenance>, + type_: &OpTy<'tcx, Provenance>, + protocol: &OpTy<'tcx, Provenance>, + sv: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let _domain = this.read_scalar(domain)?.to_i32()?; + let _type_ = this.read_scalar(type_)?.to_i32()?; + let _protocol = this.read_scalar(protocol)?.to_i32()?; + let sv = this.deref_pointer(sv)?; + + // FIXME: fail on unsupported inputs + + let fds = &mut this.machine.fds; + let sv0 = fds.insert_fd(Box::new(SocketPair)); + let sv0 = Scalar::try_from_int(sv0, sv.layout.size).unwrap(); + let sv1 = fds.insert_fd(Box::new(SocketPair)); + let sv1 = Scalar::try_from_int(sv1, sv.layout.size).unwrap(); + + this.write_scalar(sv0, &sv)?; + this.write_scalar(sv1, &sv.offset(sv.layout.size, sv.layout, this)?)?; + + Ok(Scalar::from_i32(0)) + } +} diff --git a/src/shims/x86/mod.rs b/src/shims/x86/mod.rs index 7cd397625d..7b7921219e 100644 --- a/src/shims/x86/mod.rs +++ b/src/shims/x86/mod.rs @@ -88,6 +88,19 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: this.write_immediate(*sub, &this.project_field(dest, 1)?)?; } + // Used to implement the `_mm_pause` function. + // The intrinsic is used to hint the processor that the code is in a spin-loop. + // It is compiled down to a `pause` instruction. When SSE2 is not available, + // the instruction behaves like a no-op, so it is always safe to call the + // intrinsic. + "sse2.pause" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + // Only exhibit the spin-loop hint behavior when SSE2 is enabled. + if this.tcx.sess.unstable_target_features.contains(&Symbol::intern("sse2")) { + this.yield_active_thread(); + } + } + name if name.starts_with("sse.") => { return sse::EvalContextExt::emulate_x86_sse_intrinsic( this, link_name, abi, args, dest, diff --git a/src/shims/x86/sse2.rs b/src/shims/x86/sse2.rs index 18ff5d809e..eb2cc9d37c 100644 --- a/src/shims/x86/sse2.rs +++ b/src/shims/x86/sse2.rs @@ -580,12 +580,6 @@ pub(super) trait EvalContextExt<'mir, 'tcx: 'mir>: this.copy_op(&this.project_index(&left, i)?, &this.project_index(&dest, i)?)?; } } - // Used to implement the `_mm_pause` function. - // The intrinsic is used to hint the processor that the code is in a spin-loop. - "pause" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - this.yield_active_thread(); - } _ => return Ok(EmulateForeignItemResult::NotSupported), } Ok(EmulateForeignItemResult::NeedsJumping) diff --git a/test-cargo-miri/Cargo.lock b/test-cargo-miri/Cargo.lock index d5e57a66a8..4783f79ea8 100644 --- a/test-cargo-miri/Cargo.lock +++ b/test-cargo-miri/Cargo.lock @@ -4,9 +4,9 @@ version = 3 [[package]] name = "autocfg" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +checksum = "f1fdabc7756949593fe60f30ec81974b613357de856987752631dea1e3394c80" [[package]] name = "byteorder" @@ -16,9 +16,9 @@ checksum = "0fc10e8cc6b2580fda3f36eb6dc5316657f812a3df879a44a66fc9f0fdbc4855" [[package]] name = "byteorder" -version = "1.4.3" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "cargo-miri-test" @@ -26,22 +26,23 @@ version = "0.1.0" dependencies = [ "autocfg", "byteorder 0.5.3", - "byteorder 1.4.3", + "byteorder 1.5.0", "cdylib", "exported_symbol", + "eyre", "issue_1567", "issue_1691", "issue_1705", - "issue_1760", "issue_rust_86261", - "serde_derive", + "proc-macro2", + "proc_macro_crate", ] [[package]] name = "cdylib" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", + "byteorder 1.5.0", ] [[package]] @@ -55,11 +56,27 @@ dependencies = [ name = "exported_symbol_dep" version = "0.1.0" +[[package]] +name = "eyre" +version = "0.6.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cd915d99f24784cdc19fd37ef22b97e3ff0ae756c7e492e9fbfe897d61e2aec" +dependencies = [ + "indenter", + "once_cell", +] + +[[package]] +name = "indenter" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" + [[package]] name = "issue_1567" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", + "byteorder 1.5.0", ] [[package]] @@ -70,66 +87,44 @@ version = "0.1.0" name = "issue_1705" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", + "byteorder 1.5.0", ] -[[package]] -name = "issue_1760" -version = "0.1.0" - [[package]] name = "issue_rust_86261" version = "0.1.0" [[package]] -name = "proc-macro2" -version = "1.0.66" +name = "once_cell" +version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" -dependencies = [ - "unicode-ident", -] +checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" [[package]] -name = "quote" -version = "1.0.33" +name = "proc-macro2" +version = "1.0.79" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" +checksum = "e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e" dependencies = [ - "proc-macro2", + "unicode-ident", ] [[package]] -name = "serde_derive" -version = "1.0.185" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc59dfdcbad1437773485e0367fea4b090a2e0a16d9ffc46af47764536a298ec" +name = "proc_macro_crate" +version = "0.1.0" dependencies = [ "proc-macro2", - "quote", - "syn", ] [[package]] name = "subcrate" version = "0.1.0" dependencies = [ - "byteorder 1.4.3", -] - -[[package]] -name = "syn" -version = "2.0.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", + "byteorder 1.5.0", ] [[package]] name = "unicode-ident" -version = "1.0.6" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "84a22b9f218b40614adcb3f4ff08b703773ad44fa9423e4e0d346d5db86e4ebc" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" diff --git a/test-cargo-miri/Cargo.toml b/test-cargo-miri/Cargo.toml index 1688096fd9..bfef388669 100644 --- a/test-cargo-miri/Cargo.toml +++ b/test-cargo-miri/Cargo.toml @@ -12,16 +12,21 @@ edition = "2018" byteorder = "1.0" cdylib = { path = "cdylib" } exported_symbol = { path = "exported-symbol" } +proc_macro_crate = { path = "proc-macro-crate" } issue_1567 = { path = "issue-1567" } issue_1691 = { path = "issue-1691" } issue_1705 = { path = "issue-1705" } -issue_1760 = { path = "issue-1760" } issue_rust_86261 = { path = "issue-rust-86261" } [dev-dependencies] byteorder_2 = { package = "byteorder", version = "0.5" } # to test dev-dependencies behave as expected, with renaming -# Not actually used, but exercises some unique code path (`--extern` .so file). -serde_derive = "1.0.185" +## More dependencies that we don't actually use, but add just for extra test coverage. +# These use custom build probes, let's make sure they don't explode. +# (Ideally we'd check if the probe was successful, but that's not easily possible.) +# proc-macro2 is extra exciting because it is both a host-dependency (of proc_macro_crate above) +# and a target-dependency. +proc-macro2 = "1.0" +eyre = "0.6" [build-dependencies] autocfg = "1" diff --git a/test-cargo-miri/issue-1760/Cargo.toml b/test-cargo-miri/issue-1760/Cargo.toml deleted file mode 100644 index 80925c7474..0000000000 --- a/test-cargo-miri/issue-1760/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "issue_1760" -version = "0.1.0" -authors = ["Miri Team"] -edition = "2018" - -[lib] -proc-macro = true diff --git a/test-cargo-miri/proc-macro-crate/Cargo.toml b/test-cargo-miri/proc-macro-crate/Cargo.toml new file mode 100644 index 0000000000..89652f9b04 --- /dev/null +++ b/test-cargo-miri/proc-macro-crate/Cargo.toml @@ -0,0 +1,13 @@ +[package] +# regression test for issue 1760 +name = "proc_macro_crate" +version = "0.1.0" +authors = ["Miri Team"] +edition = "2018" + +[lib] +proc-macro = true + +[dependencies] +# A common dependency of proc macros, let's make sure that works. +proc-macro2 = "1.0" diff --git a/test-cargo-miri/issue-1760/build.rs b/test-cargo-miri/proc-macro-crate/build.rs similarity index 100% rename from test-cargo-miri/issue-1760/build.rs rename to test-cargo-miri/proc-macro-crate/build.rs diff --git a/test-cargo-miri/issue-1760/src/lib.rs b/test-cargo-miri/proc-macro-crate/src/lib.rs similarity index 100% rename from test-cargo-miri/issue-1760/src/lib.rs rename to test-cargo-miri/proc-macro-crate/src/lib.rs diff --git a/test-cargo-miri/run-test.py b/test-cargo-miri/run-test.py index 21479bc4d5..cac11dff77 100755 --- a/test-cargo-miri/run-test.py +++ b/test-cargo-miri/run-test.py @@ -31,11 +31,11 @@ def cargo_miri(cmd, quiet = True): def normalize_stdout(str): str = str.replace("src\\", "src/") # normalize paths across platforms - str = re.sub("finished in \d+\.\d\ds", "finished in $TIME", str) # the time keeps changing, obviously + str = re.sub("finished in \\d+\\.\\d\\ds", "finished in $TIME", str) # the time keeps changing, obviously return str def normalize_stderr(str): - str = re.sub("Preparing a sysroot for Miri \(target: [a-z0-9_-]+\)\.\.\. done\n", "", str) # remove leading cargo-miri setup output + str = re.sub("Preparing a sysroot for Miri \\(target: [a-z0-9_-]+\\)\\.\\.\\. done\n", "", str) # remove leading cargo-miri setup output return str def check_output(actual, path, name): diff --git a/test-cargo-miri/src/lib.rs b/test-cargo-miri/src/lib.rs index 66c8aa2eac..003341d097 100644 --- a/test-cargo-miri/src/lib.rs +++ b/test-cargo-miri/src/lib.rs @@ -1,18 +1,36 @@ /// Doc-test test +/// /// ```rust /// assert!(cargo_miri_test::make_true()); /// ``` +/// +/// `no_run` test: +/// /// ```rust,no_run /// assert!(!cargo_miri_test::make_true()); /// ``` +/// +/// `compile_fail` test: +/// /// ```rust,compile_fail /// assert!(cargo_miri_test::make_true() == 5); /// ``` +/// +/// Post-monomorphization error in `compile_fail` test: +/// +/// ```rust,compile_fail +/// struct Fail(T); +/// impl Fail { +/// const C: () = panic!(); +/// } +/// +/// let _val = Fail::::C; +/// ``` #[no_mangle] pub fn make_true() -> bool { + proc_macro_crate::use_the_dependency!(); issue_1567::use_the_dependency(); issue_1705::use_the_dependency(); - issue_1760::use_the_dependency!(); issue_1691::use_me() } diff --git a/test-cargo-miri/test.default.stdout.ref b/test-cargo-miri/test.default.stdout.ref index 9a17f3d61b..922d2120be 100644 --- a/test-cargo-miri/test.default.stdout.ref +++ b/test-cargo-miri/test.default.stdout.ref @@ -10,7 +10,7 @@ running 6 tests test result: ok. 5 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out -running 4 tests -.... -test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME +running 5 tests +..... +test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME diff --git a/test-cargo-miri/test.filter.stdout.ref b/test-cargo-miri/test.filter.stdout.ref index c618956656..5c819dd532 100644 --- a/test-cargo-miri/test.filter.stdout.ref +++ b/test-cargo-miri/test.filter.stdout.ref @@ -13,5 +13,5 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out running 0 tests -test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out; finished in $TIME +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out; finished in $TIME diff --git a/tests/extern-so/libcode.version b/tests/extern-so/libcode.version deleted file mode 100644 index 0f04b9aaeb..0000000000 --- a/tests/extern-so/libcode.version +++ /dev/null @@ -1,9 +0,0 @@ -CODEABI_1.0 { - global: *add_one_int*; - *printer*; - *test_stack_spill*; - *get_unsigned_int*; - *add_int16*; - *add_short_to_long*; - local: *; -}; diff --git a/tests/extern-so/libtest.map b/tests/extern-so/libtest.map new file mode 100644 index 0000000000..a57a4dc149 --- /dev/null +++ b/tests/extern-so/libtest.map @@ -0,0 +1,12 @@ +CODEABI_1.0 { + # Define which symbols to export. + global: + add_one_int; + printer; + test_stack_spill; + get_unsigned_int; + add_int16; + add_short_to_long; + # The rest remains private. + local: *; +}; diff --git a/tests/fail/both_borrows/retag_data_race_write.rs b/tests/fail/both_borrows/retag_data_race_write.rs index eb1fe56df0..3edaf10f3d 100644 --- a/tests/fail/both_borrows/retag_data_race_write.rs +++ b/tests/fail/both_borrows/retag_data_race_write.rs @@ -17,7 +17,7 @@ fn thread_1(p: SendPtr) { fn thread_2(p: SendPtr) { let p = p.0; unsafe { - *p = 5; //~ ERROR: /Data race detected between \(1\) non-atomic (read|write) on thread `unnamed-[0-9]+` and \(2\) non-atomic write on thread `unnamed-[0-9]+`/ + *p = 5; //~ ERROR: /Data race detected between \(1\) retag (read|write) on thread `unnamed-[0-9]+` and \(2\) non-atomic write on thread `unnamed-[0-9]+`/ } } diff --git a/tests/fail/both_borrows/retag_data_race_write.stack.stderr b/tests/fail/both_borrows/retag_data_race_write.stack.stderr index c5b65e6f74..6f4b52fb88 100644 --- a/tests/fail/both_borrows/retag_data_race_write.stack.stderr +++ b/tests/fail/both_borrows/retag_data_race_write.stack.stderr @@ -1,14 +1,17 @@ -error: Undefined Behavior: Data race detected between (1) non-atomic write on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here +error: Undefined Behavior: Data race detected between (1) retag write on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here --> $DIR/retag_data_race_write.rs:LL:CC | LL | *p = 5; - | ^^^^^^ Data race detected between (1) non-atomic write on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here + | ^^^^^^ Data race detected between (1) retag write on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here | help: and (1) occurred earlier here --> $DIR/retag_data_race_write.rs:LL:CC | LL | let _r = &mut *p; | ^^^^^^^ + = help: retags occur on all (re)borrows and as well as when references are copied or moved + = help: retags permit optimizations that insert speculative reads or writes + = help: therefore from the perspective of data races, a retag has the same implications as a read or write = 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 (of the first span) on thread `unnamed-ID`: diff --git a/tests/fail/both_borrows/retag_data_race_write.tree.stderr b/tests/fail/both_borrows/retag_data_race_write.tree.stderr index 62f139f6f0..fa0012f9b2 100644 --- a/tests/fail/both_borrows/retag_data_race_write.tree.stderr +++ b/tests/fail/both_borrows/retag_data_race_write.tree.stderr @@ -1,14 +1,17 @@ -error: Undefined Behavior: Data race detected between (1) non-atomic read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here +error: Undefined Behavior: Data race detected between (1) retag read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here --> $DIR/retag_data_race_write.rs:LL:CC | LL | *p = 5; - | ^^^^^^ Data race detected between (1) non-atomic read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here + | ^^^^^^ Data race detected between (1) retag read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here | help: and (1) occurred earlier here --> $DIR/retag_data_race_write.rs:LL:CC | LL | let _r = &mut *p; | ^^^^^^^ + = help: retags occur on all (re)borrows and as well as when references are copied or moved + = help: retags permit optimizations that insert speculative reads or writes + = help: therefore from the perspective of data races, a retag has the same implications as a read or write = 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 (of the first span) on thread `unnamed-ID`: diff --git a/tests/fail/extern_static.stderr b/tests/fail/extern_static.stderr index c3de4dadb0..34bb273f04 100644 --- a/tests/fail/extern_static.stderr +++ b/tests/fail/extern_static.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `FOO` from crate `extern_static` is not supported by Miri +error: unsupported operation: extern static `FOO` is not supported by Miri --> $DIR/extern_static.rs:LL:CC | LL | let _val = unsafe { std::ptr::addr_of!(FOO) }; - | ^^^ `extern` static `FOO` from crate `extern_static` is not supported by Miri + | ^^^ extern static `FOO` is not supported by Miri | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/tests/fail/extern_static_in_const.stderr b/tests/fail/extern_static_in_const.stderr index 23f775f258..45d1632cce 100644 --- a/tests/fail/extern_static_in_const.stderr +++ b/tests/fail/extern_static_in_const.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `E` from crate `extern_static_in_const` is not supported by Miri +error: unsupported operation: extern static `E` is not supported by Miri --> $DIR/extern_static_in_const.rs:LL:CC | LL | let _val = X; - | ^ `extern` static `E` from crate `extern_static_in_const` is not supported by Miri + | ^ extern static `E` is not supported by Miri | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/tests/fail/extern_static_wrong_size.rs b/tests/fail/extern_static_wrong_size.rs index 17061f0e5c..fee3c38c25 100644 --- a/tests/fail/extern_static_wrong_size.rs +++ b/tests/fail/extern_static_wrong_size.rs @@ -6,5 +6,5 @@ extern "C" { } fn main() { - let _val = unsafe { environ }; //~ ERROR: /has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of [48] bytes and alignment of [48] bytes/ + let _val = unsafe { environ }; //~ ERROR: /with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of [48] bytes and alignment of [48] bytes/ } diff --git a/tests/fail/extern_static_wrong_size.stderr b/tests/fail/extern_static_wrong_size.stderr index c935a548f8..27699d780c 100644 --- a/tests/fail/extern_static_wrong_size.stderr +++ b/tests/fail/extern_static_wrong_size.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `environ` from crate `extern_static_wrong_size` has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes +error: unsupported operation: extern static `environ` has been declared as `extern_static_wrong_size::environ` with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes --> $DIR/extern_static_wrong_size.rs:LL:CC | LL | let _val = unsafe { environ }; - | ^^^^^^^ `extern` static `environ` from crate `extern_static_wrong_size` has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes + | ^^^^^^^ extern static `environ` has been declared as `extern_static_wrong_size::environ` with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/tests/fail/intrinsics/typed-swap-invalid-array.rs b/tests/fail/intrinsics/typed-swap-invalid-array.rs new file mode 100644 index 0000000000..89fdd2a01e --- /dev/null +++ b/tests/fail/intrinsics/typed-swap-invalid-array.rs @@ -0,0 +1,19 @@ +#![feature(core_intrinsics)] +#![feature(rustc_attrs)] + +use std::intrinsics::typed_swap; +use std::ptr::addr_of_mut; + +fn invalid_array() { + let mut a = [1_u8; 100]; + let mut b = [2_u8; 100]; + unsafe { + let a = addr_of_mut!(a).cast::<[bool; 100]>(); + let b = addr_of_mut!(b).cast::<[bool; 100]>(); + typed_swap(a, b); //~ERROR: constructing invalid value + } +} + +fn main() { + invalid_array(); +} diff --git a/tests/fail/intrinsics/typed-swap-invalid-array.stderr b/tests/fail/intrinsics/typed-swap-invalid-array.stderr new file mode 100644 index 0000000000..15f01c1c09 --- /dev/null +++ b/tests/fail/intrinsics/typed-swap-invalid-array.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: constructing invalid value at [0]: encountered 0x02, but expected a boolean + --> $DIR/typed-swap-invalid-array.rs:LL:CC + | +LL | typed_swap(a, b); + | ^^^^^^^^^^^^^^^^ constructing invalid value at [0]: encountered 0x02, but expected a boolean + | + = 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 `invalid_array` at $DIR/typed-swap-invalid-array.rs:LL:CC +note: inside `main` + --> $DIR/typed-swap-invalid-array.rs:LL:CC + | +LL | invalid_array(); + | ^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/tests/fail/intrinsics/typed-swap-invalid-scalar.rs b/tests/fail/intrinsics/typed-swap-invalid-scalar.rs new file mode 100644 index 0000000000..9d014a523f --- /dev/null +++ b/tests/fail/intrinsics/typed-swap-invalid-scalar.rs @@ -0,0 +1,19 @@ +#![feature(core_intrinsics)] +#![feature(rustc_attrs)] + +use std::intrinsics::typed_swap; +use std::ptr::addr_of_mut; + +fn invalid_scalar() { + let mut a = 1_u8; + let mut b = 2_u8; + unsafe { + let a = addr_of_mut!(a).cast::(); + let b = addr_of_mut!(b).cast::(); + typed_swap(a, b); //~ERROR: constructing invalid value + } +} + +fn main() { + invalid_scalar(); +} diff --git a/tests/fail/intrinsics/typed-swap-invalid-scalar.stderr b/tests/fail/intrinsics/typed-swap-invalid-scalar.stderr new file mode 100644 index 0000000000..262ca202f9 --- /dev/null +++ b/tests/fail/intrinsics/typed-swap-invalid-scalar.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: constructing invalid value: encountered 0x02, but expected a boolean + --> $DIR/typed-swap-invalid-scalar.rs:LL:CC + | +LL | typed_swap(a, b); + | ^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x02, but expected a boolean + | + = 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 `invalid_scalar` at $DIR/typed-swap-invalid-scalar.rs:LL:CC +note: inside `main` + --> $DIR/typed-swap-invalid-scalar.rs:LL:CC + | +LL | invalid_scalar(); + | ^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/tests/fail/intrinsics/unchecked_add1.rs b/tests/fail/intrinsics/unchecked_add1.rs index 13265d0fb0..3f8b4e5515 100644 --- a/tests/fail/intrinsics/unchecked_add1.rs +++ b/tests/fail/intrinsics/unchecked_add1.rs @@ -1,5 +1,3 @@ -#![feature(unchecked_math)] - fn main() { // MAX overflow let _val = unsafe { 40000u16.unchecked_add(30000) }; //~ ERROR: overflow executing `unchecked_add` diff --git a/tests/fail/intrinsics/unchecked_add2.rs b/tests/fail/intrinsics/unchecked_add2.rs index 229f50321d..3283dbf8e7 100644 --- a/tests/fail/intrinsics/unchecked_add2.rs +++ b/tests/fail/intrinsics/unchecked_add2.rs @@ -1,5 +1,3 @@ -#![feature(unchecked_math)] - fn main() { // MIN overflow let _val = unsafe { (-30000i16).unchecked_add(-8000) }; //~ ERROR: overflow executing `unchecked_add` diff --git a/tests/fail/intrinsics/unchecked_mul1.rs b/tests/fail/intrinsics/unchecked_mul1.rs index 810d3418dc..2feed7759e 100644 --- a/tests/fail/intrinsics/unchecked_mul1.rs +++ b/tests/fail/intrinsics/unchecked_mul1.rs @@ -1,4 +1,3 @@ -#![feature(unchecked_math)] fn main() { // MAX overflow let _val = unsafe { 300u16.unchecked_mul(250u16) }; //~ ERROR: overflow executing `unchecked_mul` diff --git a/tests/fail/intrinsics/unchecked_mul2.rs b/tests/fail/intrinsics/unchecked_mul2.rs index 421019542a..42cd509404 100644 --- a/tests/fail/intrinsics/unchecked_mul2.rs +++ b/tests/fail/intrinsics/unchecked_mul2.rs @@ -1,4 +1,3 @@ -#![feature(unchecked_math)] fn main() { // MIN overflow let _val = unsafe { 1_000_000_000i32.unchecked_mul(-4) }; //~ ERROR: overflow executing `unchecked_mul` diff --git a/tests/fail/intrinsics/unchecked_sub1.rs b/tests/fail/intrinsics/unchecked_sub1.rs index c6e0066674..e5178bf4ef 100644 --- a/tests/fail/intrinsics/unchecked_sub1.rs +++ b/tests/fail/intrinsics/unchecked_sub1.rs @@ -1,4 +1,3 @@ -#![feature(unchecked_math)] fn main() { // MIN overflow let _val = unsafe { 14u32.unchecked_sub(22) }; //~ ERROR: overflow executing `unchecked_sub` diff --git a/tests/fail/intrinsics/unchecked_sub2.rs b/tests/fail/intrinsics/unchecked_sub2.rs index 65aa292e21..ac9fd1e5d0 100644 --- a/tests/fail/intrinsics/unchecked_sub2.rs +++ b/tests/fail/intrinsics/unchecked_sub2.rs @@ -1,4 +1,3 @@ -#![feature(unchecked_math)] fn main() { // MAX overflow let _val = unsafe { 30000i16.unchecked_sub(-7000) }; //~ ERROR: overflow executing `unchecked_sub` diff --git a/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr b/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr index a4249d2e88..e5dbe74988 100644 --- a/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr +++ b/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri +error: unsupported operation: extern static `_dispatch_queue_attr_concurrent` is not supported by Miri --> $DIR/issue-miri-3288-ice-symbolic-alignment-extern-static.rs:LL:CC | LL | let _val = *DISPATCH_QUEUE_CONCURRENT; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri + | ^^^^^^^^^^^^^^^^^^^^^^^^^ extern static `_dispatch_queue_attr_concurrent` is not supported by Miri | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/tests/fail/provenance/ptr_int_unexposed.rs b/tests/fail/provenance/ptr_int_unexposed.rs index 20fd330699..f89378fcb3 100644 --- a/tests/fail/provenance/ptr_int_unexposed.rs +++ b/tests/fail/provenance/ptr_int_unexposed.rs @@ -7,6 +7,6 @@ fn main() { let x_usize: usize = x_ptr.addr(); // Cast back an address that did *not* get exposed. - let ptr = std::ptr::from_exposed_addr::(x_usize); + let ptr = std::ptr::with_exposed_provenance::(x_usize); assert_eq!(unsafe { *ptr }, 3); //~ ERROR: is a dangling pointer } diff --git a/tests/fail/provenance/ptr_invalid.rs b/tests/fail/provenance/ptr_invalid.rs index 730859684a..512473cd89 100644 --- a/tests/fail/provenance/ptr_invalid.rs +++ b/tests/fail/provenance/ptr_invalid.rs @@ -4,6 +4,6 @@ fn main() { let x = 42; let xptr = &x as *const i32; - let xptr_invalid = std::ptr::without_provenance::(xptr.expose_addr()); + let xptr_invalid = std::ptr::without_provenance::(xptr.expose_provenance()); let _val = unsafe { *xptr_invalid }; //~ ERROR: is a dangling pointer } diff --git a/tests/fail/provenance/strict_provenance_cast.rs b/tests/fail/provenance/strict_provenance_cast.rs index 106cf4d804..d7b54f640f 100644 --- a/tests/fail/provenance/strict_provenance_cast.rs +++ b/tests/fail/provenance/strict_provenance_cast.rs @@ -3,5 +3,5 @@ fn main() { let addr = &0 as *const i32 as usize; - let _ptr = std::ptr::from_exposed_addr::(addr); //~ ERROR: integer-to-pointer casts and `ptr::from_exposed_addr` are not supported + let _ptr = std::ptr::with_exposed_provenance::(addr); //~ ERROR: integer-to-pointer casts and `ptr::with_exposed_provenance` are not supported } diff --git a/tests/fail/provenance/strict_provenance_cast.stderr b/tests/fail/provenance/strict_provenance_cast.stderr index a110ed4ebb..8c61b66ac4 100644 --- a/tests/fail/provenance/strict_provenance_cast.stderr +++ b/tests/fail/provenance/strict_provenance_cast.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: integer-to-pointer casts and `ptr::from_exposed_addr` are not supported with `-Zmiri-strict-provenance` +error: unsupported operation: integer-to-pointer casts and `ptr::with_exposed_provenance` are not supported with `-Zmiri-strict-provenance` --> $DIR/strict_provenance_cast.rs:LL:CC | -LL | let _ptr = std::ptr::from_exposed_addr::(addr); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer casts and `ptr::from_exposed_addr` are not supported with `-Zmiri-strict-provenance` +LL | let _ptr = std::ptr::with_exposed_provenance::(addr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer casts and `ptr::with_exposed_provenance` are not supported with `-Zmiri-strict-provenance` | = help: use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead = note: BACKTRACE: diff --git a/tests/fail/stacked_borrows/exposed_only_ro.rs b/tests/fail/stacked_borrows/exposed_only_ro.rs index b0e4cceb98..608ab71891 100644 --- a/tests/fail/stacked_borrows/exposed_only_ro.rs +++ b/tests/fail/stacked_borrows/exposed_only_ro.rs @@ -6,7 +6,7 @@ fn main() { let mut x = 0; let _fool = &mut x as *mut i32; // this would have fooled the old untagged pointer logic - let addr = (&x as *const i32).expose_addr(); - let ptr = std::ptr::from_exposed_addr_mut::(addr); + let addr = (&x as *const i32).expose_provenance(); + let ptr = std::ptr::with_exposed_provenance_mut::(addr); unsafe { *ptr = 0 }; //~ ERROR: /write access using .* no exposed tags have suitable permission in the borrow stack/ } diff --git a/tests/fail/stacked_borrows/retag_data_race_protected_read.rs b/tests/fail/stacked_borrows/retag_data_race_protected_read.rs index 5db89c89b7..3de517055e 100644 --- a/tests/fail/stacked_borrows/retag_data_race_protected_read.rs +++ b/tests/fail/stacked_borrows/retag_data_race_protected_read.rs @@ -13,7 +13,7 @@ fn main() { let ptr = ptr; // We do a protected mutable retag (but no write!) in this thread. fn retag(_x: &mut i32) {} - retag(unsafe { &mut *ptr.0 }); //~ERROR: Data race detected between (1) non-atomic read on thread `main` and (2) non-atomic write on thread `unnamed-1` + retag(unsafe { &mut *ptr.0 }); //~ERROR: Data race detected between (1) non-atomic read on thread `main` and (2) retag write of type `i32` on thread `unnamed-1` }); // We do a read in the main thread. diff --git a/tests/fail/stacked_borrows/retag_data_race_protected_read.stderr b/tests/fail/stacked_borrows/retag_data_race_protected_read.stderr index 2ce757013d..47ae4b5d46 100644 --- a/tests/fail/stacked_borrows/retag_data_race_protected_read.stderr +++ b/tests/fail/stacked_borrows/retag_data_race_protected_read.stderr @@ -1,14 +1,17 @@ -error: Undefined Behavior: Data race detected between (1) non-atomic read on thread `main` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here +error: Undefined Behavior: Data race detected between (1) non-atomic read on thread `main` and (2) retag write of type `i32` on thread `unnamed-ID` at ALLOC. (2) just happened here --> $DIR/retag_data_race_protected_read.rs:LL:CC | LL | retag(unsafe { &mut *ptr.0 }); - | ^^^^^^^^^^^ Data race detected between (1) non-atomic read on thread `main` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here + | ^^^^^^^^^^^ Data race detected between (1) non-atomic read on thread `main` and (2) retag write of type `i32` on thread `unnamed-ID` at ALLOC. (2) just happened here | help: and (1) occurred earlier here --> $DIR/retag_data_race_protected_read.rs:LL:CC | LL | unsafe { ptr.0.read() }; | ^^^^^^^^^^^^ + = help: retags occur on all (re)borrows and as well as when references are copied or moved + = help: retags permit optimizations that insert speculative reads or writes + = help: therefore from the perspective of data races, a retag has the same implications as a read or write = 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 (of the first span) on thread `unnamed-ID`: diff --git a/tests/fail/stacked_borrows/retag_data_race_read.rs b/tests/fail/stacked_borrows/retag_data_race_read.rs index 01a2e9ac47..25c92ddf6c 100644 --- a/tests/fail/stacked_borrows/retag_data_race_read.rs +++ b/tests/fail/stacked_borrows/retag_data_race_read.rs @@ -15,7 +15,7 @@ fn thread_1(p: SendPtr) { fn thread_2(p: SendPtr) { let p = p.0; unsafe { - *p = 5; //~ ERROR: Data race detected between (1) non-atomic read on thread `unnamed-1` and (2) non-atomic write on thread `unnamed-2` + *p = 5; //~ ERROR: Data race detected between (1) retag read on thread `unnamed-1` and (2) non-atomic write on thread `unnamed-2` } } diff --git a/tests/fail/stacked_borrows/retag_data_race_read.stderr b/tests/fail/stacked_borrows/retag_data_race_read.stderr index d3c8d14e2a..9fe9fbeda4 100644 --- a/tests/fail/stacked_borrows/retag_data_race_read.stderr +++ b/tests/fail/stacked_borrows/retag_data_race_read.stderr @@ -1,14 +1,17 @@ -error: Undefined Behavior: Data race detected between (1) non-atomic read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here +error: Undefined Behavior: Data race detected between (1) retag read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here --> $DIR/retag_data_race_read.rs:LL:CC | LL | *p = 5; - | ^^^^^^ Data race detected between (1) non-atomic read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here + | ^^^^^^ Data race detected between (1) retag read on thread `unnamed-ID` and (2) non-atomic write on thread `unnamed-ID` at ALLOC. (2) just happened here | help: and (1) occurred earlier here --> $DIR/retag_data_race_read.rs:LL:CC | LL | let _r = &*p; | ^^^ + = help: retags occur on all (re)borrows and as well as when references are copied or moved + = help: retags permit optimizations that insert speculative reads or writes + = help: therefore from the perspective of data races, a retag has the same implications as a read or write = 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 (of the first span) on thread `unnamed-ID`: diff --git a/tests/fail/tree_borrows/spurious_read.rs b/tests/fail/tree_borrows/spurious_read.rs index 3f39dcb4b7..50ef0ceef9 100644 --- a/tests/fail/tree_borrows/spurious_read.rs +++ b/tests/fail/tree_borrows/spurious_read.rs @@ -89,7 +89,7 @@ fn retagx_retagy_retx_writey_rety() { // - retag `y` protected // - (wait for the other thread to return so that there is no foreign protector when we write) // - attempt a write through `y`. - // - (UB should have occured by now, but the next step would be to + // - (UB should have occurred by now, but the next step would be to // remove `y`'s protector) let thread_y = thread::spawn(move || { let b = (2, by); diff --git a/tests/pass-dep/shims/libc-fs.rs b/tests/pass-dep/shims/libc-fs.rs index fafeb9e055..4cfc1843a7 100644 --- a/tests/pass-dep/shims/libc-fs.rs +++ b/tests/pass-dep/shims/libc-fs.rs @@ -16,6 +16,10 @@ mod utils; fn main() { test_dup_stdout_stderr(); test_canonicalize_too_long(); + test_rename(); + test_ftruncate::(libc::ftruncate); + #[cfg(target_os = "linux")] + test_ftruncate::(libc::ftruncate64); test_readlink(); test_file_open_unix_allow_two_args(); test_file_open_unix_needs_three_args(); @@ -133,6 +137,65 @@ fn test_readlink() { assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); } +fn test_rename() { + let path1 = prepare("miri_test_libc_fs_source.txt"); + let path2 = prepare("miri_test_libc_fs_rename_destination.txt"); + + let file = File::create(&path1).unwrap(); + drop(file); + + let c_path1 = CString::new(path1.as_os_str().as_bytes()).expect("CString::new failed"); + let c_path2 = CString::new(path2.as_os_str().as_bytes()).expect("CString::new failed"); + + // Renaming should succeed + unsafe { libc::rename(c_path1.as_ptr(), c_path2.as_ptr()) }; + // Check that old file path isn't present + assert_eq!(ErrorKind::NotFound, path1.metadata().unwrap_err().kind()); + // Check that the file has moved successfully + assert!(path2.metadata().unwrap().is_file()); + + // Renaming a nonexistent file should fail + let res = unsafe { libc::rename(c_path1.as_ptr(), c_path2.as_ptr()) }; + assert_eq!(res, -1); + assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound); + + remove_file(&path2).unwrap(); +} + +fn test_ftruncate>( + ftruncate: unsafe extern "C" fn(fd: libc::c_int, length: T) -> libc::c_int, +) { + // libc::off_t is i32 in target i686-unknown-linux-gnu + // https://docs.rs/libc/latest/i686-unknown-linux-gnu/libc/type.off_t.html + + let bytes = b"hello"; + let path = prepare("miri_test_libc_fs_ftruncate.txt"); + let mut file = File::create(&path).unwrap(); + file.write(bytes).unwrap(); + file.sync_all().unwrap(); + assert_eq!(file.metadata().unwrap().len(), 5); + + let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed"); + let fd = unsafe { libc::open(c_path.as_ptr(), libc::O_RDWR) }; + + // Truncate to a bigger size + let mut res = unsafe { ftruncate(fd, T::from(10)) }; + assert_eq!(res, 0); + assert_eq!(file.metadata().unwrap().len(), 10); + + // Write after truncate + file.write(b"dup").unwrap(); + file.sync_all().unwrap(); + assert_eq!(file.metadata().unwrap().len(), 10); + + // Truncate to smaller size + res = unsafe { ftruncate(fd, T::from(2)) }; + assert_eq!(res, 0); + assert_eq!(file.metadata().unwrap().len(), 2); + + remove_file(&path).unwrap(); +} + #[cfg(target_os = "linux")] fn test_o_tmpfile_flag() { use std::fs::{create_dir, OpenOptions}; diff --git a/tests/pass-dep/shims/posix_memalign.rs b/tests/pass-dep/shims/posix_memalign.rs index 5cf62995fb..db66b21341 100644 --- a/tests/pass-dep/shims/posix_memalign.rs +++ b/tests/pass-dep/shims/posix_memalign.rs @@ -1,6 +1,6 @@ //@ignore-target-windows: No libc on Windows -#![feature(pointer_is_aligned)] +#![feature(pointer_is_aligned_to)] #![feature(strict_provenance)] use core::ptr; diff --git a/tests/pass/async-closure-captures.rs b/tests/pass/async-closure-captures.rs new file mode 100644 index 0000000000..cac26bfe14 --- /dev/null +++ b/tests/pass/async-closure-captures.rs @@ -0,0 +1,125 @@ +// Same as rustc's `tests/ui/async-await/async-closures/captures.rs`, keep in sync + +#![feature(async_closure, noop_waker)] + +use std::future::Future; +use std::pin::pin; +use std::task::*; + +pub fn block_on(fut: impl Future) -> T { + let mut fut = pin!(fut); + let ctx = &mut Context::from_waker(Waker::noop()); + + loop { + match fut.as_mut().poll(ctx) { + Poll::Pending => {} + Poll::Ready(t) => break t, + } + } +} + +fn main() { + block_on(async_main()); +} + +async fn call(f: &impl async Fn() -> T) -> T { + f().await +} + +async fn call_once(f: impl async FnOnce() -> T) -> T { + f().await +} + +#[derive(Debug)] +#[allow(unused)] +struct Hello(i32); + +async fn async_main() { + // Capture something by-ref + { + let x = Hello(0); + let c = async || { + println!("{x:?}"); + }; + call(&c).await; + call_once(c).await; + + let x = &Hello(1); + let c = async || { + println!("{x:?}"); + }; + call(&c).await; + call_once(c).await; + } + + // Capture something and consume it (force to `AsyncFnOnce`) + { + let x = Hello(2); + let c = async || { + println!("{x:?}"); + drop(x); + }; + call_once(c).await; + } + + // Capture something with `move`, don't consume it + { + let x = Hello(3); + let c = async move || { + println!("{x:?}"); + }; + call(&c).await; + call_once(c).await; + + let x = &Hello(4); + let c = async move || { + println!("{x:?}"); + }; + call(&c).await; + call_once(c).await; + } + + // Capture something with `move`, also consume it (so `AsyncFnOnce`) + { + let x = Hello(5); + let c = async move || { + println!("{x:?}"); + drop(x); + }; + call_once(c).await; + } + + fn force_fnonce(f: impl async FnOnce() -> T) -> impl async FnOnce() -> T { + f + } + + // Capture something with `move`, but infer to `AsyncFnOnce` + { + let x = Hello(6); + let c = force_fnonce(async move || { + println!("{x:?}"); + }); + call_once(c).await; + + let x = &Hello(7); + let c = force_fnonce(async move || { + println!("{x:?}"); + }); + call_once(c).await; + } + + // Capture something by-ref, but infer to `AsyncFnOnce` + { + let x = Hello(8); + let c = force_fnonce(async || { + println!("{x:?}"); + }); + call_once(c).await; + + let x = &Hello(9); + let c = force_fnonce(async || { + println!("{x:?}"); + }); + call_once(c).await; + } +} diff --git a/tests/pass/async-closure-captures.stdout b/tests/pass/async-closure-captures.stdout new file mode 100644 index 0000000000..42a7999b2d --- /dev/null +++ b/tests/pass/async-closure-captures.stdout @@ -0,0 +1,14 @@ +Hello(0) +Hello(0) +Hello(1) +Hello(1) +Hello(2) +Hello(3) +Hello(3) +Hello(4) +Hello(4) +Hello(5) +Hello(6) +Hello(7) +Hello(8) +Hello(9) diff --git a/tests/pass/async-closure-drop.rs b/tests/pass/async-closure-drop.rs new file mode 100644 index 0000000000..9b2fc2948b --- /dev/null +++ b/tests/pass/async-closure-drop.rs @@ -0,0 +1,40 @@ +#![feature(async_closure, noop_waker, async_fn_traits)] + +use std::future::Future; +use std::pin::pin; +use std::task::*; + +pub fn block_on(fut: impl Future) -> T { + let mut fut = pin!(fut); + let ctx = &mut Context::from_waker(Waker::noop()); + + loop { + match fut.as_mut().poll(ctx) { + Poll::Pending => {} + Poll::Ready(t) => break t, + } + } +} + +async fn call_once(f: impl async FnOnce(DropMe)) { + f(DropMe("world")).await; +} + +#[derive(Debug)] +struct DropMe(&'static str); + +impl Drop for DropMe { + fn drop(&mut self) { + println!("{}", self.0); + } +} + +pub fn main() { + block_on(async { + let b = DropMe("hello"); + let async_closure = async move |a: DropMe| { + println!("{a:?} {b:?}"); + }; + call_once(async_closure).await; + }); +} diff --git a/tests/pass/async-closure-drop.stdout b/tests/pass/async-closure-drop.stdout new file mode 100644 index 0000000000..34cfdedc44 --- /dev/null +++ b/tests/pass/async-closure-drop.stdout @@ -0,0 +1,3 @@ +DropMe("world") DropMe("hello") +world +hello diff --git a/tests/pass/async-closure.rs b/tests/pass/async-closure.rs index 9b2fc2948b..2f7ec2b9e6 100644 --- a/tests/pass/async-closure.rs +++ b/tests/pass/async-closure.rs @@ -1,6 +1,7 @@ #![feature(async_closure, noop_waker, async_fn_traits)] use std::future::Future; +use std::ops::{AsyncFnMut, AsyncFnOnce}; use std::pin::pin; use std::task::*; @@ -16,25 +17,36 @@ pub fn block_on(fut: impl Future) -> T { } } -async fn call_once(f: impl async FnOnce(DropMe)) { - f(DropMe("world")).await; +async fn call_mut(f: &mut impl AsyncFnMut(i32)) { + f(0).await; } -#[derive(Debug)] -struct DropMe(&'static str); +async fn call_once(f: impl AsyncFnOnce(i32)) { + f(1).await; +} -impl Drop for DropMe { - fn drop(&mut self) { - println!("{}", self.0); - } +async fn call_normal>(f: &impl Fn(i32) -> F) { + f(0).await; +} + +async fn call_normal_once>(f: impl FnOnce(i32) -> F) { + f(1).await; } pub fn main() { block_on(async { - let b = DropMe("hello"); - let async_closure = async move |a: DropMe| { - println!("{a:?} {b:?}"); + let b = 2i32; + let mut async_closure = async move |a: i32| { + println!("{a} {b}"); }; + call_mut(&mut async_closure).await; call_once(async_closure).await; + + // No-capture closures implement `Fn`. + let async_closure = async move |a: i32| { + println!("{a}"); + }; + call_normal(&async_closure).await; + call_normal_once(async_closure).await; }); } diff --git a/tests/pass/async-closure.stdout b/tests/pass/async-closure.stdout index 34cfdedc44..7baae1aa94 100644 --- a/tests/pass/async-closure.stdout +++ b/tests/pass/async-closure.stdout @@ -1,3 +1,4 @@ -DropMe("world") DropMe("hello") -world -hello +0 2 +1 2 +0 +1 diff --git a/tests/pass/box.stack.stderr b/tests/pass/box.stack.stderr index f6e208cea9..1a4d52ee31 100644 --- a/tests/pass/box.stack.stderr +++ b/tests/pass/box.stack.stderr @@ -4,11 +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::from_exposed_addr`, + = 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: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation. + = 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 `from_exposed_addr` semantics. + = 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. = note: BACKTRACE: = note: inside `into_raw` at $DIR/box.rs:LL:CC diff --git a/tests/pass/extern_types.stack.stderr b/tests/pass/extern_types.stack.stderr index 2e18f69305..275d718129 100644 --- a/tests/pass/extern_types.stack.stderr +++ b/tests/pass/extern_types.stack.stderr @@ -4,11 +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::from_exposed_addr`, + = 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: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation. + = 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 `from_exposed_addr` semantics. + = 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. = note: BACKTRACE: = note: inside `main` at $DIR/extern_types.rs:LL:CC diff --git a/tests/pass/intrinsics-x86-pause-without-sse2.rs b/tests/pass/intrinsics-x86-pause-without-sse2.rs new file mode 100644 index 0000000000..c8b92fd545 --- /dev/null +++ b/tests/pass/intrinsics-x86-pause-without-sse2.rs @@ -0,0 +1,25 @@ +// 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=-sse2 + +#[cfg(target_arch = "x86")] +use std::arch::x86::*; +#[cfg(target_arch = "x86_64")] +use std::arch::x86_64::*; + +fn main() { + assert!(!is_x86_feature_detected!("sse2")); + + unsafe { + // This is a SSE2 intrinsic, but it behaves as a no-op when SSE2 + // is not available, so it is always safe to call. + _mm_pause(); + } +} diff --git a/tests/pass/intrinsics-x86-sse2.rs b/tests/pass/intrinsics-x86-sse2.rs index e636d6c8aa..e0088b9eb2 100644 --- a/tests/pass/intrinsics-x86-sse2.rs +++ b/tests/pass/intrinsics-x86-sse2.rs @@ -54,6 +54,11 @@ mod tests { } } + fn test_mm_pause() { + unsafe { _mm_pause() } + } + test_mm_pause(); + #[target_feature(enable = "sse2")] unsafe fn test_mm_avg_epu8() { let (a, b) = (_mm_set1_epi8(3), _mm_set1_epi8(9)); diff --git a/tests/pass/portable-simd-ptrs.rs b/tests/pass/portable-simd-ptrs.rs index 70ba5636c6..096ec78da1 100644 --- a/tests/pass/portable-simd-ptrs.rs +++ b/tests/pass/portable-simd-ptrs.rs @@ -7,6 +7,6 @@ use std::simd::prelude::*; fn main() { // Pointer casts let _val: Simd<*const u8, 4> = Simd::<*const i32, 4>::splat(ptr::null()).cast(); - let addrs = Simd::<*const i32, 4>::splat(ptr::null()).expose_addr(); - let _ptrs = Simd::<*const i32, 4>::from_exposed_addr(addrs); + let addrs = Simd::<*const i32, 4>::splat(ptr::null()).expose_provenance(); + let _ptrs = Simd::<*const i32, 4>::with_exposed_provenance(addrs); } diff --git a/tests/pass/portable-simd.rs b/tests/pass/portable-simd.rs index 399913a757..cdb441b450 100644 --- a/tests/pass/portable-simd.rs +++ b/tests/pass/portable-simd.rs @@ -526,6 +526,23 @@ fn simd_intrinsics() { } } +fn simd_float_intrinsics() { + use intrinsics::*; + + // These are just smoke tests to ensure the intrinsics can be called. + unsafe { + let a = f32x4::splat(10.0); + simd_fsqrt(a); + simd_fsin(a); + simd_fcos(a); + simd_fexp(a); + simd_fexp2(a); + simd_flog(a); + simd_flog2(a); + simd_flog10(a); + } +} + fn simd_masked_loadstore() { // The buffer is deliberarely too short, so reading the last element would be UB. let buf = [3i32; 3]; @@ -559,5 +576,6 @@ fn main() { simd_gather_scatter(); simd_round(); simd_intrinsics(); + simd_float_intrinsics(); simd_masked_loadstore(); } diff --git a/tests/pass/ptr_int_from_exposed.rs b/tests/pass/ptr_int_from_exposed.rs index d8d57679e6..5690d7865b 100644 --- a/tests/pass/ptr_int_from_exposed.rs +++ b/tests/pass/ptr_int_from_exposed.rs @@ -10,9 +10,9 @@ fn ptr_roundtrip_out_of_bounds() { let x: i32 = 3; let x_ptr = &x as *const i32; - let x_usize = x_ptr.wrapping_offset(128).expose_addr(); + let x_usize = x_ptr.wrapping_offset(128).expose_provenance(); - let ptr = ptr::from_exposed_addr::(x_usize).wrapping_offset(-128); + let ptr = ptr::with_exposed_provenance::(x_usize).wrapping_offset(-128); assert_eq!(unsafe { *ptr }, 3); } @@ -24,10 +24,10 @@ fn ptr_roundtrip_confusion() { let x_ptr = &x as *const i32; let y_ptr = &y as *const i32; - let x_usize = x_ptr.expose_addr(); - let y_usize = y_ptr.expose_addr(); + let x_usize = x_ptr.expose_provenance(); + let y_usize = y_ptr.expose_provenance(); - let ptr = ptr::from_exposed_addr::(y_usize); + let ptr = ptr::with_exposed_provenance::(y_usize); let ptr = ptr.with_addr(x_usize); assert_eq!(unsafe { *ptr }, 0); } @@ -37,9 +37,9 @@ fn ptr_roundtrip_imperfect() { let x: u8 = 3; let x_ptr = &x as *const u8; - let x_usize = x_ptr.expose_addr() + 128; + let x_usize = x_ptr.expose_provenance() + 128; - let ptr = ptr::from_exposed_addr::(x_usize).wrapping_offset(-128); + let ptr = ptr::with_exposed_provenance::(x_usize).wrapping_offset(-128); assert_eq!(unsafe { *ptr }, 3); } @@ -48,10 +48,10 @@ fn ptr_roundtrip_null() { let x = &42; let x_ptr = x as *const i32; let x_null_ptr = x_ptr.with_addr(0); // addr 0, but still the provenance of x - let null = x_null_ptr.expose_addr(); + let null = x_null_ptr.expose_provenance(); assert_eq!(null, 0); - let x_null_ptr_copy = ptr::from_exposed_addr::(null); // just a roundtrip, so has provenance of x (angelically) + let x_null_ptr_copy = ptr::with_exposed_provenance::(null); // just a roundtrip, so has provenance of x (angelically) let x_ptr_copy = x_null_ptr_copy.with_addr(x_ptr.addr()); // addr of x and provenance of x assert_eq!(unsafe { *x_ptr_copy }, 42); } diff --git a/tests/pass/stacked-borrows/int-to-ptr.rs b/tests/pass/stacked-borrows/int-to-ptr.rs index e467356dd0..c89d79b42e 100644 --- a/tests/pass/stacked-borrows/int-to-ptr.rs +++ b/tests/pass/stacked-borrows/int-to-ptr.rs @@ -17,7 +17,7 @@ fn example(variant: bool) { unsafe { fn not_so_innocent(x: &mut u32) -> usize { let x_raw4 = x as *mut u32; - x_raw4.expose_addr() + x_raw4.expose_provenance() } let mut c = 42u32; @@ -26,7 +26,7 @@ fn example(variant: bool) { // stack: [..., Unique(1)] let x_raw2 = x_unique1 as *mut u32; - let x_raw2_addr = x_raw2.expose_addr(); + let x_raw2_addr = x_raw2.expose_provenance(); // stack: [..., Unique(1), SharedRW(2)] let x_unique3 = &mut *x_raw2; @@ -39,7 +39,7 @@ fn example(variant: bool) { // 4 is the "obvious" choice (topmost tag, what we used to do with untagged pointers). // And indeed if `variant == true` it is the only possible choice. // But if `variant == false` then 2 is the only possible choice! - let x_wildcard = ptr::from_exposed_addr_mut::(x_raw2_addr); + let x_wildcard = ptr::with_exposed_provenance_mut::(x_raw2_addr); if variant { // If we picked 2, this will invalidate 3. diff --git a/tests/pass/stacked-borrows/issue-miri-2389.stderr b/tests/pass/stacked-borrows/issue-miri-2389.stderr index f3ba052ae5..7cbfad3942 100644 --- a/tests/pass/stacked-borrows/issue-miri-2389.stderr +++ b/tests/pass/stacked-borrows/issue-miri-2389.stderr @@ -4,11 +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::from_exposed_addr`, + = 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: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation. + = 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 `from_exposed_addr` semantics. + = 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. = note: BACKTRACE: = note: inside `main` at $DIR/issue-miri-2389.rs:LL:CC diff --git a/tests/pass/stacked-borrows/unknown-bottom-gc.rs b/tests/pass/stacked-borrows/unknown-bottom-gc.rs index 5bb4e879c3..55356814a1 100644 --- a/tests/pass/stacked-borrows/unknown-bottom-gc.rs +++ b/tests/pass/stacked-borrows/unknown-bottom-gc.rs @@ -9,7 +9,7 @@ fn main() { // Expose the allocation and use the exposed pointer, creating an unknown bottom unsafe { - let p: *mut u8 = ptr::from_exposed_addr::(ptr.expose_addr()) as *mut u8; + let p: *mut u8 = ptr::with_exposed_provenance::(ptr.expose_provenance()) as *mut u8; *p = 1; } diff --git a/tests/ui.rs b/tests/ui.rs index 7f363ccdfe..7e8d140118 100644 --- a/tests/ui.rs +++ b/tests/ui.rs @@ -1,11 +1,15 @@ -use colored::*; -use regex::bytes::Regex; use std::ffi::OsString; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::{env, process::Command}; -use ui_test::{color_eyre::Result, Config, Match, Mode, OutputConflictHandling}; -use ui_test::{status_emitter, CommandBuilder, Format, RustfixMode}; + +use colored::*; +use regex::bytes::Regex; +use ui_test::color_eyre::eyre::{Context, Result}; +use ui_test::{ + status_emitter, CommandBuilder, Config, Format, Match, Mode, OutputConflictHandling, + RustfixMode, +}; fn miri_path() -> PathBuf { PathBuf::from(option_env!("MIRI").unwrap_or(env!("CARGO_BIN_EXE_miri"))) @@ -41,44 +45,26 @@ fn build_so_for_c_ffi_tests() -> PathBuf { // This is to avoid automatically adding `malloc`, etc. // Source: https://anadoxin.org/blog/control-over-symbol-exports-in-gcc.html/ "-fPIC", - "-Wl,--version-script=tests/extern-so/libcode.version", + "-Wl,--version-script=tests/extern-so/libtest.map", ]) .output() .expect("failed to generate shared object file for testing external C function calls"); if !cc_output.status.success() { - panic!("error in generating shared object file for testing external C function calls"); + panic!( + "error in generating shared object file for testing external C function calls:\n{}", + String::from_utf8_lossy(&cc_output.stderr), + ); } so_file_path } -fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> Config { +/// Does *not* set any args or env vars, since it is shared between the test runner and +/// run_dep_mode. +fn miri_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> Config { // Miri is rustc-like, so we create a default builder for rustc and modify it let mut program = CommandBuilder::rustc(); program.program = miri_path(); - // Add some flags we always want. - program.args.push("-Dwarnings".into()); - program.args.push("-Dunused".into()); - program.args.push("-Ainternal_features".into()); - if let Ok(extra_flags) = env::var("MIRIFLAGS") { - for flag in extra_flags.split_whitespace() { - program.args.push(flag.into()); - } - } - program.args.push("-Zui-testing".into()); - program.args.push("--target".into()); - program.args.push(target.into()); - - // If we're on linux, and we're testing the extern-so functionality, - // then build the shared object file for testing external C function calls - // and push the relevant compiler flag. - if cfg!(target_os = "linux") && path.starts_with("tests/extern-so/") { - let so_file_path = build_so_for_c_ffi_tests(); - let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file="); - flag.push(so_file_path.into_os_string()); - program.args.push(flag); - } - let mut config = Config { target: Some(target.to_owned()), stderr_filters: STDERR.clone(), @@ -116,13 +102,37 @@ fn run_tests( with_dependencies: bool, tmpdir: &Path, ) -> Result<()> { - let mut config = test_config(target, path, mode, with_dependencies); + let mut config = miri_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()))); + // If a test ICEs, we want to see a backtrace. + config.program.envs.push(("RUST_BACKTRACE".into(), Some("1".into()))); + + // Add some flags we always want. + config.program.args.push("-Dwarnings".into()); + config.program.args.push("-Dunused".into()); + config.program.args.push("-Ainternal_features".into()); + if let Ok(extra_flags) = env::var("MIRIFLAGS") { + for flag in extra_flags.split_whitespace() { + config.program.args.push(flag.into()); + } + } + config.program.args.push("-Zui-testing".into()); + config.program.args.push("--target".into()); + config.program.args.push(target.into()); + + // If we're testing the extern-so functionality, then build the shared object file for testing + // external C function calls and push the relevant compiler flag. + if path.starts_with("tests/extern-so/") { + assert!(cfg!(target_os = "linux")); + let so_file_path = build_so_for_c_ffi_tests(); + let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file="); + flag.push(so_file_path.into_os_string()); + config.program.args.push(flag); + } // Handle command-line arguments. let args = ui_test::Args::test()?; @@ -223,7 +233,7 @@ fn ui( with_dependencies: Dependencies, tmpdir: &Path, ) -> Result<()> { - let msg = format!("## Running ui tests in {path} against miri for {target}"); + let msg = format!("## Running ui tests in {path} for {target}"); eprintln!("{}", msg.green().bold()); let with_dependencies = match with_dependencies { @@ -231,6 +241,7 @@ fn ui( WithoutDependencies => false, }; run_tests(mode, path, target, with_dependencies, tmpdir) + .with_context(|| format!("ui tests in {path} for {target} failed")) } fn get_target() -> String { @@ -285,13 +296,12 @@ fn main() -> Result<()> { fn run_dep_mode(target: String, mut args: impl Iterator) -> Result<()> { let path = args.next().expect("./miri run-dep must be followed by a file name"); - let mut config = test_config( + let config = miri_config( &target, "", Mode::Yolo { rustfix: RustfixMode::Disabled }, /* with dependencies */ true, ); - config.program.args.clear(); // We want to give the user full control over flags let dep_args = config.build_dependencies()?; let mut cmd = config.program.build(&config.out_dir);