diff options
45 files changed, 758 insertions, 534 deletions
diff --git a/src/tools/miri/.github/workflows/sysroots.yml b/src/tools/miri/.github/workflows/sysroots.yml index 73a10768db9..6a4f44ddd50 100644 --- a/src/tools/miri/.github/workflows/sysroots.yml +++ b/src/tools/miri/.github/workflows/sysroots.yml @@ -13,7 +13,7 @@ jobs: name: Build the sysroots runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Build the sysroots run: | cargo install -f rustup-toolchain-install-master diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index dbb0e8a2925..d8636915ea8 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -187,7 +187,7 @@ Here is an example job for GitHub Actions: name: "Miri" runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install Miri run: | rustup toolchain install nightly --component miri @@ -216,9 +216,9 @@ degree documented below): - For every other target with OS `linux`, `macos`, or `windows`, Miri should generally work, but we make no promises and we don't run tests for such targets. - We have unofficial support (not maintained by the Miri team itself) for some further operating systems. + - `solaris` / `illumos`: maintained by @devnexen. Supports `std::{env, thread, sync}`, but not `std::fs`. - `freebsd`: **maintainer wanted**. Supports `std::env` and parts of `std::{thread, fs}`, but not `std::sync`. - `android`: **maintainer wanted**. Support very incomplete, but a basic "hello world" works. - - `solaris` / `illumos`: maintained by @devnexen. Support very incomplete, but a basic "hello world" works. - `wasm`: **maintainer wanted**. Support very incomplete, not even standard output works, but an empty `main` function works. - For targets on other operating systems, Miri might fail before even reaching the `main` function. diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 1f66b6fa776..c7be71662bd 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -26,7 +26,7 @@ time ./miri install # We enable all features to make sure the Stacked Borrows consistency check runs. echo "Building debug version of Miri" export CARGO_EXTRA_FLAGS="$CARGO_EXTRA_FLAGS --all-features" -time ./miri build --all-targets # the build that all the `./miri test` below will use +time ./miri build # the build that all the `./miri test` below will use endgroup @@ -66,7 +66,7 @@ function run_tests { time MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test $TARGET_FLAG tests/{pass,panic} fi if [ -n "${MANY_SEEDS-}" ]; then - # Also run some many-seeds tests. + # Also run some many-seeds tests. (Also tests `./miri run`.) time for FILE in tests/many-seeds/*.rs; do ./miri run "--many-seeds=0..$MANY_SEEDS" $TARGET_FLAG "$FILE" done @@ -75,6 +75,8 @@ function run_tests { # Check that the benchmarks build and run, but only once. time HYPERFINE="hyperfine -w0 -r1" ./miri bench $TARGET_FLAG fi + # Smoke-test `./miri run --dep`. + ./miri run $TARGET_FLAG --dep tests/pass-dep/getrandom.rs ## test-cargo-miri # On Windows, there is always "python", not "python3" or "python2". @@ -148,11 +150,11 @@ case $HOST_TARGET in # Partially supported targets (tier 2) BASIC="empty_main integer vec string btreemap hello hashmap heap_alloc align" # ensures we have the basics: stdout/stderr, system allocator, randomness (for HashMap initialization) UNIX="panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there - TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs - TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname libc-time fs - TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX threadname pthread-sync available-parallelism libc-time tls - TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC $UNIX threadname pthread-sync available-parallelism libc-time tls - TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX + TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname pthread time fs + TEST_TARGET=i686-unknown-freebsd run_tests_minimal $BASIC $UNIX threadname pthread time fs + TEST_TARGET=x86_64-unknown-illumos run_tests_minimal $BASIC $UNIX thread sync available-parallelism time tls + TEST_TARGET=x86_64-pc-solaris run_tests_minimal $BASIC $UNIX thread sync available-parallelism time tls + TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX pthread --skip threadname --skip pthread_cond_timedwait TEST_TARGET=wasm32-wasip2 run_tests_minimal empty_main wasm heap_alloc libc-mem TEST_TARGET=wasm32-unknown-unknown run_tests_minimal empty_main wasm TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std diff --git a/src/tools/miri/miri-script/Cargo.lock b/src/tools/miri/miri-script/Cargo.lock index 5e792abac17..8f19576c51d 100644 --- a/src/tools/miri/miri-script/Cargo.lock +++ b/src/tools/miri/miri-script/Cargo.lock @@ -61,9 +61,9 @@ checksum = "11157ac094ffbdde99aa67b23417ebdd801842852b500e395a45a9c0aac03e4a" [[package]] name = "errno" -version = "0.3.8" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a258e46cdc063eb8519c00b9fc845fc47bcfca4130e2f08e88665ceda8474245" +checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" dependencies = [ "libc", "windows-sys 0.52.0", @@ -99,6 +99,12 @@ dependencies = [ ] [[package]] +name = "itoa" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49f1f14873335454500d59611f1cf4a4b0f786f9ac11f4312a78e4cf2566695b" + +[[package]] name = "libc" version = "0.2.153" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -117,9 +123,15 @@ dependencies = [ [[package]] name = "linux-raw-sys" -version = "0.4.13" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01cda141df6706de531b6c46c3a33ecca755538219bd484262fa09410c13539c" +checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" + +[[package]] +name = "memchr" +version = "2.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "miri-script" @@ -131,6 +143,7 @@ dependencies = [ "itertools", "path_macro", "rustc_version", + "serde_json", "shell-words", "walkdir", "which", @@ -204,9 +217,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.31" +version = "0.38.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ea3e1a662af26cd7a3ba09c0297a31af215563ecf42817c98df621387f4e949" +checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f" dependencies = [ "bitflags 2.4.2", "errno", @@ -216,6 +229,12 @@ dependencies = [ ] [[package]] +name = "ryu" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3cb5ba0dc43242ce17de99c180e96db90b235b8a9fdc9543c96d2209116bd9f" + +[[package]] name = "same-file" version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -231,6 +250,38 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca" [[package]] +name = "serde" +version = "1.0.210" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8e3592472072e6e22e0a54d5904d9febf8508f65fb8552499a1abc7d1078c3a" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.210" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "243902eda00fad750862fc144cea25caca5e20d615af0a81bee94ca738f1df1f" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.128" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ff5456707a1de34e7e37f2a6fd3d3f808c318259cbd01ab6377795054b483d8" +dependencies = [ + "itoa", + "memchr", + "ryu", + "serde", +] + +[[package]] name = "shell-words" version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -347,7 +398,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.3", + "windows-targets 0.52.6", ] [[package]] @@ -367,17 +418,18 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d380ba1dc7187569a8a9e91ed34b8ccfc33123bbacb8c0aed2d1ad7f3ef2dc5f" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.3", - "windows_aarch64_msvc 0.52.3", - "windows_i686_gnu 0.52.3", - "windows_i686_msvc 0.52.3", - "windows_x86_64_gnu 0.52.3", - "windows_x86_64_gnullvm 0.52.3", - "windows_x86_64_msvc 0.52.3", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", + "windows_i686_gnullvm", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -388,9 +440,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68e5dcfb9413f53afd9c8f86e56a7b4d86d9a2fa26090ea2dc9e40fba56c6ec6" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" [[package]] name = "windows_aarch64_msvc" @@ -400,9 +452,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8dab469ebbc45798319e69eebf92308e541ce46760b49b18c6b3fe5e8965b30f" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_i686_gnu" @@ -412,9 +464,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.3" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a4e9b6a7cac734a8b4138a4e1044eac3404d8326b6c0f939276560687a033fb" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" [[package]] name = "windows_i686_msvc" @@ -424,9 +482,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28b0ec9c422ca95ff34a78755cfa6ad4a51371da2a5ace67500cf7ca5f232c58" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" [[package]] name = "windows_x86_64_gnu" @@ -436,9 +494,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "704131571ba93e89d7cd43482277d6632589b18ecf4468f591fbae0a8b101614" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" [[package]] name = "windows_x86_64_gnullvm" @@ -448,9 +506,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42079295511643151e98d61c38c0acc444e52dd42ab456f7ccfd5152e8ecf21c" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" [[package]] name = "windows_x86_64_msvc" @@ -460,9 +518,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.3" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0770833d60a970638e989b3fa9fd2bb1aaadcf88963d1659fd7d9990196ed2d6" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "xshell" diff --git a/src/tools/miri/miri-script/Cargo.toml b/src/tools/miri/miri-script/Cargo.toml index 631d3a82102..2922c24d6c0 100644 --- a/src/tools/miri/miri-script/Cargo.toml +++ b/src/tools/miri/miri-script/Cargo.toml @@ -23,3 +23,4 @@ xshell = "0.2.6" rustc_version = "0.4" dunce = "1.0.4" directories = "5" +serde_json = "1" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index a9a80175901..7348acac5a9 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -494,23 +494,21 @@ impl Command { flags: Vec<String>, ) -> Result<()> { let mut e = MiriEnv::new()?; + + // Preparation: get a sysroot, and get the miri binary. + let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?; + let miri_bin = + e.build_get_binary(".").context("failed to get filename of miri executable")?; + // More flags that we will pass before `flags` // (because `flags` may contain `--`). let mut early_flags = Vec::<OsString>::new(); - - // Add target, edition to flags. if let Some(target) = &target { early_flags.push("--target".into()); early_flags.push(target.into()); } - if verbose { - early_flags.push("--verbose".into()); - } early_flags.push("--edition".into()); early_flags.push(edition.as_deref().unwrap_or("2021").into()); - - // 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()); @@ -523,18 +521,19 @@ impl Command { let run_miri = |e: &MiriEnv, seed_flag: Option<String>| -> Result<()> { // The basic command that executes the Miri driver. let mut cmd = if dep { + // We invoke the test suite as that has all the logic for running with dependencies. e.cargo_cmd(".", "test") .args(&["--test", "ui"]) .args(quiet_flag) .arg("--") .args(&["--miri-run-dep-mode"]) } else { - e.cargo_cmd(".", "run").args(quiet_flag).arg("--") + cmd!(e.sh, "{miri_bin}") }; cmd.set_quiet(!verbose); // Add Miri flags let mut cmd = cmd.args(&miri_flags).args(&seed_flag).args(&early_flags).args(&flags); - // For `--dep` we also need to set the env var. + // For `--dep` we also need to set the target in the env var. if dep { if let Some(target) = &target { cmd = cmd.env("MIRI_TEST_TARGET", target); diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 35c604b407e..9d1a8e4fb1d 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -1,10 +1,11 @@ use std::ffi::{OsStr, OsString}; +use std::io::BufRead; use std::ops::Range; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; -use std::thread; +use std::{env, iter, thread}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use dunce::canonicalize; use path_macro::path; use xshell::{cmd, Cmd, Shell}; @@ -73,8 +74,11 @@ impl MiriEnv { let rustflags = { let mut flags = OsString::new(); // We set the rpath so that Miri finds the private rustc libraries it needs. - flags.push("-C link-args=-Wl,-rpath,"); - flags.push(libdir); + // (This only makes sense on Unix.) + if cfg!(unix) { + flags.push("-C link-args=-Wl,-rpath,"); + flags.push(&libdir); + } // Enable rustc-specific lints (ignored without `-Zunstable-options`). flags.push( " -Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes", @@ -88,6 +92,14 @@ impl MiriEnv { }; sh.set_var("RUSTFLAGS", rustflags); + // On Windows, the `-Wl,-rpath,` above does not help. Instead we add the libdir to the PATH, + // so that Windows can find the DLLs. + if cfg!(windows) { + let old_path = sh.var("PATH")?; + let new_path = env::join_paths(iter::once(libdir).chain(env::split_paths(&old_path)))?; + sh.set_var("PATH", new_path); + } + // 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); @@ -126,21 +138,40 @@ impl MiriEnv { 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. - // 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); + // We build all targets, since building *just* the bin target doesnot include + // `dev-dependencies` and that changes feature resolution. This also gets us more + // parallelism in `./miri test` as we build Miri and its tests together. + let mut cmd = + self.cargo_cmd(crate_dir, "build").args(&["--all-targets"]).args(quiet_flag).args(args); cmd.set_quiet(quiet); cmd.run()?; Ok(()) } + /// Returns the path to the main crate binary. Assumes that `build` has been called before. + pub fn build_get_binary(&self, crate_dir: impl AsRef<OsStr>) -> Result<PathBuf> { + let cmd = + self.cargo_cmd(crate_dir, "build").args(&["--all-targets", "--message-format=json"]); + let output = cmd.output()?; + let mut bin = None; + for line in output.stdout.lines() { + let line = line?; + if line.starts_with("{") { + let json: serde_json::Value = serde_json::from_str(&line)?; + if json["reason"] == "compiler-artifact" + && !json["profile"]["test"].as_bool().unwrap() + && !json["executable"].is_null() + { + if bin.is_some() { + bail!("found two binaries in cargo output"); + } + bin = Some(PathBuf::from(json["executable"].as_str().unwrap())) + } + } + } + bin.ok_or_else(|| anyhow!("found no binary in cargo output")) + } + 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(()) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 3fdad2a91e9..3f4d095fc19 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -54fdef7799d9ff9470bb5cabd29fde9471a99eaa +e2dc1a1c0f97a90319181a721ab317210307617a diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index f686b331ad6..b5b43f589f6 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -1165,7 +1165,7 @@ impl FrameState { } else { // This can fail to exist if `race_detecting` was false when the allocation // occurred, in which case we can backdate this to the beginning of time. - let clocks = clocks.entry(local).or_insert_with(Default::default); + let clocks = clocks.entry(local).or_default(); clocks.write = thread_clocks.clock[index]; clocks.write_type = NaWriteType::Write; } @@ -1186,7 +1186,7 @@ impl FrameState { // This can fail to exist if `race_detecting` was false when the allocation // occurred, in which case we can backdate this to the beginning of time. let mut clocks = self.local_clocks.borrow_mut(); - let clocks = clocks.entry(local).or_insert_with(Default::default); + let clocks = clocks.entry(local).or_default(); clocks.read = thread_clocks.clock[index]; } diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index d709e4e6eae..9c2c6ae1330 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -1,7 +1,6 @@ use std::collections::VecDeque; use rustc_index::Idx; -use rustc_middle::ty::layout::TyAndLayout; use super::sync::EvalContextExtPriv as _; use super::vector_clock::VClock; @@ -30,14 +29,12 @@ impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn init_once_get_or_create_id( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, ) -> InterpResult<'tcx, InitOnceId> { let this = self.eval_context_mut(); this.get_or_create_id( - lock_op, - lock_layout, + lock, offset, |ecx| &mut ecx.machine.sync.init_onces, |_| Ok(Default::default()), diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 97e910df6a2..1f910d885ca 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -1,10 +1,11 @@ +use std::any::Any; use std::collections::{hash_map::Entry, VecDeque}; use std::ops::Not; use std::time::Duration; use rustc_data_structures::fx::FxHashMap; use rustc_index::{Idx, IndexVec}; -use rustc_middle::ty::layout::TyAndLayout; +use rustc_target::abi::Size; use super::init_once::InitOnce; use super::vector_clock::VClock; @@ -66,27 +67,6 @@ pub(super) use declare_id; declare_id!(MutexId); -/// The mutex kind. -#[derive(Debug, Clone, Copy)] -#[non_exhaustive] -pub enum MutexKind { - Invalid, - Normal, - Default, - Recursive, - ErrorCheck, -} - -#[derive(Debug)] -/// Additional data that may be used by shim implementations. -pub struct AdditionalMutexData { - /// The mutex kind, used by some mutex implementations like pthreads mutexes. - pub kind: MutexKind, - - /// The address of the mutex. - pub address: u64, -} - /// The mutex state. #[derive(Default, Debug)] struct Mutex { @@ -100,7 +80,7 @@ struct Mutex { clock: VClock, /// Additional data that can be set by shim implementations. - data: Option<AdditionalMutexData>, + data: Option<Box<dyn Any>>, } declare_id!(RwLockId); @@ -137,6 +117,9 @@ struct RwLock { /// locks. /// This is only relevant when there is an active reader. clock_current_readers: VClock, + + /// Additional data that can be set by shim implementations. + data: Option<Box<dyn Any>>, } declare_id!(CondvarId); @@ -151,6 +134,9 @@ struct Condvar { /// Contains the clock of the last thread to /// perform a condvar-signal. clock: VClock, + + /// Additional data that can be set by shim implementations. + data: Option<Box<dyn Any>>, } /// The futex state. @@ -196,21 +182,21 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { #[inline] fn get_or_create_id<Id: SyncId + Idx, T>( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>, create_obj: impl for<'a> FnOnce(&'a mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>, ) -> InterpResult<'tcx, Option<Id>> { let this = self.eval_context_mut(); - let value_place = - this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?; + let offset = Size::from_bytes(offset); + assert!(lock.layout.size >= offset + this.machine.layouts.u32.size); + let id_place = lock.offset(offset, this.machine.layouts.u32, this)?; let next_index = get_objs(this).next_index(); // Since we are lazy, this update has to be atomic. let (old, success) = this .atomic_compare_exchange_scalar( - &value_place, + &id_place, &ImmTy::from_uint(0u32, this.machine.layouts.u32), Scalar::from_u32(next_index.to_u32()), AtomicRwOrd::Relaxed, // deliberately *no* synchronization @@ -248,18 +234,18 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { /// - `obj` must be the new sync object. fn create_id<Id: SyncId + Idx, T>( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>, obj: T, ) -> InterpResult<'tcx, Id> { let this = self.eval_context_mut(); - let value_place = - this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?; + let offset = Size::from_bytes(offset); + assert!(lock.layout.size >= offset + this.machine.layouts.u32.size); + let id_place = lock.offset(offset, this.machine.layouts.u32, this)?; let new_index = get_objs(this).push(obj); - this.write_scalar(Scalar::from_u32(new_index.to_u32()), &value_place)?; + this.write_scalar(Scalar::from_u32(new_index.to_u32()), &id_place)?; Ok(new_index) } @@ -292,15 +278,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Eagerly create and initialize a new mutex. fn mutex_create( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, - data: Option<AdditionalMutexData>, + data: Option<Box<dyn Any>>, ) -> InterpResult<'tcx, MutexId> { let this = self.eval_context_mut(); this.create_id( - lock_op, - lock_layout, + lock, offset, |ecx| &mut ecx.machine.sync.mutexes, Mutex { data, ..Default::default() }, @@ -311,17 +295,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// `initialize_data` must return any additional data that a user wants to associate with the mutex. fn mutex_get_or_create_id( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, initialize_data: impl for<'a> FnOnce( &'a mut MiriInterpCx<'tcx>, - ) -> InterpResult<'tcx, Option<AdditionalMutexData>>, + ) -> InterpResult<'tcx, Option<Box<dyn Any>>>, ) -> InterpResult<'tcx, MutexId> { let this = self.eval_context_mut(); this.get_or_create_id( - lock_op, - lock_layout, + lock, offset, |ecx| &mut ecx.machine.sync.mutexes, |ecx| initialize_data(ecx).map(|data| Mutex { data, ..Default::default() }), @@ -330,48 +312,84 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Retrieve the additional data stored for a mutex. - fn mutex_get_data<'a>(&'a mut self, id: MutexId) -> Option<&'a AdditionalMutexData> + fn mutex_get_data<'a, T: 'static>(&'a mut self, id: MutexId) -> Option<&'a T> where 'tcx: 'a, { let this = self.eval_context_ref(); - this.machine.sync.mutexes[id].data.as_ref() + this.machine.sync.mutexes[id].data.as_deref().and_then(|p| p.downcast_ref::<T>()) } fn rwlock_get_or_create_id( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, + initialize_data: impl for<'a> FnOnce( + &'a mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, Option<Box<dyn Any>>>, ) -> InterpResult<'tcx, RwLockId> { let this = self.eval_context_mut(); this.get_or_create_id( - lock_op, - lock_layout, + lock, offset, |ecx| &mut ecx.machine.sync.rwlocks, - |_| Ok(Default::default()), + |ecx| initialize_data(ecx).map(|data| RwLock { data, ..Default::default() }), )? .ok_or_else(|| err_ub_format!("rwlock has invalid ID").into()) } + /// Retrieve the additional data stored for a rwlock. + fn rwlock_get_data<'a, T: 'static>(&'a mut self, id: RwLockId) -> Option<&'a T> + where + 'tcx: 'a, + { + let this = self.eval_context_ref(); + this.machine.sync.rwlocks[id].data.as_deref().and_then(|p| p.downcast_ref::<T>()) + } + + /// Eagerly create and initialize a new condvar. + fn condvar_create( + &mut self, + condvar: &MPlaceTy<'tcx>, + offset: u64, + data: Option<Box<dyn Any>>, + ) -> InterpResult<'tcx, CondvarId> { + let this = self.eval_context_mut(); + this.create_id( + condvar, + offset, + |ecx| &mut ecx.machine.sync.condvars, + Condvar { data, ..Default::default() }, + ) + } + fn condvar_get_or_create_id( &mut self, - lock_op: &OpTy<'tcx>, - lock_layout: TyAndLayout<'tcx>, + lock: &MPlaceTy<'tcx>, offset: u64, + initialize_data: impl for<'a> FnOnce( + &'a mut MiriInterpCx<'tcx>, + ) -> InterpResult<'tcx, Option<Box<dyn Any>>>, ) -> InterpResult<'tcx, CondvarId> { let this = self.eval_context_mut(); this.get_or_create_id( - lock_op, - lock_layout, + lock, offset, |ecx| &mut ecx.machine.sync.condvars, - |_| Ok(Default::default()), + |ecx| initialize_data(ecx).map(|data| Condvar { data, ..Default::default() }), )? .ok_or_else(|| err_ub_format!("condvar has invalid ID").into()) } + /// Retrieve the additional data stored for a condvar. + fn condvar_get_data<'a, T: 'static>(&'a mut self, id: CondvarId) -> Option<&'a T> + where + 'tcx: 'a, + { + let this = self.eval_context_ref(); + this.machine.sync.condvars[id].data.as_deref().and_then(|p| p.downcast_ref::<T>()) + } + #[inline] /// Get the id of the thread that currently owns this lock. fn mutex_get_owner(&mut self, id: MutexId) -> ThreadId { diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 85b4b02ff5f..8a59206943d 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -133,10 +133,7 @@ pub use crate::concurrency::{ cpu_affinity::MAX_CPUS, data_race::{AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _}, init_once::{EvalContextExt as _, InitOnceId}, - sync::{ - AdditionalMutexData, CondvarId, EvalContextExt as _, MutexId, MutexKind, RwLockId, - SynchronizationObjects, - }, + sync::{CondvarId, EvalContextExt as _, MutexId, RwLockId, SynchronizationObjects}, thread::{ BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager, TimeoutAnchor, TimeoutClock, UnblockCallback, diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index 3ca5f6bb2df..48bf959538b 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -21,7 +21,7 @@ pub(crate) enum FlockOp { Unlock, } -/// Represents an open file descriptor. +/// Represents an open file description. pub trait FileDescription: std::fmt::Debug + Any { fn name(&self) -> &'static str; @@ -303,7 +303,7 @@ pub struct FdTable { impl VisitProvenance for FdTable { fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // All our FileDescriptor do not have any tags. + // All our FileDescription instances do not have any tags. } } @@ -337,18 +337,18 @@ impl FdTable { } pub fn insert(&mut self, fd_ref: FileDescriptionRef) -> i32 { - self.insert_ref_with_min_fd(fd_ref, 0) + self.insert_with_min_num(fd_ref, 0) } - /// Insert a file description, giving it a file descriptor that is at least `min_fd`. - fn insert_ref_with_min_fd(&mut self, file_handle: FileDescriptionRef, min_fd: i32) -> i32 { + /// Insert a file description, giving it a file descriptor that is at least `min_fd_num`. + fn insert_with_min_num(&mut self, file_handle: FileDescriptionRef, min_fd_num: i32) -> i32 { // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in // between used FDs, the find_map combinator will return it. If the first such unused FD // is after all other used FDs, the find_map combinator will return None, and we will use // the FD following the greatest FD thus far. let candidate_new_fd = - self.fds.range(min_fd..).zip(min_fd..).find_map(|((fd, _fh), counter)| { - if *fd != counter { + self.fds.range(min_fd_num..).zip(min_fd_num..).find_map(|((fd_num, _fd), counter)| { + if *fd_num != counter { // There was a gap in the fds stored, return the first unused one // (note that this relies on BTreeMap iterating in key order) Some(counter) @@ -357,61 +357,61 @@ impl FdTable { None } }); - let new_fd = candidate_new_fd.unwrap_or_else(|| { + let new_fd_num = candidate_new_fd.unwrap_or_else(|| { // find_map ran out of BTreeMap entries before finding a free fd, use one plus the // maximum fd in the map - self.fds.last_key_value().map(|(fd, _)| fd.strict_add(1)).unwrap_or(min_fd) + self.fds.last_key_value().map(|(fd_num, _)| fd_num.strict_add(1)).unwrap_or(min_fd_num) }); - self.fds.try_insert(new_fd, file_handle).unwrap(); - new_fd + self.fds.try_insert(new_fd_num, file_handle).unwrap(); + new_fd_num } - pub fn get(&self, fd: i32) -> Option<FileDescriptionRef> { - let fd = self.fds.get(&fd)?; + pub fn get(&self, fd_num: i32) -> Option<FileDescriptionRef> { + let fd = self.fds.get(&fd_num)?; Some(fd.clone()) } - pub fn remove(&mut self, fd: i32) -> Option<FileDescriptionRef> { - self.fds.remove(&fd) + pub fn remove(&mut self, fd_num: i32) -> Option<FileDescriptionRef> { + self.fds.remove(&fd_num) } - pub fn is_fd(&self, fd: i32) -> bool { - self.fds.contains_key(&fd) + pub fn is_fd_num(&self, fd_num: i32) -> bool { + self.fds.contains_key(&fd_num) } } impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { - fn dup(&mut self, old_fd: i32) -> InterpResult<'tcx, Scalar> { + fn dup(&mut self, old_fd_num: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let Some(dup_fd) = this.machine.fds.get(old_fd) else { + let Some(fd) = this.machine.fds.get(old_fd_num) 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))) + Ok(Scalar::from_i32(this.machine.fds.insert(fd))) } - fn dup2(&mut self, old_fd: i32, new_fd: i32) -> InterpResult<'tcx, Scalar> { + fn dup2(&mut self, old_fd_num: i32, new_fd_num: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let Some(dup_fd) = this.machine.fds.get(old_fd) else { + let Some(fd) = this.machine.fds.get(old_fd_num) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - if new_fd != old_fd { + if new_fd_num != old_fd_num { // Close new_fd if it is previously opened. // 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) { + if let Some(old_new_fd) = this.machine.fds.fds.insert(new_fd_num, fd) { // Ignore close error (not interpreter's) according to dup2() doc. - file_description.close(this.machine.communicate(), this)?.ok(); + old_new_fd.close(this.machine.communicate(), this)?.ok(); } } - Ok(Scalar::from_i32(new_fd)) + Ok(Scalar::from_i32(new_fd_num)) } - fn flock(&mut self, fd: i32, op: i32) -> InterpResult<'tcx, Scalar> { + fn flock(&mut self, fd_num: i32, op: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let Some(file_descriptor) = this.machine.fds.get(fd) else { + let Some(fd) = this.machine.fds.get(fd_num) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; @@ -436,8 +436,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("unsupported flags {:#x}", op); }; - let result = file_descriptor.flock(this.machine.communicate(), parsed_op)?; - drop(file_descriptor); + let result = fd.flock(this.machine.communicate(), parsed_op)?; + drop(fd); // return `0` if flock is successful let result = result.map(|()| 0i32); Ok(Scalar::from_i32(this.try_unwrap_io_result(result)?)) @@ -452,7 +452,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { args.len() ); } - let fd = this.read_scalar(&args[0])?.to_i32()?; + let fd_num = this.read_scalar(&args[0])?.to_i32()?; let cmd = this.read_scalar(&args[1])?.to_i32()?; // We only support getting the flags for a descriptor. @@ -461,7 +461,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // `FD_CLOEXEC` value without checking if the flag is set for the file because `std` // always sets this flag when opening a file. However we still need to check that the // file itself is open. - Ok(Scalar::from_i32(if this.machine.fds.is_fd(fd) { + Ok(Scalar::from_i32(if this.machine.fds.is_fd_num(fd_num) { this.eval_libc_i32("FD_CLOEXEC") } else { this.fd_not_found()? @@ -481,9 +481,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } let start = this.read_scalar(&args[2])?.to_i32()?; - match this.machine.fds.get(fd) { - Some(dup_fd) => - Ok(Scalar::from_i32(this.machine.fds.insert_ref_with_min_fd(dup_fd, start))), + match this.machine.fds.get(fd_num) { + Some(fd) => Ok(Scalar::from_i32(this.machine.fds.insert_with_min_num(fd, start))), None => Ok(Scalar::from_i32(this.fd_not_found()?)), } } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") { @@ -494,7 +493,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(-1)); } - this.ffullsync_fd(fd) + this.ffullsync_fd(fd_num) } else { throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd); } @@ -503,12 +502,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn close(&mut self, fd_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let fd = this.read_scalar(fd_op)?.to_i32()?; + let fd_num = this.read_scalar(fd_op)?.to_i32()?; - let Some(file_description) = this.machine.fds.remove(fd) else { + let Some(fd) = this.machine.fds.remove(fd_num) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; - let result = file_description.close(this.machine.communicate(), this)?; + let result = fd.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)?)) @@ -532,16 +531,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// and keeps the cursor unchanged. fn read( &mut self, - fd: i32, + fd_num: i32, buf: Pointer, count: u64, offset: Option<i128>, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - // Isolation check is done via `FileDescriptor` trait. + // Isolation check is done via `FileDescription` trait. - trace!("Reading from FD {}, size {}", fd, count); + trace!("Reading from FD {}, size {}", fd_num, count); // Check that the *entire* buffer is actually valid memory. this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; @@ -554,7 +553,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(fd) else { + let Some(fd) = this.machine.fds.get(fd_num) else { trace!("read: FD not found"); return Ok(Scalar::from_target_isize(this.fd_not_found()?, this)); }; @@ -597,14 +596,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn write( &mut self, - fd: i32, + fd_num: i32, buf: Pointer, count: u64, offset: Option<i128>, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - // Isolation check is done via `FileDescriptor` trait. + // Isolation check is done via `FileDescription` trait. // Check that the *entire* buffer is actually valid memory. this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; @@ -618,7 +617,7 @@ 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(fd) else { + let Some(fd) = this.machine.fds.get(fd_num) else { return Ok(Scalar::from_target_isize(this.fd_not_found()?, this)); }; diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index e00758bb98d..e1697a47415 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -554,10 +554,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Ok(Scalar::from_i32(this.try_unwrap_io_result(fd)?)) } - fn lseek64(&mut self, fd: i32, offset: i128, whence: i32) -> InterpResult<'tcx, Scalar> { + fn lseek64(&mut self, fd_num: i32, offset: i128, whence: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - // Isolation check is done via `FileDescriptor` trait. + // Isolation check is done via `FileDescription` trait. let seek_from = if whence == this.eval_libc_i32("SEEK_SET") { if offset < 0 { @@ -580,13 +580,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let communicate = this.machine.communicate(); - let Some(file_description) = this.machine.fds.get(fd) else { + let Some(fd) = this.machine.fds.get(fd_num) else { return Ok(Scalar::from_i64(this.fd_not_found()?)); }; - let result = file_description - .seek(communicate, seek_from)? - .map(|offset| i64::try_from(offset).unwrap()); - drop(file_description); + let result = fd.seek(communicate, seek_from)?.map(|offset| i64::try_from(offset).unwrap()); + drop(fd); let result = this.try_unwrap_io_result(result)?; Ok(Scalar::from_i64(result)) @@ -721,7 +719,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let metadata = match FileMetadata::from_fd(this, fd)? { + let metadata = match FileMetadata::from_fd_num(this, fd)? { Some(metadata) => metadata, None => return Ok(Scalar::from_i32(-1)), }; @@ -808,7 +806,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // If the path is empty, and the AT_EMPTY_PATH flag is set, we query the open file // represented by dirfd, whether it's a directory or otherwise. let metadata = if path.as_os_str().is_empty() && empty_path_flag { - FileMetadata::from_fd(this, dirfd)? + FileMetadata::from_fd_num(this, dirfd)? } else { FileMetadata::from_path(this, &path, follow_symlink)? }; @@ -1260,7 +1258,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { })) } - fn ftruncate64(&mut self, fd: i32, length: i128) -> InterpResult<'tcx, Scalar> { + fn ftruncate64(&mut self, fd_num: i32, length: i128) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); // Reject if isolation is enabled. @@ -1270,30 +1268,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let Some(file_description) = this.machine.fds.get(fd) else { + let Some(fd) = this.machine.fds.get(fd_num) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; // FIXME: Support ftruncate64 for all FDs - let FileHandle { file, writable } = - file_description.downcast::<FileHandle>().ok_or_else(|| { - err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors") - })?; + let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| { + err_unsup_format!("`ftruncate64` is only supported on file-backed file descriptors") + })?; if *writable { if let Ok(length) = length.try_into() { let result = file.set_len(length); - drop(file_description); + drop(fd); let result = this.try_unwrap_io_result(result.map(|_| 0i32))?; Ok(Scalar::from_i32(result)) } else { - drop(file_description); + drop(fd); let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; Ok(Scalar::from_i32(-1)) } } else { - drop(file_description); + drop(fd); // The file is not writable let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; @@ -1321,18 +1318,17 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { self.ffullsync_fd(fd) } - fn ffullsync_fd(&mut self, fd: i32) -> InterpResult<'tcx, Scalar> { + fn ffullsync_fd(&mut self, fd_num: i32) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let Some(file_description) = this.machine.fds.get(fd) else { + let Some(fd) = this.machine.fds.get(fd_num) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; // Only regular files support synchronization. - let FileHandle { file, writable } = - file_description.downcast::<FileHandle>().ok_or_else(|| { - err_unsup_format!("`fsync` is only supported on file-backed file descriptors") - })?; + let FileHandle { file, writable } = fd.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); - drop(file_description); + drop(fd); Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) } @@ -1348,16 +1344,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let Some(file_description) = this.machine.fds.get(fd) else { + let Some(fd) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; // Only regular files support synchronization. - let FileHandle { file, writable } = - file_description.downcast::<FileHandle>().ok_or_else(|| { - err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors") - })?; + let FileHandle { file, writable } = fd.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); - drop(file_description); + drop(fd); Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) } @@ -1396,18 +1391,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let Some(file_description) = this.machine.fds.get(fd) else { + let Some(fd) = this.machine.fds.get(fd) else { return Ok(Scalar::from_i32(this.fd_not_found()?)); }; // Only regular files support synchronization. - let FileHandle { file, writable } = - file_description.downcast::<FileHandle>().ok_or_else(|| { - err_unsup_format!( - "`sync_data_range` is only supported on file-backed file descriptors" - ) - })?; + let FileHandle { file, writable } = fd.downcast::<FileHandle>().ok_or_else(|| { + err_unsup_format!("`sync_data_range` is only supported on file-backed file descriptors") + })?; let io_result = maybe_sync_file(file, *writable, File::sync_data); - drop(file_description); + drop(fd); Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) } @@ -1699,15 +1691,15 @@ impl FileMetadata { FileMetadata::from_meta(ecx, metadata) } - fn from_fd<'tcx>( + fn from_fd_num<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - fd: i32, + fd_num: i32, ) -> InterpResult<'tcx, Option<FileMetadata>> { - let Some(file_description) = ecx.machine.fds.get(fd) else { + let Some(fd) = ecx.machine.fds.get(fd_num) else { return ecx.fd_not_found().map(|_: i32| None); }; - let file = &file_description + let file = &fd .downcast::<FileHandle>() .ok_or_else(|| { err_unsup_format!( @@ -1717,7 +1709,7 @@ impl FileMetadata { .file; let metadata = file.metadata(); - drop(file_description); + drop(fd); FileMetadata::from_meta(ecx, metadata) } diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index ee86cf5f26d..d91ce45e101 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -51,7 +51,7 @@ impl EpollEventInstance { #[derive(Clone, Debug)] pub struct EpollEventInterest { /// The file descriptor value of the file description registered. - file_descriptor: i32, + fd_num: i32, /// The events bitmask retrieved from `epoll_event`. events: u32, /// The data retrieved from `epoll_event`. @@ -62,7 +62,7 @@ pub struct EpollEventInterest { /// Ready list of the epoll instance under which this EpollEventInterest is registered. ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>, /// The file descriptor value that this EpollEventInterest is registered under. - epfd: i32, + epfd_num: i32, } /// EpollReadyEvents reflects the readiness of a file description. @@ -339,11 +339,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Create an epoll_interest. let interest = Rc::new(RefCell::new(EpollEventInterest { - file_descriptor: fd, + fd_num: fd, events, data, ready_list: Rc::clone(ready_list), - epfd: epfd_value, + epfd_num: epfd_value, })); if op == epoll_ctl_add { @@ -553,7 +553,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if is_updated { // Edge-triggered notification only notify one thread even if there are // multiple threads block on the same epfd. - let epfd = this.machine.fds.get(epoll_interest.borrow().epfd).unwrap(); + let epfd = this.machine.fds.get(epoll_interest.borrow().epfd_num).unwrap(); // This unwrap can never fail because if the current epoll instance were // closed and its epfd value reused, the upgrade of weak_epoll_interest @@ -615,7 +615,7 @@ fn check_and_update_one_event_interest<'tcx>( // 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 epoll_key = (id, epoll_event_interest.fd_num); let ready_list = &mut epoll_event_interest.ready_list.borrow_mut(); let event_instance = EpollEventInstance::new(flags, epoll_event_interest.data); // Triggers the notification by inserting it to the ready list. diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index 882c08cca15..8f9237a1d46 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -14,12 +14,13 @@ use crate::*; impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { - fn os_unfair_lock_getid(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> { + fn os_unfair_lock_getid(&mut self, lock_ptr: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> { let this = self.eval_context_mut(); + let lock = this.deref_pointer(lock_ptr)?; // os_unfair_lock holds a 32-bit value, is initialized with zero and // must be assumed to be opaque. Therefore, we can just store our // internal mutex ID in the structure without anyone noticing. - this.mutex_get_or_create_id(lock_op, this.libc_ty_layout("os_unfair_lock"), 0, |_| Ok(None)) + this.mutex_get_or_create_id(&lock, 0, |_| Ok(None)) } } diff --git a/src/tools/miri/src/shims/unix/mem.rs b/src/tools/miri/src/shims/unix/mem.rs index de5a5d0759c..33ed0e26982 100644 --- a/src/tools/miri/src/shims/unix/mem.rs +++ b/src/tools/miri/src/shims/unix/mem.rs @@ -42,10 +42,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let map_shared = this.eval_libc_i32("MAP_SHARED"); let map_fixed = this.eval_libc_i32("MAP_FIXED"); - // This is a horrible hack, but on MacOS and Solaris the guard page mechanism uses mmap + // This is a horrible hack, but on MacOS and Solarish the guard page mechanism uses mmap // in a way we do not support. We just give it the return value it expects. if this.frame_in_std() - && matches!(&*this.tcx.sess.target.os, "macos" | "solaris") + && matches!(&*this.tcx.sess.target.os, "macos" | "solaris" | "illumos") && (flags & map_fixed) != 0 { return Ok(Scalar::from_maybe_pointer(Pointer::from_addr_invalid(addr), this)); diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 57cc9cf4618..fea994663c0 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -11,17 +11,17 @@ use crate::*; #[inline] fn mutexattr_kind_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { Ok(match &*ecx.tcx.sess.target.os { - "linux" | "illumos" | "solaris" | "macos" => 0, + "linux" | "illumos" | "solaris" | "macos" | "freebsd" | "android" => 0, os => throw_unsup_format!("`pthread_mutexattr` is not supported on {os}"), }) } fn mutexattr_get_kind<'tcx>( ecx: &MiriInterpCx<'tcx>, - attr_op: &OpTy<'tcx>, + attr_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, i32> { ecx.deref_pointer_and_read( - attr_op, + attr_ptr, mutexattr_kind_offset(ecx)?, ecx.libc_ty_layout("pthread_mutexattr_t"), ecx.machine.layouts.i32, @@ -31,11 +31,11 @@ fn mutexattr_get_kind<'tcx>( fn mutexattr_set_kind<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - attr_op: &OpTy<'tcx>, + attr_ptr: &OpTy<'tcx>, kind: i32, ) -> InterpResult<'tcx, ()> { ecx.deref_pointer_and_write( - attr_op, + attr_ptr, mutexattr_kind_offset(ecx)?, Scalar::from_i32(kind), ecx.libc_ty_layout("pthread_mutexattr_t"), @@ -43,29 +43,40 @@ fn mutexattr_set_kind<'tcx>( ) } -/// A flag that allows to distinguish `PTHREAD_MUTEX_NORMAL` from -/// `PTHREAD_MUTEX_DEFAULT`. Since in `glibc` they have the same numeric values, -/// but different behaviour, we need a way to distinguish them. We do this by -/// setting this bit flag to the `PTHREAD_MUTEX_NORMAL` mutexes. See the comment -/// in `pthread_mutexattr_settype` function. -const PTHREAD_MUTEX_NORMAL_FLAG: i32 = 0x8000000; - -fn is_mutex_kind_default<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tcx, bool> { - Ok(kind == ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT")) +/// To differentiate "the mutex kind has not been changed" from +/// "the mutex kind has been set to PTHREAD_MUTEX_DEFAULT and that is +/// equal to some other mutex kind", we make the default value of this +/// field *not* PTHREAD_MUTEX_DEFAULT but this special flag. +const PTHREAD_MUTEX_KIND_UNCHANGED: i32 = 0x8000000; + +/// The mutex kind. +#[derive(Debug, Clone, Copy)] +pub enum MutexKind { + Normal, + Default, + Recursive, + ErrorCheck, } -fn is_mutex_kind_normal<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tcx, bool> { - let mutex_normal_kind = ecx.eval_libc_i32("PTHREAD_MUTEX_NORMAL"); - Ok(kind == (mutex_normal_kind | PTHREAD_MUTEX_NORMAL_FLAG)) +#[derive(Debug)] +/// Additional data that we attach with each mutex instance. +pub struct AdditionalMutexData { + /// The mutex kind, used by some mutex implementations like pthreads mutexes. + pub kind: MutexKind, + + /// The address of the mutex. + pub address: u64, } -// pthread_mutex_t is between 24 and 48 bytes, depending on the platform. +// pthread_mutex_t is between 4 and 48 bytes, depending on the platform. // We ignore the platform layout and store our own fields: // - id: u32 fn mutex_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { + // When adding a new OS, make sure we also support all its static initializers in + // `mutex_kind_from_static_initializer`! let offset = match &*ecx.tcx.sess.target.os { - "linux" | "illumos" | "solaris" => 0, + "linux" | "illumos" | "solaris" | "freebsd" | "android" => 0, // macOS stores a signature in the first bytes, so we have to move to offset 4. "macos" => 4, os => throw_unsup_format!("`pthread_mutex` is not supported on {os}"), @@ -77,15 +88,28 @@ fn mutex_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { // recursive or error checking mutexes. We should also add thme in this sanity check. static SANITY: AtomicBool = AtomicBool::new(false); if !SANITY.swap(true, Ordering::Relaxed) { - let static_initializer = ecx.eval_path(&["libc", "PTHREAD_MUTEX_INITIALIZER"]); - let id_field = static_initializer - .offset(Size::from_bytes(offset), ecx.machine.layouts.u32, ecx) - .unwrap(); - let id = ecx.read_scalar(&id_field).unwrap().to_u32().unwrap(); - assert_eq!( - id, 0, - "PTHREAD_MUTEX_INITIALIZER is incompatible with our pthread_mutex layout: id is not 0" - ); + let check_static_initializer = |name| { + let static_initializer = ecx.eval_path(&["libc", name]); + let id_field = static_initializer + .offset(Size::from_bytes(offset), ecx.machine.layouts.u32, ecx) + .unwrap(); + let id = ecx.read_scalar(&id_field).unwrap().to_u32().unwrap(); + assert_eq!(id, 0, "{name} is incompatible with our pthread_mutex layout: id is not 0"); + }; + + check_static_initializer("PTHREAD_MUTEX_INITIALIZER"); + // Check non-standard initializers. + match &*ecx.tcx.sess.target.os { + "linux" => { + check_static_initializer("PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP"); + check_static_initializer("PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP"); + check_static_initializer("PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP"); + } + "illumos" | "solaris" | "macos" | "freebsd" | "android" => { + // No non-standard initializers. + } + os => throw_unsup_format!("`pthread_mutex` is not supported on {os}"), + } } Ok(offset) @@ -94,15 +118,13 @@ fn mutex_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { /// Eagerly create and initialize a new mutex. fn mutex_create<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - mutex_op: &OpTy<'tcx>, - kind: i32, + mutex_ptr: &OpTy<'tcx>, + kind: MutexKind, ) -> InterpResult<'tcx> { - // FIXME: might be worth changing mutex_create to take the mplace - // rather than the `OpTy`. - let address = ecx.read_pointer(mutex_op)?.addr().bytes(); - let kind = translate_kind(ecx, kind)?; - let data = Some(AdditionalMutexData { address, kind }); - ecx.mutex_create(mutex_op, ecx.libc_ty_layout("pthread_mutex_t"), mutex_id_offset(ecx)?, data)?; + let mutex = ecx.deref_pointer(mutex_ptr)?; + let address = mutex.ptr().addr().bytes(); + let data = Box::new(AdditionalMutexData { address, kind }); + ecx.mutex_create(&mutex, mutex_id_offset(ecx)?, Some(data))?; Ok(()) } @@ -112,27 +134,23 @@ fn mutex_create<'tcx>( /// return an error if it has. fn mutex_get_id<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - mutex_op: &OpTy<'tcx>, + mutex_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, MutexId> { - let address = ecx.read_pointer(mutex_op)?.addr().bytes(); - - // FIXME: might be worth changing mutex_get_or_create_id to take the mplace - // rather than the `OpTy`. - let id = ecx.mutex_get_or_create_id( - mutex_op, - ecx.libc_ty_layout("pthread_mutex_t"), - mutex_id_offset(ecx)?, - |ecx| { - // This is called if a static initializer was used and the lock has not been assigned - // an ID yet. We have to determine the mutex kind from the static initializer. - let kind = kind_from_static_initializer(ecx, mutex_op)?; - - Ok(Some(AdditionalMutexData { kind, address })) - }, - )?; + let mutex = ecx.deref_pointer(mutex_ptr)?; + let address = mutex.ptr().addr().bytes(); + + let id = ecx.mutex_get_or_create_id(&mutex, mutex_id_offset(ecx)?, |ecx| { + // This is called if a static initializer was used and the lock has not been assigned + // an ID yet. We have to determine the mutex kind from the static initializer. + let kind = mutex_kind_from_static_initializer(ecx, &mutex)?; + + Ok(Some(Box::new(AdditionalMutexData { kind, address }))) + })?; // Check that the mutex has not been moved since last use. - let data = ecx.mutex_get_data(id).expect("data should be always exist for pthreads"); + let data = ecx + .mutex_get_data::<AdditionalMutexData>(id) + .expect("data should always exist for pthreads"); if data.address != address { throw_ub_format!("pthread_mutex_t can't be moved after first use") } @@ -141,51 +159,64 @@ fn mutex_get_id<'tcx>( } /// Returns the kind of a static initializer. -fn kind_from_static_initializer<'tcx>( +fn mutex_kind_from_static_initializer<'tcx>( ecx: &MiriInterpCx<'tcx>, - mutex_op: &OpTy<'tcx>, + mutex: &MPlaceTy<'tcx>, ) -> InterpResult<'tcx, MutexKind> { - // Only linux has static initializers other than PTHREAD_MUTEX_DEFAULT. - let kind = match &*ecx.tcx.sess.target.os { + Ok(match &*ecx.tcx.sess.target.os { + // Only linux has static initializers other than PTHREAD_MUTEX_DEFAULT. "linux" => { let offset = if ecx.pointer_size().bytes() == 8 { 16 } else { 12 }; - - ecx.deref_pointer_and_read( - mutex_op, - offset, - ecx.libc_ty_layout("pthread_mutex_t"), - ecx.machine.layouts.i32, - )? - .to_i32()? + let kind_place = + mutex.offset(Size::from_bytes(offset), ecx.machine.layouts.i32, ecx)?; + let kind = ecx.read_scalar(&kind_place)?.to_i32()?; + // Here we give PTHREAD_MUTEX_DEFAULT priority so that + // PTHREAD_MUTEX_INITIALIZER behaves like `pthread_mutex_init` with a NULL argument. + if kind == ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT") { + MutexKind::Default + } else { + mutex_translate_kind(ecx, kind)? + } } - | "illumos" | "solaris" | "macos" => ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT"), - os => throw_unsup_format!("`pthread_mutex` is not supported on {os}"), - }; - - translate_kind(ecx, kind) + _ => MutexKind::Default, + }) } -fn translate_kind<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tcx, MutexKind> { - Ok(if is_mutex_kind_default(ecx, kind)? { - MutexKind::Default - } else if is_mutex_kind_normal(ecx, kind)? { +fn mutex_translate_kind<'tcx>( + ecx: &MiriInterpCx<'tcx>, + kind: i32, +) -> InterpResult<'tcx, MutexKind> { + Ok(if kind == (ecx.eval_libc_i32("PTHREAD_MUTEX_NORMAL")) { MutexKind::Normal } else if kind == ecx.eval_libc_i32("PTHREAD_MUTEX_ERRORCHECK") { MutexKind::ErrorCheck } else if kind == ecx.eval_libc_i32("PTHREAD_MUTEX_RECURSIVE") { MutexKind::Recursive + } else if kind == ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT") + || kind == PTHREAD_MUTEX_KIND_UNCHANGED + { + // We check this *last* since PTHREAD_MUTEX_DEFAULT may be numerically equal to one of the + // others, and we want an explicit `mutexattr_settype` to work as expected. + MutexKind::Default } else { throw_unsup_format!("unsupported type of mutex: {kind}"); }) } -// pthread_rwlock_t is between 32 and 56 bytes, depending on the platform. +// pthread_rwlock_t is between 4 and 56 bytes, depending on the platform. // We ignore the platform layout and store our own fields: // - id: u32 +#[derive(Debug)] +/// Additional data that we attach with each rwlock instance. +pub struct AdditionalRwLockData { + /// The address of the rwlock. + pub address: u64, +} + fn rwlock_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { let offset = match &*ecx.tcx.sess.target.os { - "linux" | "illumos" | "solaris" => 0, + "linux" | "illumos" | "solaris" | "freebsd" | "android" => 0, // macOS stores a signature in the first bytes, so we have to move to offset 4. "macos" => 4, os => throw_unsup_format!("`pthread_rwlock` is not supported on {os}"), @@ -211,13 +242,24 @@ fn rwlock_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { fn rwlock_get_id<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - rwlock_op: &OpTy<'tcx>, + rwlock_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, RwLockId> { - ecx.rwlock_get_or_create_id( - rwlock_op, - ecx.libc_ty_layout("pthread_rwlock_t"), - rwlock_id_offset(ecx)?, - ) + let rwlock = ecx.deref_pointer(rwlock_ptr)?; + let address = rwlock.ptr().addr().bytes(); + + let id = ecx.rwlock_get_or_create_id(&rwlock, rwlock_id_offset(ecx)?, |_| { + Ok(Some(Box::new(AdditionalRwLockData { address }))) + })?; + + // Check that the rwlock has not been moved since last use. + let data = ecx + .rwlock_get_data::<AdditionalRwLockData>(id) + .expect("data should always exist for pthreads"); + if data.address != address { + throw_ub_format!("pthread_rwlock_t can't be moved after first use") + } + + Ok(id) } // pthread_condattr_t. @@ -227,7 +269,7 @@ fn rwlock_get_id<'tcx>( #[inline] fn condattr_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { Ok(match &*ecx.tcx.sess.target.os { - "linux" | "illumos" | "solaris" => 0, + "linux" | "illumos" | "solaris" | "freebsd" | "android" => 0, // macOS does not have a clock attribute. os => throw_unsup_format!("`pthread_condattr` clock field is not supported on {os}"), }) @@ -235,10 +277,10 @@ fn condattr_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u fn condattr_get_clock_id<'tcx>( ecx: &MiriInterpCx<'tcx>, - attr_op: &OpTy<'tcx>, + attr_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, i32> { ecx.deref_pointer_and_read( - attr_op, + attr_ptr, condattr_clock_offset(ecx)?, ecx.libc_ty_layout("pthread_condattr_t"), ecx.machine.layouts.i32, @@ -246,13 +288,26 @@ fn condattr_get_clock_id<'tcx>( .to_i32() } +fn cond_translate_clock_id<'tcx>( + ecx: &MiriInterpCx<'tcx>, + raw_id: i32, +) -> InterpResult<'tcx, ClockId> { + Ok(if raw_id == ecx.eval_libc_i32("CLOCK_REALTIME") { + ClockId::Realtime + } else if raw_id == ecx.eval_libc_i32("CLOCK_MONOTONIC") { + ClockId::Monotonic + } else { + throw_unsup_format!("unsupported clock id: {raw_id}"); + }) +} + fn condattr_set_clock_id<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - attr_op: &OpTy<'tcx>, + attr_ptr: &OpTy<'tcx>, clock_id: i32, ) -> InterpResult<'tcx, ()> { ecx.deref_pointer_and_write( - attr_op, + attr_ptr, condattr_clock_offset(ecx)?, Scalar::from_i32(clock_id), ecx.libc_ty_layout("pthread_condattr_t"), @@ -260,14 +315,13 @@ fn condattr_set_clock_id<'tcx>( ) } -// pthread_cond_t. +// pthread_cond_t can be only 4 bytes in size, depending on the platform. // We ignore the platform layout and store our own fields: // - id: u32 -// - clock: i32 fn cond_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { let offset = match &*ecx.tcx.sess.target.os { - "linux" | "illumos" | "solaris" => 0, + "linux" | "illumos" | "solaris" | "freebsd" | "android" => 0, // macOS stores a signature in the first bytes, so we have to move to offset 4. "macos" => 4, os => throw_unsup_format!("`pthread_cond` is not supported on {os}"), @@ -291,88 +345,42 @@ fn cond_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> { Ok(offset) } -/// Determines whether this clock represents the real-time clock, CLOCK_REALTIME. -fn is_cond_clock_realtime<'tcx>(ecx: &MiriInterpCx<'tcx>, clock_id: i32) -> bool { - // To ensure compatibility with PTHREAD_COND_INITIALIZER on all platforms, - // we can't just compare with CLOCK_REALTIME: on Solarish, PTHREAD_COND_INITIALIZER - // makes the clock 0 but CLOCK_REALTIME is 3. - // However, we need to always be able to distinguish this from CLOCK_MONOTONIC. - clock_id == ecx.eval_libc_i32("CLOCK_REALTIME") - || (clock_id == 0 && clock_id != ecx.eval_libc_i32("CLOCK_MONOTONIC")) +#[derive(Debug, Clone, Copy)] +enum ClockId { + Realtime, + Monotonic, } -fn cond_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> u64 { - // macOS doesn't have a clock attribute, but to keep the code uniform we store - // a clock ID in the pthread_cond_t anyway. There's enough space. - let offset = 8; +#[derive(Debug)] +/// Additional data that we attach with each cond instance. +struct AdditionalCondData { + /// The address of the cond. + address: u64, - // Sanity-check this against PTHREAD_COND_INITIALIZER (but only once): - // the clock must start out as CLOCK_REALTIME. - static SANITY: AtomicBool = AtomicBool::new(false); - if !SANITY.swap(true, Ordering::Relaxed) { - let static_initializer = ecx.eval_path(&["libc", "PTHREAD_COND_INITIALIZER"]); - let id_field = static_initializer - .offset(Size::from_bytes(offset), ecx.machine.layouts.i32, ecx) - .unwrap(); - let id = ecx.read_scalar(&id_field).unwrap().to_i32().unwrap(); - assert!( - is_cond_clock_realtime(ecx, id), - "PTHREAD_COND_INITIALIZER is incompatible with our pthread_cond layout: clock is not CLOCK_REALTIME" - ); - } - - offset + /// The clock id of the cond. + clock_id: ClockId, } fn cond_get_id<'tcx>( ecx: &mut MiriInterpCx<'tcx>, - cond_op: &OpTy<'tcx>, + cond_ptr: &OpTy<'tcx>, ) -> InterpResult<'tcx, CondvarId> { - ecx.condvar_get_or_create_id( - cond_op, - ecx.libc_ty_layout("pthread_cond_t"), - cond_id_offset(ecx)?, - ) -} + let cond = ecx.deref_pointer(cond_ptr)?; + let address = cond.ptr().addr().bytes(); + let id = ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?, |_ecx| { + // This used the static initializer. The clock there is always CLOCK_REALTIME. + Ok(Some(Box::new(AdditionalCondData { address, clock_id: ClockId::Realtime }))) + })?; -fn cond_reset_id<'tcx>( - ecx: &mut MiriInterpCx<'tcx>, - cond_op: &OpTy<'tcx>, -) -> InterpResult<'tcx, ()> { - ecx.deref_pointer_and_write( - cond_op, - cond_id_offset(ecx)?, - Scalar::from_i32(0), - ecx.libc_ty_layout("pthread_cond_t"), - ecx.machine.layouts.u32, - ) -} - -fn cond_get_clock_id<'tcx>( - ecx: &MiriInterpCx<'tcx>, - cond_op: &OpTy<'tcx>, -) -> InterpResult<'tcx, i32> { - ecx.deref_pointer_and_read( - cond_op, - cond_clock_offset(ecx), - ecx.libc_ty_layout("pthread_cond_t"), - ecx.machine.layouts.i32, - )? - .to_i32() -} + // Check that the mutex has not been moved since last use. + let data = ecx + .condvar_get_data::<AdditionalCondData>(id) + .expect("data should always exist for pthreads"); + if data.address != address { + throw_ub_format!("pthread_cond_t can't be moved after first use") + } -fn cond_set_clock_id<'tcx>( - ecx: &mut MiriInterpCx<'tcx>, - cond_op: &OpTy<'tcx>, - clock_id: i32, -) -> InterpResult<'tcx, ()> { - ecx.deref_pointer_and_write( - cond_op, - cond_clock_offset(ecx), - Scalar::from_i32(clock_id), - ecx.libc_ty_layout("pthread_cond_t"), - ecx.machine.layouts.i32, - ) + Ok(id) } impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -380,8 +388,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_mutexattr_init(&mut self, attr_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); - let default_kind = this.eval_libc_i32("PTHREAD_MUTEX_DEFAULT"); - mutexattr_set_kind(this, attr_op, default_kind)?; + mutexattr_set_kind(this, attr_op, PTHREAD_MUTEX_KIND_UNCHANGED)?; Ok(()) } @@ -394,30 +401,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let kind = this.read_scalar(kind_op)?.to_i32()?; - if kind == this.eval_libc_i32("PTHREAD_MUTEX_NORMAL") { - // In `glibc` implementation, the numeric values of - // `PTHREAD_MUTEX_NORMAL` and `PTHREAD_MUTEX_DEFAULT` are equal. - // However, a mutex created by explicitly passing - // `PTHREAD_MUTEX_NORMAL` type has in some cases different behaviour - // from the default mutex for which the type was not explicitly - // specified. For a more detailed discussion, please see - // https://github.com/rust-lang/miri/issues/1419. - // - // To distinguish these two cases in already constructed mutexes, we - // use the same trick as glibc: for the case when - // `pthread_mutexattr_settype` is called explicitly, we set the - // `PTHREAD_MUTEX_NORMAL_FLAG` flag. - let normal_kind = kind | PTHREAD_MUTEX_NORMAL_FLAG; - // Check that after setting the flag, the kind is distinguishable - // from all other kinds. - assert_ne!(normal_kind, this.eval_libc_i32("PTHREAD_MUTEX_DEFAULT")); - assert_ne!(normal_kind, this.eval_libc_i32("PTHREAD_MUTEX_ERRORCHECK")); - assert_ne!(normal_kind, this.eval_libc_i32("PTHREAD_MUTEX_RECURSIVE")); - mutexattr_set_kind(this, attr_op, normal_kind)?; - } else if kind == this.eval_libc_i32("PTHREAD_MUTEX_DEFAULT") + if kind == this.eval_libc_i32("PTHREAD_MUTEX_NORMAL") + || kind == this.eval_libc_i32("PTHREAD_MUTEX_DEFAULT") || kind == this.eval_libc_i32("PTHREAD_MUTEX_ERRORCHECK") || kind == this.eval_libc_i32("PTHREAD_MUTEX_RECURSIVE") { + // Make sure we do not mix this up with the "unchanged" kind. + assert_ne!(kind, PTHREAD_MUTEX_KIND_UNCHANGED); mutexattr_set_kind(this, attr_op, kind)?; } else { let einval = this.eval_libc_i32("EINVAL"); @@ -461,9 +451,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let attr = this.read_pointer(attr_op)?; let kind = if this.ptr_is_null(attr)? { - this.eval_libc_i32("PTHREAD_MUTEX_DEFAULT") + MutexKind::Default } else { - mutexattr_get_kind(this, attr_op)? + mutex_translate_kind(this, mutexattr_get_kind(this, attr_op)?)? }; mutex_create(this, mutex_op, kind)?; @@ -479,8 +469,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let id = mutex_get_id(this, mutex_op)?; - let kind = - this.mutex_get_data(id).expect("data should always exist for pthread mutexes").kind; + let kind = this + .mutex_get_data::<AdditionalMutexData>(id) + .expect("data should always exist for pthread mutexes") + .kind; let ret = if this.mutex_is_locked(id) { let owner_thread = this.mutex_get_owner(id); @@ -498,10 +490,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.mutex_lock(id); 0 } - _ => - throw_unsup_format!( - "called pthread_mutex_lock on an unsupported type of mutex" - ), } } } else { @@ -517,8 +505,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let id = mutex_get_id(this, mutex_op)?; - let kind = - this.mutex_get_data(id).expect("data should always exist for pthread mutexes").kind; + let kind = this + .mutex_get_data::<AdditionalMutexData>(id) + .expect("data should always exist for pthread mutexes") + .kind; Ok(Scalar::from_i32(if this.mutex_is_locked(id) { let owner_thread = this.mutex_get_owner(id); @@ -532,10 +522,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.mutex_lock(id); 0 } - _ => - throw_unsup_format!( - "called pthread_mutex_trylock on an unsupported type of mutex" - ), } } } else { @@ -549,8 +535,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let id = mutex_get_id(this, mutex_op)?; - let kind = - this.mutex_get_data(id).expect("data should always exist for pthread mutexes").kind; + let kind = this + .mutex_get_data::<AdditionalMutexData>(id) + .expect("data should always exist for pthread mutexes") + .kind; if let Some(_old_locked_count) = this.mutex_unlock(id)? { // The mutex was locked by the current thread. @@ -570,10 +558,6 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ), MutexKind::ErrorCheck | MutexKind::Recursive => Ok(Scalar::from_i32(this.eval_libc_i32("EPERM"))), - _ => - throw_unsup_format!( - "called pthread_mutex_unlock on an unsupported type of mutex" - ), } } } @@ -581,15 +565,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_mutex_destroy(&mut self, mutex_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); + // Reading the field also has the side-effect that we detect double-`destroy` + // since we make the field unint below. let id = mutex_get_id(this, mutex_op)?; if this.mutex_is_locked(id) { throw_ub_format!("destroyed a locked mutex"); } - // Destroying an uninit pthread_mutex is UB, so check to make sure it's not uninit. - mutex_get_id(this, mutex_op)?; - // This might lead to false positives, see comment in pthread_mutexattr_destroy this.write_uninit( &this.deref_pointer_as(mutex_op, this.libc_ty_layout("pthread_mutex_t"))?, @@ -691,15 +674,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_rwlock_destroy(&mut self, rwlock_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); + // Reading the field also has the side-effect that we detect double-`destroy` + // since we make the field unint below. let id = rwlock_get_id(this, rwlock_op)?; if this.rwlock_is_locked(id) { throw_ub_format!("destroyed a locked rwlock"); } - // Destroying an uninit pthread_rwlock is UB, so check to make sure it's not uninit. - rwlock_get_id(this, rwlock_op)?; - // This might lead to false positives, see comment in pthread_mutexattr_destroy this.write_uninit( &this.deref_pointer_as(rwlock_op, this.libc_ty_layout("pthread_rwlock_t"))?, @@ -789,11 +771,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } else { condattr_get_clock_id(this, attr_op)? }; - - // Write 0 to use the same code path as the static initializers. - cond_reset_id(this, cond_op)?; - - cond_set_clock_id(this, cond_op, clock_id)?; + let clock_id = cond_translate_clock_id(this, clock_id)?; + + let cond = this.deref_pointer(cond_op)?; + let address = cond.ptr().addr().bytes(); + this.condvar_create( + &cond, + cond_id_offset(this)?, + Some(Box::new(AdditionalCondData { address, clock_id })), + )?; Ok(()) } @@ -848,7 +834,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let mutex_id = mutex_get_id(this, mutex_op)?; // Extract the timeout. - let clock_id = cond_get_clock_id(this, cond_op)?; + let clock_id = this + .condvar_get_data::<AdditionalCondData>(id) + .expect("additional data should always be present for pthreads") + .clock_id; let duration = match this .read_timespec(&this.deref_pointer_as(abstime_op, this.libc_ty_layout("timespec"))?)? { @@ -859,13 +848,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(()); } }; - let timeout_clock = if is_cond_clock_realtime(this, clock_id) { - this.check_no_isolation("`pthread_cond_timedwait` with `CLOCK_REALTIME`")?; - TimeoutClock::RealTime - } else if clock_id == this.eval_libc_i32("CLOCK_MONOTONIC") { - TimeoutClock::Monotonic - } else { - throw_unsup_format!("unsupported clock id: {}", clock_id); + let timeout_clock = match clock_id { + ClockId::Realtime => { + this.check_no_isolation("`pthread_cond_timedwait` with `CLOCK_REALTIME`")?; + TimeoutClock::RealTime + } + ClockId::Monotonic => TimeoutClock::Monotonic, }; this.condvar_wait( @@ -883,15 +871,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_cond_destroy(&mut self, cond_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); + // Reading the field also has the side-effect that we detect double-`destroy` + // since we make the field unint below. let id = cond_get_id(this, cond_op)?; if this.condvar_is_awaited(id) { throw_ub_format!("destroying an awaited conditional variable"); } - // Destroying an uninit pthread_cond is UB, so check to make sure it's not uninit. - cond_get_id(this, cond_op)?; - cond_get_clock_id(this, cond_op)?; - // This might lead to false positives, see comment in pthread_mutexattr_destroy this.write_uninit(&this.deref_pointer_as(cond_op, this.libc_ty_layout("pthread_cond_t"))?)?; // FIXME: delete interpreter state associated with this condvar. diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index e1fbb77037c..51b0129356b 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -10,9 +10,10 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // Windows sync primitives are pointer sized. // We only use the first 4 bytes for the id. - fn init_once_get_id(&mut self, init_once_op: &OpTy<'tcx>) -> InterpResult<'tcx, InitOnceId> { + fn init_once_get_id(&mut self, init_once_ptr: &OpTy<'tcx>) -> InterpResult<'tcx, InitOnceId> { let this = self.eval_context_mut(); - this.init_once_get_or_create_id(init_once_op, this.windows_ty_layout("INIT_ONCE"), 0) + let init_once = this.deref_pointer(init_once_ptr)?; + this.init_once_get_or_create_id(&init_once, 0) } /// Returns `true` if we were succssful, `false` if we would block. diff --git a/src/tools/miri/test_dependencies/Cargo.lock b/src/tools/miri/test_dependencies/Cargo.lock index 39d41281728..9a4431eb704 100644 --- a/src/tools/miri/test_dependencies/Cargo.lock +++ b/src/tools/miri/test_dependencies/Cargo.lock @@ -172,6 +172,7 @@ dependencies = [ name = "miri-test-deps" version = "0.1.0" dependencies = [ + "cfg-if", "getrandom 0.1.16", "getrandom 0.2.15", "libc", diff --git a/src/tools/miri/test_dependencies/Cargo.toml b/src/tools/miri/test_dependencies/Cargo.toml index c24422df26c..e7eff46afca 100644 --- a/src/tools/miri/test_dependencies/Cargo.toml +++ b/src/tools/miri/test_dependencies/Cargo.toml @@ -11,6 +11,7 @@ edition = "2021" # all dependencies (and their transitive ones) listed here can be used in `tests/`. libc = "0.2" num_cpus = "1.10.1" +cfg-if = "1" getrandom_01 = { package = "getrandom", version = "0.1" } getrandom_02 = { package = "getrandom", version = "0.2", features = ["js"] } diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr new file mode 100644 index 00000000000..a15451cb319 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: pthread_cond_t can't be moved after first use + --> $DIR/libc_pthread_cond_move.rs:LL:CC + | +LL | libc::pthread_cond_destroy(cond2.as_mut_ptr()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_cond_t can't be moved after first use + | + = 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 `check` at $DIR/libc_pthread_cond_move.rs:LL:CC +note: inside `main` + --> $DIR/libc_pthread_cond_move.rs:LL:CC + | +LL | check() + | ^^^^^^^ + +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-dep/concurrency/libc_pthread_cond_move.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.rs new file mode 100644 index 00000000000..e4e84eb9fd0 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.rs @@ -0,0 +1,37 @@ +//@revisions: static_initializer init +//@ignore-target-windows: No pthreads on Windows + +/// Test that moving a pthread_cond between uses fails. + +fn main() { + check() +} + +#[cfg(init)] +fn check() { + unsafe { + use core::mem::MaybeUninit; + let mut cond = MaybeUninit::<libc::pthread_cond_t>::uninit(); + + libc::pthread_cond_init(cond.as_mut_ptr(), std::ptr::null()); + + // move pthread_cond_t + let mut cond2 = cond; + + libc::pthread_cond_destroy(cond2.as_mut_ptr()); //~[init] ERROR: pthread_cond_t can't be moved after first use + } +} + +#[cfg(static_initializer)] +fn check() { + unsafe { + let mut cond = libc::PTHREAD_COND_INITIALIZER; + + libc::pthread_cond_signal(&mut cond as *mut _); + + // move pthread_cond_t + let mut cond2 = cond; + + libc::pthread_cond_destroy(&mut cond2 as *mut _); //~[static_initializer] ERROR: pthread_cond_t can't be moved after first use + } +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr new file mode 100644 index 00000000000..4e4188e2a12 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: pthread_cond_t can't be moved after first use + --> $DIR/libc_pthread_cond_move.rs:LL:CC + | +LL | libc::pthread_cond_destroy(&mut cond2 as *mut _); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_cond_t can't be moved after first use + | + = 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 `check` at $DIR/libc_pthread_cond_move.rs:LL:CC +note: inside `main` + --> $DIR/libc_pthread_cond_move.rs:LL:CC + | +LL | check() + | ^^^^^^^ + +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-dep/concurrency/libc_pthread_mutex_default_deadlock.rs b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_default_deadlock.rs index 1038b8988f9..6723f2c6e77 100644 --- a/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_default_deadlock.rs +++ b/src/tools/miri/tests/fail-dep/concurrency/libc_pthread_mutex_default_deadlock.rs @@ -4,7 +4,8 @@ fn main() { unsafe { - let mutexattr: libc::pthread_mutexattr_t = std::mem::zeroed(); + let mut mutexattr: libc::pthread_mutexattr_t = std::mem::zeroed(); + assert_eq!(libc::pthread_mutexattr_init(&mut mutexattr as *mut _), 0); let mut mutex: libc::pthread_mutex_t = std::mem::zeroed(); assert_eq!(libc::pthread_mutex_init(&mut mutex as *mut _, &mutexattr as *const _), 0); assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0); diff --git a/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs new file mode 100644 index 00000000000..b51bae79849 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs @@ -0,0 +1,14 @@ +//@ignore-target-windows: No pthreads on Windows + +fn main() { + unsafe { + let mut rw = libc::PTHREAD_RWLOCK_INITIALIZER; + + libc::pthread_rwlock_rdlock(&mut rw as *mut _); + + // Move rwlock + let mut rw2 = rw; + + libc::pthread_rwlock_unlock(&mut rw2 as *mut _); //~ ERROR: pthread_rwlock_t can't be moved after first use + } +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr new file mode 100644 index 00000000000..8a5ec4aa98d --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: pthread_rwlock_t can't be moved after first use + --> $DIR/libx_pthread_rwlock_moved.rs:LL:CC + | +LL | libc::pthread_rwlock_unlock(&mut rw2 as *mut _); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_rwlock_t can't be moved after first use + | + = 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/libx_pthread_rwlock_moved.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/data_race/local_variable_read_race.rs b/src/tools/miri/tests/fail/data_race/local_variable_read_race.rs index 80d2b7b7c12..16a23f595ee 100644 --- a/src/tools/miri/tests/fail/data_race/local_variable_read_race.rs +++ b/src/tools/miri/tests/fail/data_race/local_variable_read_race.rs @@ -29,7 +29,7 @@ fn main() { // when it actually happened), we'd miss the UB in this test. // Also, the UB error should point at the write above, not the addr-of here. P.store(std::ptr::addr_of_mut!(val), Relaxed); - + // Wait for the thread to be done. t1.join().unwrap(); diff --git a/src/tools/miri/tests/fail/data_race/local_variable_write_race.rs b/src/tools/miri/tests/fail/data_race/local_variable_write_race.rs index eabbe4403c6..7e00573146c 100644 --- a/src/tools/miri/tests/fail/data_race/local_variable_write_race.rs +++ b/src/tools/miri/tests/fail/data_race/local_variable_write_race.rs @@ -28,7 +28,7 @@ fn main() { // when it actually happened), we'd miss the UB in this test. // Also, the UB error should point at the write above, not the addr-of here. P.store(std::ptr::addr_of_mut!(val), Relaxed); - + // Wait for the thread to be done. t1.join().unwrap(); diff --git a/src/tools/miri/tests/fail/provenance/int_copy_looses_provenance3.rs b/src/tools/miri/tests/fail/provenance/int_copy_looses_provenance3.rs index 48a48ce4587..d6bbfd7d94c 100644 --- a/src/tools/miri/tests/fail/provenance/int_copy_looses_provenance3.rs +++ b/src/tools/miri/tests/fail/provenance/int_copy_looses_provenance3.rs @@ -11,7 +11,7 @@ enum E { // Doing a copy at integer type should lose provenance. // This tests the case where provenacne is hiding in the discriminant of an enum. fn main() { - assert_eq!(mem::size_of::<E>(), 2*mem::size_of::<usize>()); + assert_eq!(mem::size_of::<E>(), 2 * mem::size_of::<usize>()); // We want to store provenance in the enum discriminant, but the value still needs to // be valid atfor the type. So we split provenance and data. diff --git a/src/tools/miri/tests/fail/uninit/padding-enum.rs b/src/tools/miri/tests/fail/uninit/padding-enum.rs index 3852ac5c477..a9628799b7d 100644 --- a/src/tools/miri/tests/fail/uninit/padding-enum.rs +++ b/src/tools/miri/tests/fail/uninit/padding-enum.rs @@ -7,17 +7,20 @@ enum E { Some(&'static (), &'static (), usize), } -fn main() { unsafe { - let mut p: mem::MaybeUninit<E> = mem::MaybeUninit::zeroed(); - // The copy when `E` is returned from `transmute` should destroy padding - // (even when we use `write_unaligned`, which under the hood uses an untyped copy). - p.as_mut_ptr().write_unaligned(mem::transmute((0usize, 0usize, 0usize))); - // This is a `None`, so everything but the discriminant is padding. - assert!(matches!(*p.as_ptr(), E::None)); +fn main() { + unsafe { + let mut p: mem::MaybeUninit<E> = mem::MaybeUninit::zeroed(); + // The copy when `E` is returned from `transmute` should destroy padding + // (even when we use `write_unaligned`, which under the hood uses an untyped copy). + p.as_mut_ptr().write_unaligned(mem::transmute((0usize, 0usize, 0usize))); + // This is a `None`, so everything but the discriminant is padding. + assert!(matches!(*p.as_ptr(), E::None)); - // Turns out the discriminant is (currently) stored - // in the 2nd pointer, so the first half is padding. - let c = &p as *const _ as *const u8; - let _val = *c.add(0); // Get a padding byte. - //~^ERROR: uninitialized -} } + // Turns out the discriminant is (currently) stored + // in the 2nd pointer, so the first half is padding. + let c = &p as *const _ as *const u8; + // Read a padding byte. + let _val = *c.add(0); + //~^ERROR: uninitialized + } +} diff --git a/src/tools/miri/tests/fail/uninit/padding-enum.stderr b/src/tools/miri/tests/fail/uninit/padding-enum.stderr index c571f188740..312cf6d20fa 100644 --- a/src/tools/miri/tests/fail/uninit/padding-enum.stderr +++ b/src/tools/miri/tests/fail/uninit/padding-enum.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory --> $DIR/padding-enum.rs:LL:CC | -LL | let _val = *c.add(0); // Get a padding byte. - | ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory +LL | let _val = *c.add(0); + | ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory | = 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 diff --git a/src/tools/miri/tests/fail/uninit/padding-struct.rs b/src/tools/miri/tests/fail/uninit/padding-struct.rs index dd3be503439..4cdc6f3a104 100644 --- a/src/tools/miri/tests/fail/uninit/padding-struct.rs +++ b/src/tools/miri/tests/fail/uninit/padding-struct.rs @@ -3,9 +3,12 @@ use std::mem; #[repr(C)] struct Pair(u8, u16); -fn main() { unsafe { - let p: Pair = mem::transmute(0u32); // The copy when `Pair` is returned from `transmute` should destroy padding. - let c = &p as *const _ as *const u8; - let _val = *c.add(1); // Get the padding byte. - //~^ERROR: uninitialized -} } +fn main() { + unsafe { + let p: Pair = mem::transmute(0u32); // The copy when `Pair` is returned from `transmute` should destroy padding. + let c = &p as *const _ as *const u8; + // Read the padding byte. + let _val = *c.add(1); + //~^ERROR: uninitialized + } +} diff --git a/src/tools/miri/tests/fail/uninit/padding-struct.stderr b/src/tools/miri/tests/fail/uninit/padding-struct.stderr index 8dc40a482ac..19e3969279b 100644 --- a/src/tools/miri/tests/fail/uninit/padding-struct.stderr +++ b/src/tools/miri/tests/fail/uninit/padding-struct.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory --> $DIR/padding-struct.rs:LL:CC | -LL | let _val = *c.add(1); // Get the padding byte. - | ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory +LL | let _val = *c.add(1); + | ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory | = 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 diff --git a/src/tools/miri/tests/fail/uninit/padding-union.rs b/src/tools/miri/tests/fail/uninit/padding-union.rs index 2e9e0a40d6c..294578607b3 100644 --- a/src/tools/miri/tests/fail/uninit/padding-union.rs +++ b/src/tools/miri/tests/fail/uninit/padding-union.rs @@ -6,9 +6,12 @@ union U { field: (u8, u16), } -fn main() { unsafe { - let p: U = mem::transmute(0u32); // The copy when `U` is returned from `transmute` should destroy padding. - let c = &p as *const _ as *const [u8; 4]; - let _val = *c; // Read the entire thing, definitely contains the padding byte. - //~^ERROR: uninitialized -} } +fn main() { + unsafe { + let p: U = mem::transmute(0u32); // The copy when `U` is returned from `transmute` should destroy padding. + let c = &p as *const _ as *const [u8; 4]; + // Read the entire thing, definitely contains the padding byte. + let _val = *c; + //~^ERROR: uninitialized + } +} diff --git a/src/tools/miri/tests/fail/uninit/padding-union.stderr b/src/tools/miri/tests/fail/uninit/padding-union.stderr index 04002da4f19..61a8d1c6ba6 100644 --- a/src/tools/miri/tests/fail/uninit/padding-union.stderr +++ b/src/tools/miri/tests/fail/uninit/padding-union.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: constructing invalid value at [1]: encountered uninitialized memory, but expected an integer --> $DIR/padding-union.rs:LL:CC | -LL | let _val = *c; // Read the entire thing, definitely contains the padding byte. - | ^^ constructing invalid value at [1]: encountered uninitialized memory, but expected an integer +LL | let _val = *c; + | ^^ constructing invalid value at [1]: encountered uninitialized memory, but expected an integer | = 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 diff --git a/src/tools/miri/tests/fail/uninit/padding-wide-ptr.rs b/src/tools/miri/tests/fail/uninit/padding-wide-ptr.rs index 0403a9caba6..4e363dbf81e 100644 --- a/src/tools/miri/tests/fail/uninit/padding-wide-ptr.rs +++ b/src/tools/miri/tests/fail/uninit/padding-wide-ptr.rs @@ -3,16 +3,19 @@ use std::mem; // If this is `None`, the metadata becomes padding. type T = Option<&'static str>; -fn main() { unsafe { - let mut p: mem::MaybeUninit<T> = mem::MaybeUninit::zeroed(); - // The copy when `T` is returned from `transmute` should destroy padding - // (even when we use `write_unaligned`, which under the hood uses an untyped copy). - p.as_mut_ptr().write_unaligned(mem::transmute((0usize, 0usize))); - // Null epresents `None`. - assert!(matches!(*p.as_ptr(), None)); +fn main() { + unsafe { + let mut p: mem::MaybeUninit<T> = mem::MaybeUninit::zeroed(); + // The copy when `T` is returned from `transmute` should destroy padding + // (even when we use `write_unaligned`, which under the hood uses an untyped copy). + p.as_mut_ptr().write_unaligned(mem::transmute((0usize, 0usize))); + // Null epresents `None`. + assert!(matches!(*p.as_ptr(), None)); - // The second part, with the length, becomes padding. - let c = &p as *const _ as *const u8; - let _val = *c.add(mem::size_of::<*const u8>()); // Get a padding byte. - //~^ERROR: uninitialized -} } + // The second part, with the length, becomes padding. + let c = &p as *const _ as *const u8; + // Read a padding byte. + let _val = *c.add(mem::size_of::<*const u8>()); + //~^ERROR: uninitialized + } +} diff --git a/src/tools/miri/tests/fail/uninit/padding-wide-ptr.stderr b/src/tools/miri/tests/fail/uninit/padding-wide-ptr.stderr index 0da72550b2e..24194c4b02a 100644 --- a/src/tools/miri/tests/fail/uninit/padding-wide-ptr.stderr +++ b/src/tools/miri/tests/fail/uninit/padding-wide-ptr.stderr @@ -1,8 +1,8 @@ error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory --> $DIR/padding-wide-ptr.rs:LL:CC | -LL | let _val = *c.add(mem::size_of::<*const u8>()); // Get a padding byte. - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory +LL | let _val = *c.add(mem::size_of::<*const u8>()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory | = 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 diff --git a/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_timedwait.rs b/src/tools/miri/tests/pass-dep/libc/libc_pthread_cond_timedwait.rs index d758168c7c3..d758168c7c3 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_timedwait.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc_pthread_cond_timedwait.rs diff --git a/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_timedwait_isolated.rs b/src/tools/miri/tests/pass-dep/libc/libc_pthread_cond_timedwait_isolated.rs index f1a3c5dc10d..f1a3c5dc10d 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_timedwait_isolated.rs +++ b/src/tools/miri/tests/pass-dep/libc/libc_pthread_cond_timedwait_isolated.rs diff --git a/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs b/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs index d66cd3bbb03..8be42b50897 100644 --- a/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs +++ b/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs @@ -10,16 +10,42 @@ fn main() { .collect::<String>(); fn set_thread_name(name: &CStr) -> i32 { - #[cfg(any(target_os = "linux", target_os = "illumos", target_os = "solaris"))] - return unsafe { libc::pthread_setname_np(libc::pthread_self(), name.as_ptr().cast()) }; - #[cfg(target_os = "freebsd")] - unsafe { - // pthread_set_name_np does not return anything - libc::pthread_set_name_np(libc::pthread_self(), name.as_ptr().cast()); - return 0; - }; - #[cfg(target_os = "macos")] - return unsafe { libc::pthread_setname_np(name.as_ptr().cast()) }; + cfg_if::cfg_if! { + if #[cfg(any(target_os = "linux", target_os = "illumos", target_os = "solaris"))] { + unsafe { libc::pthread_setname_np(libc::pthread_self(), name.as_ptr().cast()) } + } else if #[cfg(target_os = "freebsd")] { + // pthread_set_name_np does not return anything + unsafe { libc::pthread_set_name_np(libc::pthread_self(), name.as_ptr().cast()) }; + 0 + } else if #[cfg(target_os = "macos")] { + unsafe { libc::pthread_setname_np(name.as_ptr().cast()) } + } else { + compile_error!("set_thread_name not supported for this OS") + } + } + } + + fn get_thread_name(name: &mut [u8]) -> i32 { + cfg_if::cfg_if! { + if #[cfg(any( + target_os = "linux", + target_os = "illumos", + target_os = "solaris", + target_os = "macos" + ))] { + unsafe { + libc::pthread_getname_np(libc::pthread_self(), name.as_mut_ptr().cast(), name.len()) + } + } else if #[cfg(target_os = "freebsd")] { + // pthread_get_name_np does not return anything + unsafe { + libc::pthread_get_name_np(libc::pthread_self(), name.as_mut_ptr().cast(), name.len()) + }; + 0 + } else { + compile_error!("get_thread_name not supported for this OS") + } + } } let result = thread::Builder::new().name(long_name.clone()).spawn(move || { @@ -28,14 +54,7 @@ fn main() { // But the system is limited -- make sure we successfully set a truncation. let mut buf = vec![0u8; long_name.len() + 1]; - #[cfg(not(target_os = "freebsd"))] - unsafe { - libc::pthread_getname_np(libc::pthread_self(), buf.as_mut_ptr().cast(), buf.len()) - }; - #[cfg(target_os = "freebsd")] - unsafe { - libc::pthread_get_name_np(libc::pthread_self(), buf.as_mut_ptr().cast(), buf.len()) - }; + assert_eq!(get_thread_name(&mut buf), 0); let cstr = CStr::from_bytes_until_nul(&buf).unwrap(); assert!(cstr.to_bytes().len() >= 15, "name is too short: len={}", cstr.to_bytes().len()); // POSIX seems to promise at least 15 chars assert!(long_name.as_bytes().starts_with(cstr.to_bytes())); diff --git a/src/tools/miri/tests/pass/arrays.rs b/src/tools/miri/tests/pass/arrays.rs index b0c6f54cab8..5cb04d0f8be 100644 --- a/src/tools/miri/tests/pass/arrays.rs +++ b/src/tools/miri/tests/pass/arrays.rs @@ -62,7 +62,9 @@ fn debug() { } fn huge_zst() { - fn id<T>(x: T) -> T { x } + fn id<T>(x: T) -> T { + x + } // A "huge" zero-sized array. Make sure we don't loop over it in any part of Miri. let val = [(); usize::MAX]; diff --git a/src/tools/miri/tests/pass/async-niche-aliasing.rs b/src/tools/miri/tests/pass/async-niche-aliasing.rs index 7f19afb33e4..ab82c929a94 100644 --- a/src/tools/miri/tests/pass/async-niche-aliasing.rs +++ b/src/tools/miri/tests/pass/async-niche-aliasing.rs @@ -3,10 +3,10 @@ use std::{ future::Future, + mem::MaybeUninit, pin::Pin, sync::Arc, task::{Context, Poll, Wake}, - mem::MaybeUninit, }; struct ThingAdder<'a> { diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation.rs b/src/tools/miri/tests/pass/shims/time-with-isolation.rs index 645d42ad975..e7b16244123 100644 --- a/src/tools/miri/tests/pass/shims/time-with-isolation.rs +++ b/src/tools/miri/tests/pass/shims/time-with-isolation.rs @@ -41,9 +41,9 @@ fn test_block_for_one_second() { /// Ensures that we get the same behavior across all targets. fn test_deterministic() { let begin = Instant::now(); - for _ in 0..100_000 {} + for _ in 0..10_000 {} let time = begin.elapsed(); - println!("The loop took around {}s", time.as_secs()); + println!("The loop took around {}ms", time.as_millis()); println!("(It's fine for this number to change when you `--bless` this test.)") } diff --git a/src/tools/miri/tests/pass/shims/time-with-isolation.stdout b/src/tools/miri/tests/pass/shims/time-with-isolation.stdout index ff5889bacd5..2d7fb5f4a61 100644 --- a/src/tools/miri/tests/pass/shims/time-with-isolation.stdout +++ b/src/tools/miri/tests/pass/shims/time-with-isolation.stdout @@ -1,2 +1,2 @@ -The loop took around 12s +The loop took around 1250ms (It's fine for this number to change when you `--bless` this test.) |