diff options
84 files changed, 1825 insertions, 714 deletions
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..1c76354eaee 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -173,24 +173,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 +309,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=<id1>,<id2>,...` 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=<tag1>,<tag2>,...` 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 <https://github.com/rust-analyzer/rust-analyzer/issues/10793>. -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/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 <https://github.com/rust-analyzer/rust-analyzer/issues/10793>. -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<String>) -> 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<String>) -> 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 clippy(flags: Vec<String>) -> Result<()> { + fn doc(flags: Vec<String>) -> 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)?; + e.doc(".", &flags)?; + e.doc("cargo-miri", &flags)?; Ok(()) } - fn cargo(flags: Vec<String>) -> Result<()> { + fn clippy(flags: Vec<String>) -> 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<String>, target: Option<String>) -> 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<String>| -> Result<()> { + let run_miri = |e: &MiriEnv, seed_flag: Option<String>| -> 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<String>, }, + /// Build documentation + Doc { + /// Flags that are passed through to `cargo doc`. + flags: Vec<String>, + }, /// 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<String>, }, - /// Runs just `cargo <flags>` with the Miri-specific environment variables. - /// Mainly meant to be invoked by rust-analyzer. - Cargo { flags: Vec<String> }, /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. Bench { target: Option<String>, @@ -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<PathBuf> { const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR"); @@ -28,13 +28,14 @@ pub fn flagsplit(flags: &str) -> Vec<String> { } /// 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<String>, + cargo_extra_flags: Vec<String>, /// 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<OsStr>, 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<OsStr>, args: impl IntoIterator<Item = impl AsRef<OsStr>>, ) -> 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<OsStr>, - args: &[String], - quiet: bool, - ) -> Result<()> { - let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + pub fn build(&self, crate_dir: impl AsRef<OsStr>, 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<OsStr>, 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<OsStr>, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "check").arg("--all-targets").args(args).run()?; Ok(()) } - pub fn clippy(&self, manifest_path: impl AsRef<OsStr>, 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<OsStr>, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "doc").args(args).run()?; Ok(()) } - pub fn test(&self, manifest_path: impl AsRef<OsStr>, 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<OsStr>, args: &[String]) -> Result<()> { + self.cargo_cmd(crate_dir, "clippy").arg("--all-targets").args(args).run()?; + Ok(()) + } + + pub fn test(&self, crate_dir: impl AsRef<OsStr>, 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<Item = Result<PathBuf, walkdir::Error>>, - 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<u32>, - 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<u64> = 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::<NonZero<u64>>(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<u64>; - /// Tracking pointer provenance #[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] pub struct BorTag(NonZero<u64>); @@ -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<AllocId, BorTag>, - /// 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<BorTag, ProtectorKind>, /// The pointer ids to trace tracked_pointer_tags: FxHashSet<BorTag>, - /// The call ids to trace - tracked_call_ids: FxHashSet<CallId>, /// 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<BorTag>, - tracked_call_ids: FxHashSet<CallId>, 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<PermissionPriv> { + // 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<dyn Iterator<Item = Self>> { 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(<bool>::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<Self, ()> { 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/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<u64>, Option<String>, 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..0850a8f24d9 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<u64>, /// The stacked borrows pointer ids to report about pub tracked_pointer_tags: FxHashSet<BorTag>, - /// The stacked borrows call IDs to report about - pub tracked_call_ids: FxHashSet<CallId>, /// The allocation ids to report about. pub tracked_alloc_ids: FxHashSet<AllocId>, /// 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,12 +457,8 @@ pub fn eval_entry<'tcx>( ecx.handle_ice(); panic::resume_unwind(panic_payload) }); - let res = match res { - Err(res) => res, - // `Ok` can never happen - #[cfg(bootstrap)] - Ok(never) => match never {}, - }; + // `Ok` can never happen. + let Err(res) = res; // Machine cleanup. Only do this if all threads have terminated; threads that are still running // might cause Stacked Borrows errors (https://github.com/rust-lang/miri/issues/2396). 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<P: Projectable<'tcx, Provenance>>( &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<u64>> { @@ -80,6 +84,7 @@ pub trait FileDescription: std::fmt::Debug + Any { fn close<'tcx>( self: Box<Self>, _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<T: Any>(&self) -> Option<&T> { + pub fn downcast<T: Any>(&self) -> Option<&T> { (self as &dyn Any).downcast_ref() } - - #[inline(always)] - pub fn downcast_mut<T: Any>(&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<usize>> { // 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<RefCell<Box<dyn FileDescription>>>); +pub struct FileDescWithId<T: FileDescription + ?Sized> { + id: FdId, + file_description: Box<T>, +} -impl FileDescriptionRef { - fn new(fd: impl FileDescription) -> Self { - FileDescriptionRef(Rc::new(RefCell::new(Box::new(fd)))) - } +#[derive(Clone, Debug)] +pub struct FileDescriptionRef(Rc<FileDescWithId<dyn FileDescription>>); - pub fn borrow(&self) -> Ref<'_, dyn FileDescription> { - Ref::map(self.0.borrow(), |fd| fd.as_ref()) +impl Deref for FileDescriptionRef { + type Target = dyn FileDescription; + + fn deref(&self) -> &Self::Target { + &*self.0.file_description } +} - pub fn borrow_mut(&self) -> RefMut<'_, dyn FileDescription> { - RefMut::map(self.0.borrow_mut(), |fd| fd.as_mut()) +impl FileDescriptionRef { + fn new(fd: impl FileDescription, id: FdId) -> Self { + FileDescriptionRef(Rc::new(FileDescWithId { id, file_description: Box::new(fd) })) } - 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<FileDescWithId<dyn FileDescription>>, +} + +impl WeakFileDescriptionRef { + pub fn upgrade(&self) -> Option<FileDescriptionRef> { + 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<i32, FileDescriptionRef>, + pub fds: BTreeMap<i32, FileDescriptionRef>, + /// 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<Ref<'_, dyn FileDescription>> { - let fd = self.fds.get(&fd)?; - Some(fd.borrow()) - } - - pub fn get_mut(&self, fd: i32) -> Option<RefMut<'_, dyn FileDescription>> { - let fd = self.fds.get(&fd)?; - Some(fd.borrow_mut()) - } - - pub fn get_ref(&self, fd: i32) -> Option<FileDescriptionRef> { + pub fn get(&self, fd: i32) -> Option<FileDescriptionRef> { 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..6c35281ecf2 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -815,6 +815,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 +882,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 +896,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<usize>> { 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<usize>> { 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<u64>> { 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<Self>, 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::<FileHandle>().ok_or_else(|| { + file_description.downcast::<FileHandle>().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::<FileHandle>().ok_or_else(|| { + file_description.downcast::<FileHandle>().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::<FileHandle>().ok_or_else(|| { + file_description.downcast::<FileHandle>().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::<FileHandle>().ok_or_else(|| { + file_description.downcast::<FileHandle>().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::<FileHandle>() + .downcast::<FileHandle>() .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..d529a9b63e6 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -1,32 +1,104 @@ +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<i32, EpollEvent>, + /// A map of EpollEventInterests registered under this epoll instance. + /// Each entry is differentiated using FdId and file descriptor value. + interest_list: RefCell<BTreeMap<(FdId, i32), Rc<RefCell<EpollEventInterest>>>>, + /// 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<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>, } -/// 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: /// /// <https://man7.org/linux/man-pages/man2/epoll_ctl.2.html> #[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. + /// <https://man7.org/linux/man-pages/man3/epoll_event.3type.html> + data: u64, + /// Ready list of the epoll instance under which this EpollEventInterest is registered. + ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>, +} + +/// EpollReadyEvents reflects the readiness of a file description. +pub struct EpollReadyEvents { + /// The associated file is available for read(2) operations. + pub epollin: bool, + /// The associated file is available for write(2) operations. + pub epollout: bool, + /// Stream socket peer closed connection, or shut down writing + /// half of connection. + pub epollrdhup: bool, +} + +impl EpollReadyEvents { + pub fn new() -> Self { + EpollReadyEvents { epollin: false, epollout: false, epollrdhup: 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 mut bitmask = 0; + if self.epollin { + bitmask |= epollin; + } + if self.epollout { + bitmask |= epollout; + } + if self.epollrdhup { + bitmask |= epollrdhup; + } + bitmask + } +} + +impl Epoll { + fn get_ready_list(&self) -> Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>> { + Rc::clone(&self.ready_list) + } } impl FileDescription for Epoll { @@ -37,11 +109,51 @@ impl FileDescription for Epoll { fn close<'tcx>( self: Box<Self>, _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<FdId, Vec<Weak<RefCell<EpollEventInterest>>>>); + +impl EpollInterestTable { + pub(crate) fn new() -> Self { + EpollInterestTable(BTreeMap::new()) + } + + pub fn insert_epoll_interest(&mut self, id: FdId, fd: Weak<RefCell<EpollEventInterest>>) { + 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<Weak<RefCell<EpollEventInterest>>>> { + self.0.get(&id) + } + + pub fn get_epoll_interest_mut( + &mut self, + id: FdId, + ) -> Option<&mut Vec<Weak<RefCell<EpollEventInterest>>>> { + 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 +176,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 +205,142 @@ 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"); + + // 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::<Epoll>() + .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 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; + 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 != 0 { + throw_unsup_format!( + "epoll_ctl: encountered unknown unsupported flags {:#x}", + flags + ); + } - 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::<Epoll>() - .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; + 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)?; - epfd.file_descriptors.insert(fd, event); - Ok(Scalar::from_i32(0)) + 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 epoll_key = (id, fd); + + // 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)); }; - let epfd = epfd - .downcast_mut::<Epoll>() - .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; + // All related Weak<EpollEventInterest> will fail to upgrade after the drop. + drop(epoll_interest); - 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 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 +375,98 @@ 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 maxevents = this.read_scalar(maxevents)?.to_i32()?; + let event = this.deref_pointer_as( + events_op, + this.libc_array_ty_layout("epoll_event", maxevents.try_into().unwrap()), + )?; + let timeout = this.read_scalar(timeout)?.to_i32()?; - let Some(mut epfd) = this.machine.fds.get_mut(epfd) else { + if epfd <= 0 { + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + return Ok(Scalar::from_i32(-1)); + } + // 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::<Epoll>() + let epoll_file_description = epfd + .downcast::<Epoll>() .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(&event)?; + + while let Some((epoll_key, epoll_return)) = ready_list.pop_first() { + // If the file description is fully close, the entry for corresponding FdID in the + // global epoll event interest table would be empty. + if this.machine.epoll_interests.get_epoll_interest(epoll_key.0).is_some() { + // Return notification to the caller if the file description is not fully closed. + if let Some(des) = array_iter.next(this)? { + this.write_int_fields_named( + &[ + ("events", epoll_return.events.into()), + ("u64", epoll_return.data.into()), + ], + &des.1, + )?; + num_of_events = num_of_events.checked_add(1).unwrap(); + } 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(()) } } 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<u64>, is_nonblock: bool, - clock: VClock, + clock: RefCell<VClock>, } 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<Self>, _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..8cfa659d90a 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -17,6 +17,7 @@ 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 _; diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs index 455820a9e6e..75d4d0dbbe4 100644 --- a/src/tools/miri/src/shims/unix/socket.rs +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -1,9 +1,10 @@ -use std::cell::RefCell; +use std::cell::{OnceCell, RefCell}; use std::collections::VecDeque; use std::io; use std::io::{Error, ErrorKind, Read}; -use std::rc::{Rc, Weak}; +use crate::shims::unix::fd::{FileDescriptionRef, WeakFileDescriptionRef}; +use crate::shims::unix::linux::epoll::{EpollReadyEvents, EvalContextExt as _}; use crate::shims::unix::*; use crate::{concurrency::VClock, *}; @@ -15,10 +16,12 @@ 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<RefCell<Buffer>>, - readbuf: Rc<RefCell<Buffer>>, + /// The buffer we are reading from. + readbuf: RefCell<Buffer>, + /// The `SocketPair` 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<WeakFileDescriptionRef>, is_nonblock: bool, } @@ -26,10 +29,18 @@ struct SocketPair { struct Buffer { buf: VecDeque<u8>, 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 Buffer { + fn new() -> Self { + Buffer { buf: VecDeque::new(), clock: VClock::default() } + } +} + +impl SocketPair { + fn peer_fd(&self) -> &WeakFileDescriptionRef { + self.peer_fd.get().unwrap() + } } impl FileDescription for SocketPair { @@ -37,35 +48,68 @@ impl FileDescription for SocketPair { "socketpair" } + fn get_epoll_ready_events<'tcx>(&self) -> InterpResult<'tcx, EpollReadyEvents> { + // We only check the status of EPOLLIN, EPOLLOUT 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. + let readbuf = self.readbuf.borrow(); + if !readbuf.buf.is_empty() { + epoll_ready_events.epollin = true; + } + + // Check if is writable. + if let Some(peer_fd) = self.peer_fd().upgrade() { + let writebuf = &peer_fd.downcast::<SocketPair>().unwrap().readbuf.borrow(); + let data_size = writebuf.buf.len(); + let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(data_size); + if available_space != 0 { + epoll_ready_events.epollout = true; + } + } else { + // Peer FD has been closed. + epoll_ready_events.epollrdhup = 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; + } + Ok(epoll_ready_events) + } + fn close<'tcx>( self: Box<Self>, _communicate_allowed: bool, + ecx: &mut MiriInterpCx<'tcx>, ) -> 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; - }; + if let Some(peer_fd) = self.peer_fd().upgrade() { + // 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>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &mut [u8], ecx: &mut MiriInterpCx<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { 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)); } + let mut readbuf = self.readbuf.borrow_mut(); if readbuf.buf.is_empty() { - if !readbuf.buf_has_writer { - // Socketpair with no writer and empty buffer. + 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 { @@ -88,14 +132,31 @@ impl FileDescription for SocketPair { // 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>( - &mut self, + &self, + _self_ref: &FileDescriptionRef, _communicate_allowed: bool, bytes: &[u8], ecx: &mut MiriInterpCx<'tcx>, @@ -107,12 +168,13 @@ impl FileDescription for SocketPair { return Ok(Ok(0)); } - let Some(writebuf) = self.writebuf.upgrade() else { + // 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 mut writebuf = writebuf.borrow_mut(); + let mut writebuf = peer_fd.downcast::<SocketPair>().unwrap().readbuf.borrow_mut(); let data_size = writebuf.buf.len(); let available_space = MAX_SOCKETPAIR_BUFFER_CAPACITY.strict_sub(data_size); if available_space == 0 { @@ -131,6 +193,14 @@ impl FileDescription for SocketPair { // 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)); } } @@ -194,36 +264,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } - 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), + // Generate file descriptions. + let fds = &mut this.machine.fds; + let fd0 = fds.new_ref(SocketPair { + readbuf: RefCell::new(Buffer::new()), + peer_fd: OnceCell::new(), is_nonblock: is_sock_nonblock, - }; - - let socketpair_1 = SocketPair { - writebuf: Rc::downgrade(&buffer2), - readbuf: Rc::clone(&buffer1), + }); + let fd1 = fds.new_ref(SocketPair { + readbuf: RefCell::new(Buffer::new()), + peer_fd: OnceCell::new(), 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); + // Make the file descriptions point to each other. + fd0.downcast::<SocketPair>().unwrap().peer_fd.set(fd1.downgrade()).unwrap(); + fd1.downcast::<SocketPair>().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)?)?; 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::<rustc_apfloat::ieee::Double>(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::<F>()?; let right = right.to_scalar().to_float::<F>()?; @@ -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::<F>( - 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::<F>(this, which, &left, &right)?; + let res = bin_op_float::<F>(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) - /// - /// <https://www.felixcloutier.com/x86/sqrtss> - /// <https://www.felixcloutier.com/x86/sqrtps> - Sqrt, /// Approximation of 1/x /// /// <https://www.felixcloutier.com/x86/rcpss> @@ -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::<Single>(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::<Double>(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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected --> $DIR/aliasing_mut2.rs:LL:CC | LL | pub fn safe(x: &i32, y: &mut i32) { - | ^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID + | ^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] 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 <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected --> $DIR/aliasing_mut4.rs:LL:CC | LL | pub fn safe(x: &i32, y: &mut Cell<i32>) { - | ^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID + | ^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is weakly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is weakly protected --> $DIR/box_noalias_violation.rs:LL:CC | LL | *y - | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is weakly protected because it is an argument of call ID + | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> $DIR/illegal_write6.rs:LL:CC | LL | unsafe { *y = 2 }; - | ^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | unsafe { *x = 0 }; - | ^^^^^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] 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 <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected --> $DIR/invalidate_against_protector3.rs:LL:CC | LL | unsafe { *x = 0 }; - | ^^^^^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> $DIR/arg_inplace_mutate.rs:LL:CC | LL | unsafe { ptr.write(S(0)) }; - | ^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> $DIR/arg_inplace_observe_during.rs:LL:CC | LL | unsafe { ptr.read() }; - | ^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 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 index c8e0782eff2..c8e0782eff2 100644 --- 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 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 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 <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected + --> $DIR/return_pointer_aliasing_read.rs:LL:CC | LL | unsafe { ptr.read() }; - | ^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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: <TAG> 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: <TAG> 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 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 <TAG> (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 <TAG> (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 <TAG> (currently Active) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <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 <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 <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 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 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 <TAG> 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected + --> $DIR/return_pointer_aliasing_write.rs:LL:CC | LL | unsafe { ptr.write(0) }; - | ^^^^^^^^^^^^ - | | - | attempting a write access using <TAG> 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 <TAG> because that would remove [Unique for <TAG>] 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: <TAG> 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: <TAG> 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: <TAG> 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 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 <TAG> (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 <TAG> (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 <TAG> (currently Active) to become Disabled = help: protected tags must never be Disabled help: the accessed tag <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 <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 <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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected + --> $DIR/return_pointer_aliasing_write_tail_call.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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: <TAG> 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: <TAG> 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 <TAG> (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 <TAG> (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 <TAG> (root of the allocation) is foreign to the protected tag <TAG> (i.e., it is not a child) + = help: this foreign write access would cause the protected tag <TAG> (currently Active) to become Disabled + = help: protected tags must never be Disabled +help: the accessed tag <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 <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 <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 <TAG>] is strongly protected by call ID +error: Undefined Behavior: deallocating while item [Unique for <TAG>] is strongly protected --> RUSTLIB/alloc/src/alloc.rs:LL:CC | LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [Unique for <TAG>] is strongly protected by call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> $DIR/drop_in_place_protector.rs:LL:CC | LL | let _val = *P; - | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | let _val = unsafe { *x }; - | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is strongly protected because it is an argument of call ID + | ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] 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 | └─┬──<TAG=root of the allocation> -| RsM | └─┬──<TAG=base> -| RsM | ├─┬──<TAG=x> -| RsM | │ └─┬──<TAG=caller:x> -| Rs | │ └────<TAG=callee:x> Strongly protected -| RsM | └────<TAG=y, callee:y, caller:y> +| ReIM| └─┬──<TAG=base> +| ReIM| ├─┬──<TAG=x> +| ReIM| │ └─┬──<TAG=caller:x> +| Res | │ └────<TAG=callee:x> Strongly protected +| ReIM| └────<TAG=y, callee:y, caller:y> ────────────────────────────────────────────────── error: Undefined Behavior: write access through <TAG> (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 | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=n> -| Rs | ├─┬──<TAG=x> -| Rs | │ └─┬──<TAG=caller:x> -| Rs | │ └────<TAG=callee:x> Strongly protected -| Rs | └────<TAG=y, callee:y, caller:y> +| Res | └─┬──<TAG=n> +| Res | ├─┬──<TAG=x> +| Res | │ └─┬──<TAG=caller:x> +| Res | │ └────<TAG=callee:x> Strongly protected +| Res | └────<TAG=y, callee:y, caller:y> ────────────────────────────────────────────────── error: Undefined Behavior: write access through <TAG> (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..841761e53dd --- /dev/null +++ b/src/tools/miri/tests/pass-dep/libc/libc-epoll.rs @@ -0,0 +1,505 @@ +//@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_epoll_ctl_mod(); + test_epoll_ctl_del(); + test_pointer(); + test_two_same_fd_in_same_epoll_instance(); +} + +// 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<const N: usize>(epfd: i32, expected_notifications: &[(u32, u64)]) -> bool { + 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()); + let slice = unsafe { std::slice::from_raw_parts(array_ptr, res.try_into().unwrap()) }; + let mut return_events = slice.iter(); + let mut expected_notifications = expected_notifications.iter(); + while let Some(return_event) = return_events.next() { + if let Some(notification) = expected_notifications.next() { + let event = return_event.events; + let data = return_event.u64; + assert_eq!(event, notification.0); + assert_eq!(data, notification.1); + } else { + return false; + } + } + // There shouldn't be any more expected. + return expected_notifications.next().is_none(); +} + +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_ne!(res, -1); + + // 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(); + assert!(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. + assert!(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. + assert!(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).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + assert!(check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)])); +} + +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); + + // 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. + // (Not using checked cast as EPOLLET wraps around.) + 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_ne!(res, -1); + + // 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(); + assert!(check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)])); + + // Test 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_MOD, fds[1], &mut ev) }; + assert_ne!(res, -1); + + // Close the other side of the socketpair to invoke EPOLLRDHUP. + let res = unsafe { libc::close(fds[0]) }; + assert_eq!(res, 0); + + // Check result from epoll_wait. + let expected_event = u32::try_from(libc::EPOLLRDHUP | libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fds[1]).unwrap(); + assert!(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_ne!(res, -1); + + // Test EPOLL_CTL_DEL. + assert!(check_epoll_wait::<0>(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_ne!(res, -1); + let res = unsafe { libc::epoll_ctl(epfd2, libc::EPOLL_CTL_ADD, fds[1], &mut ev) }; + assert_ne!(res, -1); + + // 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(); + assert!(check_epoll_wait::<8>(epfd1, &[(expected_event, expected_value)])); + assert!(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_ne!(res, -1); + let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, newfd, &mut ev) }; + assert_ne!(res, -1); + + // 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; + assert!(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_ne!(res, -1); + + // Check result from epoll_wait. + let expected_event = u32::try_from(libc::EPOLLIN | libc::EPOLLOUT).unwrap(); + let expected_value = u64::try_from(fd).unwrap(); + assert!(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::<u64>::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_ne!(res, -1); +} + +// 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_ne!(res, -1); + 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_ne!(res, -1); + + // 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; + assert!(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; + assert!(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_ne!(res, -1); + + // 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. + assert!(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_ne!(res, -1); + + // 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; + assert!(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. + assert!(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_ne!(res, -1); + + // 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(); + assert!(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_ne!(res, -1); + 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_ne!(res, -1); + + // 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; + assert!(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. + // But in real system, no notification will be provided here. + let expected_event = u32::try_from(libc::EPOLLOUT).unwrap(); + let expected_value = fds[1] as u64; + assert!(check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)])); + + // 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; + assert!(check_epoll_wait::<8>(epfd, &[(expected_event, expected_value)])); +} 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/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 | └─┬──<TAG=root of the allocation> -| RsM | └────<TAG=data, x, y> +| ReIM| └────<TAG=data, x, y> ────────────────────────────────────────────────── ────────────────────────────────────────────────── 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 | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=data> -| Rs | └────<TAG=x> +| Res | └─┬──<TAG=data> +| Res | └────<TAG=x> ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=data> -| Rs | └─┬──<TAG=x> -| Rs | └─┬──<TAG=caller:x> -| Rs | └────<TAG=callee:x> Strongly protected +| Res | └─┬──<TAG=data> +| Res | └─┬──<TAG=x> +| Res | └─┬──<TAG=caller:x> +| Res | └────<TAG=callee:x> Strongly protected ────────────────────────────────────────────────── ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=data> -| Rs | ├─┬──<TAG=x> -| Rs | │ └─┬──<TAG=caller:x> -| Rs | │ └────<TAG=callee:x> -| Rs | └────<TAG=y> +| Res | └─┬──<TAG=data> +| Res | ├─┬──<TAG=x> +| Res | │ └─┬──<TAG=caller:x> +| Res | │ └────<TAG=callee:x> +| Res | └────<TAG=y> ────────────────────────────────────────────────── ────────────────────────────────────────────────── 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 | └─┬──<TAG=root of the allocation> -| Rs | Act | Rs | Act | Rs | Act | Rs | Act | Rs | └─┬──<TAG=data, data> +| Res | Act | Res | Act | Res | Act | Res | Act | Res | └─┬──<TAG=data, data> |-----| Act |-----|?Dis |-----|?Dis |-----|?Dis |-----| ├────<TAG=data[1]> |-----|-----|-----| Act |-----|?Dis |-----|?Dis |-----| ├────<TAG=data[10]> |-----|-----|-----|-----|-----| Frz |-----|?Dis |-----| ├────<TAG=data[100]> 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 | └─┬──<TAG=root of the allocation> | Act | └─┬──<TAG=parent> | Frz | ├────<TAG=x> -| Rs | └────<TAG=y> +| Res | └────<TAG=y> ────────────────────────────────────────────────── 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 | └─┬──<TAG=root of the allocation> -| RsM | └─┬──<TAG=base> -| RsM | ├─┬──<TAG=x> -| RsM | │ └─┬──<TAG=caller:x> -| RsC | │ └────<TAG=callee:x> -| RsM | └────<TAG=y, caller:y, callee:y> +| ReIM| └─┬──<TAG=base> +| ReIM| ├─┬──<TAG=x> +| ReIM| │ └─┬──<TAG=caller:x> +| ResC| │ └────<TAG=callee:x> +| ReIM| └────<TAG=y, caller:y, callee:y> ────────────────────────────────────────────────── [interior mut] Foreign Read: Re* -> Re* ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 8 | Act | └─┬──<TAG=root of the allocation> -| RsM | └─┬──<TAG=base> -| RsM | ├────<TAG=x> -| RsM | └────<TAG=y> +| ReIM| └─┬──<TAG=base> +| ReIM| ├────<TAG=x> +| ReIM| └────<TAG=y> ────────────────────────────────────────────────── [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 | └─┬──<TAG=root of the allocation> | Act | └─┬──<TAG=base> -| RsM | ├────<TAG=x> +| ReIM| ├────<TAG=x> | Act | └────<TAG=y> ────────────────────────────────────────────────── [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 | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=base> -| Rs | ├─┬──<TAG=x> -| Rs | │ └─┬──<TAG=caller:x> -| RsC | │ └────<TAG=callee:x> -| Rs | └────<TAG=y, caller:y, callee:y> +| Res | └─┬──<TAG=base> +| Res | ├─┬──<TAG=x> +| Res | │ └─┬──<TAG=caller:x> +| ResC| │ └────<TAG=callee:x> +| Res | └────<TAG=y, caller:y, callee:y> ────────────────────────────────────────────────── [] Foreign Read: Res -> Res ────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 | Act | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=base> -| Rs | ├────<TAG=x> -| Rs | └────<TAG=y> +| Res | └─┬──<TAG=base> +| Res | ├────<TAG=x> +| Res | └────<TAG=y> ────────────────────────────────────────────────── [] 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 | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=base> -| Rs | └────<TAG=raw, uniq, uniq> +| Res | └─┬──<TAG=base> +| Res | └────<TAG=raw, uniq, uniq> ────────────────────────────────────────────────── ────────────────────────────────────────────────── 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 | └─┬──<TAG=root of the allocation> -| Rs | └─┬──<TAG=base> -| Rs | └─┬──<TAG=raw> +| Res | └─┬──<TAG=base> +| Res | └─┬──<TAG=raw> |-----| └────<TAG=uniq, uniq> ────────────────────────────────────────────────── ────────────────────────────────────────────────── 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 | └─┬──<TAG=root of the allocation> -| Rs | └────<TAG=base.as_ptr(), base.as_ptr(), raw_parts.0, reconstructed.as_ptr(), reconstructed.as_ptr()> +| Res | └────<TAG=base.as_ptr(), base.as_ptr(), raw_parts.0, reconstructed.as_ptr(), reconstructed.as_ptr()> ────────────────────────────────────────────────── 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 // - // |- <XYZ: uniq> - // '- <XYZ: uniq> + // ├─ <TAG=base.as_ptr()> + // └─ <TAG=base.as_ptr()> // // then `nth_parent` is not big enough. // The correct value for `nth_parent` should be the minimum // integer for which the output shows // - // '- <XYZ: uniq, uniq> + // └─ <TAG=base.as_ptr(), base.as_ptr(), ...> // ) // // 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 | └─┬──<TAG=root of the allocation> -|-----| ├────<TAG=base.as_ptr()> -|-----| ├────<TAG=base.as_ptr()> -|-----| └─┬──<TAG=raw_parts.0> -|-----| ├────<TAG=reconstructed.as_ptr()> -|-----| └────<TAG=reconstructed.as_ptr()> +|-----| └─┬──<TAG=base.as_ptr(), base.as_ptr()> +|-----| └─┬──<TAG=raw_parts.0> +|-----| └────<TAG=reconstructed.as_ptr(), reconstructed.as_ptr()> ────────────────────────────────────────────────── 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<String> { // 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 = |