diff --git a/src/tools/miri/.cargo/config.toml b/src/tools/miri/.cargo/config.toml new file mode 100644 index 00000000000..42e7c2c4818 --- /dev/null +++ b/src/tools/miri/.cargo/config.toml @@ -0,0 +1,9 @@ +[unstable] +profile-rustflags = true + +# Add back the containing directory of the packages we have to refer to using --manifest-path. +# Per-package profiles avoid adding this to build dependencies. +[profile.dev.package."cargo-miri"] +rustflags = ["--remap-path-prefix", "=cargo-miri"] +[profile.dev.package."miri-script"] +rustflags = ["--remap-path-prefix", "=miri-script"] diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index fc4e484fa38..22c833a5488 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -60,7 +60,7 @@ jobs: - name: clippy (all features) run: ./miri clippy --all-features -- -D warnings - name: rustdoc - run: RUSTDOCFLAGS="-Dwarnings" ./miri cargo doc --document-private-items + run: RUSTDOCFLAGS="-Dwarnings" ./miri doc --document-private-items # These jobs doesn't actually test anything, but they're only used to tell # bors the build completed, as there is no practical way to detect when a @@ -123,6 +123,8 @@ jobs: run: | git config --global user.name 'The Miri Cronjob Bot' git config --global user.email 'miri@cron.bot' + - name: Install nightly toolchain + run: rustup toolchain install nightly --profile minimal - name: get changes from rustc run: ./miri rustc-pull - name: Install rustup-toolchain-install-master diff --git a/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml index 8f54b5b8d81..bf5749a7b17 100644 --- a/src/tools/miri/.github/workflows/setup/action.yml +++ b/src/tools/miri/.github/workflows/setup/action.yml @@ -35,6 +35,10 @@ runs: run: cargo install -f rustup-toolchain-install-master hyperfine shell: bash + - name: Install nightly toolchain + run: rustup toolchain install nightly --profile minimal + shell: bash + - name: Install "master" toolchain run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 9067cbc6032..ca03a9b16e3 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -8,10 +8,43 @@ find useful. Check out the issues on this GitHub repository for some ideas. In particular, look for the green `E-*` labels which mark issues that should be rather well-suited for onboarding. For more ideas or help with hacking on Miri, you can -contact us (`oli-obk` and `RalfJ`) on the [Rust Zulip]. +contact us on the [Rust Zulip]. See the [Rust website](https://www.rust-lang.org/governance/teams/compiler#team-miri) +for a list of Miri maintainers. [Rust Zulip]: https://rust-lang.zulipchat.com +### Larger-scale contributions + +If you are thinking about making a larger-scale contribution -- in particular anything that needs +more than can reasonably fit in a single PR to be feature-complete -- then please talk to us before +writing significant amounts of code. Generally, we will ask that you follow a three-step "project" +process for such contributions: + +1. Clearly define the **goal** of the project. This defines the scope of the project, i.e. which + part of which APIs should be supported. If this involves functions that expose a big API surface + with lots of flags, the project may want to support only a tiny subset of flags; that should be + documented. A good way to express the goal is with one or more test cases that Miri should be + able to successfully execute when the project is completed. It is a good idea to get feedback + from team members already at this stage to ensure that the project is reasonably scoped and + aligns with our interests. +2. Make a **design** for how to realize the goal. A larger project will likely have to do global + changes to Miri, like adding new global state to the `Machine` type or new methods to the + `FileDescription` trait. Often we have to iterate on those changes, which can quite substantially + change how the final implementation looks like. + + The design should be reasonably concrete, i.e. for new global state or methods the corresponding + Rust types and method signatures should be spelled out. We realize that it can be hard to make a + design without doing implementation work, in particular if you are not yet familiar with the + codebase. Doing draft implementations in phase 2 of this process is perfectly fine, just please + be aware that we might request fundamental changes that can require significantly reworking what + you already did. If you open a PR in this stage, please clearly indicate that this project is + still in the design stage. + +3. Finish the **implementation** and have it reviewed. + +This process is largely informal, and its primary goal is to more clearly communicate expectations. +Please get in touch with us if you have any questions! + ## Preparing the build environment Miri heavily relies on internal and unstable rustc interfaces to execute MIR, @@ -173,24 +206,24 @@ to `.vscode/settings.json` in your local Miri clone: "cargo-miri/Cargo.toml", "miri-script/Cargo.toml", ], + "rust-analyzer.check.invocationLocation": "root", + "rust-analyzer.check.invocationStrategy": "once", "rust-analyzer.check.overrideCommand": [ "env", "MIRI_AUTO_OPS=no", "./miri", - "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.invocationLocation": "root", + "rust-analyzer.cargo.buildScripts.invocationStrategy": "once", "rust-analyzer.cargo.buildScripts.overrideCommand": [ "env", "MIRI_AUTO_OPS=no", "./miri", - "cargo", "check", "--message-format=json", - "--all-targets", ], } ``` @@ -309,6 +342,7 @@ anyone but Miri itself. They are used to communicate between different Miri binaries, and as such worth documenting: * `CARGO_EXTRA_FLAGS` is understood by `./miri` and passed to all host cargo invocations. + It is reserved for CI usage; setting the wrong flags this way can easily confuse the script. * `MIRI_BE_RUSTC` can be set to `host` or `target`. It tells the Miri driver to actually not interpret the code but compile it like rustc would. With `target`, Miri sets some compiler flags to prepare the code for interpretation; with `host`, this is not done. diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index a0bff386a71..5821adb96ce 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -414,10 +414,6 @@ to Miri failing to detect cases of undefined behavior in a program. being allocated or freed. This helps in debugging memory leaks and use after free bugs. Specifying this argument multiple times does not overwrite the previous values, instead it appends its values to the list. Listing an id multiple times has no effect. -* `-Zmiri-track-call-id=,,...` shows a backtrace when the given call ids are - assigned to a stack frame. This helps in debugging UB related to Stacked - Borrows "protectors". Specifying this argument multiple times does not overwrite the previous - values, instead it appends its values to the list. Listing an id multiple times has no effect. * `-Zmiri-track-pointer-tag=,,...` shows a backtrace when a given pointer tag is created and when (if ever) it is popped from a borrow stack (which is where the tag becomes invalid and any future use of it will error). This helps you in finding out why UB is diff --git a/src/tools/miri/cargo-miri/miri b/src/tools/miri/cargo-miri/miri deleted file mode 100755 index cf3ad06788a..00000000000 --- a/src/tools/miri/cargo-miri/miri +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -# RA invokes `./miri cargo ...` for each workspace, so we need to forward that to the main `miri` -# script. See . -exec "$(dirname "$0")"/../miri "$@" diff --git a/src/tools/miri/cargo-miri/src/util.rs b/src/tools/miri/cargo-miri/src/util.rs index 5f2794e2244..56f38de8de6 100644 --- a/src/tools/miri/cargo-miri/src/util.rs +++ b/src/tools/miri/cargo-miri/src/util.rs @@ -93,12 +93,9 @@ pub fn find_miri() -> PathBuf { if let Some(path) = env::var_os("MIRI") { return path.into(); } + // Assume it is in the same directory as ourselves. let mut path = std::env::current_exe().expect("current executable path invalid"); - if cfg!(windows) { - path.set_file_name("miri.exe"); - } else { - path.set_file_name("miri"); - } + path.set_file_name(format!("miri{}", env::consts::EXE_SUFFIX)); path } diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 3e90ecc5c03..1f66b6fa776 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -150,8 +150,8 @@ case $HOST_TARGET in UNIX="panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs - TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX threadname pthread-sync available-parallelism libc-time - TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC $UNIX threadname pthread-sync available-parallelism libc-time + TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX threadname pthread-sync available-parallelism libc-time tls + TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC $UNIX threadname pthread-sync available-parallelism libc-time tls TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX TEST_TARGET=wasm32-wasip2 run_tests_minimal empty_main wasm heap_alloc libc-mem TEST_TARGET=wasm32-unknown-unknown run_tests_minimal empty_main wasm diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 07383bb59eb..5d07ad9e249 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -1,8 +1,15 @@ #!/usr/bin/env bash set -e +# We want to call the binary directly, so we need to know where it ends up. +MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target +# If stdout is not a terminal and we are not on CI, assume that we are being invoked by RA, and use JSON output. +if ! [ -t 1 ] && [ -z "$CI" ]; then + MESSAGE_FORMAT="--message-format=json" +fi +# We need a nightly toolchain, for the `profile-rustflags` cargo feature. +cargo +nightly build $CARGO_EXTRA_FLAGS --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml \ + -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" $MESSAGE_FORMAT || \ + ( echo "Failed to build miri-script. Is the 'nightly' toolchain installed?"; exit 1 ) # Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through # rustup (that sets it's own environmental variables), which is undesirable. -MIRI_SCRIPT_TARGET_DIR="$(dirname "$0")"/miri-script/target -cargo +stable build $CARGO_EXTRA_FLAGS -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml || \ - ( echo "Failed to build miri-script. Is the 'stable' toolchain installed?"; exit 1 ) "$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" diff --git a/src/tools/miri/miri-script/miri b/src/tools/miri/miri-script/miri deleted file mode 100755 index cf3ad06788a..00000000000 --- a/src/tools/miri/miri-script/miri +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/sh -# RA invokes `./miri cargo ...` for each workspace, so we need to forward that to the main `miri` -# script. See . -exec "$(dirname "$0")"/../miri "$@" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index fc205040baf..a9a80175901 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -1,12 +1,12 @@ use std::env; use std::ffi::{OsStr, OsString}; use std::io::Write; +use std::net; use std::ops::Not; use std::ops::Range; use std::path::PathBuf; use std::process; -use std::thread; -use std::time; +use std::time::Duration; use anyhow::{anyhow, bail, Context, Result}; use path_macro::path; @@ -19,9 +19,10 @@ use crate::Command; /// Used for rustc syncs. const JOSH_FILTER: &str = ":rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri"; -const JOSH_PORT: &str = "42042"; +const JOSH_PORT: u16 = 42042; impl MiriEnv { + /// Prepares the environment: builds miri and cargo-miri and a sysroot. /// Returns the location of the sysroot. /// /// If the target is None the sysroot will be built for the host machine. @@ -34,12 +35,10 @@ impl MiriEnv { // Sysroot already set, use that. return Ok(miri_sysroot.into()); } - let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); - let Self { toolchain, cargo_extra_flags, .. } = &self; // Make sure everything is built. Also Miri itself. - self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?; - self.build(&manifest_path, &[], quiet)?; + self.build(".", &[], quiet)?; + self.build("cargo-miri", &[], quiet)?; let target_flag = if let Some(target) = &target { vec![OsStr::new("--target"), target.as_ref()] @@ -56,10 +55,12 @@ impl MiriEnv { eprintln!(); } - let mut cmd = cmd!(self.sh, - "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- - miri setup --print-sysroot {target_flag...}" - ); + let mut cmd = self + .cargo_cmd("cargo-miri", "run") + .arg("--quiet") + .arg("--") + .args(&["miri", "setup", "--print-sysroot"]) + .args(target_flag); cmd.set_quiet(quiet); let output = cmd.read()?; self.sh.set_var("MIRI_SYSROOT", &output); @@ -105,13 +106,11 @@ impl Command { let mut cmd = process::Command::new("josh-proxy"); cmd.arg("--local").arg(local_dir); cmd.arg("--remote").arg("https://github.com"); - cmd.arg("--port").arg(JOSH_PORT); + cmd.arg("--port").arg(JOSH_PORT.to_string()); cmd.arg("--no-background"); cmd.stdout(process::Stdio::null()); cmd.stderr(process::Stdio::null()); let josh = cmd.spawn().context("failed to start josh-proxy, make sure it is installed")?; - // Give it some time so hopefully the port is open. (100ms was not enough.) - thread::sleep(time::Duration::from_millis(200)); // Create a wrapper that stops it on drop. struct Josh(process::Child); @@ -125,7 +124,7 @@ impl Command { .output() .expect("failed to SIGINT josh-proxy"); // Sadly there is no "wait with timeout"... so we just give it some time to finish. - thread::sleep(time::Duration::from_millis(100)); + std::thread::sleep(Duration::from_millis(100)); // Now hopefully it is gone. if self.0.try_wait().expect("failed to wait for josh-proxy").is_some() { return; @@ -139,7 +138,20 @@ impl Command { } } - Ok(Josh(josh)) + // Wait until the port is open. We try every 10ms until 1s passed. + for _ in 0..100 { + // This will generally fail immediately when the port is still closed. + let josh_ready = net::TcpStream::connect_timeout( + &net::SocketAddr::from(([127, 0, 0, 1], JOSH_PORT)), + Duration::from_millis(1), + ); + if josh_ready.is_ok() { + return Ok(Josh(josh)); + } + // Not ready yet. + std::thread::sleep(Duration::from_millis(10)); + } + bail!("Even after waiting for 1s, josh-proxy is still not available.") } pub fn exec(self) -> Result<()> { @@ -151,8 +163,8 @@ impl Command { | Command::Test { .. } | Command::Run { .. } | Command::Fmt { .. } - | Command::Clippy { .. } - | Command::Cargo { .. } => Self::auto_actions()?, + | Command::Doc { .. } + | Command::Clippy { .. } => Self::auto_actions()?, | Command::Toolchain { .. } | Command::Bench { .. } | Command::RustcPull { .. } @@ -166,9 +178,9 @@ impl Command { Command::Test { bless, flags, target } => Self::test(bless, flags, target), Command::Run { dep, verbose, many_seeds, target, edition, flags } => Self::run(dep, verbose, many_seeds, target, edition, flags), + Command::Doc { flags } => Self::doc(flags), Command::Fmt { flags } => Self::fmt(flags), Command::Clippy { flags } => Self::clippy(flags), - Command::Cargo { flags } => Self::cargo(flags), Command::Bench { target, benches } => Self::bench(target, benches), Command::Toolchain { flags } => Self::toolchain(flags), Command::RustcPull { commit } => Self::rustc_pull(commit.clone()), @@ -236,6 +248,8 @@ impl Command { } // Make sure josh is running. let josh = Self::start_josh()?; + let josh_url = + format!("http://localhost:{JOSH_PORT}/rust-lang/rust.git@{commit}{JOSH_FILTER}.git"); // Update rust-version file. As a separate commit, since making it part of // the merge has confused the heck out of josh in the past. @@ -250,7 +264,7 @@ impl Command { .context("FAILED to commit rust-version file, something went wrong")?; // Fetch given rustc commit. - cmd!(sh, "git fetch http://localhost:{JOSH_PORT}/rust-lang/rust.git@{commit}{JOSH_FILTER}.git") + cmd!(sh, "git fetch {josh_url}") .run() .inspect_err(|_| { // Try to un-do the previous `git commit`, to leave the repo in the state we found it. @@ -294,6 +308,8 @@ impl Command { } // Make sure josh is running. let josh = Self::start_josh()?; + let josh_url = + format!("http://localhost:{JOSH_PORT}/{github_user}/rust.git{JOSH_FILTER}.git"); // Find a repo we can do our preparation in. if let Ok(rustc_git) = env::var("RUSTC_GIT") { @@ -338,20 +354,11 @@ impl Command { // Do the actual push. sh.change_dir(miri_dir()?); println!("Pushing miri changes..."); - cmd!( - sh, - "git push http://localhost:{JOSH_PORT}/{github_user}/rust.git{JOSH_FILTER}.git HEAD:{branch}" - ) - .run()?; + cmd!(sh, "git push {josh_url} HEAD:{branch}").run()?; println!(); // Do a round-trip check to make sure the push worked as expected. - cmd!( - sh, - "git fetch http://localhost:{JOSH_PORT}/{github_user}/rust.git{JOSH_FILTER}.git {branch}" - ) - .ignore_stderr() - .read()?; + cmd!(sh, "git fetch {josh_url} {branch}").ignore_stderr().read()?; let head = cmd!(sh, "git rev-parse HEAD").read()?; let fetch_head = cmd!(sh, "git rev-parse FETCH_HEAD").read()?; if head != fetch_head { @@ -427,39 +434,37 @@ impl Command { fn build(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; - e.build(path!(e.miri_dir / "Cargo.toml"), &flags, /* quiet */ false)?; - e.build(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags, /* quiet */ false)?; + e.build(".", &flags, /* quiet */ false)?; + e.build("cargo-miri", &flags, /* quiet */ false)?; Ok(()) } fn check(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; - e.check(path!(e.miri_dir / "Cargo.toml"), &flags)?; - e.check(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; + e.check(".", &flags)?; + e.check("cargo-miri", &flags)?; + Ok(()) + } + + fn doc(flags: Vec) -> Result<()> { + let e = MiriEnv::new()?; + e.doc(".", &flags)?; + e.doc("cargo-miri", &flags)?; Ok(()) } fn clippy(flags: Vec) -> Result<()> { let e = MiriEnv::new()?; - e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?; - e.clippy(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; - e.clippy(path!(e.miri_dir / "miri-script" / "Cargo.toml"), &flags)?; - Ok(()) - } - - fn cargo(flags: Vec) -> Result<()> { - let e = MiriEnv::new()?; - let toolchain = &e.toolchain; - // We carefully kept the working dir intact, so this will run cargo *on the workspace in the - // current working dir*, not on the main Miri workspace. That is exactly what RA needs. - cmd!(e.sh, "cargo +{toolchain} {flags...}").run()?; + e.clippy(".", &flags)?; + e.clippy("cargo-miri", &flags)?; + e.clippy("miri-script", &flags)?; Ok(()) } fn test(bless: bool, mut flags: Vec, target: Option) -> Result<()> { let mut e = MiriEnv::new()?; - // Prepare a sysroot. + // Prepare a sysroot. (Also builds cargo-miri, which we need.) e.build_miri_sysroot(/* quiet */ false, target.as_deref())?; // Forward information to test harness. @@ -476,7 +481,7 @@ impl Command { // Then test, and let caller control flags. // Only in root project as `cargo-miri` has no tests. - e.test(path!(e.miri_dir / "Cargo.toml"), &flags)?; + e.test(".", &flags)?; Ok(()) } @@ -504,32 +509,27 @@ impl Command { early_flags.push("--edition".into()); early_flags.push(edition.as_deref().unwrap_or("2021").into()); - // Prepare a sysroot, add it to the flags. + // Prepare a sysroot, add it to the flags. (Also builds cargo-miri, which we need.) let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?; early_flags.push("--sysroot".into()); early_flags.push(miri_sysroot.into()); // Compute everything needed to 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 quiet_flag = if verbose { None } else { Some("--quiet") }; // This closure runs the command with the given `seed_flag` added between the MIRIFLAGS and // the `flags` given on the command-line. - let run_miri = |sh: &Shell, seed_flag: Option| -> Result<()> { + let run_miri = |e: &MiriEnv, seed_flag: Option| -> Result<()> { // The basic command that executes the Miri driver. let mut cmd = if dep { - cmd!( - sh, - "cargo +{toolchain} {quiet_flag...} test {extra_flags...} --manifest-path {miri_manifest} --test ui -- --miri-run-dep-mode" - ) + e.cargo_cmd(".", "test") + .args(&["--test", "ui"]) + .args(quiet_flag) + .arg("--") + .args(&["--miri-run-dep-mode"]) } else { - cmd!( - sh, - "cargo +{toolchain} {quiet_flag...} run {extra_flags...} --manifest-path {miri_manifest} --" - ) + e.cargo_cmd(".", "run").args(quiet_flag).arg("--") }; cmd.set_quiet(!verbose); // Add Miri flags @@ -545,14 +545,14 @@ impl Command { }; // Run the closure once or many times. if let Some(seed_range) = many_seeds { - e.run_many_times(seed_range, |sh, seed| { + e.run_many_times(seed_range, |e, seed| { eprintln!("Trying seed: {seed}"); - run_miri(sh, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| { + run_miri(e, Some(format!("-Zmiri-seed={seed}"))).inspect_err(|_| { eprintln!("FAILING SEED: {seed}"); }) })?; } else { - run_miri(&e.sh, None)?; + run_miri(&e, None)?; } Ok(()) } @@ -579,6 +579,6 @@ impl Command { .filter_ok(|item| item.file_type().is_file()) .map_ok(|item| item.into_path()); - e.format_files(files, &e.toolchain[..], &config_path, &flags) + e.format_files(files, &config_path, &flags) } } diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index c4f0d808d93..92148237107 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -48,6 +48,11 @@ pub enum Command { /// Flags that are passed through to `miri`. flags: Vec, }, + /// Build documentation + Doc { + /// Flags that are passed through to `cargo doc`. + flags: Vec, + }, /// Format all sources and tests. Fmt { /// Flags that are passed through to `rustfmt`. @@ -58,9 +63,6 @@ pub enum Command { /// Flags that are passed through to `cargo clippy`. flags: Vec, }, - /// Runs just `cargo ` with the Miri-specific environment variables. - /// Mainly meant to be invoked by rust-analyzer. - Cargo { flags: Vec }, /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. Bench { target: Option, @@ -151,6 +153,7 @@ fn main() -> Result<()> { let command = match args.next_raw().as_deref() { Some("build") => Command::Build { flags: args.remainder() }, Some("check") => Command::Check { flags: args.remainder() }, + Some("doc") => Command::Doc { flags: args.remainder() }, Some("test") => { let mut target = None; let mut bless = false; @@ -205,7 +208,6 @@ fn main() -> Result<()> { } Some("fmt") => Command::Fmt { flags: args.remainder() }, Some("clippy") => Command::Clippy { flags: args.remainder() }, - Some("cargo") => Command::Cargo { flags: args.remainder() }, Some("install") => Command::Install { flags: args.remainder() }, Some("bench") => { let mut target = None; diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index e1b77be192e..35c604b407e 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -7,7 +7,7 @@ use std::thread; use anyhow::{anyhow, Context, Result}; use dunce::canonicalize; use path_macro::path; -use xshell::{cmd, Shell}; +use xshell::{cmd, Cmd, Shell}; pub fn miri_dir() -> std::io::Result { const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR"); @@ -28,13 +28,14 @@ pub fn flagsplit(flags: &str) -> Vec { } /// Some extra state we track for building Miri, such as the right RUSTFLAGS. +#[derive(Clone)] pub struct MiriEnv { /// miri_dir is the root of the miri repository checkout we are working in. pub miri_dir: PathBuf, /// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations. - pub toolchain: String, + toolchain: String, /// Extra flags to pass to cargo. - pub cargo_extra_flags: Vec, + cargo_extra_flags: Vec, /// The rustc sysroot pub sysroot: PathBuf, /// The shell we use. @@ -54,15 +55,14 @@ impl MiriEnv { // Determine some toolchain properties if !libdir.exists() { - println!("Something went wrong determining the library dir."); - println!("I got {} but that does not exist.", libdir.display()); - println!("Please report a bug at https://github.com/rust-lang/miri/issues."); + eprintln!("Something went wrong determining the library dir."); + eprintln!("I got {} but that does not exist.", libdir.display()); + eprintln!("Please report a bug at https://github.com/rust-lang/miri/issues."); std::process::exit(2); } - // Share target dir between `miri` and `cargo-miri`. - let target_dir = std::env::var_os("CARGO_TARGET_DIR") - .unwrap_or_else(|| path!(miri_dir / "target").into()); - sh.set_var("CARGO_TARGET_DIR", target_dir); + + // Hard-code the target dir, since we rely on all binaries ending up in the same spot. + sh.set_var("CARGO_TARGET_DIR", path!(miri_dir / "target")); // We configure dev builds to not be unusably slow. let devel_opt_level = @@ -91,61 +91,73 @@ impl MiriEnv { // Get extra flags for cargo. let cargo_extra_flags = std::env::var("CARGO_EXTRA_FLAGS").unwrap_or_default(); let cargo_extra_flags = flagsplit(&cargo_extra_flags); + if cargo_extra_flags.iter().any(|a| a == "--release" || a.starts_with("--profile")) { + // This makes binaries end up in different paths, let's not do that. + eprintln!( + "Passing `--release` or `--profile` in `CARGO_EXTRA_FLAGS` will totally confuse miri-script, please don't do that." + ); + std::process::exit(1); + } Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags }) } + pub fn cargo_cmd(&self, crate_dir: impl AsRef, cmd: &str) -> Cmd<'_> { + let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + let manifest_path = path!(self.miri_dir / crate_dir.as_ref() / "Cargo.toml"); + cmd!( + self.sh, + "cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path}" + ) + } + pub fn install_to_sysroot( &self, path: impl AsRef, args: impl IntoIterator>, ) -> Result<()> { let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self; + let path = path!(self.miri_dir / path.as_ref()); // Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains. + // (Not using `cargo_cmd` as `install` is special and doesn't use `--manifest-path`.) cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?; Ok(()) } - pub fn build( - &self, - manifest_path: impl AsRef, - args: &[String], - quiet: bool, - ) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + pub fn build(&self, crate_dir: impl AsRef, args: &[String], quiet: bool) -> Result<()> { let quiet_flag = if quiet { Some("--quiet") } else { None }; // We build the tests as well, (a) to avoid having rebuilds when building the tests later // and (b) to have more parallelism during the build of Miri and its tests. - let mut cmd = cmd!( - self.sh, - "cargo +{toolchain} build --bins --tests {cargo_extra_flags...} --manifest-path {manifest_path} {quiet_flag...} {args...}" - ); + // This means `./miri run` without `--dep` will build Miri twice (for the sysroot with + // dev-dependencies, and then for running without dev-dependencies), but the way more common + // `./miri test` will avoid building Miri twice. + let mut cmd = self + .cargo_cmd(crate_dir, "build") + .args(&["--bins", "--tests"]) + .args(quiet_flag) + .args(args); cmd.set_quiet(quiet); cmd.run()?; Ok(()) } - pub fn check(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; - cmd!(self.sh, "cargo +{toolchain} check {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}") - .run()?; + pub fn check(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "check").arg("--all-targets").args(args).run()?; Ok(()) } - pub fn clippy(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; - cmd!(self.sh, "cargo +{toolchain} clippy {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}") - .run()?; + pub fn doc(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "doc").args(args).run()?; Ok(()) } - pub fn test(&self, manifest_path: impl AsRef, args: &[String]) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; - cmd!( - self.sh, - "cargo +{toolchain} test {cargo_extra_flags...} --manifest-path {manifest_path} {args...}" - ) - .run()?; + pub fn clippy(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "clippy").arg("--all-targets").args(args).run()?; + Ok(()) + } + + pub fn test(&self, crate_dir: impl AsRef, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "test").args(args).run()?; Ok(()) } @@ -155,7 +167,6 @@ impl MiriEnv { pub fn format_files( &self, files: impl Iterator>, - toolchain: &str, config_path: &Path, flags: &[String], ) -> anyhow::Result<()> { @@ -166,6 +177,7 @@ impl MiriEnv { // Format in batches as not all our files fit into Windows' command argument limit. for batch in &files.chunks(256) { // Build base command. + let toolchain = &self.toolchain; let mut cmd = cmd!( self.sh, "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" @@ -197,7 +209,7 @@ impl MiriEnv { pub fn run_many_times( &self, range: Range, - run: impl Fn(&Shell, u32) -> Result<()> + Sync, + run: impl Fn(&Self, u32) -> Result<()> + Sync, ) -> Result<()> { // `next` is atomic so threads can concurrently fetch their next value to run. let next = AtomicU32::new(range.start); @@ -207,10 +219,10 @@ impl MiriEnv { let mut handles = Vec::new(); // Spawn one worker per core. for _ in 0..thread::available_parallelism()?.get() { - // Create a copy of the shell for this thread. - let local_shell = self.sh.clone(); + // Create a copy of the environment for this thread. + let local_miri = self.clone(); let handle = s.spawn(|| -> Result<()> { - let local_shell = local_shell; // move the copy into this thread. + let local_miri = local_miri; // move the copy into this thread. // Each worker thread keeps asking for numbers until we're all done. loop { let cur = next.fetch_add(1, Ordering::Relaxed); @@ -219,7 +231,7 @@ impl MiriEnv { break; } // Run the command with this seed. - run(&local_shell, cur).inspect_err(|_| { + run(&local_miri, cur).inspect_err(|_| { // If we failed, tell everyone about this. failed.store(true, Ordering::Relaxed); })?; diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index b74f9759ebe..c3f4f4b5d82 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -29e924841f06bb181d87494eba2783761bc1ddec +f24a6ba06f4190d8ec4f22d1baa800e64b1900cb diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index e13e54c3309..14f43f576d3 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -581,17 +581,6 @@ fn main() { show_error!("-Zmiri-track-pointer-tag requires nonzero arguments"); } } - } else if let Some(param) = arg.strip_prefix("-Zmiri-track-call-id=") { - let ids: Vec = parse_comma_list(param).unwrap_or_else(|err| { - show_error!("-Zmiri-track-call-id requires a comma separated list of valid `u64` arguments: {err}") - }); - for id in ids.into_iter().map(miri::CallId::new) { - if let Some(id) = id { - miri_config.tracked_call_ids.insert(id); - } else { - show_error!("-Zmiri-track-call-id requires a nonzero argument"); - } - } } else if let Some(param) = arg.strip_prefix("-Zmiri-track-alloc-id=") { let ids = parse_comma_list::>(param).unwrap_or_else(|err| { show_error!("-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {err}") diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index d537a7fbc17..7a3d76a9beb 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -12,8 +12,6 @@ use crate::*; pub mod stacked_borrows; pub mod tree_borrows; -pub type CallId = NonZero; - /// Tracking pointer provenance #[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct BorTag(NonZero); @@ -57,9 +55,6 @@ impl fmt::Debug for BorTag { /// Per-call-stack-frame data for borrow tracking #[derive(Debug)] pub struct FrameState { - /// The ID of the call this frame corresponds to. - call_id: CallId, - /// If this frame is protecting any tags, they are listed here. We use this list to do /// incremental updates of the global list of protected tags stored in the /// `stacked_borrows::GlobalState` upon function return, and if we attempt to pop a protected @@ -93,18 +88,13 @@ pub struct GlobalStateInner { /// The root tag is the one used for the initial pointer. /// We need this in a separate table to handle cyclic statics. root_ptr_tags: FxHashMap, - /// Next unused call ID (for protectors). - next_call_id: CallId, /// All currently protected tags. - /// An item is protected if its tag is in this set, *and* it has the "protected" bit set. /// We add tags to this when they are created with a protector in `reborrow`, and /// we remove tags from this when the call which is protecting them returns, in /// `GlobalStateInner::end_call`. See `Stack::item_invalidated` for more details. protected_tags: FxHashMap, /// The pointer ids to trace tracked_pointer_tags: FxHashSet, - /// The call ids to trace - tracked_call_ids: FxHashSet, /// Whether to recurse into datatypes when searching for pointers to retag. retag_fields: RetagFields, /// Whether `core::ptr::Unique` gets special (`Box`-like) handling. @@ -168,7 +158,6 @@ impl GlobalStateInner { pub fn new( borrow_tracker_method: BorrowTrackerMethod, tracked_pointer_tags: FxHashSet, - tracked_call_ids: FxHashSet, retag_fields: RetagFields, unique_is_unique: bool, ) -> Self { @@ -176,10 +165,8 @@ impl GlobalStateInner { borrow_tracker_method, next_ptr_tag: BorTag::one(), root_ptr_tags: FxHashMap::default(), - next_call_id: NonZero::new(1).unwrap(), protected_tags: FxHashMap::default(), tracked_pointer_tags, - tracked_call_ids, retag_fields, unique_is_unique, } @@ -192,14 +179,8 @@ impl GlobalStateInner { id } - pub fn new_frame(&mut self, machine: &MiriMachine<'_>) -> FrameState { - let call_id = self.next_call_id; - trace!("new_frame: Assigning call ID {}", call_id); - if self.tracked_call_ids.contains(&call_id) { - machine.emit_diagnostic(NonHaltingDiagnostic::CreatedCallId(call_id)); - } - self.next_call_id = NonZero::new(call_id.get() + 1).unwrap(); - FrameState { call_id, protected_tags: SmallVec::new() } + pub fn new_frame(&mut self) -> FrameState { + FrameState { protected_tags: SmallVec::new() } } fn end_call(&mut self, frame: &machine::FrameExtra<'_>) { @@ -252,7 +233,6 @@ impl BorrowTrackerMethod { RefCell::new(GlobalStateInner::new( self, config.tracked_pointer_tags.clone(), - config.tracked_call_ids.clone(), config.retag_fields, config.unique_is_unique, )) @@ -346,7 +326,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn print_borrow_state(&mut self, alloc_id: AllocId, show_unnamed: bool) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; + let Some(borrow_tracker) = &this.machine.borrow_tracker else { + eprintln!("attempted to print borrow state, but no borrow state is being tracked"); + return Ok(()); + }; + let method = borrow_tracker.borrow().borrow_tracker_method; match method { BorrowTrackerMethod::StackedBorrows => this.print_stacks(alloc_id), BorrowTrackerMethod::TreeBorrows => this.print_tree(alloc_id, show_unnamed), diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 87d9057cb89..12eeaae0eff 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -429,30 +429,14 @@ impl<'history, 'ecx, 'tcx> DiagnosticCx<'history, 'ecx, 'tcx> { ProtectorKind::WeakProtector => "weakly protected", ProtectorKind::StrongProtector => "strongly protected", }; - let item_tag = item.tag(); - let call_id = self - .machine - .threads - .all_stacks() - .flat_map(|(_id, stack)| stack) - .map(|frame| { - frame.extra.borrow_tracker.as_ref().expect("we should have borrow tracking data") - }) - .find(|frame| frame.protected_tags.iter().any(|(_, tag)| tag == &item_tag)) - .map(|frame| frame.call_id) - .unwrap(); // FIXME: Surely we should find something, but a panic seems wrong here? match self.operation { Operation::Dealloc(_) => - err_sb_ub( - format!("deallocating while item {item:?} is {protected} by call {call_id:?}",), - vec![], - None, - ), + err_sb_ub(format!("deallocating while item {item:?} is {protected}",), vec![], None), Operation::Retag(RetagOp { orig_tag: tag, .. }) | Operation::Access(AccessOp { tag, .. }) => err_sb_ub( format!( - "not granting access to tag {tag:?} because that would remove {item:?} which is {protected} because it is an argument of call {call_id:?}", + "not granting access to tag {tag:?} because that would remove {item:?} which is {protected}", ), vec![], tag.and_then(|tag| self.get_logs_relevant_to(tag, Some(item.tag()))), diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs index b9a52e4966c..13846710615 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/item.rs @@ -7,9 +7,12 @@ use crate::borrow_tracker::BorTag; pub struct Item(u64); // An Item contains 3 bitfields: -// * Bits 0-61 store a BorTag -// * Bits 61-63 store a Permission -// * Bit 64 stores a flag which indicates if we have a protector +// * Bits 0-61 store a BorTag. +// * Bits 61-63 store a Permission. +// * Bit 64 stores a flag which indicates if we might have a protector. +// This is purely an optimization: if the bit is set, the tag *might* be +// in `protected_tags`, but if the bit is not set then the tag is definitely +// not in `protected_tags`. const TAG_MASK: u64 = u64::MAX >> 3; const PERM_MASK: u64 = 0x3 << 61; const PROTECTED_MASK: u64 = 0x1 << 63; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 44f42d5fb9c..722cb79c66b 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -1,10 +1,6 @@ use rustc_middle::{ mir::{Mutability, RetagKind}, - ty::{ - self, - layout::{HasParamEnv, HasTyCtxt}, - Ty, - }, + ty::{self, layout::HasParamEnv, Ty}, }; use rustc_span::def_id::DefId; use rustc_target::abi::{Abi, Size}; @@ -146,10 +142,9 @@ impl<'tcx> NewPermission { // interior mutability and protectors interact poorly. // To eliminate the case of Protected Reserved IM we override interior mutability // in the case of a protected reference: protected references are always considered - // "freeze". + // "freeze" in their reservation phase. let initial_state = match mutability { - Mutability::Mut if ty_is_unpin => - Permission::new_reserved(ty_is_freeze || is_protected), + Mutability::Mut if ty_is_unpin => Permission::new_reserved(ty_is_freeze, is_protected), Mutability::Not if ty_is_freeze => Permission::new_frozen(), // Raw pointers never enter this function so they are not handled. // However raw pointers are not the only pointers that take the parent @@ -176,10 +171,12 @@ impl<'tcx> NewPermission { // Regular `Unpin` box, give it `noalias` but only a weak protector // because it is valid to deallocate it within the function. let ty_is_freeze = ty.is_freeze(*cx.tcx, cx.param_env()); + let protected = kind == RetagKind::FnEntry; + let initial_state = Permission::new_reserved(ty_is_freeze, protected); Self { zero_size, - initial_state: Permission::new_reserved(ty_is_freeze), - protector: (kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), + initial_state, + protector: protected.then_some(ProtectorKind::WeakProtector), } }) } @@ -521,11 +518,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn tb_protect_place(&mut self, place: &MPlaceTy<'tcx>) -> InterpResult<'tcx, MPlaceTy<'tcx>> { let this = self.eval_context_mut(); + // Note: if we were to inline `new_reserved` below we would find out that + // `ty_is_freeze` is eventually unused because it appears in a `ty_is_freeze || true`. + // We are nevertheless including it here for clarity. + let ty_is_freeze = place.layout.ty.is_freeze(*this.tcx, this.param_env()); // Retag it. With protection! That is the entire point. let new_perm = NewPermission { - initial_state: Permission::new_reserved( - place.layout.ty.is_freeze(this.tcx(), this.param_env()), - ), + initial_state: Permission::new_reserved(ty_is_freeze, /* protected */ true), zero_size: false, protector: Some(ProtectorKind::StrongProtector), }; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 8e23257b6c0..5461edb51d3 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -8,10 +8,16 @@ use crate::AccessKind; /// The activation states of a pointer. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] enum PermissionPriv { - /// represents: a local reference that has not yet been written to; - /// allows: child reads, foreign reads, foreign writes if type is freeze; + /// represents: a local mutable reference that has not yet been written to; + /// allows: child reads, foreign reads; /// affected by: child writes (becomes Active), - /// rejects: foreign writes (Disabled, except if type is not freeze). + /// rejects: foreign writes (Disabled). + /// + /// `ReservedFrz` is mostly for types that are `Freeze` (no interior mutability). + /// If the type has interior mutability, see `ReservedIM` instead. + /// (Note: since the discovery of `tests/fail/tree_borrows/reservedim_spurious_write.rs`, + /// we also use `ReservedFreeze` for mutable references that were retagged with a protector + /// independently of interior mutability) /// /// special case: behaves differently when protected, which is where `conflicted` /// is relevant @@ -22,12 +28,12 @@ enum PermissionPriv { /// - foreign-read then child-write is UB due to `conflicted`, /// - child-write then foreign-read is UB since child-write will activate and then /// foreign-read disables a protected `Active`, which is UB. - /// - /// Note: since the discovery of `tests/fail/tree_borrows/reservedim_spurious_write.rs`, - /// `ty_is_freeze` does not strictly mean that the type has no interior mutability, - /// it could be an interior mutable type that lost its interior mutability privileges - /// when retagged with a protector. - Reserved { ty_is_freeze: bool, conflicted: bool }, + ReservedFrz { conflicted: bool }, + /// Alternative version of `ReservedFrz` made for types with interior mutability. + /// allows: child reads, foreign reads, foreign writes (extra); + /// affected by: child writes (becomes Active); + /// rejects: nothing. + ReservedIM, /// represents: a unique pointer; /// allows: child reads, child writes; /// rejects: foreign reads (Frozen), foreign writes (Disabled). @@ -59,17 +65,14 @@ impl PartialOrd for PermissionPriv { (_, Frozen) => Less, (Active, _) => Greater, (_, Active) => Less, - ( - Reserved { ty_is_freeze: f1, conflicted: c1 }, - Reserved { ty_is_freeze: f2, conflicted: c2 }, - ) => { - // No transition ever changes `ty_is_freeze`. - if f1 != f2 { - return None; - } + (ReservedIM, ReservedIM) => Equal, + (ReservedFrz { conflicted: c1 }, ReservedFrz { conflicted: c2 }) => { // `bool` is ordered such that `false <= true`, so this works as intended. c1.cmp(c2) } + // Versions of `Reserved` with different interior mutability are incomparable with each + // other. + (ReservedIM, ReservedFrz { .. }) | (ReservedFrz { .. }, ReservedIM) => return None, }) } } @@ -77,7 +80,12 @@ impl PartialOrd for PermissionPriv { impl PermissionPriv { /// Check if `self` can be the initial state of a pointer. fn is_initial(&self) -> bool { - matches!(self, Reserved { conflicted: false, .. } | Frozen) + matches!(self, ReservedFrz { conflicted: false } | Frozen | ReservedIM) + } + + /// Reject `ReservedIM` that cannot exist in the presence of a protector. + fn compatible_with_protector(&self) -> bool { + !matches!(self, ReservedIM) } } @@ -93,7 +101,7 @@ mod transition { Disabled => return None, // The inner data `ty_is_freeze` of `Reserved` is always irrelevant for Read // accesses, since the data is not being mutated. Hence the `{ .. }`. - readable @ (Reserved { .. } | Active | Frozen) => readable, + readable @ (ReservedFrz { .. } | ReservedIM | Active | Frozen) => readable, }) } @@ -109,11 +117,16 @@ mod transition { // Someone else read. To make sure we won't write before function exit, // we set the "conflicted" flag, which will disallow writes while we are protected. - Reserved { ty_is_freeze, .. } if protected => - Reserved { ty_is_freeze, conflicted: true }, + ReservedFrz { .. } if protected => ReservedFrz { conflicted: true }, // Before activation and without protectors, foreign reads are fine. // That's the entire point of 2-phase borrows. - res @ Reserved { .. } => res, + res @ (ReservedFrz { .. } | ReservedIM) => { + // Even though we haven't checked `ReservedIM if protected` separately, + // it is a state that cannot occur because under a protector we only + // create `ReservedFrz` never `ReservedIM`. + assert!(!protected); + res + } Active => if protected { // We wrote, someone else reads -- that's bad. @@ -134,10 +147,10 @@ mod transition { // If the `conflicted` flag is set, then there was a foreign read during // the function call that is still ongoing (still `protected`), // this is UB (`noalias` violation). - Reserved { conflicted: true, .. } if protected => return None, + ReservedFrz { conflicted: true } if protected => return None, // A write always activates the 2-phase borrow, even with interior // mutability - Reserved { .. } | Active => Active, + ReservedFrz { .. } | ReservedIM | Active => Active, Frozen | Disabled => return None, }) } @@ -145,15 +158,15 @@ mod transition { /// A non-child node was write-accessed: this makes everything `Disabled` except for /// non-protected interior mutable `Reserved` which stay the same. fn foreign_write(state: PermissionPriv, protected: bool) -> Option { + // There is no explicit dependency on `protected`, but recall that interior mutable + // types receive a `ReservedFrz` instead of `ReservedIM` when retagged under a protector, + // so the result of this function does indirectly depend on (past) protector status. Some(match state { - // FIXME: since the fix related to reservedim_spurious_write, it is now possible - // to express these transitions of the state machine without an explicit dependency - // on `protected`: because `ty_is_freeze: false` implies `!protected` then - // the line handling `Reserved { .. } if protected` could be deleted. - // This will however require optimizations to the exhaustive tests because - // fewer initial conditions are valid. - Reserved { .. } if protected => Disabled, - res @ Reserved { ty_is_freeze: false, .. } => res, + res @ ReservedIM => { + // We can never create a `ReservedIM` under a protector, only `ReservedFrz`. + assert!(!protected); + res + } _ => Disabled, }) } @@ -208,9 +221,23 @@ impl Permission { Self { inner: Active } } - /// Default initial permission of a reborrowed mutable reference. - pub fn new_reserved(ty_is_freeze: bool) -> Self { - Self { inner: Reserved { ty_is_freeze, conflicted: false } } + /// Default initial permission of a reborrowed mutable reference that is either + /// protected or not interior mutable. + fn new_reserved_frz() -> Self { + Self { inner: ReservedFrz { conflicted: false } } + } + + /// Default initial permission of an unprotected interior mutable reference. + fn new_reserved_im() -> Self { + Self { inner: ReservedIM } + } + + /// Wrapper around `new_reserved_frz` and `new_reserved_im` that decides + /// which to call based on the interior mutability and the retag kind (whether there + /// is a protector is relevant because being protected takes priority over being + /// interior mutable) + pub fn new_reserved(ty_is_freeze: bool, protected: bool) -> Self { + if ty_is_freeze || protected { Self::new_reserved_frz() } else { Self::new_reserved_im() } } /// Default initial permission of a reborrowed shared reference. @@ -224,6 +251,11 @@ impl Permission { Self { inner: Disabled } } + /// Reject `ReservedIM` that cannot exist in the presence of a protector. + pub fn compatible_with_protector(&self) -> bool { + self.inner.compatible_with_protector() + } + /// Apply the transition to the inner PermissionPriv. pub fn perform_access( kind: AccessKind, @@ -279,12 +311,9 @@ pub mod diagnostics { f, "{}", match self { - Reserved { ty_is_freeze: true, conflicted: false } => "Reserved", - Reserved { ty_is_freeze: true, conflicted: true } => "Reserved (conflicted)", - Reserved { ty_is_freeze: false, conflicted: false } => - "Reserved (interior mutable)", - Reserved { ty_is_freeze: false, conflicted: true } => - "Reserved (interior mutable, conflicted)", + ReservedFrz { conflicted: false } => "Reserved", + ReservedFrz { conflicted: true } => "Reserved (conflicted)", + ReservedIM => "Reserved (interior mutable)", Active => "Active", Frozen => "Frozen", Disabled => "Disabled", @@ -312,10 +341,9 @@ pub mod diagnostics { // and also as `diagnostics::DisplayFmtPermission.uninit` otherwise // alignment will be incorrect. match self.inner { - Reserved { ty_is_freeze: true, conflicted: false } => "Rs ", - Reserved { ty_is_freeze: true, conflicted: true } => "RsC ", - Reserved { ty_is_freeze: false, conflicted: false } => "RsM ", - Reserved { ty_is_freeze: false, conflicted: true } => "RsCM", + ReservedFrz { conflicted: false } => "Res ", + ReservedFrz { conflicted: true } => "ResC", + ReservedIM => "ReIM", Active => "Act ", Frozen => "Frz ", Disabled => "Dis ", @@ -325,13 +353,14 @@ pub mod diagnostics { impl PermTransition { /// Readable explanation of the consequences of an event. - /// Fits in the sentence "This accessed caused {trans.summary()}". + /// Fits in the sentence "This transition corresponds to {trans.summary()}". pub fn summary(&self) -> &'static str { assert!(self.is_possible()); + assert!(!self.is_noop()); match (self.from, self.to) { (_, Active) => "the first write to a 2-phase borrowed mutable reference", (_, Frozen) => "a loss of write permissions", - (Reserved { conflicted: false, .. }, Reserved { conflicted: true, .. }) => + (ReservedFrz { conflicted: false }, ReservedFrz { conflicted: true }) => "a temporary loss of write permissions until function exit", (Frozen, Disabled) => "a loss of read permissions", (_, Disabled) => "a loss of read and write permissions", @@ -380,28 +409,33 @@ pub mod diagnostics { (Frozen, Frozen) => true, (Active, Frozen) => true, (Disabled, Disabled) => true, - (Reserved { conflicted: true, .. }, Reserved { conflicted: true, .. }) => - true, + ( + ReservedFrz { conflicted: true, .. }, + ReservedFrz { conflicted: true, .. }, + ) => true, // A pointer being `Disabled` is a strictly stronger source of // errors than it being `Frozen`. If we try to access a `Disabled`, // then where it became `Frozen` (or `Active` or `Reserved`) is the least // of our concerns for now. - (Reserved { conflicted: true, .. } | Active | Frozen, Disabled) => false, - (Reserved { conflicted: true, .. }, Frozen) => false, + (ReservedFrz { conflicted: true } | Active | Frozen, Disabled) => false, + (ReservedFrz { conflicted: true }, Frozen) => false, // `Active` and `Reserved` have all permissions, so a // `ChildAccessForbidden(Reserved | Active)` can never exist. - (_, Active) | (_, Reserved { conflicted: false, .. }) => + (_, Active) | (_, ReservedFrz { conflicted: false }) => unreachable!("this permission cannot cause an error"), - // No transition has `Reserved(conflicted=false)` as its `.to` unless it's a noop. - (Reserved { conflicted: false, .. }, _) => + // No transition has `Reserved { conflicted: false }` or `ReservedIM` + // as its `.to` unless it's a noop. + (ReservedFrz { conflicted: false } | ReservedIM, _) => unreachable!("self is a noop transition"), // All transitions produced in normal executions (using `apply_access`) // change permissions in the order `Reserved -> Active -> Frozen -> Disabled`. // We assume that the error was triggered on the same location that // the transition `self` applies to, so permissions found must be increasing // in the order `self.from < self.to <= insufficient.inner` - (Active | Frozen | Disabled, Reserved { .. }) | (Disabled, Frozen) => + (Active | Frozen | Disabled, ReservedFrz { .. } | ReservedIM) + | (Disabled, Frozen) + | (ReservedFrz { .. }, ReservedIM) => unreachable!("permissions between self and err must be increasing"), } } @@ -415,8 +449,10 @@ pub mod diagnostics { // conflicted. (Active, Active) => true, (Frozen, Frozen) => true, - (Reserved { conflicted: true, .. }, Reserved { conflicted: true, .. }) => - true, + ( + ReservedFrz { conflicted: true, .. }, + ReservedFrz { conflicted: true, .. }, + ) => true, // If the error is a transition `Frozen -> Disabled`, then we don't really // care whether before that was `Reserved -> Active -> Frozen` or // `Frozen` directly. @@ -429,23 +465,23 @@ pub mod diagnostics { // -> Reserved { conflicted: true }` is inexistant or irrelevant, // and so is the `Reserved { conflicted: false } -> Active` (Active, Frozen) => false, - (Reserved { conflicted: true, .. }, _) => false, + (ReservedFrz { conflicted: true }, _) => false, (_, Disabled) => unreachable!( "permission that results in Disabled should not itself be Disabled in the first place" ), - // No transition has `Reserved { conflicted: false }` as its `.to` + // No transition has `Reserved { conflicted: false }` or `ReservedIM` as its `.to` // unless it's a noop. - (Reserved { conflicted: false, .. }, _) => + (ReservedFrz { conflicted: false } | ReservedIM, _) => unreachable!("self is a noop transition"), // Permissions only evolve in the order `Reserved -> Active -> Frozen -> Disabled`, // so permissions found must be increasing in the order // `self.from < self.to <= forbidden.from < forbidden.to`. - (Disabled, Reserved { .. } | Active | Frozen) - | (Frozen, Reserved { .. } | Active) - | (Active, Reserved { .. }) => + (Disabled, ReservedFrz { .. } | ReservedIM | Active | Frozen) + | (Frozen, ReservedFrz { .. } | ReservedIM | Active) + | (Active, ReservedFrz { .. } | ReservedIM) => unreachable!("permissions between self and err must be increasing"), } } @@ -466,9 +502,9 @@ pub mod diagnostics { #[cfg(test)] impl Permission { - pub fn is_reserved_with_conflicted(&self, expected_conflicted: bool) -> bool { + pub fn is_reserved_frz_with_conflicted(&self, expected_conflicted: bool) -> bool { match self.inner { - Reserved { conflicted, .. } => conflicted == expected_conflicted, + ReservedFrz { conflicted } => conflicted == expected_conflicted, _ => false, } } @@ -482,10 +518,9 @@ mod propagation_optimization_checks { impl Exhaustive for PermissionPriv { fn exhaustive() -> Box> { Box::new( - vec![Active, Frozen, Disabled].into_iter().chain( - <[bool; 2]>::exhaustive() - .map(|[ty_is_freeze, conflicted]| Reserved { ty_is_freeze, conflicted }), - ), + vec![Active, Frozen, Disabled, ReservedIM] + .into_iter() + .chain(::exhaustive().map(|conflicted| ReservedFrz { conflicted })), ) } } @@ -525,6 +560,9 @@ mod propagation_optimization_checks { // We thus eliminate from this test and all other tests // the case where the tag is initially unprotected and later becomes protected. precondition!(old_protected || !new_protected); + if old_protected { + precondition!(old.compatible_with_protector()); + } for (access, rel_pos) in <(AccessKind, AccessRelatedness)>::exhaustive() { if let Some(new) = perform_access(access, rel_pos, old, old_protected) { assert_eq!( @@ -546,6 +584,9 @@ mod propagation_optimization_checks { for old in PermissionPriv::exhaustive() { for [old_protected, new_protected] in <[bool; 2]>::exhaustive() { precondition!(old_protected || !new_protected); + if old_protected { + precondition!(old.compatible_with_protector()); + } for rel_pos in AccessRelatedness::exhaustive() { precondition!(rel_pos.is_foreign()); if let Some(new) = perform_access(old_access, rel_pos, old, old_protected) { @@ -570,6 +611,9 @@ mod propagation_optimization_checks { reach.insert((start, start)); for (access, rel) in <(AccessKind, AccessRelatedness)>::exhaustive() { for prot in bool::exhaustive() { + if prot { + precondition!(start.compatible_with_protector()); + } if let Some(end) = transition::perform_access(access, rel, start, prot) { reach.insert((start, end)); } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs index 73717014e89..654419c099d 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs @@ -14,6 +14,15 @@ impl Exhaustive for LocationState { } } +impl LocationState { + /// Ensure that the current internal state can exist at the same time as a protector. + /// In practice this only eliminates `ReservedIM` that is never used in the presence + /// of a protector (we instead emit `ReservedFrz` on retag). + pub fn compatible_with_protector(&self) -> bool { + self.permission.compatible_with_protector() + } +} + #[test] #[rustfmt::skip] // Exhaustive check that for any starting configuration loc, @@ -30,6 +39,9 @@ fn all_read_accesses_commute() { // so the two read accesses occur under the same protector. for protected in bool::exhaustive() { for loc in LocationState::exhaustive() { + if protected { + precondition!(loc.compatible_with_protector()); + } // Apply 1 then 2. Failure here means that there is UB in the source // and we skip the check in the target. let mut loc12 = loc; @@ -61,8 +73,8 @@ fn protected_enforces_noalias() { // We want to check pairs of accesses where one is foreign and one is not. precondition!(rel1.is_foreign() != rel2.is_foreign()); for [kind1, kind2] in <[AccessKind; 2]>::exhaustive() { - for mut state in LocationState::exhaustive() { - let protected = true; + let protected = true; + for mut state in LocationState::exhaustive().filter(|s| s.compatible_with_protector()) { precondition!(state.perform_access(kind1, rel1, protected).is_ok()); precondition!(state.perform_access(kind2, rel2, protected).is_ok()); // If these were both allowed, it must have been two reads. @@ -387,6 +399,9 @@ mod spurious_read { fn retag_y(self, new_y: LocStateProt) -> Result { assert!(new_y.is_initial()); + if new_y.prot && !new_y.state.compatible_with_protector() { + return Err(()); + } // `xy_rel` changes to "mutually foreign" now: `y` can no longer be a parent of `x`. Self { y: new_y, xy_rel: RelPosXY::MutuallyForeign, ..self } .read_if_initialized(PtrSelector::Y) @@ -511,7 +526,7 @@ mod spurious_read { } #[test] - // `Reserved(aliased=false)` and `Reserved(aliased=true)` are properly indistinguishable + // `Reserved { conflicted: false }` and `Reserved { conflicted: true }` are properly indistinguishable // under the conditions where we want to insert a spurious read. fn reserved_aliased_protected_indistinguishable() { let source = LocStateProtPair { @@ -521,14 +536,16 @@ mod spurious_read { prot: true, }, y: LocStateProt { - state: LocationState::new_uninit(Permission::new_reserved(false)), + state: LocationState::new_uninit(Permission::new_reserved( + /* freeze */ true, /* protected */ true, + )), prot: true, }, }; let acc = TestAccess { ptr: PtrSelector::X, kind: AccessKind::Read }; let target = source.clone().perform_test_access(&acc).unwrap(); - assert!(source.y.state.permission.is_reserved_with_conflicted(false)); - assert!(target.y.state.permission.is_reserved_with_conflicted(true)); + assert!(source.y.state.permission.is_reserved_frz_with_conflicted(false)); + assert!(target.y.state.permission.is_reserved_frz_with_conflicted(true)); assert!(!source.distinguishable::<(), ()>(&target)) } @@ -563,7 +580,13 @@ mod spurious_read { precondition!(x_retag_perm.initialized); // And `x` just got retagged, so it must be initial. precondition!(x_retag_perm.permission.is_initial()); + // As stated earlier, `x` is always protected in the patterns we consider here. + precondition!(x_retag_perm.compatible_with_protector()); for y_protected in bool::exhaustive() { + // Finally `y` that is optionally protected must have a compatible permission. + if y_protected { + precondition!(y_current_perm.compatible_with_protector()); + } v.push(Pattern { xy_rel, x_retag_perm, y_current_perm, y_protected }); } } diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index 9145ef32c52..c23e5737280 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -26,27 +26,6 @@ pub(super) struct InitOnce { clock: VClock, } -impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} -trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { - /// Provides the closure with the next InitOnceId. Creates that InitOnce if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn init_once_get_or_create(&mut self, existing: F) -> InterpResult<'tcx, InitOnceId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, InitOnceId) -> InterpResult<'tcx, Option>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.init_onces.next_index(); - if let Some(old) = existing(this, next_index)? { - Ok(old) - } else { - let new_index = this.machine.sync.init_onces.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } - } -} - impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn init_once_get_or_create_id( @@ -56,9 +35,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, InitOnceId> { let this = self.eval_context_mut(); - this.init_once_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.init_onces)? + .ok_or_else(|| err_ub_format!("init_once has invalid ID").into()) } #[inline] diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index d0c9a4600e8..d972831c768 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -164,25 +164,29 @@ pub struct SynchronizationObjects { impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Lazily initialize the ID of this Miri sync structure. - /// ('0' indicates uninit.) + /// If memory stores '0', that indicates uninit and we generate a new instance. + /// Returns `None` if memory stores a non-zero invalid ID. + /// + /// `get_objs` must return the `IndexVec` that stores all the objects of this type. #[inline] - fn get_or_create_id( + fn get_or_create_id( &mut self, - next_id: Id, lock_op: &OpTy<'tcx>, lock_layout: TyAndLayout<'tcx>, offset: u64, + get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec, ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); let value_place = this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?; + let next_index = get_objs(this).next_index(); // Since we are lazy, this update has to be atomic. let (old, success) = this .atomic_compare_exchange_scalar( &value_place, &ImmTy::from_uint(0u32, this.machine.layouts.u32), - Scalar::from_u32(next_id.to_u32()), + Scalar::from_u32(next_index.to_u32()), AtomicRwOrd::Relaxed, // deliberately *no* synchronization AtomicReadOrd::Relaxed, false, @@ -190,76 +194,22 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { .to_scalar_pair(); Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") { - // Caller of the closure needs to allocate next_id - None + // We set the in-memory ID to `next_index`, now also create this object in the machine + // state. + let new_index = get_objs(this).push(T::default()); + assert_eq!(next_index, new_index); + Some(new_index) } else { - Some(Id::from_u32(old.to_u32().expect("layout is u32"))) + let id = Id::from_u32(old.to_u32().expect("layout is u32")); + if get_objs(this).get(id).is_none() { + // The in-memory ID is invalid. + None + } else { + Some(id) + } }) } - /// Provides the closure with the next MutexId. Creates that mutex if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn mutex_get_or_create(&mut self, existing: F) -> InterpResult<'tcx, MutexId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, MutexId) -> InterpResult<'tcx, Option>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.mutexes.next_index(); - if let Some(old) = existing(this, next_index)? { - if this.machine.sync.mutexes.get(old).is_none() { - throw_ub_format!("mutex has invalid ID"); - } - Ok(old) - } else { - let new_index = this.machine.sync.mutexes.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } - } - - /// Provides the closure with the next RwLockId. Creates that RwLock if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn rwlock_get_or_create(&mut self, existing: F) -> InterpResult<'tcx, RwLockId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, RwLockId) -> InterpResult<'tcx, Option>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.rwlocks.next_index(); - if let Some(old) = existing(this, next_index)? { - if this.machine.sync.rwlocks.get(old).is_none() { - throw_ub_format!("rwlock has invalid ID"); - } - Ok(old) - } else { - let new_index = this.machine.sync.rwlocks.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } - } - - /// Provides the closure with the next CondvarId. Creates that Condvar if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn condvar_get_or_create(&mut self, existing: F) -> InterpResult<'tcx, CondvarId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, CondvarId) -> InterpResult<'tcx, Option>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.condvars.next_index(); - if let Some(old) = existing(this, next_index)? { - if this.machine.sync.condvars.get(old).is_none() { - throw_ub_format!("condvar has invalid ID"); - } - Ok(old) - } else { - let new_index = this.machine.sync.condvars.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } - } - fn condvar_reacquire_mutex( &mut self, mutex: MutexId, @@ -293,9 +243,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, MutexId> { let this = self.eval_context_mut(); - this.mutex_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.mutexes)? + .ok_or_else(|| err_ub_format!("mutex has invalid ID").into()) } fn rwlock_get_or_create_id( @@ -305,9 +254,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, RwLockId> { let this = self.eval_context_mut(); - this.rwlock_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.rwlocks)? + .ok_or_else(|| err_ub_format!("rwlock has invalid ID").into()) } fn condvar_get_or_create_id( @@ -317,9 +265,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, CondvarId> { let this = self.eval_context_mut(); - this.condvar_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.condvars)? + .ok_or_else(|| err_ub_format!("condvar has invalid ID").into()) } #[inline] diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 1bed55743d4..92f344d13b7 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -116,7 +116,6 @@ pub enum NonHaltingDiagnostic { CreatedPointerTag(NonZero, Option, Option<(AllocId, AllocRange, ProvenanceExtra)>), /// This `Item` was popped from the borrow stack. The string explains the reason. PoppedPointerTag(Item, String), - CreatedCallId(CallId), CreatedAlloc(AllocId, Size, Align, MemoryKind), FreedAlloc(AllocId), AccessedAlloc(AllocId, AccessKind), @@ -607,7 +606,6 @@ impl<'tcx> MiriMachine<'tcx> { ("reborrow of reference to `extern type`".to_string(), DiagLevel::Warning), CreatedPointerTag(..) | PoppedPointerTag(..) - | CreatedCallId(..) | CreatedAlloc(..) | AccessedAlloc(..) | FreedAlloc(..) @@ -625,7 +623,6 @@ impl<'tcx> MiriMachine<'tcx> { "created tag {tag:?} with {perm} at {alloc_id:?}{range:?} derived from {orig_tag:?}" ), PoppedPointerTag(item, cause) => format!("popped tracked tag for item {item:?}{cause}"), - CreatedCallId(id) => format!("function call with id {id}"), CreatedAlloc(AllocId(id), size, align, kind) => format!( "created {kind} allocation of {size} bytes (alignment {align} bytes) with id {id}", diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index d781188cd0c..bb623c66892 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -118,8 +118,6 @@ pub struct MiriConfig { pub seed: Option, /// The stacked borrows pointer ids to report about pub tracked_pointer_tags: FxHashSet, - /// The stacked borrows call IDs to report about - pub tracked_call_ids: FxHashSet, /// The allocation ids to report about. pub tracked_alloc_ids: FxHashSet, /// For the tracked alloc ids, also report read/write accesses. @@ -183,7 +181,6 @@ impl Default for MiriConfig { args: vec![], seed: None, tracked_pointer_tags: FxHashSet::default(), - tracked_call_ids: FxHashSet::default(), tracked_alloc_ids: FxHashSet::default(), track_alloc_accesses: false, data_race_detector: true, @@ -460,10 +457,13 @@ pub fn eval_entry<'tcx>( ecx.handle_ice(); panic::resume_unwind(panic_payload) }); + // `Ok` can never happen. + #[cfg(not(bootstrap))] + let Err(res) = res; + #[cfg(bootstrap)] let res = match res { Err(res) => res, // `Ok` can never happen - #[cfg(bootstrap)] Ok(never) => match never {}, }; diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 1bdf9f06dcd..04837456212 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -371,6 +371,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { path_ty_layout(this, &["std", "sys", "pal", "windows", "c", name]) } + /// Helper function to get `TyAndLayout` of an array that consists of `libc` type. + fn libc_array_ty_layout(&self, name: &str, size: u64) -> TyAndLayout<'tcx> { + let this = self.eval_context_ref(); + let elem_ty_layout = this.libc_ty_layout(name); + let array_ty = Ty::new_array(*this.tcx, elem_ty_layout.ty, size); + this.layout_of(array_ty).unwrap() + } + /// Project to the given *named* field (which must be a struct or union type). fn project_field_named>( &self, diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 7a11e353f9d..f796e845a23 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -55,9 +55,9 @@ extern crate either; extern crate tracing; // The rustc crates we need -extern crate rustc_attr; extern crate rustc_apfloat; extern crate rustc_ast; +extern crate rustc_attr; extern crate rustc_const_eval; extern crate rustc_data_structures; extern crate rustc_errors; @@ -123,9 +123,7 @@ pub use crate::borrow_tracker::stacked_borrows::{ EvalContextExt as _, Item, Permission, Stack, Stacks, }; pub use crate::borrow_tracker::tree_borrows::{EvalContextExt as _, Tree}; -pub use crate::borrow_tracker::{ - BorTag, BorrowTrackerMethod, CallId, EvalContextExt as _, RetagFields, -}; +pub use crate::borrow_tracker::{BorTag, BorrowTrackerMethod, EvalContextExt as _, RetagFields}; pub use crate::clock::{Clock, Instant}; pub use crate::concurrency::{ cpu_affinity::MAX_CPUS, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 754f8cf1381..a7493d48d6a 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -455,6 +455,9 @@ pub struct MiriMachine<'tcx> { /// The table of directory descriptors. pub(crate) dirs: shims::DirTable, + /// The list of all EpollEventInterest. + pub(crate) epoll_interests: shims::EpollInterestTable, + /// This machine's monotone clock. pub(crate) clock: Clock, @@ -649,6 +652,7 @@ impl<'tcx> MiriMachine<'tcx> { isolated_op: config.isolated_op, validation: config.validation, fds: shims::FdTable::init(config.mute_stdout_stderr), + epoll_interests: shims::EpollInterestTable::new(), dirs: Default::default(), layouts, threads, @@ -787,6 +791,7 @@ impl VisitProvenance for MiriMachine<'_> { data_race, alloc_addresses, fds, + epoll_interests:_, tcx: _, isolated_op: _, validation: _, @@ -1370,7 +1375,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { let borrow_tracker = ecx.machine.borrow_tracker.as_ref(); let extra = FrameExtra { - borrow_tracker: borrow_tracker.map(|bt| bt.borrow_mut().new_frame(&ecx.machine)), + borrow_tracker: borrow_tracker.map(|bt| bt.borrow_mut().new_frame()), catch_unwind: None, timing, is_user_relevant: ecx.machine.is_user_relevant(&frame), diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 7f6f63ff5e7..d40dbdba80f 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -8,7 +8,7 @@ use rustc_middle::mir; use rustc_middle::ty; use rustc_span::Symbol; use rustc_target::{ - abi::{Align, Size}, + abi::{Align, AlignFromBytesError, Size}, spec::abi::Abi, }; @@ -199,9 +199,20 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { if i128::from(size) > this.tcx.data_layout.pointer_size.signed_int_max() { throw_ub_format!("creating an allocation larger than half the address space"); } - if !align.is_power_of_two() { - throw_ub_format!("creating allocation with non-power-of-two alignment {}", align); + if let Err(e) = Align::from_bytes(align) { + match e { + AlignFromBytesError::TooLarge(_) => { + throw_unsup_format!( + "creating allocation with alignment {align} exceeding rustc's maximum \ + supported value" + ); + } + AlignFromBytesError::NotPowerOfTwo(_) => { + throw_ub_format!("creating allocation with non-power-of-two alignment {align}"); + } + } } + Ok(()) } @@ -289,8 +300,12 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let [id, show_unnamed] = this.check_shim(abi, Abi::Rust, link_name, args)?; let id = this.read_scalar(id)?.to_u64()?; let show_unnamed = this.read_scalar(show_unnamed)?.to_bool()?; - if let Some(id) = std::num::NonZero::new(id) { - this.print_borrow_state(AllocId(id), show_unnamed)?; + if let Some(id) = std::num::NonZero::new(id).map(AllocId) + && this.get_alloc_info(id).2 == AllocKind::LiveData + { + this.print_borrow_state(id, show_unnamed)?; + } else { + eprintln!("{id} is not the ID of a live data allocation"); } } "miri_pointer_name" => { diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index a41a2883c91..7d5349f26b1 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -17,7 +17,7 @@ pub mod panic; pub mod time; pub mod tls; -pub use unix::{DirTable, FdTable}; +pub use unix::{DirTable, EpollInterestTable, FdTable}; /// What needs to be done after emulating an item (a shim or an intrinsic) is done. pub enum EmulateItemResult { diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 1b25ef05769..e3b9835e360 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -2,13 +2,15 @@ //! standard file descriptors (stdin/stdout/stderr). use std::any::Any; -use std::cell::{Ref, RefCell, RefMut}; use std::collections::BTreeMap; use std::io::{self, ErrorKind, IsTerminal, Read, SeekFrom, Write}; +use std::ops::Deref; use std::rc::Rc; +use std::rc::Weak; use rustc_target::abi::Size; +use crate::shims::unix::linux::epoll::EpollReadyEvents; use crate::shims::unix::*; use crate::*; @@ -25,7 +27,8 @@ pub trait FileDescription: std::fmt::Debug + Any { /// Reads as much as possible into the given buffer, and returns the number of bytes read. fn read<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, _bytes: &mut [u8], _ecx: &mut MiriInterpCx<'tcx>, @@ -35,7 +38,8 @@ pub trait FileDescription: std::fmt::Debug + Any { /// Writes as much as possible from the given buffer, and returns the number of bytes written. fn write<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, _bytes: &[u8], _ecx: &mut MiriInterpCx<'tcx>, @@ -46,7 +50,7 @@ pub trait FileDescription: std::fmt::Debug + Any { /// Reads as much as possible into the given buffer from a given offset, /// and returns the number of bytes read. fn pread<'tcx>( - &mut self, + &self, _communicate_allowed: bool, _bytes: &mut [u8], _offset: u64, @@ -58,7 +62,7 @@ pub trait FileDescription: std::fmt::Debug + Any { /// Writes as much as possible from the given buffer starting at a given offset, /// and returns the number of bytes written. fn pwrite<'tcx>( - &mut self, + &self, _communicate_allowed: bool, _bytes: &[u8], _offset: u64, @@ -70,7 +74,7 @@ pub trait FileDescription: std::fmt::Debug + Any { /// Seeks to the given offset (which can be relative to the beginning, end, or current position). /// Returns the new position from the start of the stream. fn seek<'tcx>( - &mut self, + &self, _communicate_allowed: bool, _offset: SeekFrom, ) -> InterpResult<'tcx, io::Result> { @@ -80,6 +84,7 @@ pub trait FileDescription: std::fmt::Debug + Any { fn close<'tcx>( self: Box, _communicate_allowed: bool, + _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { throw_unsup_format!("cannot close {}", self.name()); } @@ -97,18 +102,18 @@ pub trait FileDescription: std::fmt::Debug + Any { // so we use a default impl here. false } + + /// Check the readiness of file description. + fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + throw_unsup_format!("{}: epoll does not support this file description", self.name()); + } } impl dyn FileDescription { #[inline(always)] - pub fn downcast_ref(&self) -> Option<&T> { + pub fn downcast(&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 FileDescription for io::Stdin { @@ -117,7 +122,8 @@ impl FileDescription for io::Stdin { } fn read<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, communicate_allowed: bool, bytes: &mut [u8], _ecx: &mut MiriInterpCx<'tcx>, @@ -126,7 +132,7 @@ impl FileDescription for io::Stdin { // 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)) + Ok(Read::read(&mut { self }, bytes)) } fn is_tty(&self, communicate_allowed: bool) -> bool { @@ -140,13 +146,14 @@ impl FileDescription for io::Stdout { } fn write<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &[u8], _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result> { // We allow writing to stderr even with isolation enabled. - let result = Write::write(self, bytes); + 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 @@ -168,7 +175,8 @@ impl FileDescription for io::Stderr { } fn write<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &[u8], _ecx: &mut MiriInterpCx<'tcx>, @@ -193,7 +201,8 @@ impl FileDescription for NullOutput { } fn write<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &[u8], _ecx: &mut MiriInterpCx<'tcx>, @@ -203,36 +212,85 @@ impl FileDescription for NullOutput { } } +/// Structure contains both the file description and its unique identifier. #[derive(Clone, Debug)] -pub struct FileDescriptionRef(Rc>>); +pub struct FileDescWithId { + id: FdId, + file_description: Box, +} + +#[derive(Clone, Debug)] +pub struct FileDescriptionRef(Rc>); + +impl Deref for FileDescriptionRef { + type Target = dyn FileDescription; + + fn deref(&self) -> &Self::Target { + &*self.0.file_description + } +} impl FileDescriptionRef { - fn new(fd: impl FileDescription) -> Self { - FileDescriptionRef(Rc::new(RefCell::new(Box::new(fd)))) + fn new(fd: impl FileDescription, id: FdId) -> Self { + FileDescriptionRef(Rc::new(FileDescWithId { id, file_description: Box::new(fd) })) } - pub fn borrow(&self) -> Ref<'_, dyn FileDescription> { - Ref::map(self.0.borrow(), |fd| fd.as_ref()) - } - - pub fn borrow_mut(&self) -> RefMut<'_, dyn FileDescription> { - RefMut::map(self.0.borrow_mut(), |fd| fd.as_mut()) - } - - pub fn close<'ctx>(self, communicate_allowed: bool) -> InterpResult<'ctx, io::Result<()>> { + pub fn close<'tcx>( + self, + communicate_allowed: bool, + ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result<()>> { // Destroy this `Rc` using `into_inner` so we can call `close` instead of // implicitly running the destructor of the file description. + let id = self.get_id(); match Rc::into_inner(self.0) { - Some(fd) => RefCell::into_inner(fd).close(communicate_allowed), + Some(fd) => { + // Remove entry from the global epoll_event_interest table. + ecx.machine.epoll_interests.remove(id); + + fd.file_description.close(communicate_allowed, ecx) + } None => Ok(Ok(())), } } + + pub fn downgrade(&self) -> WeakFileDescriptionRef { + WeakFileDescriptionRef { weak_ref: Rc::downgrade(&self.0) } + } + + pub fn get_id(&self) -> FdId { + self.0.id + } } +/// Holds a weak reference to the actual file description. +#[derive(Clone, Debug, Default)] +pub struct WeakFileDescriptionRef { + weak_ref: Weak>, +} + +impl WeakFileDescriptionRef { + pub fn upgrade(&self) -> Option { + if let Some(file_desc_with_id) = self.weak_ref.upgrade() { + return Some(FileDescriptionRef(file_desc_with_id)); + } + None + } +} + +/// A unique id for file descriptions. While we could use the address, considering that +/// is definitely unique, the address would expose interpreter internal state when used +/// for sorting things. So instead we generate a unique id per file description that stays +/// the same even if a file descriptor is duplicated and gets a new integer file descriptor. +#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd)] +pub struct FdId(usize); + /// The file descriptor table #[derive(Debug)] pub struct FdTable { - fds: BTreeMap, + pub fds: BTreeMap, + /// Unique identifier for file description, used to differentiate between various file description. + next_file_description_id: FdId, } impl VisitProvenance for FdTable { @@ -243,7 +301,7 @@ impl VisitProvenance for FdTable { impl FdTable { fn new() -> Self { - FdTable { fds: BTreeMap::new() } + FdTable { fds: BTreeMap::new(), next_file_description_id: FdId(0) } } pub(crate) fn init(mute_stdout_stderr: bool) -> FdTable { let mut fds = FdTable::new(); @@ -258,10 +316,20 @@ impl FdTable { fds } + pub fn new_ref(&mut self, fd: impl FileDescription) -> FileDescriptionRef { + let file_handle = FileDescriptionRef::new(fd, self.next_file_description_id); + self.next_file_description_id = FdId(self.next_file_description_id.0.strict_add(1)); + file_handle + } + /// Insert a new file description to the FdTable. pub fn insert_new(&mut self, fd: impl FileDescription) -> i32 { - let file_handle = FileDescriptionRef::new(fd); - self.insert_ref_with_min_fd(file_handle, 0) + let fd_ref = self.new_ref(fd); + self.insert(fd_ref) + } + + pub fn insert(&mut self, fd_ref: FileDescriptionRef) -> i32 { + self.insert_ref_with_min_fd(fd_ref, 0) } /// Insert a file description, giving it a file descriptor that is at least `min_fd`. @@ -291,17 +359,7 @@ impl FdTable { new_fd } - pub fn get(&self, fd: i32) -> Option> { - let fd = self.fds.get(&fd)?; - Some(fd.borrow()) - } - - pub fn get_mut(&self, fd: i32) -> Option> { - let fd = self.fds.get(&fd)?; - Some(fd.borrow_mut()) - } - - pub fn get_ref(&self, fd: i32) -> Option { + pub fn get(&self, fd: i32) -> Option { let fd = self.fds.get(&fd)?; Some(fd.clone()) } @@ -320,7 +378,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let Some(dup_fd) = this.machine.fds.get_ref(old_fd) else { + let Some(dup_fd) = this.machine.fds.get(old_fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; Ok(Scalar::from_i32(this.machine.fds.insert_ref_with_min_fd(dup_fd, 0))) @@ -329,7 +387,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let Some(dup_fd) = this.machine.fds.get_ref(old_fd) else { + let Some(dup_fd) = this.machine.fds.get(old_fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; if new_fd != old_fd { @@ -337,7 +395,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // If old_fd and new_fd point to the same description, then `dup_fd` ensures we keep the underlying file description alive. if let Some(file_description) = this.machine.fds.fds.insert(new_fd, dup_fd) { // Ignore close error (not interpreter's) according to dup2() doc. - file_description.close(this.machine.communicate())?.ok(); + file_description.close(this.machine.communicate(), this)?.ok(); } } Ok(Scalar::from_i32(new_fd)) @@ -415,7 +473,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } let start = this.read_scalar(&args[2])?.to_i32()?; - match this.machine.fds.get_ref(fd) { + match this.machine.fds.get(fd) { Some(dup_fd) => Ok(Scalar::from_i32(this.machine.fds.insert_ref_with_min_fd(dup_fd, start))), None => Ok(Scalar::from_i32(this.fd_not_found()?)), @@ -442,7 +500,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let Some(file_description) = this.machine.fds.remove(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - let result = file_description.close(this.machine.communicate())?; + let result = file_description.close(this.machine.communicate(), this)?; // return `0` if close is successful let result = result.map(|()| 0i32); Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) @@ -488,7 +546,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let communicate = this.machine.communicate(); // We temporarily dup the FD to be able to retain mutable access to `this`. - let Some(fd) = this.machine.fds.get_ref(fd) else { + let Some(fd) = this.machine.fds.get(fd) else { trace!("read: FD not found"); return Ok(Scalar::from_target_isize(this.fd_not_found()?, this)); }; @@ -499,17 +557,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // `usize::MAX` because it is bounded by the host's `isize`. let mut bytes = vec![0; usize::try_from(count).unwrap()]; let result = match offset { - None => fd.borrow_mut().read(communicate, &mut bytes, this), + None => fd.read(&fd, communicate, &mut bytes, this), Some(offset) => { let Ok(offset) = u64::try_from(offset) else { let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; return Ok(Scalar::from_target_isize(-1, this)); }; - fd.borrow_mut().pread(communicate, &mut bytes, offset, this) + fd.pread(communicate, &mut bytes, offset, this) } }; - drop(fd); // `File::read` never returns a value larger than `count`, so this cannot fail. match result?.map(|c| i64::try_from(c).unwrap()) { @@ -553,22 +610,21 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?.to_owned(); // We temporarily dup the FD to be able to retain mutable access to `this`. - let Some(fd) = this.machine.fds.get_ref(fd) else { + let Some(fd) = this.machine.fds.get(fd) else { return Ok(Scalar::from_target_isize(this.fd_not_found()?, this)); }; let result = match offset { - None => fd.borrow_mut().write(communicate, &bytes, this), + None => fd.write(&fd, communicate, &bytes, this), Some(offset) => { let Ok(offset) = u64::try_from(offset) else { let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; return Ok(Scalar::from_target_isize(-1, this)); }; - fd.borrow_mut().pwrite(communicate, &bytes, offset, this) + fd.pwrite(communicate, &bytes, offset, this) } }; - drop(fd); let result = result?.map(|c| i64::try_from(c).unwrap()); Ok(Scalar::from_target_isize(this.try_unwrap_io_result(result)?, this)) diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 57930f9807d..273a99b3116 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -288,14 +288,25 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(result, dest)?; } - // Sockets + // Unnamed sockets and pipes "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)?; } + "pipe" => { + let [pipefd] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.pipe2(pipefd, /*flags*/ None)?; + this.write_scalar(result, dest)?; + } + "pipe2" => { + let [pipefd, flags] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.pipe2(pipefd, Some(flags))?; + this.write_scalar(result, dest)?; + } // Time "gettimeofday" => { @@ -815,6 +826,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.handle_miri_start_unwind(payload)?; return Ok(EmulateItemResult::NeedsUnwind); } + "getuid" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + // For now, just pretend we always have this fixed UID. + this.write_int(UID, 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. @@ -877,13 +893,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_null(dest)?; } - "getuid" - if this.frame_in_std() => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - // For now, just pretend we always have this fixed UID. - this.write_int(super::UID, dest)?; - } - "getpwuid_r" | "__posix_getpwuid_r" if this.frame_in_std() => { // getpwuid_r is the standard name, __posix_getpwuid_r is used on solarish @@ -898,7 +907,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let result = this.deref_pointer(result)?; // Must be for "us". - if uid != crate::shims::unix::UID { + if uid != UID { throw_unsup_format!("`getpwuid_r` on other users is not supported"); } diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index d93374db818..80f4b89bf34 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -12,6 +12,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_target::abi::Size; use crate::shims::os_str::bytes_to_os_str; +use crate::shims::unix::fd::FileDescriptionRef; use crate::shims::unix::*; use crate::*; use shims::time::system_time_to_duration; @@ -30,27 +31,29 @@ impl FileDescription for FileHandle { } fn read<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, communicate_allowed: bool, bytes: &mut [u8], _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); - Ok(self.file.read(bytes)) + Ok((&mut &self.file).read(bytes)) } fn write<'tcx>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, communicate_allowed: bool, bytes: &[u8], _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); - Ok(self.file.write(bytes)) + Ok((&mut &self.file).write(bytes)) } fn pread<'tcx>( - &mut self, + &self, communicate_allowed: bool, bytes: &mut [u8], offset: u64, @@ -60,13 +63,13 @@ impl FileDescription for FileHandle { // Emulates pread using seek + read + seek to restore cursor position. // Correctness of this emulation relies on sequential nature of Miri execution. // The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`. + let file = &mut &self.file; let mut f = || { - let cursor_pos = self.file.stream_position()?; - self.file.seek(SeekFrom::Start(offset))?; - let res = self.file.read(bytes); + let cursor_pos = file.stream_position()?; + file.seek(SeekFrom::Start(offset))?; + let res = file.read(bytes); // Attempt to restore cursor position even if the read has failed - self.file - .seek(SeekFrom::Start(cursor_pos)) + file.seek(SeekFrom::Start(cursor_pos)) .expect("failed to restore file position, this shouldn't be possible"); res }; @@ -74,7 +77,7 @@ impl FileDescription for FileHandle { } fn pwrite<'tcx>( - &mut self, + &self, communicate_allowed: bool, bytes: &[u8], offset: u64, @@ -84,13 +87,13 @@ impl FileDescription for FileHandle { // Emulates pwrite using seek + write + seek to restore cursor position. // Correctness of this emulation relies on sequential nature of Miri execution. // The closure is used to emulate `try` block, since we "bubble" `io::Error` using `?`. + let file = &mut &self.file; let mut f = || { - let cursor_pos = self.file.stream_position()?; - self.file.seek(SeekFrom::Start(offset))?; - let res = self.file.write(bytes); + let cursor_pos = file.stream_position()?; + file.seek(SeekFrom::Start(offset))?; + let res = file.write(bytes); // Attempt to restore cursor position even if the write has failed - self.file - .seek(SeekFrom::Start(cursor_pos)) + file.seek(SeekFrom::Start(cursor_pos)) .expect("failed to restore file position, this shouldn't be possible"); res }; @@ -98,17 +101,18 @@ impl FileDescription for FileHandle { } fn seek<'tcx>( - &mut self, + &self, communicate_allowed: bool, offset: SeekFrom, ) -> InterpResult<'tcx, io::Result> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); - Ok(self.file.seek(offset)) + Ok((&mut &self.file).seek(offset)) } fn close<'tcx>( self: Box, communicate_allowed: bool, + _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); // We sync the file if it was opened in a mode different than read-only. @@ -576,7 +580,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let communicate = this.machine.communicate(); - let Some(mut file_description) = this.machine.fds.get_mut(fd) else { + let Some(file_description) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i64(this.fd_not_found()?)); }; let result = file_description @@ -1272,7 +1276,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // FIXME: Support ftruncate64 for all FDs let FileHandle { file, writable } = - file_description.downcast_ref::().ok_or_else(|| { + file_description.downcast::().ok_or_else(|| { err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors") })?; @@ -1324,7 +1328,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; // Only regular files support synchronization. let FileHandle { file, writable } = - file_description.downcast_ref::().ok_or_else(|| { + file_description.downcast::().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); @@ -1349,7 +1353,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; // Only regular files support synchronization. let FileHandle { file, writable } = - file_description.downcast_ref::().ok_or_else(|| { + file_description.downcast::().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); @@ -1397,7 +1401,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; // Only regular files support synchronization. let FileHandle { file, writable } = - file_description.downcast_ref::().ok_or_else(|| { + file_description.downcast::().ok_or_else(|| { err_unsup_format!( "`sync_data_range` is only supported on file-backed file descriptors" ) @@ -1704,7 +1708,7 @@ impl FileMetadata { }; let file = &file_description - .downcast_ref::() + .downcast::() .ok_or_else(|| { err_unsup_format!( "obtaining metadata is only supported on file-backed file descriptors" diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 9127db3d004..fb1e0afdf9e 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -1,32 +1,127 @@ +use std::cell::RefCell; +use std::collections::BTreeMap; use std::io; +use std::rc::{Rc, Weak}; -use rustc_data_structures::fx::FxHashMap; - +use crate::shims::unix::fd::{FdId, FileDescriptionRef}; use crate::shims::unix::*; use crate::*; /// 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, + /// A map of EpollEventInterests registered under this epoll instance. + /// Each entry is differentiated using FdId and file descriptor value. + interest_list: RefCell>>>, + /// A map of EpollEventInstance that will be returned when `epoll_wait` is called. + /// Similar to interest_list, the entry is also differentiated using FdId + /// and file descriptor value. + // This is an Rc because EpollInterest need to hold a reference to update + // it. + ready_list: Rc>>, } -/// Epoll Events associate events with data. -/// These fields are currently unused by miri. -/// This matches the `epoll_event` struct defined +/// EpollEventInstance contains information that will be returned by epoll_wait. +#[derive(Debug)] +pub struct EpollEventInstance { + /// Xor-ed event types that happened to the file description. + events: u32, + /// Original data retrieved from `epoll_event` during `epoll_ctl`. + data: u64, +} + +impl EpollEventInstance { + pub fn new(events: u32, data: u64) -> EpollEventInstance { + EpollEventInstance { events, data } + } +} + +/// EpollEventInterest registers the file description information to an epoll +/// instance during a successful `epoll_ctl` call. It also stores additional +/// information needed to check and update readiness state for `epoll_wait`. +/// +/// `events` and `data` field 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)] +pub struct EpollEventInterest { + /// The file descriptor value of the file description registered. + file_descriptor: i32, + /// The events bitmask retrieved from `epoll_event`. events: u32, - /// `Scalar` is used to represent the - /// `epoll_data` type union. - #[allow(dead_code)] - data: Scalar, + /// The data retrieved from `epoll_event`. + /// libc's data field in epoll_event can store integer or pointer, + /// but only u64 is supported for now. + /// + data: u64, + /// Ready list of the epoll instance under which this EpollEventInterest is registered. + ready_list: Rc>>, +} + +/// EpollReadyEvents reflects the readiness of a file description. +pub struct EpollReadyEvents { + /// The associated file is available for read(2) operations, in the sense that a read will not block. + /// (I.e., returning EOF is considered "ready".) + pub epollin: bool, + /// The associated file is available for write(2) operations, in the sense that a write will not block. + pub epollout: bool, + /// Stream socket peer closed connection, or shut down writing + /// half of connection. + pub epollrdhup: bool, + /// For stream socket, this event merely indicates that the peer + /// closed its end of the channel. + /// Unlike epollrdhup, this should only be set when the stream is fully closed. + /// epollrdhup also gets set when only the write half is closed, which is possible + /// via `shutdown(_, SHUT_WR)`. + pub epollhup: bool, + /// Error condition happened on the associated file descriptor. + pub epollerr: bool, +} + +impl EpollReadyEvents { + pub fn new() -> Self { + EpollReadyEvents { + epollin: false, + epollout: false, + epollrdhup: false, + epollhup: false, + epollerr: false, + } + } + + pub fn get_event_bitmask<'tcx>(&self, ecx: &MiriInterpCx<'tcx>) -> u32 { + let epollin = ecx.eval_libc_u32("EPOLLIN"); + let epollout = ecx.eval_libc_u32("EPOLLOUT"); + let epollrdhup = ecx.eval_libc_u32("EPOLLRDHUP"); + let epollhup = ecx.eval_libc_u32("EPOLLHUP"); + let epollerr = ecx.eval_libc_u32("EPOLLERR"); + + let mut bitmask = 0; + if self.epollin { + bitmask |= epollin; + } + if self.epollout { + bitmask |= epollout; + } + if self.epollrdhup { + bitmask |= epollrdhup; + } + if self.epollhup { + bitmask |= epollhup; + } + if self.epollerr { + bitmask |= epollerr; + } + bitmask + } +} + +impl Epoll { + fn get_ready_list(&self) -> Rc>> { + Rc::clone(&self.ready_list) + } } impl FileDescription for Epoll { @@ -37,11 +132,51 @@ impl FileDescription for Epoll { fn close<'tcx>( self: Box, _communicate_allowed: bool, + _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { Ok(Ok(())) } } +/// The table of all EpollEventInterest. +/// The BTreeMap key is the FdId of an active file description registered with +/// any epoll instance. The value is a list of EpollEventInterest associated +/// with that file description. +pub struct EpollInterestTable(BTreeMap>>>); + +impl EpollInterestTable { + pub(crate) fn new() -> Self { + EpollInterestTable(BTreeMap::new()) + } + + pub fn insert_epoll_interest(&mut self, id: FdId, fd: Weak>) { + match self.0.get_mut(&id) { + Some(fds) => { + fds.push(fd); + } + None => { + let vec = vec![fd]; + self.0.insert(id, vec); + } + } + } + + pub fn get_epoll_interest(&self, id: FdId) -> Option<&Vec>>> { + self.0.get(&id) + } + + pub fn get_epoll_interest_mut( + &mut self, + id: FdId, + ) -> Option<&mut Vec>>> { + self.0.get_mut(&id) + } + + pub fn remove(&mut self, id: FdId) { + self.0.remove(&id); + } +} + impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// This function returns a file descriptor referring to the new `Epoll` instance. This file @@ -64,6 +199,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } + let mut epoll_instance = Epoll::default(); + epoll_instance.ready_list = Rc::new(RefCell::new(BTreeMap::new())); + let fd = this.machine.fds.insert_new(Epoll::default()); Ok(Scalar::from_i32(fd)) } @@ -90,48 +228,156 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let epfd = this.read_scalar(epfd)?.to_i32()?; + let epfd_value = this.read_scalar(epfd)?.to_i32()?; let op = this.read_scalar(op)?.to_i32()?; let fd = this.read_scalar(fd)?.to_i32()?; - let _event = this.read_scalar(event)?.to_pointer(this)?; + let event = this.deref_pointer_as(event, this.libc_ty_layout("epoll_event"))?; let epoll_ctl_add = this.eval_libc_i32("EPOLL_CTL_ADD"); let epoll_ctl_mod = this.eval_libc_i32("EPOLL_CTL_MOD"); let epoll_ctl_del = this.eval_libc_i32("EPOLL_CTL_DEL"); + let epollin = this.eval_libc_u32("EPOLLIN"); + let epollout = this.eval_libc_u32("EPOLLOUT"); + let epollrdhup = this.eval_libc_u32("EPOLLRDHUP"); + let epollet = this.eval_libc_u32("EPOLLET"); + let epollhup = this.eval_libc_u32("EPOLLHUP"); + let epollerr = this.eval_libc_u32("EPOLLERR"); + + // Fail on unsupported operations. + if op & epoll_ctl_add != epoll_ctl_add + && op & epoll_ctl_mod != epoll_ctl_mod + && op & epoll_ctl_del != epoll_ctl_del + { + throw_unsup_format!("epoll_ctl: encountered unknown unsupported operation {:#x}", op); + } + + // Check if epfd is a valid epoll file descriptor. + let Some(epfd) = this.machine.fds.get(epfd_value) else { + return Ok(Scalar::from_i32(this.fd_not_found()?)); + }; + let epoll_file_description = epfd + .downcast::() + .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; + + let mut interest_list = epoll_file_description.interest_list.borrow_mut(); + let ready_list = &epoll_file_description.ready_list; + + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(Scalar::from_i32(this.fd_not_found()?)); + }; + let id = file_descriptor.get_id(); if op == epoll_ctl_add || op == epoll_ctl_mod { - let event = this.deref_pointer_as(event, this.libc_ty_layout("epoll_event"))?; + // Read event bitmask and data from epoll_event passed by caller. + let mut events = this.read_scalar(&this.project_field(&event, 0)?)?.to_u32()?; + let data = this.read_scalar(&this.project_field(&event, 1)?)?.to_u64()?; - let events = this.project_field(&event, 0)?; - let events = this.read_scalar(&events)?.to_u32()?; - let data = this.project_field(&event, 1)?; - let data = this.read_scalar(&data)?; - let event = EpollEvent { events, data }; + // Unset the flag we support to discover if any unsupported flags are used. + let mut flags = events; + // epoll_wait(2) will always wait for epollhup and epollerr; it is not + // necessary to set it in events when calling epoll_ctl(). + // So we will always set these two event types. + events |= epollhup; + events |= epollerr; - let Some(mut epfd) = this.machine.fds.get_mut(epfd) else { - return Ok(Scalar::from_i32(this.fd_not_found()?)); - }; - let epfd = epfd - .downcast_mut::() - .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; + if events & epollet != epollet { + // We only support edge-triggered notification for now. + throw_unsup_format!("epoll_ctl: epollet flag must be included."); + } else { + flags &= !epollet; + } + if flags & epollin == epollin { + flags &= !epollin; + } + if flags & epollout == epollout { + flags &= !epollout; + } + if flags & epollrdhup == epollrdhup { + flags &= !epollrdhup; + } + if flags & epollhup == epollhup { + flags &= !epollhup; + } + if flags & epollerr == epollerr { + flags &= !epollerr; + } + if flags != 0 { + throw_unsup_format!( + "epoll_ctl: encountered unknown unsupported flags {:#x}", + flags + ); + } - epfd.file_descriptors.insert(fd, event); - Ok(Scalar::from_i32(0)) + let epoll_key = (id, fd); + + // Check the existence of fd in the interest list. + if op == epoll_ctl_add { + if interest_list.contains_key(&epoll_key) { + let eexist = this.eval_libc("EEXIST"); + this.set_last_error(eexist)?; + return Ok(Scalar::from_i32(-1)); + } + } else { + if !interest_list.contains_key(&epoll_key) { + let enoent = this.eval_libc("ENOENT"); + this.set_last_error(enoent)?; + return Ok(Scalar::from_i32(-1)); + } + } + + let id = file_descriptor.get_id(); + // Create an epoll_interest. + let interest = Rc::new(RefCell::new(EpollEventInterest { + file_descriptor: fd, + events, + data, + ready_list: Rc::clone(ready_list), + })); + + if op == epoll_ctl_add { + // Insert an epoll_interest to global epoll_interest list. + this.machine.epoll_interests.insert_epoll_interest(id, Rc::downgrade(&interest)); + interest_list.insert(epoll_key, interest); + } else { + // Directly modify the epoll_interest so the global epoll_event_interest table + // will be updated too. + let mut epoll_interest = interest_list.get_mut(&epoll_key).unwrap().borrow_mut(); + epoll_interest.events = events; + epoll_interest.data = data; + } + + // Readiness will be updated immediately when the epoll_event_interest is added or modified. + this.check_and_update_readiness(&file_descriptor)?; + + return Ok(Scalar::from_i32(0)); } else if op == epoll_ctl_del { - let Some(mut epfd) = this.machine.fds.get_mut(epfd) else { - return Ok(Scalar::from_i32(this.fd_not_found()?)); - }; - let epfd = epfd - .downcast_mut::() - .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; + let epoll_key = (id, fd); - epfd.file_descriptors.remove(&fd); - Ok(Scalar::from_i32(0)) - } else { - let einval = this.eval_libc("EINVAL"); - this.set_last_error(einval)?; - Ok(Scalar::from_i32(-1)) + // Remove epoll_event_interest from interest_list. + let Some(epoll_interest) = interest_list.remove(&epoll_key) else { + let enoent = this.eval_libc("ENOENT"); + this.set_last_error(enoent)?; + return Ok(Scalar::from_i32(-1)); + }; + // All related Weak will fail to upgrade after the drop. + drop(epoll_interest); + + // Remove related epoll_interest from ready list. + ready_list.borrow_mut().remove(&epoll_key); + + // Remove dangling EpollEventInterest from its global table. + // .unwrap() below should succeed because the file description id must have registered + // at least one epoll_interest, if not, it will fail when removing epoll_interest from + // interest list. + this.machine + .epoll_interests + .get_epoll_interest_mut(id) + .unwrap() + .retain(|event| event.upgrade().is_some()); + + return Ok(Scalar::from_i32(0)); } + Ok(Scalar::from_i32(-1)) } /// The `epoll_wait()` system call waits for events on the `Epoll` @@ -166,25 +412,116 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn epoll_wait( &mut self, epfd: &OpTy<'tcx>, - events: &OpTy<'tcx>, + events_op: &OpTy<'tcx>, maxevents: &OpTy<'tcx>, timeout: &OpTy<'tcx>, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let epfd = this.read_scalar(epfd)?.to_i32()?; - let _events = this.read_scalar(events)?.to_pointer(this)?; - let _maxevents = this.read_scalar(maxevents)?.to_i32()?; - let _timeout = this.read_scalar(timeout)?.to_i32()?; + let events = this.read_immediate(events_op)?; + let maxevents = this.read_scalar(maxevents)?.to_i32()?; + let timeout = this.read_scalar(timeout)?.to_i32()?; - let Some(mut epfd) = this.machine.fds.get_mut(epfd) else { + if epfd <= 0 || maxevents <= 0 { + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + return Ok(Scalar::from_i32(-1)); + } + + // This needs to come after the maxevents value check, or else maxevents.try_into().unwrap() + // will fail. + let events = this.deref_pointer_as( + &events, + this.libc_array_ty_layout("epoll_event", maxevents.try_into().unwrap()), + )?; + + // FIXME: Implement blocking support + if timeout != 0 { + throw_unsup_format!("epoll_wait: timeout value can only be 0"); + } + + let Some(epfd) = this.machine.fds.get(epfd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - let _epfd = epfd - .downcast_mut::() + let epoll_file_description = epfd + .downcast::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; - // 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"); + let ready_list = epoll_file_description.get_ready_list(); + let mut ready_list = ready_list.borrow_mut(); + let mut num_of_events: i32 = 0; + let mut array_iter = this.project_array_fields(&events)?; + + while let Some(des) = array_iter.next(this)? { + if let Some(epoll_event_instance) = ready_list_next(this, &mut ready_list) { + this.write_int_fields_named( + &[ + ("events", epoll_event_instance.events.into()), + ("u64", epoll_event_instance.data.into()), + ], + &des.1, + )?; + num_of_events = num_of_events.strict_add(1); + } else { + break; + } + } + Ok(Scalar::from_i32(num_of_events)) + } + + /// For a specific file description, get its ready events and update the corresponding ready + /// list. This function should be called whenever an event causes more bytes or an EOF to become + /// newly readable from an FD, and whenever more bytes can be written to an FD or no more future + /// writes are possible. + /// + /// This *will* report an event if anyone is subscribed to it, without any further filtering, so + /// do not call this function when an FD didn't have anything happen to it! + fn check_and_update_readiness(&self, fd_ref: &FileDescriptionRef) -> InterpResult<'tcx, ()> { + let this = self.eval_context_ref(); + let id = fd_ref.get_id(); + // Get a list of EpollEventInterest that is associated to a specific file description. + if let Some(epoll_interests) = this.machine.epoll_interests.get_epoll_interest(id) { + let epoll_ready_events = fd_ref.get_epoll_ready_events()?; + // Get the bitmask of ready events. + let ready_events = epoll_ready_events.get_event_bitmask(this); + + for weak_epoll_interest in epoll_interests { + if let Some(epoll_interest) = weak_epoll_interest.upgrade() { + // This checks if any of the events specified in epoll_event_interest.events + // match those in ready_events. + let epoll_event_interest = epoll_interest.borrow(); + let flags = epoll_event_interest.events & ready_events; + // If there is any event that we are interested in being specified as ready, + // insert an epoll_return to the ready list. + if flags != 0 { + let epoll_key = (id, epoll_event_interest.file_descriptor); + let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); + let event_instance = + EpollEventInstance::new(flags, epoll_event_interest.data); + ready_list.insert(epoll_key, event_instance); + } + } + } + } + Ok(()) } } + +/// This function takes in ready list and returns EpollEventInstance with file description +/// that is not closed. +fn ready_list_next( + ecx: &MiriInterpCx<'_>, + ready_list: &mut BTreeMap<(FdId, i32), EpollEventInstance>, +) -> Option { + while let Some((epoll_key, epoll_event_instance)) = ready_list.pop_first() { + // This ensures that we only return events that we are interested. The FD might have been closed since + // the event was generated, in which case we are not interested anymore. + // When a file description is fully closed, it gets removed from `machine.epoll_interests`, + // so we skip events whose FD is not in that map anymore. + if ecx.machine.epoll_interests.get_epoll_interest(epoll_key.0).is_some() { + return Some(epoll_event_instance); + } + } + return None; +} diff --git a/src/tools/miri/src/shims/unix/linux/eventfd.rs b/src/tools/miri/src/shims/unix/linux/eventfd.rs index 4ab8760d930..77d16a032aa 100644 --- a/src/tools/miri/src/shims/unix/linux/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux/eventfd.rs @@ -1,10 +1,13 @@ //! Linux `eventfd` implementation. +use std::cell::{Cell, RefCell}; use std::io; use std::io::{Error, ErrorKind}; use std::mem; use rustc_target::abi::Endian; +use crate::shims::unix::fd::FileDescriptionRef; +use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _}; use crate::shims::unix::*; use crate::{concurrency::VClock, *}; @@ -25,9 +28,9 @@ const MAX_COUNTER: u64 = u64::MAX - 1; 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. - counter: u64, + counter: Cell, is_nonblock: bool, - clock: VClock, + clock: RefCell, } impl FileDescription for Event { @@ -35,16 +38,29 @@ impl FileDescription for Event { "event" } + fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + // We only check the status of EPOLLIN and EPOLLOUT flags for eventfd. If other event flags + // need to be supported in the future, the check should be added here. + + Ok(EpollReadyEvents { + epollin: self.counter.get() != 0, + epollout: self.counter.get() != MAX_COUNTER, + ..EpollReadyEvents::new() + }) + } + fn close<'tcx>( self: Box, _communicate_allowed: bool, + _ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<()>> { Ok(Ok(())) } /// Read the counter in the buffer and return the counter if succeeded. fn read<'tcx>( - &mut self, + &self, + self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &mut [u8], ecx: &mut MiriInterpCx<'tcx>, @@ -54,7 +70,8 @@ impl FileDescription for Event { return Ok(Err(Error::from(ErrorKind::InvalidInput))); }; // Block when counter == 0. - if self.counter == 0 { + let counter = self.counter.get(); + if counter == 0 { if self.is_nonblock { return Ok(Err(Error::from(ErrorKind::WouldBlock))); } else { @@ -63,13 +80,17 @@ impl FileDescription for Event { } } else { // Synchronize with all prior `write` calls to this FD. - ecx.acquire_clock(&self.clock); + ecx.acquire_clock(&self.clock.borrow()); // Return the counter in the host endianness using the buffer provided by caller. *bytes = match ecx.tcx.sess.target.endian { - Endian::Little => self.counter.to_le_bytes(), - Endian::Big => self.counter.to_be_bytes(), + Endian::Little => counter.to_le_bytes(), + Endian::Big => counter.to_be_bytes(), }; - self.counter = 0; + self.counter.set(0); + // When any of the event happened, we check and update the status of all supported event + // types for current file description. + ecx.check_and_update_readiness(self_ref)?; + return Ok(Ok(U64_ARRAY_SIZE)); } } @@ -87,7 +108,8 @@ impl FileDescription for Event { /// supplied buffer is less than 8 bytes, or if an attempt is /// made to write the value 0xffffffffffffffff. fn write<'tcx>( - &mut self, + &self, + self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &[u8], ecx: &mut MiriInterpCx<'tcx>, @@ -107,13 +129,13 @@ impl FileDescription for Event { } // If the addition does not let the counter to exceed the maximum value, update the counter. // Else, block. - match self.counter.checked_add(num) { + match self.counter.get().checked_add(num) { Some(new_count @ 0..=MAX_COUNTER) => { // Future `read` calls will synchronize with this write, so update the FD clock. if let Some(clock) = &ecx.release_clock() { - self.clock.join(clock); + self.clock.borrow_mut().join(clock); } - self.counter = new_count; + self.counter.set(new_count); } None | Some(u64::MAX) => { if self.is_nonblock { @@ -124,6 +146,10 @@ impl FileDescription for Event { } } }; + // When any of the event happened, we check and update the status of all supported event + // types for current file description. + ecx.check_and_update_readiness(self_ref)?; + Ok(Ok(U64_ARRAY_SIZE)) } } @@ -178,11 +204,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("eventfd: encountered unknown unsupported flags {:#x}", flags); } - let fd = this.machine.fds.insert_new(Event { - counter: val.into(), + let fds = &mut this.machine.fds; + + let fd_value = fds.insert_new(Event { + counter: Cell::new(val.into()), is_nonblock, - clock: VClock::default(), + clock: RefCell::new(VClock::default()), }); - Ok(Scalar::from_i32(fd)) + + Ok(Scalar::from_i32(fd_value)) } } diff --git a/src/tools/miri/src/shims/unix/mod.rs b/src/tools/miri/src/shims/unix/mod.rs index dc9068fddde..7da6d7b02a2 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -4,9 +4,9 @@ mod env; mod fd; mod fs; mod mem; -mod socket; mod sync; mod thread; +mod unnamed_socket; mod android; mod freebsd; @@ -17,14 +17,15 @@ mod solarish; pub use env::UnixEnvVars; pub use fd::{FdTable, FileDescription}; pub use fs::DirTable; +pub use linux::epoll::EpollInterestTable; // All the Unix-specific extension traits pub use env::EvalContextExt as _; 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 _; +pub use unnamed_socket::EvalContextExt as _; // Make up some constants. const UID: u32 = 1000; diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs deleted file mode 100644 index 455820a9e6e..00000000000 --- a/src/tools/miri/src/shims/unix/socket.rs +++ /dev/null @@ -1,232 +0,0 @@ -use std::cell::RefCell; -use std::collections::VecDeque; -use std::io; -use std::io::{Error, ErrorKind, Read}; -use std::rc::{Rc, Weak}; - -use crate::shims::unix::*; -use crate::{concurrency::VClock, *}; - -/// The maximum capacity of the socketpair buffer in bytes. -/// This number is arbitrary as the value can always -/// be configured in the real system. -const MAX_SOCKETPAIR_BUFFER_CAPACITY: usize = 212992; - -/// Pair of connected sockets. -#[derive(Debug)] -struct SocketPair { - // By making the write link weak, a `write` can detect when all readers are - // gone, and trigger EPIPE as appropriate. - writebuf: Weak>, - readbuf: Rc>, - is_nonblock: bool, -} - -#[derive(Debug)] -struct Buffer { - buf: VecDeque, - clock: VClock, - /// Indicates if there is at least one active writer to this buffer. - /// If all writers of this buffer are dropped, buf_has_writer becomes false and we - /// indicate EOF instead of blocking. - buf_has_writer: bool, -} - -impl FileDescription for SocketPair { - fn name(&self) -> &'static str { - "socketpair" - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result<()>> { - // This is used to signal socketfd of other side that there is no writer to its readbuf. - // If the upgrade fails, there is no need to update as all read ends have been dropped. - if let Some(writebuf) = self.writebuf.upgrade() { - writebuf.borrow_mut().buf_has_writer = false; - }; - Ok(Ok(())) - } - - fn read<'tcx>( - &mut self, - _communicate_allowed: bool, - bytes: &mut [u8], - ecx: &mut MiriInterpCx<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - let request_byte_size = bytes.len(); - let mut readbuf = self.readbuf.borrow_mut(); - - // Always succeed on read size 0. - if request_byte_size == 0 { - return Ok(Ok(0)); - } - - if readbuf.buf.is_empty() { - if !readbuf.buf_has_writer { - // Socketpair with no writer and empty buffer. - // 0 bytes successfully read indicates end-of-file. - return Ok(Ok(0)); - } else { - if self.is_nonblock { - // Non-blocking socketpair with writer and empty buffer. - // https://linux.die.net/man/2/read - // EAGAIN or EWOULDBLOCK can be returned for socket, - // POSIX.1-2001 allows either error to be returned for this case. - // Since there is no ErrorKind for EAGAIN, WouldBlock is used. - return Ok(Err(Error::from(ErrorKind::WouldBlock))); - } else { - // Blocking socketpair with writer and empty buffer. - // FIXME: blocking is currently not supported - throw_unsup_format!("socketpair read: blocking isn't supported yet"); - } - } - } - - // Synchronize with all previous writes to this buffer. - // FIXME: this over-synchronizes; a more precise approach would be to - // only sync with the writes whose data we will read. - ecx.acquire_clock(&readbuf.clock); - // Do full read / partial read based on the space available. - // Conveniently, `read` exists on `VecDeque` and has exactly the desired behavior. - let actual_read_size = readbuf.buf.read(bytes).unwrap(); - return Ok(Ok(actual_read_size)); - } - - fn write<'tcx>( - &mut self, - _communicate_allowed: bool, - bytes: &[u8], - ecx: &mut MiriInterpCx<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - let write_size = bytes.len(); - // Always succeed on write size 0. - // ("If count is zero and fd refers to a file other than a regular file, the results are not specified.") - if write_size == 0 { - return Ok(Ok(0)); - } - - let Some(writebuf) = self.writebuf.upgrade() else { - // If the upgrade from Weak to Rc fails, it indicates that all read ends have been - // closed. - return Ok(Err(Error::from(ErrorKind::BrokenPipe))); - }; - let mut writebuf = writebuf.borrow_mut(); - let data_size = writebuf.buf.len(); - let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(data_size); - if available_space == 0 { - if self.is_nonblock { - // Non-blocking socketpair with a full buffer. - return Ok(Err(Error::from(ErrorKind::WouldBlock))); - } else { - // Blocking socketpair with a full buffer. - throw_unsup_format!("socketpair write: blocking isn't supported yet"); - } - } - // Remember this clock so `read` can synchronize with us. - if let Some(clock) = &ecx.release_clock() { - writebuf.clock.join(clock); - } - // Do full write / partial write based on the space available. - let actual_write_size = write_size.min(available_space); - writebuf.buf.extend(&bytes[..actual_write_size]); - return Ok(Ok(actual_write_size)); - } -} - -impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} -pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { - /// For more information on the arguments see the socketpair manpage: - /// - fn socketpair( - &mut self, - domain: &OpTy<'tcx>, - type_: &OpTy<'tcx>, - protocol: &OpTy<'tcx>, - sv: &OpTy<'tcx>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let domain = this.read_scalar(domain)?.to_i32()?; - let mut type_ = this.read_scalar(type_)?.to_i32()?; - let protocol = this.read_scalar(protocol)?.to_i32()?; - let sv = this.deref_pointer(sv)?; - - let mut is_sock_nonblock = false; - - // Parse and remove the type flags that we support. If type != 0 after removing, - // unsupported flags are used. - if type_ & this.eval_libc_i32("SOCK_STREAM") == this.eval_libc_i32("SOCK_STREAM") { - type_ &= !(this.eval_libc_i32("SOCK_STREAM")); - } - - // SOCK_NONBLOCK only exists on Linux. - if this.tcx.sess.target.os == "linux" { - if type_ & this.eval_libc_i32("SOCK_NONBLOCK") == this.eval_libc_i32("SOCK_NONBLOCK") { - is_sock_nonblock = true; - type_ &= !(this.eval_libc_i32("SOCK_NONBLOCK")); - } - if type_ & this.eval_libc_i32("SOCK_CLOEXEC") == this.eval_libc_i32("SOCK_CLOEXEC") { - type_ &= !(this.eval_libc_i32("SOCK_CLOEXEC")); - } - } - - // Fail on unsupported input. - // AF_UNIX and AF_LOCAL are synonyms, so we accept both in case - // their values differ. - if domain != this.eval_libc_i32("AF_UNIX") && domain != this.eval_libc_i32("AF_LOCAL") { - throw_unsup_format!( - "socketpair: domain {:#x} is unsupported, only AF_UNIX \ - and AF_LOCAL are allowed", - domain - ); - } else if type_ != 0 { - throw_unsup_format!( - "socketpair: type {:#x} is unsupported, only SOCK_STREAM, \ - SOCK_CLOEXEC and SOCK_NONBLOCK are allowed", - type_ - ); - } else if protocol != 0 { - throw_unsup_format!( - "socketpair: socket protocol {protocol} is unsupported, \ - only 0 is allowed", - ); - } - - let buffer1 = Rc::new(RefCell::new(Buffer { - buf: VecDeque::new(), - clock: VClock::default(), - buf_has_writer: true, - })); - - let buffer2 = Rc::new(RefCell::new(Buffer { - buf: VecDeque::new(), - clock: VClock::default(), - buf_has_writer: true, - })); - - let socketpair_0 = SocketPair { - writebuf: Rc::downgrade(&buffer1), - readbuf: Rc::clone(&buffer2), - is_nonblock: is_sock_nonblock, - }; - - let socketpair_1 = SocketPair { - writebuf: Rc::downgrade(&buffer2), - readbuf: Rc::clone(&buffer1), - is_nonblock: is_sock_nonblock, - }; - - let fds = &mut this.machine.fds; - let sv0 = fds.insert_new(socketpair_0); - let sv1 = fds.insert_new(socketpair_1); - let sv0 = Scalar::from_int(sv0, sv.layout.size); - let sv1 = Scalar::from_int(sv1, sv.layout.size); - - 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/tools/miri/src/shims/unix/unnamed_socket.rs b/src/tools/miri/src/shims/unix/unnamed_socket.rs new file mode 100644 index 00000000000..745f27398d0 --- /dev/null +++ b/src/tools/miri/src/shims/unix/unnamed_socket.rs @@ -0,0 +1,386 @@ +//! This implements "anonymous" sockets, that do not correspond to anything on the host system and +//! are entirely implemented inside Miri. +//! We also use the same infrastructure to implement unnamed pipes. + +use std::cell::{Cell, OnceCell, RefCell}; +use std::collections::VecDeque; +use std::io; +use std::io::{Error, ErrorKind, Read}; + +use crate::shims::unix::fd::{FileDescriptionRef, WeakFileDescriptionRef}; +use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _}; +use crate::shims::unix::*; +use crate::{concurrency::VClock, *}; + +/// The maximum capacity of the socketpair buffer in bytes. +/// This number is arbitrary as the value can always +/// be configured in the real system. +const MAX_SOCKETPAIR_BUFFER_CAPACITY: usize = 212992; + +/// One end of a pair of connected unnamed sockets. +#[derive(Debug)] +struct AnonSocket { + /// The buffer we are reading from, or `None` if this is the writing end of a pipe. + /// (In that case, the peer FD will be the reading end of that pipe.) + readbuf: Option>, + /// The `AnonSocket` file descriptor that is our "peer", and that holds the buffer we are + /// writing to. This is a weak reference because the other side may be closed before us; all + /// future writes will then trigger EPIPE. + peer_fd: OnceCell, + /// Indicates whether the peer has lost data when the file description is closed. + /// This flag is set to `true` if the peer's `readbuf` is non-empty at the time + /// of closure. + peer_lost_data: Cell, + is_nonblock: bool, +} + +#[derive(Debug)] +struct Buffer { + buf: VecDeque, + clock: VClock, +} + +impl Buffer { + fn new() -> Self { + Buffer { buf: VecDeque::new(), clock: VClock::default() } + } +} + +impl AnonSocket { + fn peer_fd(&self) -> &WeakFileDescriptionRef { + self.peer_fd.get().unwrap() + } +} + +impl FileDescription for AnonSocket { + fn name(&self) -> &'static str { + "socketpair" + } + + fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + // We only check the status of EPOLLIN, EPOLLOUT, EPOLLHUP and EPOLLRDHUP flags. + // If other event flags need to be supported in the future, the check should be added here. + + let mut epoll_ready_events = EpollReadyEvents::new(); + + // Check if it is readable. + if let Some(readbuf) = &self.readbuf { + if !readbuf.borrow().buf.is_empty() { + epoll_ready_events.epollin = true; + } + } else { + // Without a read buffer, reading never blocks, so we are always ready. + epoll_ready_events.epollin = true; + } + + // Check if is writable. + if let Some(peer_fd) = self.peer_fd().upgrade() { + if let Some(writebuf) = &peer_fd.downcast::().unwrap().readbuf { + let data_size = writebuf.borrow().buf.len(); + let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(data_size); + if available_space != 0 { + epoll_ready_events.epollout = true; + } + } else { + // Without a write buffer, writing never blocks. + epoll_ready_events.epollout = true; + } + } else { + // Peer FD has been closed. This always sets both the RDHUP and HUP flags + // as we do not support `shutdown` that could be used to partially close the stream. + epoll_ready_events.epollrdhup = true; + epoll_ready_events.epollhup = true; + // Since the peer is closed, even if no data is available reads will return EOF and + // writes will return EPIPE. In other words, they won't block, so we mark this as ready + // for read and write. + epoll_ready_events.epollin = true; + epoll_ready_events.epollout = true; + // If there is data lost in peer_fd, set EPOLLERR. + if self.peer_lost_data.get() { + epoll_ready_events.epollerr = true; + } + } + Ok(epoll_ready_events) + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result<()>> { + if let Some(peer_fd) = self.peer_fd().upgrade() { + // If the current readbuf is non-empty when the file description is closed, + // notify the peer that data lost has happened in current file description. + if let Some(readbuf) = &self.readbuf { + if !readbuf.borrow().buf.is_empty() { + peer_fd.downcast::().unwrap().peer_lost_data.set(true); + } + } + // Notify peer fd that close has happened, since that can unblock reads and writes. + ecx.check_and_update_readiness(&peer_fd)?; + } + Ok(Ok(())) + } + + fn read<'tcx>( + &self, + _self_ref: &FileDescriptionRef, + _communicate_allowed: bool, + bytes: &mut [u8], + ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + let request_byte_size = bytes.len(); + + // Always succeed on read size 0. + if request_byte_size == 0 { + return Ok(Ok(0)); + } + + let Some(readbuf) = &self.readbuf else { + // FIXME: This should return EBADF, but there's no nice way to do that as there's no + // corresponding ErrorKind variant. + throw_unsup_format!("reading from the write end of a pipe"); + }; + let mut readbuf = readbuf.borrow_mut(); + if readbuf.buf.is_empty() { + if self.peer_fd().upgrade().is_none() { + // Socketpair with no peer and empty buffer. + // 0 bytes successfully read indicates end-of-file. + return Ok(Ok(0)); + } else { + if self.is_nonblock { + // Non-blocking socketpair with writer and empty buffer. + // https://linux.die.net/man/2/read + // EAGAIN or EWOULDBLOCK can be returned for socket, + // POSIX.1-2001 allows either error to be returned for this case. + // Since there is no ErrorKind for EAGAIN, WouldBlock is used. + return Ok(Err(Error::from(ErrorKind::WouldBlock))); + } else { + // Blocking socketpair with writer and empty buffer. + // FIXME: blocking is currently not supported + throw_unsup_format!("socketpair read: blocking isn't supported yet"); + } + } + } + + // Synchronize with all previous writes to this buffer. + // FIXME: this over-synchronizes; a more precise approach would be to + // only sync with the writes whose data we will read. + ecx.acquire_clock(&readbuf.clock); + + // Do full read / partial read based on the space available. + // Conveniently, `read` exists on `VecDeque` and has exactly the desired behavior. + let actual_read_size = readbuf.buf.read(bytes).unwrap(); + + // Need to drop before others can access the readbuf again. + drop(readbuf); + + // A notification should be provided for the peer file description even when it can + // only write 1 byte. This implementation is not compliant with the actual Linux kernel + // implementation. For optimization reasons, the kernel will only mark the file description + // as "writable" when it can write more than a certain number of bytes. Since we + // don't know what that *certain number* is, we will provide a notification every time + // a read is successful. This might result in our epoll emulation providing more + // notifications than the real system. + if let Some(peer_fd) = self.peer_fd().upgrade() { + ecx.check_and_update_readiness(&peer_fd)?; + } + + return Ok(Ok(actual_read_size)); + } + + fn write<'tcx>( + &self, + _self_ref: &FileDescriptionRef, + _communicate_allowed: bool, + bytes: &[u8], + ecx: &mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + let write_size = bytes.len(); + // Always succeed on write size 0. + // ("If count is zero and fd refers to a file other than a regular file, the results are not specified.") + if write_size == 0 { + return Ok(Ok(0)); + } + + // We are writing to our peer's readbuf. + let Some(peer_fd) = self.peer_fd().upgrade() else { + // If the upgrade from Weak to Rc fails, it indicates that all read ends have been + // closed. + return Ok(Err(Error::from(ErrorKind::BrokenPipe))); + }; + + let Some(writebuf) = &peer_fd.downcast::().unwrap().readbuf else { + // FIXME: This should return EBADF, but there's no nice way to do that as there's no + // corresponding ErrorKind variant. + throw_unsup_format!("writing to the reading end of a pipe"); + }; + let mut writebuf = writebuf.borrow_mut(); + let data_size = writebuf.buf.len(); + let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(data_size); + if available_space == 0 { + if self.is_nonblock { + // Non-blocking socketpair with a full buffer. + return Ok(Err(Error::from(ErrorKind::WouldBlock))); + } else { + // Blocking socketpair with a full buffer. + throw_unsup_format!("socketpair write: blocking isn't supported yet"); + } + } + // Remember this clock so `read` can synchronize with us. + if let Some(clock) = &ecx.release_clock() { + writebuf.clock.join(clock); + } + // Do full write / partial write based on the space available. + let actual_write_size = write_size.min(available_space); + writebuf.buf.extend(&bytes[..actual_write_size]); + + // Need to stop accessing peer_fd so that it can be notified. + drop(writebuf); + + // Notification should be provided for peer fd as it became readable. + // The kernel does this even if the fd was already readable before, so we follow suit. + ecx.check_and_update_readiness(&peer_fd)?; + + return Ok(Ok(actual_write_size)); + } +} + +impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} +pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + /// For more information on the arguments see the socketpair manpage: + /// + fn socketpair( + &mut self, + domain: &OpTy<'tcx>, + type_: &OpTy<'tcx>, + protocol: &OpTy<'tcx>, + sv: &OpTy<'tcx>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let domain = this.read_scalar(domain)?.to_i32()?; + let mut type_ = this.read_scalar(type_)?.to_i32()?; + let protocol = this.read_scalar(protocol)?.to_i32()?; + let sv = this.deref_pointer(sv)?; + + let mut is_sock_nonblock = false; + + // Parse and remove the type flags that we support. + // SOCK_NONBLOCK only exists on Linux. + if this.tcx.sess.target.os == "linux" { + if type_ & this.eval_libc_i32("SOCK_NONBLOCK") == this.eval_libc_i32("SOCK_NONBLOCK") { + is_sock_nonblock = true; + type_ &= !(this.eval_libc_i32("SOCK_NONBLOCK")); + } + if type_ & this.eval_libc_i32("SOCK_CLOEXEC") == this.eval_libc_i32("SOCK_CLOEXEC") { + type_ &= !(this.eval_libc_i32("SOCK_CLOEXEC")); + } + } + + // Fail on unsupported input. + // AF_UNIX and AF_LOCAL are synonyms, so we accept both in case + // their values differ. + if domain != this.eval_libc_i32("AF_UNIX") && domain != this.eval_libc_i32("AF_LOCAL") { + throw_unsup_format!( + "socketpair: domain {:#x} is unsupported, only AF_UNIX \ + and AF_LOCAL are allowed", + domain + ); + } else if type_ != this.eval_libc_i32("SOCK_STREAM") { + throw_unsup_format!( + "socketpair: type {:#x} is unsupported, only SOCK_STREAM, \ + SOCK_CLOEXEC and SOCK_NONBLOCK are allowed", + type_ + ); + } else if protocol != 0 { + throw_unsup_format!( + "socketpair: socket protocol {protocol} is unsupported, \ + only 0 is allowed", + ); + } + + // Generate file descriptions. + let fds = &mut this.machine.fds; + let fd0 = fds.new_ref(AnonSocket { + readbuf: Some(RefCell::new(Buffer::new())), + peer_fd: OnceCell::new(), + peer_lost_data: Cell::new(false), + is_nonblock: is_sock_nonblock, + }); + let fd1 = fds.new_ref(AnonSocket { + readbuf: Some(RefCell::new(Buffer::new())), + peer_fd: OnceCell::new(), + peer_lost_data: Cell::new(false), + is_nonblock: is_sock_nonblock, + }); + + // Make the file descriptions point to each other. + fd0.downcast::().unwrap().peer_fd.set(fd1.downgrade()).unwrap(); + fd1.downcast::().unwrap().peer_fd.set(fd0.downgrade()).unwrap(); + + // Insert the file description to the fd table, generating the file descriptors. + let sv0 = fds.insert(fd0); + let sv1 = fds.insert(fd1); + + // Return socketpair file descriptors to the caller. + let sv0 = Scalar::from_int(sv0, sv.layout.size); + let sv1 = Scalar::from_int(sv1, sv.layout.size); + this.write_scalar(sv0, &sv)?; + this.write_scalar(sv1, &sv.offset(sv.layout.size, sv.layout, this)?)?; + + Ok(Scalar::from_i32(0)) + } + + fn pipe2( + &mut self, + pipefd: &OpTy<'tcx>, + flags: Option<&OpTy<'tcx>>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let pipefd = this.deref_pointer(pipefd)?; + let flags = match flags { + Some(flags) => this.read_scalar(flags)?.to_i32()?, + None => 0, + }; + + // As usual we ignore CLOEXEC. + let cloexec = this.eval_libc_i32("O_CLOEXEC"); + if flags != 0 && flags != cloexec { + throw_unsup_format!("unsupported flags in `pipe2`"); + } + + // Generate file descriptions. + // pipefd[0] refers to the read end of the pipe. + let fds = &mut this.machine.fds; + let fd0 = fds.new_ref(AnonSocket { + readbuf: Some(RefCell::new(Buffer::new())), + peer_fd: OnceCell::new(), + peer_lost_data: Cell::new(false), + is_nonblock: false, + }); + let fd1 = fds.new_ref(AnonSocket { + readbuf: None, + peer_fd: OnceCell::new(), + peer_lost_data: Cell::new(false), + is_nonblock: false, + }); + + // Make the file descriptions point to each other. + fd0.downcast::().unwrap().peer_fd.set(fd1.downgrade()).unwrap(); + fd1.downcast::().unwrap().peer_fd.set(fd0.downgrade()).unwrap(); + + // Insert the file description to the fd table, generating the file descriptors. + let pipefd0 = fds.insert(fd0); + let pipefd1 = fds.insert(fd1); + + // Return file descriptors to the caller. + let pipefd0 = Scalar::from_int(pipefd0, pipefd.layout.size); + let pipefd1 = Scalar::from_int(pipefd1, pipefd.layout.size); + this.write_scalar(pipefd0, &pipefd)?; + this.write_scalar(pipefd1, &pipefd.offset(pipefd.layout.size, pipefd.layout, this)?)?; + + Ok(Scalar::from_i32(0)) + } +} diff --git a/src/tools/miri/src/shims/x86/avx.rs b/src/tools/miri/src/shims/x86/avx.rs index f36bb4826e4..2f6569e1823 100644 --- a/src/tools/miri/src/shims/x86/avx.rs +++ b/src/tools/miri/src/shims/x86/avx.rs @@ -73,13 +73,12 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { round_all::(this, op, rounding, dest)?; } - // Used to implement _mm256_{sqrt,rcp,rsqrt}_ps functions. + // Used to implement _mm256_{rcp,rsqrt}_ps functions. // Performs the operations on all components of `op`. - "sqrt.ps.256" | "rcp.ps.256" | "rsqrt.ps.256" => { + "rcp.ps.256" | "rsqrt.ps.256" => { let [op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let which = match unprefixed_name { - "sqrt.ps.256" => FloatUnaryOp::Sqrt, "rcp.ps.256" => FloatUnaryOp::Rcp, "rsqrt.ps.256" => FloatUnaryOp::Rsqrt, _ => unreachable!(), diff --git a/src/tools/miri/src/shims/x86/mod.rs b/src/tools/miri/src/shims/x86/mod.rs index 1bd32fce8bd..c1117e4d811 100644 --- a/src/tools/miri/src/shims/x86/mod.rs +++ b/src/tools/miri/src/shims/x86/mod.rs @@ -159,8 +159,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { #[derive(Copy, Clone)] enum FloatBinOp { - /// Arithmetic operation - Arith(mir::BinOp), /// Comparison /// /// The semantics of this operator is a case distinction: we compare the two operands, @@ -247,16 +245,11 @@ impl FloatBinOp { /// Performs `which` scalar operation on `left` and `right` and returns /// the result. fn bin_op_float<'tcx, F: rustc_apfloat::Float>( - this: &crate::MiriInterpCx<'tcx>, which: FloatBinOp, left: &ImmTy<'tcx>, right: &ImmTy<'tcx>, ) -> InterpResult<'tcx, Scalar> { match which { - FloatBinOp::Arith(which) => { - let res = this.binary_op(which, left, right)?; - Ok(res.to_scalar()) - } FloatBinOp::Cmp { gt, lt, eq, unord } => { let left = left.to_scalar().to_float::()?; let right = right.to_scalar().to_float::()?; @@ -323,7 +316,6 @@ fn bin_op_simd_float_first<'tcx, F: rustc_apfloat::Float>( assert_eq!(dest_len, right_len); let res0 = bin_op_float::( - this, which, &this.read_immediate(&this.project_index(&left, 0)?)?, &this.read_immediate(&this.project_index(&right, 0)?)?, @@ -358,7 +350,7 @@ fn bin_op_simd_float_all<'tcx, F: rustc_apfloat::Float>( let right = this.read_immediate(&this.project_index(&right, i)?)?; let dest = this.project_index(&dest, i)?; - let res = bin_op_float::(this, which, &left, &right)?; + let res = bin_op_float::(which, &left, &right)?; this.write_scalar(res, &dest)?; } @@ -367,11 +359,6 @@ fn bin_op_simd_float_all<'tcx, F: rustc_apfloat::Float>( #[derive(Copy, Clone)] enum FloatUnaryOp { - /// sqrt(x) - /// - /// - /// - Sqrt, /// Approximation of 1/x /// /// @@ -392,11 +379,6 @@ fn unary_op_f32<'tcx>( op: &ImmTy<'tcx>, ) -> InterpResult<'tcx, Scalar> { match which { - FloatUnaryOp::Sqrt => { - let op = op.to_scalar(); - // FIXME using host floats - Ok(Scalar::from_u32(f32::from_bits(op.to_u32()?).sqrt().to_bits())) - } FloatUnaryOp::Rcp => { let op = op.to_scalar().to_f32()?; let div = (Single::from_u128(1).value / op).value; diff --git a/src/tools/miri/src/shims/x86/sse.rs b/src/tools/miri/src/shims/x86/sse.rs index 32e8e8a66c1..07a4eaa85f3 100644 --- a/src/tools/miri/src/shims/x86/sse.rs +++ b/src/tools/miri/src/shims/x86/sse.rs @@ -1,5 +1,4 @@ use rustc_apfloat::ieee::Single; -use rustc_middle::mir; use rustc_span::Symbol; use rustc_target::spec::abi::Abi; @@ -29,18 +28,14 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // performed only on the first element, copying the remaining elements from the input // vector (for binary operations, from the left-hand side). match unprefixed_name { - // Used to implement _mm_{add,sub,mul,div,min,max}_ss functions. + // Used to implement _mm_{min,max}_ss functions. // Performs the operations on the first component of `left` and // `right` and copies the remaining components from `left`. - "add.ss" | "sub.ss" | "mul.ss" | "div.ss" | "min.ss" | "max.ss" => { + "min.ss" | "max.ss" => { let [left, right] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let which = match unprefixed_name { - "add.ss" => FloatBinOp::Arith(mir::BinOp::Add), - "sub.ss" => FloatBinOp::Arith(mir::BinOp::Sub), - "mul.ss" => FloatBinOp::Arith(mir::BinOp::Mul), - "div.ss" => FloatBinOp::Arith(mir::BinOp::Div), "min.ss" => FloatBinOp::Min, "max.ss" => FloatBinOp::Max, _ => unreachable!(), @@ -65,14 +60,13 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { bin_op_simd_float_all::(this, which, left, right, dest)?; } - // Used to implement _mm_{sqrt,rcp,rsqrt}_ss functions. + // Used to implement _mm_{rcp,rsqrt}_ss functions. // Performs the operations on the first component of `op` and // copies the remaining components from `op`. - "sqrt.ss" | "rcp.ss" | "rsqrt.ss" => { + "rcp.ss" | "rsqrt.ss" => { let [op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let which = match unprefixed_name { - "sqrt.ss" => FloatUnaryOp::Sqrt, "rcp.ss" => FloatUnaryOp::Rcp, "rsqrt.ss" => FloatUnaryOp::Rsqrt, _ => unreachable!(), @@ -82,11 +76,10 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } // Used to implement _mm_{sqrt,rcp,rsqrt}_ps functions. // Performs the operations on all components of `op`. - "sqrt.ps" | "rcp.ps" | "rsqrt.ps" => { + "rcp.ps" | "rsqrt.ps" => { let [op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let which = match unprefixed_name { - "sqrt.ps" => FloatUnaryOp::Sqrt, "rcp.ps" => FloatUnaryOp::Rcp, "rsqrt.ps" => FloatUnaryOp::Rsqrt, _ => unreachable!(), diff --git a/src/tools/miri/src/shims/x86/sse2.rs b/src/tools/miri/src/shims/x86/sse2.rs index 3efdd561d6c..163d74a6de4 100644 --- a/src/tools/miri/src/shims/x86/sse2.rs +++ b/src/tools/miri/src/shims/x86/sse2.rs @@ -227,46 +227,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { bin_op_simd_float_all::(this, which, left, right, dest)?; } - // Used to implement _mm_sqrt_sd functions. - // Performs the operations on the first component of `op` and - // copies the remaining components from `op`. - "sqrt.sd" => { - let [op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - let (op, op_len) = this.operand_to_simd(op)?; - let (dest, dest_len) = this.mplace_to_simd(dest)?; - - assert_eq!(dest_len, op_len); - - let op0 = this.read_scalar(&this.project_index(&op, 0)?)?.to_u64()?; - // FIXME using host floats - let res0 = Scalar::from_u64(f64::from_bits(op0).sqrt().to_bits()); - this.write_scalar(res0, &this.project_index(&dest, 0)?)?; - - for i in 1..dest_len { - this.copy_op(&this.project_index(&op, i)?, &this.project_index(&dest, i)?)?; - } - } - // Used to implement _mm_sqrt_pd functions. - // Performs the operations on all components of `op`. - "sqrt.pd" => { - let [op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - let (op, op_len) = this.operand_to_simd(op)?; - let (dest, dest_len) = this.mplace_to_simd(dest)?; - - assert_eq!(dest_len, op_len); - - for i in 0..dest_len { - let op = this.read_scalar(&this.project_index(&op, i)?)?.to_u64()?; - let dest = this.project_index(&dest, i)?; - - // FIXME using host floats - let res = Scalar::from_u64(f64::from_bits(op).sqrt().to_bits()); - - this.write_scalar(res, &dest)?; - } - } // Used to implement the _mm_cmp*_sd functions. // Performs a comparison operation on the first component of `left` // and `right`, returning 0 if false or `u64::MAX` if true. The remaining diff --git a/src/tools/miri/tests/fail-dep/tokio/sleep.rs b/src/tools/miri/tests/fail-dep/tokio/sleep.rs index d96d778e6ca..0fa5080d484 100644 --- a/src/tools/miri/tests/fail-dep/tokio/sleep.rs +++ b/src/tools/miri/tests/fail-dep/tokio/sleep.rs @@ -1,6 +1,6 @@ //@compile-flags: -Zmiri-permissive-provenance -Zmiri-backtrace=full //@only-target-x86_64-unknown-linux: support for tokio only on linux and x86 -//@error-in-other-file: returning ready events from epoll_wait is not yet implemented +//@error-in-other-file: timeout value can only be 0 //@normalize-stderr-test: " += note:.*\n" -> "" use tokio::time::{sleep, Duration, Instant}; diff --git a/src/tools/miri/tests/fail-dep/tokio/sleep.stderr b/src/tools/miri/tests/fail-dep/tokio/sleep.stderr index 6d19faab905..d5bf00fc175 100644 --- a/src/tools/miri/tests/fail-dep/tokio/sleep.stderr +++ b/src/tools/miri/tests/fail-dep/tokio/sleep.stderr @@ -1,4 +1,4 @@ -error: unsupported operation: returning ready events from epoll_wait is not yet implemented +error: unsupported operation: epoll_wait: timeout value can only be 0 --> CARGO_REGISTRY/.../epoll.rs:LL:CC | LL | / syscall!(epoll_wait( @@ -7,7 +7,7 @@ LL | | events.as_mut_ptr(), LL | | events.capacity() as i32, LL | | timeout, LL | | )) - | |__________^ returning ready events from epoll_wait is not yet implemented + | |__________^ epoll_wait: timeout value can only be 0 | = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support diff --git a/src/tools/miri/tests/fail/alloc/unsupported_big_alignment.rs b/src/tools/miri/tests/fail/alloc/unsupported_big_alignment.rs new file mode 100644 index 00000000000..08d84c461bf --- /dev/null +++ b/src/tools/miri/tests/fail/alloc/unsupported_big_alignment.rs @@ -0,0 +1,14 @@ +// Previously, attempting to allocate with an alignment greater than 2^29 would cause miri to ICE +// because rustc does not support alignments that large. +// https://github.com/rust-lang/miri/issues/3687 + +extern "Rust" { + fn __rust_alloc(size: usize, align: usize) -> *mut u8; +} + +fn main() { + unsafe { + __rust_alloc(1, 1 << 30); + //~^ERROR: exceeding rustc's maximum supported value + } +} diff --git a/src/tools/miri/tests/fail/alloc/unsupported_big_alignment.stderr b/src/tools/miri/tests/fail/alloc/unsupported_big_alignment.stderr new file mode 100644 index 00000000000..4c783b866c8 --- /dev/null +++ b/src/tools/miri/tests/fail/alloc/unsupported_big_alignment.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: creating allocation with alignment ALIGN exceeding rustc's maximum supported value + --> $DIR/unsupported_big_alignment.rs:LL:CC + | +LL | __rust_alloc(1, 1 << 30); + | ^^^^^^^^^^^^^^^^^^^^^^^^ creating allocation with alignment ALIGN exceeding rustc's maximum supported value + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/unsupported_big_alignment.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/alloc/unsupported_non_power_two_alignment.rs b/src/tools/miri/tests/fail/alloc/unsupported_non_power_two_alignment.rs new file mode 100644 index 00000000000..a4ab8094bf4 --- /dev/null +++ b/src/tools/miri/tests/fail/alloc/unsupported_non_power_two_alignment.rs @@ -0,0 +1,11 @@ +// Test non-power-of-two alignment. +extern "Rust" { + fn __rust_alloc(size: usize, align: usize) -> *mut u8; +} + +fn main() { + unsafe { + __rust_alloc(1, 3); + //~^ERROR: creating allocation with non-power-of-two alignment + } +} diff --git a/src/tools/miri/tests/fail/alloc/unsupported_non_power_two_alignment.stderr b/src/tools/miri/tests/fail/alloc/unsupported_non_power_two_alignment.stderr new file mode 100644 index 00000000000..69a6c375f47 --- /dev/null +++ b/src/tools/miri/tests/fail/alloc/unsupported_non_power_two_alignment.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: creating allocation with non-power-of-two alignment ALIGN + --> $DIR/unsupported_non_power_two_alignment.rs:LL:CC + | +LL | __rust_alloc(1, 3); + | ^^^^^^^^^^^^^^^^^^ creating allocation with non-power-of-two alignment ALIGN + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at $DIR/unsupported_non_power_two_alignment.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/both_borrows/aliasing_mut1.stack.stderr b/src/tools/miri/tests/fail/both_borrows/aliasing_mut1.stack.stderr index fe1f7060f1e..4f9e6222db2 100644 --- a/src/tools/miri/tests/fail/both_borrows/aliasing_mut1.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/aliasing_mut1.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> $DIR/aliasing_mut1.rs:LL:CC | LL | pub fn safe(x: &mut i32, y: &mut i32) { - | ^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/aliasing_mut2.stack.stderr b/src/tools/miri/tests/fail/both_borrows/aliasing_mut2.stack.stderr index c5bdfcb8fe4..54679d177da 100644 --- a/src/tools/miri/tests/fail/both_borrows/aliasing_mut2.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/aliasing_mut2.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected --> $DIR/aliasing_mut2.rs:LL:CC | LL | pub fn safe(x: &i32, y: &mut i32) { - | ^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID + | ^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/aliasing_mut4.stack.stderr b/src/tools/miri/tests/fail/both_borrows/aliasing_mut4.stack.stderr index 383eb086d1e..b3e97c92f1a 100644 --- a/src/tools/miri/tests/fail/both_borrows/aliasing_mut4.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/aliasing_mut4.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected --> $DIR/aliasing_mut4.rs:LL:CC | LL | pub fn safe(x: &i32, y: &mut Cell) { - | ^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID + | ^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.stack.stderr b/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.stack.stderr index 6e87d3ce06b..a9ea7a9e9c4 100644 --- a/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/box_noalias_violation.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is weakly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is weakly protected --> $DIR/box_noalias_violation.rs:LL:CC | LL | *y - | ^^ not granting access to tag because that would remove [Unique for ] which is weakly protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [Unique for ] which is weakly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/illegal_write6.stack.stderr b/src/tools/miri/tests/fail/both_borrows/illegal_write6.stack.stderr index 159b6cc9a8e..b5484745c41 100644 --- a/src/tools/miri/tests/fail/both_borrows/illegal_write6.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/illegal_write6.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> $DIR/illegal_write6.rs:LL:CC | LL | unsafe { *y = 2 }; - | ^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector2.stack.stderr b/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector2.stack.stderr index 5d093aeae88..11edbc3270c 100644 --- a/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector2.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector2.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | unsafe { *x = 0 }; - | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID + | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector3.stack.stderr b/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector3.stack.stderr index 8426f56004b..c6666ceac2b 100644 --- a/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector3.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/invalidate_against_protector3.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected --> $DIR/invalidate_against_protector3.rs:LL:CC | LL | unsafe { *x = 0 }; - | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID + | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr b/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr index c26c7f397b0..9f545e5687e 100644 --- a/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr b/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr index ae54da70fe2..a4111f6f5cc 100644 --- a/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr index e0d1bed6332..609426bb289 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> $DIR/arg_inplace_mutate.rs:LL:CC | LL | unsafe { ptr.write(S(0)) }; - | ^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr index 09c9a777eca..68b7c0307c8 100644 --- a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> $DIR/arg_inplace_observe_during.rs:LL:CC | LL | unsafe { ptr.read() }; - | ^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr similarity index 82% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr index eb215a2d2e8..e8b766d0b0e 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.none.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; | ^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory @@ -7,9 +7,9 @@ LL | unsafe { ptr.read() }; = 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 `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC + = note: inside `myfun` at $DIR/return_pointer_aliasing_read.rs:LL:CC note: inside `main` - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.rs similarity index 100% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.rs rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.rs diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr similarity index 81% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr index 01357f430fc..941470e9295 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.stack.stderr @@ -1,13 +1,13 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID - --> $DIR/return_pointer_aliasing.rs:LL:CC +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; - | ^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information help: was created by a SharedReadWrite retag at offsets [0x0..0x4] - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | / mir! { LL | | { @@ -18,14 +18,14 @@ LL | | } LL | | } | |_____^ help: is this argument - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; | ^^^^^^^^^^^^^^^^^^^^^ = note: BACKTRACE (of the first span): - = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC + = note: inside `myfun` at $DIR/return_pointer_aliasing_read.rs:LL:CC note: inside `main` - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr similarity index 85% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr index bbedba5a7dd..715ee330619 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_read.tree.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: read access through (root of the allocation) at ALLOC[0x0] is forbidden - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; | ^^^^^^^^^^ read access through (root of the allocation) at ALLOC[0x0] is forbidden @@ -9,7 +9,7 @@ LL | unsafe { ptr.read() }; = help: this foreign read access would cause the protected tag (currently Active) to become Disabled = help: protected tags must never be Disabled help: the accessed tag was created here - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | / mir! { LL | | { @@ -20,20 +20,20 @@ LL | | } LL | | } | |_____^ help: the protected tag was created here, in the initial state Reserved - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; | ^^^^^^^^^^^^^^^^^^^^^ help: the protected tag later transitioned to Active due to a child write access at offsets [0x0..0x4] - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; | ^^^^^^^^^^^^^^^^^^^^^ = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference = note: BACKTRACE (of the first span): - = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC + = note: inside `myfun` at $DIR/return_pointer_aliasing_read.rs:LL:CC note: inside `main` - --> $DIR/return_pointer_aliasing.rs:LL:CC + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs similarity index 73% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs index 7db641538ce..8de0b12b8b0 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.rs @@ -1,4 +1,4 @@ -// This does need an aliasing model. +// This does need an aliasing model and protectors. //@revisions: stack tree //@[tree]compile-flags: -Zmiri-tree-borrows #![feature(raw_ref_op)] @@ -14,8 +14,8 @@ pub fn main() { let _x = 0; let ptr = &raw mut _x; // We arrange for `myfun` to have a pointer that aliases - // its return place. Even just reading from that pointer is UB. - Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue()) + // its return place. Writing to that pointer is UB. + Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) } after_call = { @@ -27,7 +27,7 @@ pub fn main() { fn myfun(ptr: *mut i32) -> i32 { // This overwrites the return place, which shouldn't be possible through another pointer. unsafe { ptr.write(0) }; - //~[stack]^ ERROR: tag does not exist in the borrow stack + //~[stack]^ ERROR: strongly protected //~[tree]| ERROR: /write access .* forbidden/ 13 } diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr similarity index 56% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stack.stderr rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr index 04040827b0f..51cb270dd2e 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stack.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.stack.stderr @@ -1,16 +1,13 @@ -error: Undefined Behavior: attempting a write access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - --> $DIR/return_pointer_aliasing2.rs:LL:CC +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; - | ^^^^^^^^^^^^ - | | - | attempting a write access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of an access at ALLOC[0x0..0x4] + | ^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information help: was created by a SharedReadWrite retag at offsets [0x0..0x4] - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | / mir! { LL | | { @@ -20,18 +17,18 @@ LL | | let ptr = &raw mut _x; LL | | } LL | | } | |_____^ -help: was later invalidated at offsets [0x0..0x4] by a Unique in-place function argument/return passing protection - --> $DIR/return_pointer_aliasing2.rs:LL:CC +help: is this argument + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; | ^^^^^^^^^^^^^^^^^^^^^^^ = note: BACKTRACE (of the first span): - = note: inside `myfun` at $DIR/return_pointer_aliasing2.rs:LL:CC + = note: inside `myfun` at $DIR/return_pointer_aliasing_write.rs:LL:CC note: inside `main` - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | -LL | Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue()) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr similarity index 81% rename from src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.tree.stderr rename to src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr index 146bcfc7c47..66ca1027edc 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.tree.stderr +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write.tree.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: write access through (root of the allocation) at ALLOC[0x0] is forbidden - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; | ^^^^^^^^^^^^ write access through (root of the allocation) at ALLOC[0x0] is forbidden @@ -9,7 +9,7 @@ LL | unsafe { ptr.write(0) }; = help: this foreign write access would cause the protected tag (currently Active) to become Disabled = help: protected tags must never be Disabled help: the accessed tag was created here - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | / mir! { LL | | { @@ -20,23 +20,23 @@ LL | | } LL | | } | |_____^ help: the protected tag was created here, in the initial state Reserved - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; | ^^^^^^^^^^^^^^^^^^^^^^^ help: the protected tag later transitioned to Active due to a child write access at offsets [0x0..0x4] - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; | ^^^^^^^^^^^^^^^^^^^^^^^ = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference = note: BACKTRACE (of the first span): - = note: inside `myfun` at $DIR/return_pointer_aliasing2.rs:LL:CC + = note: inside `myfun` at $DIR/return_pointer_aliasing_write.rs:LL:CC note: inside `main` - --> $DIR/return_pointer_aliasing2.rs:LL:CC + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | -LL | Call(_x = myfun(ptr), ReturnTo(after_call), UnwindContinue()) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs new file mode 100644 index 00000000000..facc323bbc5 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.rs @@ -0,0 +1,39 @@ +// This does need an aliasing model and protectors. +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(raw_ref_op)] +#![feature(core_intrinsics)] +#![feature(custom_mir)] +#![feature(explicit_tail_calls)] +#![allow(incomplete_features)] + +use std::intrinsics::mir::*; + +#[custom_mir(dialect = "runtime", phase = "optimized")] +pub fn main() { + mir! { + { + let _x = 0; + let ptr = &raw mut _x; + // We arrange for `myfun` to have a pointer that aliases + // its return place. Writing to that pointer is UB. + Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) + } + + after_call = { + Return() + } + } +} + +fn myfun(ptr: *mut i32) -> i32 { + become myfun2(ptr) +} + +fn myfun2(ptr: *mut i32) -> i32 { + // This overwrites the return place, which shouldn't be possible through another pointer. + unsafe { ptr.write(0) }; + //~[stack]^ ERROR: strongly protected + //~[tree]| ERROR: /write access .* forbidden/ + 13 +} diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr new file mode 100644 index 00000000000..7e527a440d1 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.stack.stderr @@ -0,0 +1,37 @@ +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadWrite retag at offsets [0x0..0x4] + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let _x = 0; +LL | | let ptr = &raw mut _x; +... | +LL | | } +LL | | } + | |_____^ +help: is this argument + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `myfun2` at $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr new file mode 100644 index 00000000000..b1f2cab031e --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing_write_tail_call.tree.stderr @@ -0,0 +1,45 @@ +error: Undefined Behavior: write access through (root of the allocation) at ALLOC[0x0] is forbidden + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^ write access through (root of the allocation) at ALLOC[0x0] is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this foreign write access would cause the protected tag (currently Active) to become Disabled + = help: protected tags must never be Disabled +help: the accessed tag was created here + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let _x = 0; +LL | | let ptr = &raw mut _x; +... | +LL | | } +LL | | } + | |_____^ +help: the protected tag was created here, in the initial state Reserved + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^^^^^^^^^^^^ +help: the protected tag later transitioned to Active due to a child write access at offsets [0x0..0x4] + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^^^^^^^^^^^^ + = help: this transition corresponds to the first write to a 2-phase borrowed mutable reference + = note: BACKTRACE (of the first span): + = note: inside `myfun2` at $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | Call(*ptr = myfun(ptr), ReturnTo(after_call), UnwindContinue()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs b/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs index 9263ad381d1..860798f2ab1 100644 --- a/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs +++ b/src/tools/miri/tests/fail/intrinsics/intrinsic_target_feature.rs @@ -24,7 +24,7 @@ fn main() { unsafe { // Pass, since SSE is enabled - addss(_mm_setzero_ps(), _mm_setzero_ps()); + minss(_mm_setzero_ps(), _mm_setzero_ps()); // Fail, since SSE4.1 is not enabled dpps(_mm_setzero_ps(), _mm_setzero_ps(), 0); @@ -34,8 +34,8 @@ fn main() { #[allow(improper_ctypes)] extern "C" { - #[link_name = "llvm.x86.sse.add.ss"] - fn addss(a: __m128, b: __m128) -> __m128; + #[link_name = "llvm.x86.sse.min.ss"] + fn minss(a: __m128, b: __m128) -> __m128; #[link_name = "llvm.x86.sse41.dpps"] fn dpps(a: __m128, b: __m128, imm8: u8) -> __m128; diff --git a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr index 2cc714f935a..2a8d4f3aea0 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: deallocating while item [Unique for ] is strongly protected by call ID +error: Undefined Behavior: deallocating while item [Unique for ] is strongly protected --> RUSTLIB/alloc/src/alloc.rs:LL:CC | LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [Unique for ] is strongly protected by call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [Unique for ] is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr index 627c790f843..5147bcd458c 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/drop_in_place_protector.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> $DIR/drop_in_place_protector.rs:LL:CC | LL | let _val = *P; - | ^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr index 22a0b42cfd8..96cdce5a778 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | let _val = unsafe { *x }; - | ^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [Unique for ] which is strongly protected | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr b/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr index ce9a5b7f158..133a50938f2 100644 --- a/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.stderr @@ -2,11 +2,11 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| RsM | └─┬── -| RsM | ├─┬── -| RsM | │ └─┬── -| Rs | │ └──── Strongly protected -| RsM | └──── +| ReIM| └─┬── +| ReIM| ├─┬── +| ReIM| │ └─┬── +| Res | │ └──── Strongly protected +| ReIM| └──── ────────────────────────────────────────────────── error: Undefined Behavior: write access through (y, callee:y, caller:y) at ALLOC[0x0] is forbidden --> $DIR/cell-protected-write.rs:LL:CC diff --git a/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.stderr b/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.stderr index 41559587bda..a4dc123979e 100644 --- a/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.stderr +++ b/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.stderr @@ -2,11 +2,11 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | ├─┬── -| Rs | │ └─┬── -| Rs | │ └──── Strongly protected -| Rs | └──── +| Res | └─┬── +| Res | ├─┬── +| Res | │ └─┬── +| Res | │ └──── Strongly protected +| Res | └──── ────────────────────────────────────────────────── error: Undefined Behavior: write access through (y, callee:y, caller:y) at ALLOC[0x0] is forbidden --> $DIR/int-protected-write.rs:LL:CC diff --git a/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs new file mode 100644 index 00000000000..e28cafd3c28 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs @@ -0,0 +1,652 @@ +//@only-target-linux + +#![feature(exposed_provenance)] // Needed for fn test_pointer() +use std::convert::TryInto; +use std::mem::MaybeUninit; + +fn main() { + test_epoll_socketpair(); + test_epoll_socketpair_both_sides(); + test_socketpair_read(); + test_epoll_eventfd(); + + test_event_overwrite(); + test_not_fully_closed_fd(); + test_closed_fd(); + test_two_epoll_instance(); + test_no_notification_for_unregister_flag(); + test_epoll_ctl_mod(); + test_epoll_ctl_del(); + test_pointer(); + test_two_same_fd_in_same_epoll_instance(); + test_epoll_wait_maxevent_zero(); + test_socketpair_epollerr(); + test_epoll_lost_events(); + test_ready_list_fetching_logic(); +} + +// Using `as` cast since `EPOLLET` wraps around +const EPOLL_IN_OUT_ET: u32 = (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _; + +#[track_caller] +fn check_epoll_wait(epfd: i32, expected_notifications: &[(u32, u64)]) { + let epoll_event = libc::epoll_event { events: 0, u64: 0 }; + let mut array: [libc::epoll_event; N] = [epoll_event; N]; + let maxsize = N; + let array_ptr = array.as_mut_ptr(); + let res = unsafe { libc::epoll_wait(epfd, array_ptr, maxsize.try_into().unwrap(), 0) }; + if res < 0 { + panic!("epoll_wait failed: {}", std::io::Error::last_os_error()); + } + assert_eq!( + res, + expected_notifications.len().try_into().unwrap(), + "got wrong number of notifications" + ); + let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; + for (return_event, expected_event) in slice.iter().zip(expected_notifications.iter()) { + let event = return_event.events; + let data = return_event.u64; + assert_eq!(event, expected_event.0, "got wrong events"); + assert_eq!(data, expected_event.1, "got wrong data"); + } +} + +fn test_epoll_socketpair() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Write to fd[0] + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET|EPOLLRDHUP + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, + u64: u64::try_from(fds[1]).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Check result from epoll_wait. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); + + // Check that this is indeed using "ET" (edge-trigger) semantics: a second epoll should return nothing. + check_epoll_wait::<8>(epfd, &[]); + + // Write some more to fd[0]. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // This did not change the readiness of fd[1]. And yet, we're seeing the event reported + // again by the kernel, so Miri does the same. + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); + + // Close the peer socketpair. + let res = unsafe { libc::close(fds[0]) }; + assert_eq!(res, 0); + + // Check result from epoll_wait. + // We expect to get a read, write, HUP notification from the close since closing an FD always unblocks reads and writes on its peer. + let expected_event = + u32::try_from(libc::EPOLLRDHUP | libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLHUP).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +// This test first registers a file description with a flag that does not lead to notification, +// then EPOLL_CTL_MOD to add another flag that will lead to notification. +fn test_epoll_ctl_mod() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register fd[1] with EPOLLIN|EPOLLET. + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLET) as _, + u64: u64::try_from(fds[1]).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Check result from epoll_wait. No notification would be returned. + check_epoll_wait::<8>(epfd, &[]); + + // Use EPOLL_CTL_MOD to change to EPOLLOUT flag. + let mut ev = libc::epoll_event { + events: (libc::EPOLLOUT | libc::EPOLLET) as _, + u64: u64::try_from(fds[1]).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_MOD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Check result from epoll_wait. EPOLLOUT notification is expected. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +fn test_epoll_ctl_del() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Write to fd[0] + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: u64::try_from(fds[1]).unwrap() }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Test EPOLL_CTL_DEL. + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_DEL, fds[1], &mut ev) }; + assert_eq!(res, 0); + check_epoll_wait::<8>(epfd, &[]); +} + +// This test is for one fd registered under two different epoll instance. +fn test_two_epoll_instance() { + // Create two epoll instance. + let epfd1 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd1, -1); + let epfd2 = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd2, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Write to the socketpair. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // Register one side of the socketpair with EPOLLIN | EPOLLOUT | EPOLLET. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: u64::try_from(fds[1]).unwrap() }; + let res = unsafe { libc::epoll_ctl(epfd1, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + let res = unsafe { libc::epoll_ctl(epfd2, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Notification should be received from both instance of epoll. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + check_epoll_wait::<8>(epfd1, &[(expected_event, expected_value)]); + check_epoll_wait::<8>(epfd2, &[(expected_event, expected_value)]); +} + +// This test is for two same file description registered under the same epoll instance through dup. +// Notification should be provided for both. +fn test_two_same_fd_in_same_epoll_instance() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Dup the fd. + let newfd = unsafe { libc::dup(fds[1]) }; + assert_ne!(newfd, -1); + + // Register both fd to the same epoll instance. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: 5 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, newfd, &mut ev) }; + assert_eq!(res, 0); + + // Write to the socketpair. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + //Two notification should be received. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = 5 as u64; + check_epoll_wait::<8>( + epfd, + &[(expected_event, expected_value), (expected_event, expected_value)], + ); +} + +fn test_epoll_eventfd() { + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd = unsafe { libc::eventfd(0, flags) }; + + // Write to the eventfd instance. + let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); + let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; + assert_eq!(res, 8); + + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Register eventfd with EPOLLIN | EPOLLOUT | EPOLLET + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: u64::try_from(fd).unwrap() }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; + assert_eq!(res, 0); + + // Check result from epoll_wait. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fd).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +fn test_pointer() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET + let data = MaybeUninit::::uninit().as_ptr(); + let mut ev = + libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: data.expose_provenance() as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); +} + +// When read/write happened on one side of the socketpair, only the other side will be notified. +fn test_epoll_socketpair_both_sides() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register both fd to the same epoll instance. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[1] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Write to fds[1]. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + //Two notification should be received. + let expected_event0 = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value0 = fds[0] as u64; + let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value1 = fds[1] as u64; + check_epoll_wait::<8>( + epfd, + &[(expected_event0, expected_value0), (expected_event1, expected_value1)], + ); + + // Read from fds[0]. + let mut buf: [u8; 5] = [0; 5]; + let res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; + assert_eq!(res, 5); + assert_eq!(buf, "abcde".as_bytes()); + + // Notification should be provided for fds[1]. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[1] as u64; + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +// When file description is fully closed, epoll_wait should not provide any notification for +// that file description. +fn test_closed_fd() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd = unsafe { libc::eventfd(0, flags) }; + + // Register eventfd with EPOLLIN | EPOLLOUT | EPOLLET + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: u64::try_from(fd).unwrap() }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; + assert_eq!(res, 0); + + // Write to the eventfd instance. + let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); + let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; + assert_eq!(res, 8); + + // Close the eventfd. + let res = unsafe { libc::close(fd) }; + assert_eq!(res, 0); + + // No notification should be provided because the file description is closed. + check_epoll_wait::<8>(epfd, &[]); +} + +// When a certain file descriptor registered with epoll is closed, but the underlying file description +// is not closed, notification should still be provided. +// +// This is a quirk of epoll being described in https://man7.org/linux/man-pages/man7/epoll.7.html +// A file descriptor is removed from an interest list only after all the file descriptors +// referring to the underlying open file description have been closed. +fn test_not_fully_closed_fd() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd = unsafe { libc::eventfd(0, flags) }; + + // Dup the fd. + let newfd = unsafe { libc::dup(fd) }; + assert_ne!(newfd, -1); + + // Register eventfd with EPOLLIN | EPOLLOUT | EPOLLET + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: u64::try_from(fd).unwrap() }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; + assert_eq!(res, 0); + + // Close the original fd that being used to register with epoll. + let res = unsafe { libc::close(fd) }; + assert_eq!(res, 0); + + // Notification should still be provided because the file description is not closed. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fd as u64; + check_epoll_wait::<1>(epfd, &[(expected_event, expected_value)]); + + // Write to the eventfd instance to produce notification. + let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); + let res = unsafe { libc::write(newfd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; + assert_eq!(res, 8); + + // Close the dupped fd. + let res = unsafe { libc::close(newfd) }; + assert_eq!(res, 0); + + // No notification should be provided. + check_epoll_wait::<1>(epfd, &[]); +} + +// Each time a notification is provided, it should reflect the file description's readiness +// at the moment the latest event occurred. +fn test_event_overwrite() { + // Create an eventfd instance. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd = unsafe { libc::eventfd(0, flags) }; + + // Write to the eventfd instance. + let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes(); + let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) }; + assert_eq!(res, 8); + + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Register eventfd with EPOLLIN | EPOLLOUT | EPOLLET + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _, + u64: u64::try_from(fd).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) }; + assert_eq!(res, 0); + + // Read from the eventfd instance. + let mut buf: [u8; 8] = [0; 8]; + let res = unsafe { libc::read(fd, buf.as_mut_ptr().cast(), 8) }; + assert_eq!(res, 8); + + // Check result from epoll_wait. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fd).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +// An epoll notification will be provided for every succesful read in a socketpair. +// This behaviour differs from the real system. +fn test_socketpair_read() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register both fd to the same epoll instance. + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _, + u64: fds[0] as u64, + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET) as _, + u64: fds[1] as u64, + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Write 5 bytes to fds[1]. + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + //Two notification should be received. + let expected_event0 = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value0 = fds[0] as u64; + let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value1 = fds[1] as u64; + check_epoll_wait::<8>( + epfd, + &[(expected_event0, expected_value0), (expected_event1, expected_value1)], + ); + + // Read 3 bytes from fds[0]. + let mut buf: [u8; 3] = [0; 3]; + let res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; + assert_eq!(res, 3); + assert_eq!(buf, "abc".as_bytes()); + + // Notification will be provided in Miri. + // But in real systems, no notification will be provided here, since Linux prefers to avoid + // wakeups that are likely to lead to only small amounts of data being read/written. + // We make the test work in both cases, thus documenting the difference in behavior. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[1] as u64; + if cfg!(miri) { + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); + } else { + check_epoll_wait::<8>(epfd, &[]); + } + + // Read until the buffer is empty. + let mut buf: [u8; 2] = [0; 2]; + let res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; + assert_eq!(res, 2); + assert_eq!(buf, "de".as_bytes()); + + // Notification will be provided. + // In real system, notification will be provided too. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[1] as u64; + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +// This is to test whether flag that we don't register won't trigger notification. +fn test_no_notification_for_unregister_flag() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register fd[0] with EPOLLOUT|EPOLLET. + let mut ev = libc::epoll_event { + events: (libc::EPOLLOUT | libc::EPOLLET) as _, + u64: u64::try_from(fds[0]).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + // Write to fd[1]. + let data = "abcde".as_bytes().as_ptr(); + let res: i32 = + unsafe { libc::write(fds[1], data as *const libc::c_void, 5).try_into().unwrap() }; + assert_eq!(res, 5); + + // Check result from epoll_wait. Since we didn't register EPOLLIN flag, the notification won't + // contain EPOLLIN even though fds[0] is now readable. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fds[0]).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +fn test_epoll_wait_maxevent_zero() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + // It is ok to use uninitialised pointer here because it will error out before the + // pointer actually get accessed. + let array_ptr = MaybeUninit::::uninit().as_mut_ptr(); + let res = unsafe { libc::epoll_wait(epfd, array_ptr, 0, 0) }; + let e = std::io::Error::last_os_error(); + assert_eq!(e.raw_os_error(), Some(libc::EINVAL)); + assert_eq!(res, -1); +} + +fn test_socketpair_epollerr() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Write to fd[0] + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + + // Close fds[1]. + // EPOLLERR will be triggered if we close peer fd that still has data in its read buffer. + let res = unsafe { libc::close(fds[1]) }; + assert_eq!(res, 0); + + // Register fd[1] with EPOLLIN|EPOLLOUT|EPOLLET|EPOLLRDHUP + let mut ev = libc::epoll_event { + events: (libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLET | libc::EPOLLRDHUP) as _, + u64: u64::try_from(fds[1]).unwrap(), + }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_ne!(res, -1); + + // Check result from epoll_wait. + let expected_event = u32::try_from( + libc::EPOLLIN | libc::EPOLLOUT | libc::EPOLLHUP | libc::EPOLLRDHUP | libc::EPOLLERR, + ) + .unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)]); +} + +// This is a test for https://github.com/rust-lang/miri/issues/3812, +// epoll can lose events if they don't fit in the output buffer. +fn test_epoll_lost_events() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create a socketpair instance. + let mut fds = [-1, -1]; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Register both fd to the same epoll instance. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[0] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[0], &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fds[1] as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_eq!(res, 0); + + //Two notification should be received. But we only provide buffer for one event. + let expected_event0 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value0 = fds[0] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event0, expected_value0)]); + + // Previous event should be returned for the second epoll_wait. + let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value1 = fds[1] as u64; + check_epoll_wait::<1>(epfd, &[(expected_event1, expected_value1)]); +} + +// This is testing if closing an fd that is already in ready list will cause an empty entry in +// returned notification. +// Related discussion in https://github.com/rust-lang/miri/pull/3818#discussion_r1720679440. +fn test_ready_list_fetching_logic() { + // Create an epoll instance. + let epfd = unsafe { libc::epoll_create1(0) }; + assert_ne!(epfd, -1); + + // Create two eventfd instances. + let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC; + let fd0 = unsafe { libc::eventfd(0, flags) }; + let fd1 = unsafe { libc::eventfd(0, flags) }; + + // Register both fd to the same epoll instance. At this point, both of them are on the ready list. + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd0 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd0, &mut ev) }; + assert_eq!(res, 0); + let mut ev = libc::epoll_event { events: EPOLL_IN_OUT_ET, u64: fd1 as u64 }; + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd1, &mut ev) }; + assert_eq!(res, 0); + + // Close fd0 so the first entry in the ready list will be empty. + let res = unsafe { libc::close(fd0) }; + assert_eq!(res, 0); + + // Notification for fd1 should be returned. + let expected_event1 = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value1 = fd1 as u64; + check_epoll_wait::<1>(epfd, &[(expected_event1, expected_value1)]); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-misc.rs b/src/tools/miri/tests/pass-dep/libc/libc-misc.rs index f7e1d9faa6a..a5b944e9d42 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-misc.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-misc.rs @@ -75,11 +75,15 @@ fn test_dlsym() { assert_eq!(errno, libc::EBADF); } +fn test_getuid() { + let _val = unsafe { libc::getuid() }; +} + fn main() { test_thread_local_errno(); test_environ(); - test_dlsym(); + test_getuid(); #[cfg(target_os = "linux")] test_sigrt(); diff --git a/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs b/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs new file mode 100644 index 00000000000..5dff612bd89 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-pipe.rs @@ -0,0 +1,99 @@ +//@ignore-target-windows: No libc pipe on Windows +// test_race depends on a deterministic schedule. +//@compile-flags: -Zmiri-preemption-rate=0 +use std::thread; +fn main() { + test_pipe(); + test_pipe_threaded(); + test_race(); +} + +fn test_pipe() { + let mut fds = [-1, -1]; + let res = unsafe { libc::pipe(fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + // Read size == data available in buffer. + let data = "12345".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + let mut buf3: [u8; 5] = [0; 5]; + let res = unsafe { libc::read(fds[0], buf3.as_mut_ptr().cast(), buf3.len() as libc::size_t) }; + assert_eq!(res, 5); + assert_eq!(buf3, "12345".as_bytes()); + + // Read size > data available in buffer. + let data = "123".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 3) }; + assert_eq!(res, 3); + let mut buf4: [u8; 5] = [0; 5]; + let res = unsafe { libc::read(fds[0], buf4.as_mut_ptr().cast(), buf4.len() as libc::size_t) }; + assert_eq!(res, 3); + assert_eq!(&buf4[0..3], "123".as_bytes()); +} + +fn test_pipe_threaded() { + let mut fds = [-1, -1]; + let res = unsafe { libc::pipe(fds.as_mut_ptr()) }; + assert_eq!(res, 0); + + let thread1 = thread::spawn(move || { + let mut buf: [u8; 5] = [0; 5]; + let res: i64 = unsafe { + libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) + .try_into() + .unwrap() + }; + assert_eq!(res, 5); + assert_eq!(buf, "abcde".as_bytes()); + }); + // FIXME: we should yield here once blocking is implemented. + //thread::yield_now(); + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + thread1.join().unwrap(); + + // Read and write from different direction + let thread2 = thread::spawn(move || { + // FIXME: we should yield here once blocking is implemented. + //thread::yield_now(); + let data = "12345".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); + }); + // FIXME: we should not yield here once blocking is implemented. + thread::yield_now(); + let mut buf: [u8; 5] = [0; 5]; + let res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; + assert_eq!(res, 5); + assert_eq!(buf, "12345".as_bytes()); + thread2.join().unwrap(); +} + +fn test_race() { + static mut VAL: u8 = 0; + let mut fds = [-1, -1]; + let res = unsafe { libc::pipe(fds.as_mut_ptr()) }; + assert_eq!(res, 0); + let thread1 = thread::spawn(move || { + let mut buf: [u8; 1] = [0; 1]; + // write() from the main thread will occur before the read() here + // because preemption is disabled and the main thread yields after write(). + let res: i32 = unsafe { + libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) + .try_into() + .unwrap() + }; + assert_eq!(res, 1); + assert_eq!(buf, "a".as_bytes()); + // The read above establishes a happens-before so it is now safe to access this global variable. + unsafe { assert_eq!(VAL, 1) }; + }); + unsafe { VAL = 1 }; + let data = "a".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 1) }; + assert_eq!(res, 1); + thread::yield_now(); + thread1.join().unwrap(); +} diff --git a/src/tools/miri/tests/pass-dep/libc/libc-socketpair.rs b/src/tools/miri/tests/pass-dep/libc/libc-socketpair.rs index 324c0127ee9..15e040116de 100644 --- a/src/tools/miri/tests/pass-dep/libc/libc-socketpair.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc-socketpair.rs @@ -10,65 +10,68 @@ fn main() { fn test_socketpair() { let mut fds = [-1, -1]; - let mut res = - unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; assert_eq!(res, 0); // Read size == data available in buffer. let data = "abcde".as_bytes().as_ptr(); - res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5).try_into().unwrap() }; + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; assert_eq!(res, 5); let mut buf: [u8; 5] = [0; 5]; - res = unsafe { - libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t).try_into().unwrap() - }; + let res = unsafe { libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; assert_eq!(res, 5); assert_eq!(buf, "abcde".as_bytes()); // Read size > data available in buffer. let data = "abc".as_bytes().as_ptr(); - res = unsafe { libc::write(fds[0], data as *const libc::c_void, 3).try_into().unwrap() }; + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 3) }; assert_eq!(res, 3); let mut buf2: [u8; 5] = [0; 5]; - res = unsafe { - libc::read(fds[1], buf2.as_mut_ptr().cast(), buf2.len() as libc::size_t).try_into().unwrap() - }; + let res = unsafe { libc::read(fds[1], buf2.as_mut_ptr().cast(), buf2.len() as libc::size_t) }; assert_eq!(res, 3); assert_eq!(&buf2[0..3], "abc".as_bytes()); // Test read and write from another direction. // Read size == data available in buffer. let data = "12345".as_bytes().as_ptr(); - res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5).try_into().unwrap() }; + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; assert_eq!(res, 5); let mut buf3: [u8; 5] = [0; 5]; - res = unsafe { - libc::read(fds[0], buf3.as_mut_ptr().cast(), buf3.len() as libc::size_t).try_into().unwrap() - }; + let res = unsafe { libc::read(fds[0], buf3.as_mut_ptr().cast(), buf3.len() as libc::size_t) }; assert_eq!(res, 5); assert_eq!(buf3, "12345".as_bytes()); // Read size > data available in buffer. let data = "123".as_bytes().as_ptr(); - res = unsafe { libc::write(fds[1], data as *const libc::c_void, 3).try_into().unwrap() }; + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 3) }; assert_eq!(res, 3); let mut buf4: [u8; 5] = [0; 5]; - res = unsafe { - libc::read(fds[0], buf4.as_mut_ptr().cast(), buf4.len() as libc::size_t).try_into().unwrap() - }; + let res = unsafe { libc::read(fds[0], buf4.as_mut_ptr().cast(), buf4.len() as libc::size_t) }; assert_eq!(res, 3); assert_eq!(&buf4[0..3], "123".as_bytes()); + + // Test when happens when we close one end, with some data in the buffer. + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 3) }; + assert_eq!(res, 3); + unsafe { libc::close(fds[0]) }; + // Reading the other end should return that data, then EOF. + let mut buf: [u8; 5] = [0; 5]; + let res = unsafe { libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; + assert_eq!(res, 3); + assert_eq!(&buf[0..3], "123".as_bytes()); + let res = unsafe { libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; + assert_eq!(res, 0); // 0-sized read: EOF. + // Writing the other end should emit EPIPE. + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 1) }; + assert_eq!(res, -1); + assert_eq!(std::io::Error::last_os_error().raw_os_error(), Some(libc::EPIPE)); } fn test_socketpair_threaded() { let mut fds = [-1, -1]; - let mut res = - unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; assert_eq!(res, 0); - let data = "abcde".as_bytes().as_ptr(); - res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5).try_into().unwrap() }; - assert_eq!(res, 5); let thread1 = thread::spawn(move || { let mut buf: [u8; 5] = [0; 5]; let res: i64 = unsafe { @@ -79,28 +82,34 @@ fn test_socketpair_threaded() { assert_eq!(res, 5); assert_eq!(buf, "abcde".as_bytes()); }); + // FIXME: we should yield here once blocking is implemented. + //thread::yield_now(); + let data = "abcde".as_bytes().as_ptr(); + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 5) }; + assert_eq!(res, 5); thread1.join().unwrap(); // Read and write from different direction let thread2 = thread::spawn(move || { + // FIXME: we should yield here once blocking is implemented. + //thread::yield_now(); let data = "12345".as_bytes().as_ptr(); - let res: i64 = - unsafe { libc::write(fds[0], data as *const libc::c_void, 5).try_into().unwrap() }; + let res = unsafe { libc::write(fds[1], data as *const libc::c_void, 5) }; assert_eq!(res, 5); }); - thread2.join().unwrap(); + // FIXME: we should not yield here once blocking is implemented. + thread::yield_now(); let mut buf: [u8; 5] = [0; 5]; - res = unsafe { - libc::read(fds[1], buf.as_mut_ptr().cast(), buf.len() as libc::size_t).try_into().unwrap() - }; + let res = unsafe { libc::read(fds[0], buf.as_mut_ptr().cast(), buf.len() as libc::size_t) }; assert_eq!(res, 5); assert_eq!(buf, "12345".as_bytes()); + thread2.join().unwrap(); } + fn test_race() { static mut VAL: u8 = 0; let mut fds = [-1, -1]; - let mut res = - unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; + let res = unsafe { libc::socketpair(libc::AF_UNIX, libc::SOCK_STREAM, 0, fds.as_mut_ptr()) }; assert_eq!(res, 0); let thread1 = thread::spawn(move || { let mut buf: [u8; 1] = [0; 1]; @@ -113,11 +122,12 @@ fn test_race() { }; assert_eq!(res, 1); assert_eq!(buf, "a".as_bytes()); + // The read above establishes a happens-before so it is now safe to access this global variable. unsafe { assert_eq!(VAL, 1) }; }); unsafe { VAL = 1 }; let data = "a".as_bytes().as_ptr(); - res = unsafe { libc::write(fds[0], data as *const libc::c_void, 1).try_into().unwrap() }; + let res = unsafe { libc::write(fds[0], data as *const libc::c_void, 1) }; assert_eq!(res, 1); thread::yield_now(); thread1.join().unwrap(); diff --git a/src/tools/miri/tests/pass/tls/tls_leak_main_thread_allowed.rs b/src/tools/miri/tests/pass/tls/tls_leak_main_thread_allowed.rs index 7732e3f9217..341b2280e01 100644 --- a/src/tools/miri/tests/pass/tls/tls_leak_main_thread_allowed.rs +++ b/src/tools/miri/tests/pass/tls/tls_leak_main_thread_allowed.rs @@ -1,5 +1,4 @@ -//@ignore-target-windows: Windows uses a different mechanism for `thread_local!` -#![feature(thread_local)] +#![feature(thread_local, cfg_target_thread_local)] use std::cell::Cell; @@ -8,16 +7,20 @@ use std::cell::Cell; // // The test covers both TLS statics and the TLS macro. pub fn main() { - thread_local! { - static TLS_KEY: Cell> = Cell::new(None); - } - - TLS_KEY.with(|cell| { - cell.set(Some(Box::leak(Box::new(123)))); - }); - #[thread_local] static TLS: Cell> = Cell::new(None); TLS.set(Some(Box::leak(Box::new(123)))); + + // We can only ignore leaks on targets that use `#[thread_local]` statics to implement + // `thread_local!`. Ignore the test on targest that don't. + if cfg!(target_thread_local) { + thread_local! { + static TLS_KEY: Cell> = Cell::new(None); + } + + TLS_KEY.with(|cell| { + cell.set(Some(Box::leak(Box::new(123)))); + }); + } } diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.stderr b/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.stderr index 57caa09c888..d13e9ad0215 100644 --- a/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.stderr @@ -2,7 +2,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| RsM | └──── +| ReIM| └──── ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. diff --git a/src/tools/miri/tests/pass/tree_borrows/end-of-protector.stderr b/src/tools/miri/tests/pass/tree_borrows/end-of-protector.stderr index 69b8a17dc5e..4d77d96776d 100644 --- a/src/tools/miri/tests/pass/tree_borrows/end-of-protector.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/end-of-protector.stderr @@ -2,27 +2,27 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | └──── +| Res | └─┬── +| Res | └──── ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | └─┬── -| Rs | └─┬── -| Rs | └──── Strongly protected +| Res | └─┬── +| Res | └─┬── +| Res | └─┬── +| Res | └──── Strongly protected ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | ├─┬── -| Rs | │ └─┬── -| Rs | │ └──── -| Rs | └──── +| Res | └─┬── +| Res | ├─┬── +| Res | │ └─┬── +| Res | │ └──── +| Res | └──── ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. diff --git a/src/tools/miri/tests/pass/tree_borrows/formatting.stderr b/src/tools/miri/tests/pass/tree_borrows/formatting.stderr index 235ab68fe01..29f99034bab 100644 --- a/src/tools/miri/tests/pass/tree_borrows/formatting.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/formatting.stderr @@ -2,7 +2,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1.. 2.. 10.. 11.. 100.. 101..1000..1001..1024 | Act | Act | Act | Act | Act | Act | Act | Act | Act | └─┬── -| Rs | Act | Rs | Act | Rs | Act | Rs | Act | Rs | └─┬── +| Res | Act | Res | Act | Res | Act | Res | Act | Res | └─┬── |-----| Act |-----|?Dis |-----|?Dis |-----|?Dis |-----| ├──── |-----|-----|-----| Act |-----|?Dis |-----|?Dis |-----| ├──── |-----|-----|-----|-----|-----| Frz |-----|?Dis |-----| ├──── diff --git a/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.stderr b/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.stderr index f09aa52f1a1..d589a062111 100644 --- a/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.stderr @@ -11,5 +11,5 @@ Warning: this tree is indicative only. Some tags may have been hidden. | Act | └─┬── | Act | └─┬── | Frz | ├──── -| Rs | └──── +| Res | └──── ────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree_borrows/reserved.stderr b/src/tools/miri/tests/pass/tree_borrows/reserved.stderr index d149a4065f9..be90382640b 100644 --- a/src/tools/miri/tests/pass/tree_borrows/reserved.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/reserved.stderr @@ -3,20 +3,20 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| RsM | └─┬── -| RsM | ├─┬── -| RsM | │ └─┬── -| RsC | │ └──── -| RsM | └──── +| ReIM| └─┬── +| ReIM| ├─┬── +| ReIM| │ └─┬── +| ResC| │ └──── +| ReIM| └──── ────────────────────────────────────────────────── [interior mut] Foreign Read: Re* -> Re* ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 8 | Act | └─┬── -| RsM | └─┬── -| RsM | ├──── -| RsM | └──── +| ReIM| └─┬── +| ReIM| ├──── +| ReIM| └──── ────────────────────────────────────────────────── [interior mut] Foreign Write: Re* -> Re* ────────────────────────────────────────────────── @@ -24,7 +24,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 8 | Act | └─┬── | Act | └─┬── -| RsM | ├──── +| ReIM| ├──── | Act | └──── ────────────────────────────────────────────────── [protected] Foreign Read: Res -> Frz @@ -32,20 +32,20 @@ Warning: this tree is indicative only. Some tags may have been hidden. Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | ├─┬── -| Rs | │ └─┬── -| RsC | │ └──── -| Rs | └──── +| Res | └─┬── +| Res | ├─┬── +| Res | │ └─┬── +| ResC| │ └──── +| Res | └──── ────────────────────────────────────────────────── [] Foreign Read: Res -> Res ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | ├──── -| Rs | └──── +| Res | └─┬── +| Res | ├──── +| Res | └──── ────────────────────────────────────────────────── [] Foreign Write: Res -> Dis ────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree_borrows/unique.default.stderr b/src/tools/miri/tests/pass/tree_borrows/unique.default.stderr index 6e774e5014d..6098c855bde 100644 --- a/src/tools/miri/tests/pass/tree_borrows/unique.default.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/unique.default.stderr @@ -2,8 +2,8 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | └──── +| Res | └─┬── +| Res | └──── ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. diff --git a/src/tools/miri/tests/pass/tree_borrows/unique.uniq.stderr b/src/tools/miri/tests/pass/tree_borrows/unique.uniq.stderr index 26d9ad2ad38..960c7e216e1 100644 --- a/src/tools/miri/tests/pass/tree_borrows/unique.uniq.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/unique.uniq.stderr @@ -2,8 +2,8 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬── -| Rs | └─┬── -| Rs | └─┬── +| Res | └─┬── +| Res | └─┬── |-----| └──── ────────────────────────────────────────────────── ────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree_borrows/vec_unique.default.stderr b/src/tools/miri/tests/pass/tree_borrows/vec_unique.default.stderr index f63aa1f6834..254eba061f4 100644 --- a/src/tools/miri/tests/pass/tree_borrows/vec_unique.default.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/vec_unique.default.stderr @@ -2,5 +2,5 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 2 | Act | └─┬── -| Rs | └──── +| Res | └──── ────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs b/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs index 9debe224d45..af4c3b06931 100644 --- a/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs +++ b/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs @@ -1,5 +1,3 @@ -// FIXME: This test is broken since https://github.com/rust-lang/rust/pull/126793, -// possibly related to the additional struct between Vec and Unique. //@revisions: default uniq // We disable the GC for this test because it would change what is printed. //@compile-flags: -Zmiri-tree-borrows -Zmiri-provenance-gc=0 @@ -32,20 +30,20 @@ fn main() { // whether we got the distance correct: // If the output shows // - // |- - // '- + // ├─ + // └─ // // then `nth_parent` is not big enough. // The correct value for `nth_parent` should be the minimum // integer for which the output shows // - // '- + // └─ // ) // // Ultimately we want pointers obtained through independent // calls of `as_ptr` to be able to alias, which will probably involve // a new permission that allows aliasing when there is no protector. - let nth_parent = if cfg!(uniq) { 2 } else { 0 }; + let nth_parent = if cfg!(uniq) { 9 } else { 0 }; name!(base.as_ptr()=>nth_parent); name!(base.as_ptr()=>nth_parent); diff --git a/src/tools/miri/tests/pass/tree_borrows/vec_unique.uniq.stderr b/src/tools/miri/tests/pass/tree_borrows/vec_unique.uniq.stderr index e3796a742e9..7942e9884f4 100644 --- a/src/tools/miri/tests/pass/tree_borrows/vec_unique.uniq.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/vec_unique.uniq.stderr @@ -2,9 +2,7 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 2 | Act | └─┬── -|-----| ├──── -|-----| ├──── -|-----| └─┬── -|-----| ├──── -|-----| └──── +|-----| └─┬── +|-----| └─┬── +|-----| └──── ────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 95f8b054102..9cbcf6e42a7 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -13,7 +13,7 @@ use ui_test::{ }; fn miri_path() -> PathBuf { - PathBuf::from(option_env!("MIRI").unwrap_or(env!("CARGO_BIN_EXE_miri"))) + PathBuf::from(env::var("MIRI").unwrap_or_else(|_| env!("CARGO_BIN_EXE_miri").into())) } fn get_host() -> String { @@ -29,7 +29,7 @@ pub fn flagsplit(flags: &str) -> Vec { // Build the shared object file for testing native function calls. fn build_native_lib() -> PathBuf { - let cc = option_env!("CC").unwrap_or("cc"); + let cc = env::var("CC").unwrap_or_else(|_| "cc".into()); // Target directory that we can write to. let so_target_dir = Path::new(&env::var_os("CARGO_TARGET_DIR").unwrap()).join("miri-native-lib"); @@ -84,9 +84,11 @@ fn miri_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> if with_dependencies { // Set the `cargo-miri` binary, which we expect to be in the same folder as the `miri` binary. // (It's a separate crate, so we don't get an env var from cargo.) - let mut prog = miri_path(); - prog.set_file_name("cargo-miri"); - config.dependency_builder.program = prog; + config.dependency_builder.program = { + let mut prog = miri_path(); + prog.set_file_name(format!("cargo-miri{}", env::consts::EXE_SUFFIX)); + prog + }; let builder_args = ["miri", "run"]; // There is no `cargo miri build` so we just use `cargo miri run`. config.dependency_builder.args = builder_args.into_iter().map(Into::into).collect(); config.dependencies_crate_manifest_path =