From b5642b361f4cd4352892c32172b1ed839d46328f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 10:08:56 +0200 Subject: [PATCH 01/28] extern-so: give the version script a better name; show errors from failing to build the C lib --- .../{libcode.version => libtest.map} | 2 ++ src/tools/miri/tests/ui.rs | 20 +++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) rename src/tools/miri/tests/extern-so/{libcode.version => libtest.map} (72%) diff --git a/src/tools/miri/tests/extern-so/libcode.version b/src/tools/miri/tests/extern-so/libtest.map similarity index 72% rename from src/tools/miri/tests/extern-so/libcode.version rename to src/tools/miri/tests/extern-so/libtest.map index 0f04b9aaebb38..a9887a79a798b 100644 --- a/src/tools/miri/tests/extern-so/libcode.version +++ b/src/tools/miri/tests/extern-so/libtest.map @@ -1,9 +1,11 @@ CODEABI_1.0 { + # Define which symbols to export. global: *add_one_int*; *printer*; *test_stack_spill*; *get_unsigned_int*; *add_int16*; *add_short_to_long*; + # The rest remains private. local: *; }; diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index a75fa4cf986da..7e8d1401183b1 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -1,9 +1,10 @@ -use colored::*; -use regex::bytes::Regex; use std::ffi::OsString; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::{env, process::Command}; + +use colored::*; +use regex::bytes::Regex; use ui_test::color_eyre::eyre::{Context, Result}; use ui_test::{ status_emitter, CommandBuilder, Config, Format, Match, Mode, OutputConflictHandling, @@ -44,12 +45,15 @@ fn build_so_for_c_ffi_tests() -> PathBuf { // This is to avoid automatically adding `malloc`, etc. // Source: https://anadoxin.org/blog/control-over-symbol-exports-in-gcc.html/ "-fPIC", - "-Wl,--version-script=tests/extern-so/libcode.version", + "-Wl,--version-script=tests/extern-so/libtest.map", ]) .output() .expect("failed to generate shared object file for testing external C function calls"); if !cc_output.status.success() { - panic!("error in generating shared object file for testing external C function calls"); + panic!( + "error in generating shared object file for testing external C function calls:\n{}", + String::from_utf8_lossy(&cc_output.stderr), + ); } so_file_path } @@ -120,10 +124,10 @@ fn run_tests( config.program.args.push("--target".into()); config.program.args.push(target.into()); - // If we're on linux, and we're testing the extern-so functionality, - // then build the shared object file for testing external C function calls - // and push the relevant compiler flag. - if cfg!(target_os = "linux") && path.starts_with("tests/extern-so/") { + // If we're testing the extern-so functionality, then build the shared object file for testing + // external C function calls and push the relevant compiler flag. + if path.starts_with("tests/extern-so/") { + assert!(cfg!(target_os = "linux")); let so_file_path = build_so_for_c_ffi_tests(); let mut flag = std::ffi::OsString::from("-Zmiri-extern-so-file="); flag.push(so_file_path.into_os_string()); From 38780aa5a420c301b94345cfc372d8463ce46711 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 10:10:35 +0200 Subject: [PATCH 02/28] there is no need for these wildcards --- src/tools/miri/tests/extern-so/libtest.map | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/tests/extern-so/libtest.map b/src/tools/miri/tests/extern-so/libtest.map index a9887a79a798b..a57a4dc149feb 100644 --- a/src/tools/miri/tests/extern-so/libtest.map +++ b/src/tools/miri/tests/extern-so/libtest.map @@ -1,11 +1,12 @@ CODEABI_1.0 { # Define which symbols to export. - global: *add_one_int*; - *printer*; - *test_stack_spill*; - *get_unsigned_int*; - *add_int16*; - *add_short_to_long*; + global: + add_one_int; + printer; + test_stack_spill; + get_unsigned_int; + add_int16; + add_short_to_long; # The rest remains private. local: *; }; From 5d1e92d132208bb7347c22ac9c40577c1f285c28 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 09:36:40 +0200 Subject: [PATCH 03/28] ci: separetely control which host-only tests are run where No functional change in this commit --- src/tools/miri/ci/ci.sh | 54 +++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index cccf10a7d7032..8827cb2b60e99 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -44,7 +44,12 @@ export CARGO_EXTRA_FLAGS="$CARGO_EXTRA_FLAGS --all-features" endgroup -# Test +# Run tests. Recognizes these variables: +# - MIRI_TEST_TARGET: the target to test. Empty for host target. +# - GC_STRESS: if non-empty, run the GC stress test for the main test suite. +# - MIR_OPT: if non-empty, re-run test `pass` tests with mir-opt-level=4 +# - MANY_SEEDS: if set to N, run the "many-seeds" tests N times +# - TEST_BENCH: if non-empty, check that the benchmarks all build function run_tests { if [ -n "${MIRI_TEST_TARGET:-}" ]; then begingroup "Testing foreign architecture $MIRI_TEST_TARGET" @@ -53,18 +58,14 @@ function run_tests { fi ## ui test suite - # On the host, also stress-test the GC. - if [ -z "${MIRI_TEST_TARGET:-}" ]; then + if [ -n "${GC_STRESS:-}" ]; then MIRIFLAGS="${MIRIFLAGS:-} -Zmiri-provenance-gc=1" ./miri test else ./miri test fi - # Host-only tests - if [ -z "${MIRI_TEST_TARGET:-}" ]; then - # Running these on all targets is unlikely to catch more problems and would - # cost a lot of CI time. - + ## advanced tests + if [ -n "${MIR_OPT:-}" ]; then # Tests with optimizations (`-O` is what cargo passes, but crank MIR optimizations up all the # way, too). # Optimizations change diagnostics (mostly backtraces), so we don't check @@ -72,13 +73,15 @@ function run_tests { # We explicitly enable debug-assertions here, they are disabled by -O but we have tests # which exist to check that we panic on debug assertion failures. MIRIFLAGS="${MIRIFLAGS:-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} - + fi + if [ -n "${MANY_SEEDS:-}" ]; then # Also run some many-seeds tests. 64 seeds means this takes around a minute per test. # (Need to invoke via explicit `bash -c` for Windows.) for FILE in tests/many-seeds/*.rs; do - MIRI_SEEDS=64 ./miri many-seeds "$BASH" -c "./miri run '$FILE'" + MIRI_SEEDS=$MANY_SEEDS ./miri many-seeds "$BASH" -c "./miri run '$FILE'" done - + fi + if [ -n "${TEST_BENCH:-}" ]; then # Check that the benchmarks build and run, but without actually benchmarking. HYPERFINE="'$BASH' -c" ./miri bench fi @@ -126,34 +129,43 @@ function run_tests_minimal { ## Main Testing Logic ## -# Host target. -run_tests - -# Extra targets. # In particular, fully cover all tier 1 targets. case $HOST_TARGET in x86_64-unknown-linux-gnu) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests + # Extra tier 1 MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests - MIRI_TEST_TARGET=aarch64-apple-darwin run_tests MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests + # Extra tier 2 + MIRI_TEST_TARGET=aarch64-apple-darwin run_tests MIRI_TEST_TARGET=arm-unknown-linux-gnueabi run_tests - # Some targets are only partially supported. + # Partially supported targets (tier 2) MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align num_cpus MIRI_TEST_TARGET=i686-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple pthread-threadname libc-getentropy libc-getrandom libc-misc libc-fs atomic env align num_cpus - MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal hello integer vec panic/panic MIRI_TEST_TARGET=wasm32-wasi run_tests_minimal no_std integer strings wasm MIRI_TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std integer strings wasm - MIRI_TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std # no_std embedded architecture - MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std # JSON target file + MIRI_TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std + # Custom target JSON file + MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std ;; x86_64-apple-darwin) - MIRI_TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests + # Extra tier 1 MIRI_TEST_TARGET=x86_64-pc-windows-msvc run_tests + # Extra tier 2 + MIRI_TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture ;; i686-pc-windows-msvc) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests + # Extra tier 1 + # We really want to ensure a Linux target works on a Windows host, + # and a 64bit target works on a 32bit host. MIRI_TEST_TARGET=x86_64-unknown-linux-gnu run_tests ;; *) From 9395e1fa6fa60dd6cee4c5f37e7c649332346500 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 09:44:55 +0200 Subject: [PATCH 04/28] bash: use variable expansion that properly distinguishes null vs non-existing --- src/tools/miri/ci/ci.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 8827cb2b60e99..5c93cc7feedbd 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -51,37 +51,37 @@ endgroup # - MANY_SEEDS: if set to N, run the "many-seeds" tests N times # - TEST_BENCH: if non-empty, check that the benchmarks all build function run_tests { - if [ -n "${MIRI_TEST_TARGET:-}" ]; then + if [ -n "${MIRI_TEST_TARGET-}" ]; then begingroup "Testing foreign architecture $MIRI_TEST_TARGET" else begingroup "Testing host architecture" fi ## ui test suite - if [ -n "${GC_STRESS:-}" ]; then - MIRIFLAGS="${MIRIFLAGS:-} -Zmiri-provenance-gc=1" ./miri test + if [ -n "${GC_STRESS-}" ]; then + MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test else ./miri test fi ## advanced tests - if [ -n "${MIR_OPT:-}" ]; then + if [ -n "${MIR_OPT-}" ]; then # Tests with optimizations (`-O` is what cargo passes, but crank MIR optimizations up all the # way, too). # Optimizations change diagnostics (mostly backtraces), so we don't check # them. Also error locations change so we don't run the failing tests. # We explicitly enable debug-assertions here, they are disabled by -O but we have tests # which exist to check that we panic on debug assertion failures. - MIRIFLAGS="${MIRIFLAGS:-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} + MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} fi - if [ -n "${MANY_SEEDS:-}" ]; then + if [ -n "${MANY_SEEDS-}" ]; then # Also run some many-seeds tests. 64 seeds means this takes around a minute per test. # (Need to invoke via explicit `bash -c` for Windows.) for FILE in tests/many-seeds/*.rs; do MIRI_SEEDS=$MANY_SEEDS ./miri many-seeds "$BASH" -c "./miri run '$FILE'" done fi - if [ -n "${TEST_BENCH:-}" ]; then + if [ -n "${TEST_BENCH-}" ]; then # Check that the benchmarks build and run, but without actually benchmarking. HYPERFINE="'$BASH' -c" ./miri bench fi @@ -112,7 +112,7 @@ function run_tests { } function run_tests_minimal { - if [ -n "${MIRI_TEST_TARGET:-}" ]; then + if [ -n "${MIRI_TEST_TARGET-}" ]; then begingroup "Testing MINIMAL foreign architecture $MIRI_TEST_TARGET: only testing $@" else echo "run_tests_minimal requires MIRI_TEST_TARGET to be set" @@ -169,7 +169,7 @@ case $HOST_TARGET in MIRI_TEST_TARGET=x86_64-unknown-linux-gnu run_tests ;; *) - echo "FATAL: unknown OS" + echo "FATAL: unknown host target: $HOST_TARGET" exit 1 ;; esac From 836ea71d05584645a8ba340c5236dc0e7aa66226 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 09:45:38 +0200 Subject: [PATCH 05/28] windows ci: run many-seeds test less often --- src/tools/miri/ci/ci.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 5c93cc7feedbd..e14bc56cb78ec 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -162,7 +162,8 @@ case $HOST_TARGET in ;; i686-pc-windows-msvc) # Host - GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests + # Only smoke-test `many-seeds`; 64 runs take 15min here! + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=1 TEST_BENCH=1 run_tests # Extra tier 1 # We really want to ensure a Linux target works on a Windows host, # and a 64bit target works on a 32bit host. From e9ee19004e1b5a412cce369838f26114886d9f48 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 09:50:49 +0200 Subject: [PATCH 06/28] also control the cargo-miri env var test separately --- src/tools/miri/ci/ci.sh | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index e14bc56cb78ec..4e94ca41b7ff3 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -50,6 +50,7 @@ endgroup # - MIR_OPT: if non-empty, re-run test `pass` tests with mir-opt-level=4 # - MANY_SEEDS: if set to N, run the "many-seeds" tests N times # - TEST_BENCH: if non-empty, check that the benchmarks all build +# - CARGO_MIRI_ENV: if non-empty, set some env vars and config to potentially confuse cargo-miri function run_tests { if [ -n "${MIRI_TEST_TARGET-}" ]; then begingroup "Testing foreign architecture $MIRI_TEST_TARGET" @@ -94,14 +95,16 @@ function run_tests { PYTHON=python fi # Some environment setup that attempts to confuse the heck out of cargo-miri. - if [ "$HOST_TARGET" = x86_64-unknown-linux-gnu ]; then - # These act up on Windows (`which miri` produces a filename that does not exist?!?), - # so let's do this only on Linux. Also makes sure things work without these set. - export RUSTC=$(which rustc) # Produces a warning unless we also set MIRI + if [ -n "${CARGO_MIRI_ENV-}" ]; then + # These act up on Windows (`which miri` produces a filename that does not exist?!?). + # RUSTC is the main thing to set (it changes the first argument our wrapper will see). + # Unless MIRI is also set, that produces a warning. + export RUSTC=$(which rustc) export MIRI=$(rustc +miri --print sysroot)/bin/miri + # We entirely ignore other wrappers. + mkdir -p .cargo + echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml fi - mkdir -p .cargo - echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml # Run the actual test ${PYTHON} test-cargo-miri/run-test.py # Clean up @@ -133,7 +136,7 @@ function run_tests_minimal { case $HOST_TARGET in x86_64-unknown-linux-gnu) # Host - GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests @@ -154,9 +157,9 @@ case $HOST_TARGET in ;; x86_64-apple-darwin) # Host - GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 - MIRI_TEST_TARGET=x86_64-pc-windows-msvc run_tests + MIRI_TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests # Extra tier 2 MIRI_TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture ;; From 540f2411e6f9f039b5e4cd4369bfddf0a0479731 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 10:32:29 +0200 Subject: [PATCH 07/28] show the time that the main CI steps take --- src/tools/miri/ci/ci.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 4e94ca41b7ff3..2cd918fc2b15e 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -60,9 +60,9 @@ function run_tests { ## ui test suite if [ -n "${GC_STRESS-}" ]; then - MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test + time MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test else - ./miri test + time ./miri test fi ## advanced tests @@ -73,18 +73,18 @@ function run_tests { # them. Also error locations change so we don't run the failing tests. # We explicitly enable debug-assertions here, they are disabled by -O but we have tests # which exist to check that we panic on debug assertion failures. - MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} + time MIRIFLAGS="${MIRIFLAGS-} -O -Zmir-opt-level=4 -Cdebug-assertions=yes" MIRI_SKIP_UI_CHECKS=1 ./miri test -- tests/{pass,panic} fi if [ -n "${MANY_SEEDS-}" ]; then # Also run some many-seeds tests. 64 seeds means this takes around a minute per test. # (Need to invoke via explicit `bash -c` for Windows.) - for FILE in tests/many-seeds/*.rs; do + time for FILE in tests/many-seeds/*.rs; do MIRI_SEEDS=$MANY_SEEDS ./miri many-seeds "$BASH" -c "./miri run '$FILE'" done fi if [ -n "${TEST_BENCH-}" ]; then # Check that the benchmarks build and run, but without actually benchmarking. - HYPERFINE="'$BASH' -c" ./miri bench + time HYPERFINE="'$BASH' -c" ./miri bench fi ## test-cargo-miri @@ -106,7 +106,7 @@ function run_tests { echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml fi # Run the actual test - ${PYTHON} test-cargo-miri/run-test.py + time ${PYTHON} test-cargo-miri/run-test.py # Clean up unset RUSTC MIRI rm -rf .cargo From 660a5713f527baaa4da3a400e15d83b677ed02fb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 10:37:59 +0200 Subject: [PATCH 08/28] python: fix regex backslash escapes --- src/tools/miri/test-cargo-miri/run-test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/test-cargo-miri/run-test.py b/src/tools/miri/test-cargo-miri/run-test.py index 21479bc4d58e4..cac11dff77557 100755 --- a/src/tools/miri/test-cargo-miri/run-test.py +++ b/src/tools/miri/test-cargo-miri/run-test.py @@ -31,11 +31,11 @@ def cargo_miri(cmd, quiet = True): def normalize_stdout(str): str = str.replace("src\\", "src/") # normalize paths across platforms - str = re.sub("finished in \d+\.\d\ds", "finished in $TIME", str) # the time keeps changing, obviously + str = re.sub("finished in \\d+\\.\\d\\ds", "finished in $TIME", str) # the time keeps changing, obviously return str def normalize_stderr(str): - str = re.sub("Preparing a sysroot for Miri \(target: [a-z0-9_-]+\)\.\.\. done\n", "", str) # remove leading cargo-miri setup output + str = re.sub("Preparing a sysroot for Miri \\(target: [a-z0-9_-]+\\)\\.\\.\\. done\n", "", str) # remove leading cargo-miri setup output return str def check_output(actual, path, name): From a8edf26a61158c5074ce121dd3c65d5a9509eb38 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 10:44:18 +0200 Subject: [PATCH 09/28] move './miri check' for features to the style check job --- src/tools/miri/.github/workflows/ci.yml | 4 ++++ src/tools/miri/ci/ci.sh | 11 +++-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 2c5868d5b37a3..c5979e18a3b59 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -130,6 +130,10 @@ jobs: run: ./miri fmt --check - name: clippy run: ./miri clippy -- -D warnings + - name: clippy (no features) + run: ./miri clippy --no-default-features -- -D warnings + - name: clippy (all features) + run: ./miri clippy --all-features -- -D warnings - name: rustdoc run: RUSTDOCFLAGS="-Dwarnings" ./miri cargo doc --document-private-items diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 2cd918fc2b15e..32b58547d7c2b 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -27,20 +27,15 @@ export RUSTFLAGS="-D warnings" export CARGO_INCREMENTAL=0 export CARGO_EXTRA_FLAGS="--locked" -# Determine configuration for installed build +# Determine configuration for installed build (used by test-cargo-miri). echo "Installing release version of Miri" -./miri install - -echo "Checking various feature flag configurations" -./miri check --no-default-features # make sure this can be built -./miri check # and this, too -# `--all-features` is used for the build below, so no extra check needed. +time ./miri install # Prepare debug build for direct `./miri` invocations. # We enable all features to make sure the Stacked Borrows consistency check runs. echo "Building debug version of Miri" export CARGO_EXTRA_FLAGS="$CARGO_EXTRA_FLAGS --all-features" -./miri build --all-targets # the build that all the `./miri test` below will use +time ./miri build --all-targets # the build that all the `./miri test` below will use endgroup From 3c306c750e3a3c788f34288590b4ab7b9acb3439 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 30 Mar 2024 18:08:50 +0100 Subject: [PATCH 10/28] experiment with macOS M1 runners --- src/tools/miri/.github/workflows/ci.yml | 4 ++-- src/tools/miri/ci/ci.sh | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index c5979e18a3b59..4461706fd55d8 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -24,8 +24,8 @@ jobs: include: - os: ubuntu-latest host_target: x86_64-unknown-linux-gnu - - os: macos-latest - host_target: x86_64-apple-darwin + - os: macos-14 + host_target: aarch64-apple-darwin - os: windows-latest host_target: i686-pc-windows-msvc runs-on: ${{ matrix.os }} diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 32b58547d7c2b..f8ba612750ee9 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -135,6 +135,7 @@ case $HOST_TARGET in # Extra tier 1 MIRI_TEST_TARGET=i686-unknown-linux-gnu run_tests MIRI_TEST_TARGET=aarch64-unknown-linux-gnu run_tests + MIRI_TEST_TARGET=x86_64-apple-darwin run_tests MIRI_TEST_TARGET=i686-pc-windows-gnu run_tests MIRI_TEST_TARGET=x86_64-pc-windows-gnu run_tests # Extra tier 2 @@ -150,8 +151,8 @@ case $HOST_TARGET in # Custom target JSON file MIRI_TEST_TARGET=tests/avr.json MIRI_NO_STD=1 run_tests_minimal no_std ;; - x86_64-apple-darwin) - # Host + aarch64-apple-darwin) + # Host (tier 2) GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 MIRI_TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests From 19352685570726f14988f25836c067347f7c008f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 30 Mar 2024 18:33:04 +0100 Subject: [PATCH 11/28] reset the caches looks like the M1 runners don't like it when the cache was created on x86 also reorder the lines to be more semantically grouped --- src/tools/miri/.github/workflows/ci.yml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 4461706fd55d8..ec14fffc1d0c8 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -49,15 +49,16 @@ jobs: with: path: | # Taken from . - ~/.cargo/bin + # Cache package/registry information ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db - # contains package information of crates installed via `cargo install`. + # Cache installed binaries + ~/.cargo/bin ~/.cargo/.crates.toml ~/.cargo/.crates2.json - key: ${{ runner.os }}-cargo-reset20230315-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-cargo-reset20230315 + key: cargo-${{ runner.os }}-reset20240331-${{ hashFiles('**/Cargo.lock') }} + restore-keys: cargo-${{ runner.os }}-reset20240331 - name: Install rustup-toolchain-install-master if: ${{ steps.cache.outputs.cache-hit != 'true' }} @@ -98,15 +99,16 @@ jobs: with: path: | # Taken from . - ~/.cargo/bin + # Cache package/registry information ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db - # contains package information of crates installed via `cargo install`. + # Cache installed binaries + ~/.cargo/bin ~/.cargo/.crates.toml ~/.cargo/.crates2.json - key: ${{ runner.os }}-cargo-reset20230315-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-cargo-reset20230315 + key: cargo-${{ runner.os }}-reset20240331-${{ hashFiles('**/Cargo.lock') }} + restore-keys: cargo-${{ runner.os }}-reset20240331 - name: Install rustup-toolchain-install-master if: ${{ steps.cache.outputs.cache-hit != 'true' }} From e25c6243cadc679c34018c1bc058223ea3ad9a07 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 31 Mar 2024 14:03:45 +0200 Subject: [PATCH 12/28] update josh version and guidance --- src/tools/miri/.github/workflows/ci.yml | 2 +- src/tools/miri/CONTRIBUTING.md | 18 +++++++----------- src/tools/miri/miri-script/src/commands.rs | 2 +- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index ec14fffc1d0c8..b0dab9f509d4a 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -195,7 +195,7 @@ jobs: with: fetch-depth: 256 # get a bit more of the history - name: install josh-proxy - run: RUSTFLAGS="--cap-lints warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r22.12.06 + run: RUSTFLAGS="--cap-lints warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r23.12.04 - name: setup bot git name and email run: | git config --global user.name 'The Miri Cronjob Bot' diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 3416fb0d9ba09..60bc1d5282dd6 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -241,18 +241,20 @@ We use the [`josh` proxy](https://github.com/josh-project/josh) to transmit chan rustc and Miri repositories. You can install it as follows: ```sh -cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r22.12.06 +RUSTFLAGS="--cap-lints=warn" cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r23.12.04 ``` Josh will automatically be started and stopped by `./miri`. ### Importing changes from the rustc repo +*Note: this usually happens automatically, so these steps rarely have to be done by hand.* + We assume we start on an up-to-date master branch in the Miri repo. ```sh # Fetch and merge rustc side of the history. Takes ca 5 min the first time. -# This will also update the 'rustc-version' file. +# This will also update the `rustc-version` file. ./miri rustc-pull # Update local toolchain and apply formatting. ./miri toolchain && ./miri fmt @@ -266,12 +268,6 @@ needed. ### Exporting changes to the rustc repo -Keep in mind that pushing is the most complicated job that josh has to do -- pulling just filters -the rustc history, but pushing needs to construct a new rustc history that would filter to the given -Miri history! To avoid problems, it is a good idea to always pull immediately before you push. If -you are getting strange errors, chances are you are running into [this josh -bug](https://github.com/josh-project/josh/issues/998). In that case, please get in touch on Zulip. - We will use the josh proxy to push to your fork of rustc. Run the following in the Miri repo, assuming we are on an up-to-date master branch: @@ -280,9 +276,9 @@ assuming we are on an up-to-date master branch: ./miri rustc-push YOUR_NAME miri ``` -This will create a new branch called 'miri' in your fork, and the output should -include a link to create a rustc PR that will integrate those changes into the -main repository. +This will create a new branch called `miri` in your fork, and the output should include a link that +creates a rustc PR to integrate those changes into the main repository. If that PR has conflicts, +you need to pull rustc changes into Miri first, and then re-do the rustc push. If this fails due to authentication problems, it can help to make josh push via ssh instead of https. Add the following to your `.gitconfig`: diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 58deac66560d5..55b3b62819f5a 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -297,7 +297,7 @@ impl Command { }; // Prepare the branch. Pushing works much better if we use as base exactly // the commit that we pulled from last time, so we use the `rust-version` - // file as a good approximation of that. + // file to find out which commit that would be. println!("Preparing {github_user}/rust (base: {base})..."); if cmd!(sh, "git fetch https://github.com/{github_user}/rust {branch}") .ignore_stderr() From d6334ae4d24d51abb27e2b601efc77be133a6bc3 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Wed, 3 Apr 2024 04:55:58 +0000 Subject: [PATCH 13/28] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 187756851c78a..90194ec603864 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -5baf1e13f568b61e121953bf6a3d09faee7dd446 +b688d53a1736c17e49328a706a90829a9937a91a From 229d41731a8a25f9cd630d86250533431aefe7d6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Apr 2024 08:56:29 +0200 Subject: [PATCH 14/28] shims/unix: split general FD management from FS access; make a place for sockets --- src/tools/miri/src/machine.rs | 18 +- src/tools/miri/src/shims/unix/fd.rs | 429 +++++++++++++ .../miri/src/shims/unix/foreign_items.rs | 57 +- src/tools/miri/src/shims/unix/fs.rs | 607 +++--------------- src/tools/miri/src/shims/unix/linux/fd.rs | 67 +- .../miri/src/shims/unix/linux/fd/epoll.rs | 2 +- .../miri/src/shims/unix/linux/fd/event.rs | 2 +- .../src/shims/unix/linux/fd/socketpair.rs | 28 - .../src/shims/unix/linux/foreign_items.rs | 5 +- .../src/shims/unix/macos/foreign_items.rs | 3 +- src/tools/miri/src/shims/unix/mod.rs | 12 +- src/tools/miri/src/shims/unix/socket.rs | 66 ++ 12 files changed, 663 insertions(+), 633 deletions(-) create mode 100644 src/tools/miri/src/shims/unix/fd.rs delete mode 100644 src/tools/miri/src/shims/unix/linux/fd/socketpair.rs create mode 100644 src/tools/miri/src/shims/unix/socket.rs diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 2137de6a29bdb..14b7afcc971fd 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -31,7 +31,7 @@ use rustc_target::spec::abi::Abi; use crate::{ concurrency::{data_race, weak_memory}, - shims::unix::FileHandler, + shims::unix::FdTable, *, }; @@ -463,9 +463,9 @@ pub struct MiriMachine<'mir, 'tcx> { pub(crate) validate: bool, /// The table of file descriptors. - pub(crate) file_handler: shims::unix::FileHandler, + pub(crate) fds: shims::unix::FdTable, /// The table of directory descriptors. - pub(crate) dir_handler: shims::unix::DirHandler, + pub(crate) dirs: shims::unix::DirTable, /// This machine's monotone clock. pub(crate) clock: Clock, @@ -640,8 +640,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { tls: TlsData::default(), isolated_op: config.isolated_op, validate: config.validate, - file_handler: FileHandler::new(config.mute_stdout_stderr), - dir_handler: Default::default(), + fds: FdTable::new(config.mute_stdout_stderr), + dirs: Default::default(), layouts, threads: ThreadManager::default(), static_roots: Vec::new(), @@ -774,11 +774,11 @@ impl VisitProvenance for MiriMachine<'_, '_> { argv, cmd_line, extern_statics, - dir_handler, + dirs, borrow_tracker, data_race, alloc_addresses, - file_handler, + fds, tcx: _, isolated_op: _, validate: _, @@ -817,8 +817,8 @@ impl VisitProvenance for MiriMachine<'_, '_> { threads.visit_provenance(visit); tls.visit_provenance(visit); env_vars.visit_provenance(visit); - dir_handler.visit_provenance(visit); - file_handler.visit_provenance(visit); + dirs.visit_provenance(visit); + fds.visit_provenance(visit); data_race.visit_provenance(visit); borrow_tracker.visit_provenance(visit); alloc_addresses.visit_provenance(visit); diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs new file mode 100644 index 0000000000000..bc9348ee0e8c5 --- /dev/null +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -0,0 +1,429 @@ +//! General management of file descriptors, and support for +//! standard file descriptors (stdin/stdout/stderr). + +use std::any::Any; +use std::collections::BTreeMap; +use std::io::{self, ErrorKind, IsTerminal, Read, SeekFrom, Write}; + +use rustc_middle::ty::TyCtxt; +use rustc_target::abi::Size; + +use crate::shims::unix::*; +use crate::*; + +/// Represents an open file descriptor. +pub trait FileDescriptor: std::fmt::Debug + Any { + fn name(&self) -> &'static str; + + fn read<'tcx>( + &mut self, + _communicate_allowed: bool, + _bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot read from {}", self.name()); + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + _bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot write to {}", self.name()); + } + + fn seek<'tcx>( + &mut self, + _communicate_allowed: bool, + _offset: SeekFrom, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot seek on {}", self.name()); + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + throw_unsup_format!("cannot close {}", self.name()); + } + + /// Return a new file descriptor *that refers to the same underlying object*. + fn dup(&mut self) -> io::Result>; + + fn is_tty(&self, _communicate_allowed: bool) -> bool { + false + } +} + +impl dyn FileDescriptor { + #[inline(always)] + pub fn downcast_ref(&self) -> Option<&T> { + (self as &dyn Any).downcast_ref() + } + + #[inline(always)] + pub fn downcast_mut(&mut self) -> Option<&mut T> { + (self as &mut dyn Any).downcast_mut() + } +} + +impl FileDescriptor for io::Stdin { + fn name(&self) -> &'static str { + "stdin" + } + + fn read<'tcx>( + &mut self, + communicate_allowed: bool, + bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + if !communicate_allowed { + // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. + helpers::isolation_abort_error("`read` from stdin")?; + } + Ok(Read::read(self, bytes)) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(io::stdin())) + } + + fn is_tty(&self, communicate_allowed: bool) -> bool { + communicate_allowed && self.is_terminal() + } +} + +impl FileDescriptor for io::Stdout { + fn name(&self) -> &'static str { + "stdout" + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + // We allow writing to stderr even with isolation enabled. + let result = Write::write(&mut { self }, bytes); + // Stdout is buffered, flush to make sure it appears on the + // screen. This is the write() syscall of the interpreted + // program, we want it to correspond to a write() syscall on + // the host -- there is no good in adding extra buffering + // here. + io::stdout().flush().unwrap(); + + Ok(result) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(io::stdout())) + } + + fn is_tty(&self, communicate_allowed: bool) -> bool { + communicate_allowed && self.is_terminal() + } +} + +impl FileDescriptor for io::Stderr { + fn name(&self) -> &'static str { + "stderr" + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + // We allow writing to stderr even with isolation enabled. + // No need to flush, stderr is not buffered. + Ok(Write::write(&mut { self }, bytes)) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(io::stderr())) + } + + fn is_tty(&self, communicate_allowed: bool) -> bool { + communicate_allowed && self.is_terminal() + } +} + +/// Like /dev/null +#[derive(Debug)] +pub struct NullOutput; + +impl FileDescriptor for NullOutput { + fn name(&self) -> &'static str { + "stderr and stdout" + } + + fn write<'tcx>( + &self, + _communicate_allowed: bool, + bytes: &[u8], + _tcx: TyCtxt<'tcx>, + ) -> InterpResult<'tcx, io::Result> { + // We just don't write anything, but report to the user that we did. + Ok(Ok(bytes.len())) + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(NullOutput)) + } +} + +/// The file descriptor table +#[derive(Debug)] +pub struct FdTable { + pub fds: BTreeMap>, +} + +impl VisitProvenance for FdTable { + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { + // All our FileDescriptor do not have any tags. + } +} + +impl FdTable { + pub(crate) fn new(mute_stdout_stderr: bool) -> FdTable { + let mut fds: BTreeMap<_, Box> = BTreeMap::new(); + fds.insert(0i32, Box::new(io::stdin())); + if mute_stdout_stderr { + fds.insert(1i32, Box::new(NullOutput)); + fds.insert(2i32, Box::new(NullOutput)); + } else { + fds.insert(1i32, Box::new(io::stdout())); + fds.insert(2i32, Box::new(io::stderr())); + } + FdTable { fds } + } + + pub fn insert_fd(&mut self, file_handle: Box) -> i32 { + self.insert_fd_with_min_fd(file_handle, 0) + } + + /// Insert a new FD that is at least `min_fd`. + pub fn insert_fd_with_min_fd( + &mut self, + file_handle: Box, + min_fd: i32, + ) -> i32 { + // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in + // between used FDs, the find_map combinator will return it. If the first such unused FD + // is after all other used FDs, the find_map combinator will return None, and we will use + // the FD following the greatest FD thus far. + let candidate_new_fd = + self.fds.range(min_fd..).zip(min_fd..).find_map(|((fd, _fh), counter)| { + if *fd != counter { + // There was a gap in the fds stored, return the first unused one + // (note that this relies on BTreeMap iterating in key order) + Some(counter) + } else { + // This fd is used, keep going + None + } + }); + let new_fd = candidate_new_fd.unwrap_or_else(|| { + // find_map ran out of BTreeMap entries before finding a free fd, use one plus the + // maximum fd in the map + self.fds.last_key_value().map(|(fd, _)| fd.checked_add(1).unwrap()).unwrap_or(min_fd) + }); + + self.fds.try_insert(new_fd, file_handle).unwrap(); + new_fd + } + + pub fn get(&self, fd: i32) -> Option<&dyn FileDescriptor> { + Some(&**self.fds.get(&fd)?) + } + + pub fn get_mut(&mut self, fd: i32) -> Option<&mut dyn FileDescriptor> { + Some(&mut **self.fds.get_mut(&fd)?) + } + + pub fn remove(&mut self, fd: i32) -> Option> { + self.fds.remove(&fd) + } + + pub fn is_fd(&self, fd: i32) -> bool { + self.fds.contains_key(&fd) + } +} + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + fn fcntl(&mut self, args: &[OpTy<'tcx, Provenance>]) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + + if args.len() < 2 { + throw_ub_format!( + "incorrect number of arguments for fcntl: got {}, expected at least 2", + args.len() + ); + } + let fd = this.read_scalar(&args[0])?.to_i32()?; + let cmd = this.read_scalar(&args[1])?.to_i32()?; + + // We only support getting the flags for a descriptor. + if cmd == this.eval_libc_i32("F_GETFD") { + // Currently this is the only flag that `F_GETFD` returns. It is OK to just return the + // `FD_CLOEXEC` value without checking if the flag is set for the file because `std` + // always sets this flag when opening a file. However we still need to check that the + // file itself is open. + if this.machine.fds.is_fd(fd) { + Ok(this.eval_libc_i32("FD_CLOEXEC")) + } else { + this.fd_not_found() + } + } else if cmd == this.eval_libc_i32("F_DUPFD") + || cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC") + { + // Note that we always assume the FD_CLOEXEC flag is set for every open file, in part + // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only + // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor, + // thus they can share the same implementation here. + if args.len() < 3 { + throw_ub_format!( + "incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3", + args.len() + ); + } + let start = this.read_scalar(&args[2])?.to_i32()?; + + match this.machine.fds.get_mut(fd) { + Some(file_descriptor) => { + let dup_result = file_descriptor.dup(); + match dup_result { + Ok(dup_fd) => Ok(this.machine.fds.insert_fd_with_min_fd(dup_fd, start)), + Err(e) => { + this.set_last_error_from_io_error(e.kind())?; + Ok(-1) + } + } + } + None => this.fd_not_found(), + } + } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") { + // Reject if isolation is enabled. + if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { + this.reject_in_isolation("`fcntl`", reject_with)?; + this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; + return Ok(-1); + } + + this.ffullsync_fd(fd) + } else { + throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd); + } + } + + fn close(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let fd = this.read_scalar(fd_op)?.to_i32()?; + + Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.remove(fd) { + let result = file_descriptor.close(this.machine.communicate())?; + this.try_unwrap_io_result(result)? + } else { + this.fd_not_found()? + })) + } + + /// Function used when a file descriptor does not exist. It returns `Ok(-1)`and sets + /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses + /// `T: From` instead of `i32` directly because some fs functions return different integer + /// types (like `read`, that returns an `i64`). + fn fd_not_found>(&mut self) -> InterpResult<'tcx, T> { + let this = self.eval_context_mut(); + let ebadf = this.eval_libc("EBADF"); + this.set_last_error(ebadf)?; + Ok((-1).into()) + } + + fn read( + &mut self, + fd: i32, + buf: Pointer>, + count: u64, + ) -> InterpResult<'tcx, i64> { + let this = self.eval_context_mut(); + + // Isolation check is done via `FileDescriptor` trait. + + trace!("Reading from FD {}, size {}", fd, count); + + // Check that the *entire* buffer is actually valid memory. + this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; + + // We cap the number of read bytes to the largest value that we are able to fit in both the + // host's and target's `isize`. This saves us from having to handle overflows later. + let count = count + .min(u64::try_from(this.target_isize_max()).unwrap()) + .min(u64::try_from(isize::MAX).unwrap()); + let communicate = this.machine.communicate(); + + if let Some(file_descriptor) = this.machine.fds.get_mut(fd) { + trace!("read: FD mapped to {:?}", file_descriptor); + // We want to read at most `count` bytes. We are sure that `count` is not negative + // because it was a target's `usize`. Also we are sure that its smaller than + // `usize::MAX` because it is bounded by the host's `isize`. + let mut bytes = vec![0; usize::try_from(count).unwrap()]; + // `File::read` never returns a value larger than `count`, + // so this cannot fail. + let result = file_descriptor + .read(communicate, &mut bytes, *this.tcx)? + .map(|c| i64::try_from(c).unwrap()); + + match result { + Ok(read_bytes) => { + // If reading to `bytes` did not fail, we write those bytes to the buffer. + this.write_bytes_ptr(buf, bytes)?; + Ok(read_bytes) + } + Err(e) => { + this.set_last_error_from_io_error(e.kind())?; + Ok(-1) + } + } + } else { + trace!("read: FD not found"); + this.fd_not_found() + } + } + + fn write( + &mut self, + fd: i32, + buf: Pointer>, + count: u64, + ) -> InterpResult<'tcx, i64> { + let this = self.eval_context_mut(); + + // Isolation check is done via `FileDescriptor` trait. + + // Check that the *entire* buffer is actually valid memory. + this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; + + // We cap the number of written bytes to the largest value that we are able to fit in both the + // host's and target's `isize`. This saves us from having to handle overflows later. + let count = count + .min(u64::try_from(this.target_isize_max()).unwrap()) + .min(u64::try_from(isize::MAX).unwrap()); + let communicate = this.machine.communicate(); + + if let Some(file_descriptor) = this.machine.fds.get(fd) { + let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?; + let result = file_descriptor + .write(communicate, bytes, *this.tcx)? + .map(|c| i64::try_from(c).unwrap()); + this.try_unwrap_io_result(result) + } else { + this.fd_not_found() + } + } +} diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index 4ceda80935047..b1e1aec5880a9 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -8,8 +8,10 @@ use rustc_target::spec::abi::Abi; use crate::*; use shims::foreign_items::EmulateForeignItemResult; +use shims::unix::fd::EvalContextExt as _; use shims::unix::fs::EvalContextExt as _; use shims::unix::mem::EvalContextExt as _; +use shims::unix::socket::EvalContextExt as _; use shims::unix::sync::EvalContextExt as _; use shims::unix::thread::EvalContextExt as _; @@ -51,7 +53,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // See `fn emulate_foreign_item_inner` in `shims/foreign_items.rs` for the general pattern. #[rustfmt::skip] match link_name.as_str() { - // Environment related shims + // Environment variables "getenv" => { let [name] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.getenv(name)?; @@ -79,25 +81,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_scalar(Scalar::from_i32(result), dest)?; } - // File related shims - "open" | "open64" => { - // `open` is variadic, the third argument is only present when the second argument has O_CREAT (or on linux O_TMPFILE, but miri doesn't support that) set - this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; - let result = this.open(args)?; - this.write_scalar(Scalar::from_i32(result), dest)?; - } - "close" => { - let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let result = this.close(fd)?; - this.write_scalar(result, dest)?; - } - "fcntl" => { - // `fcntl` is variadic. The argument count is checked based on the first argument - // in `this.fcntl()`, so we do not use `check_shim` here. - this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; - let result = this.fcntl(args)?; - this.write_scalar(Scalar::from_i32(result), dest)?; - } + // File descriptors "read" => { let [fd, buf, count] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let fd = this.read_scalar(fd)?.to_i32()?; @@ -116,6 +100,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Now, `result` is the value we return back to the program. this.write_scalar(Scalar::from_target_isize(result, this), dest)?; } + "close" => { + let [fd] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let result = this.close(fd)?; + this.write_scalar(result, dest)?; + } + "fcntl" => { + // `fcntl` is variadic. The argument count is checked based on the first argument + // in `this.fcntl()`, so we do not use `check_shim` here. + this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; + let result = this.fcntl(args)?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } + + // File and file system access + "open" | "open64" => { + // `open` is variadic, the third argument is only present when the second argument has O_CREAT (or on linux O_TMPFILE, but miri doesn't support that) set + this.check_abi_and_shim_symbol_clash(abi, Abi::C { unwind: false }, link_name)?; + let result = this.open(args)?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } "unlink" => { let [path] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.unlink(path)?; @@ -219,7 +223,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_scalar(Scalar::from_i32(result), dest)?; } - // Time related shims + // Sockets + "socketpair" => { + let [domain, type_, protocol, sv] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + let result = this.socketpair(domain, type_, protocol, sv)?; + this.write_scalar(result, dest)?; + } + + // Time "gettimeofday" => { let [tv, tz] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.gettimeofday(tv, tz)?; @@ -598,7 +611,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if bufsize > 256 { let err = this.eval_libc("EIO"); this.set_last_error(err)?; - this.write_scalar(Scalar::from_i32(-1), dest)? + this.write_scalar(Scalar::from_i32(-1), dest)?; } else { this.gen_random(buf, bufsize)?; this.write_scalar(Scalar::from_i32(0), dest)?; diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index b141ca4a019c3..31076fdfaf626 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -1,6 +1,6 @@ -use std::any::Any; +//! File and file system access + use std::borrow::Cow; -use std::collections::BTreeMap; use std::fs::{ read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir, }; @@ -13,70 +13,16 @@ use rustc_middle::ty::TyCtxt; use rustc_target::abi::Size; use crate::shims::os_str::bytes_to_os_str; +use crate::shims::unix::*; use crate::*; use shims::time::system_time_to_duration; #[derive(Debug)] -pub struct FileHandle { +struct FileHandle { file: File, writable: bool, } -pub trait FileDescriptor: std::fmt::Debug + Any { - fn name(&self) -> &'static str; - - fn read<'tcx>( - &mut self, - _communicate_allowed: bool, - _bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot read from {}", self.name()); - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - _bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot write to {}", self.name()); - } - - fn seek<'tcx>( - &mut self, - _communicate_allowed: bool, - _offset: SeekFrom, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot seek on {}", self.name()); - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - throw_unsup_format!("cannot close {}", self.name()); - } - - fn dup(&mut self) -> io::Result>; - - fn is_tty(&self, _communicate_allowed: bool) -> bool { - false - } -} - -impl dyn FileDescriptor { - #[inline(always)] - pub fn downcast_ref(&self) -> Option<&T> { - (self as &dyn Any).downcast_ref() - } - - #[inline(always)] - pub fn downcast_mut(&mut self) -> Option<&mut T> { - (self as &mut dyn Any).downcast_mut() - } -} - impl FileDescriptor for FileHandle { fn name(&self) -> &'static str { "FILE" @@ -147,172 +93,6 @@ impl FileDescriptor for FileHandle { } } -impl FileDescriptor for io::Stdin { - fn name(&self) -> &'static str { - "stdin" - } - - fn read<'tcx>( - &mut self, - communicate_allowed: bool, - bytes: &mut [u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - if !communicate_allowed { - // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. - helpers::isolation_abort_error("`read` from stdin")?; - } - Ok(Read::read(self, bytes)) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(io::stdin())) - } - - fn is_tty(&self, communicate_allowed: bool) -> bool { - communicate_allowed && self.is_terminal() - } -} - -impl FileDescriptor for io::Stdout { - fn name(&self) -> &'static str { - "stdout" - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - // We allow writing to stderr even with isolation enabled. - let result = Write::write(&mut { self }, bytes); - // Stdout is buffered, flush to make sure it appears on the - // screen. This is the write() syscall of the interpreted - // program, we want it to correspond to a write() syscall on - // the host -- there is no good in adding extra buffering - // here. - io::stdout().flush().unwrap(); - - Ok(result) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(io::stdout())) - } - - fn is_tty(&self, communicate_allowed: bool) -> bool { - communicate_allowed && self.is_terminal() - } -} - -impl FileDescriptor for io::Stderr { - fn name(&self) -> &'static str { - "stderr" - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - // We allow writing to stderr even with isolation enabled. - // No need to flush, stderr is not buffered. - Ok(Write::write(&mut { self }, bytes)) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(io::stderr())) - } - - fn is_tty(&self, communicate_allowed: bool) -> bool { - communicate_allowed && self.is_terminal() - } -} - -#[derive(Debug)] -struct NullOutput; - -impl FileDescriptor for NullOutput { - fn name(&self) -> &'static str { - "stderr and stdout" - } - - fn write<'tcx>( - &self, - _communicate_allowed: bool, - bytes: &[u8], - _tcx: TyCtxt<'tcx>, - ) -> InterpResult<'tcx, io::Result> { - // We just don't write anything, but report to the user that we did. - Ok(Ok(bytes.len())) - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(NullOutput)) - } -} - -#[derive(Debug)] -pub struct FileHandler { - pub handles: BTreeMap>, -} - -impl VisitProvenance for FileHandler { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // All our FileDescriptor do not have any tags. - } -} - -impl FileHandler { - pub(crate) fn new(mute_stdout_stderr: bool) -> FileHandler { - let mut handles: BTreeMap<_, Box> = BTreeMap::new(); - handles.insert(0i32, Box::new(io::stdin())); - if mute_stdout_stderr { - handles.insert(1i32, Box::new(NullOutput)); - handles.insert(2i32, Box::new(NullOutput)); - } else { - handles.insert(1i32, Box::new(io::stdout())); - handles.insert(2i32, Box::new(io::stderr())); - } - FileHandler { handles } - } - - pub fn insert_fd(&mut self, file_handle: Box) -> i32 { - self.insert_fd_with_min_fd(file_handle, 0) - } - - fn insert_fd_with_min_fd(&mut self, file_handle: Box, min_fd: i32) -> i32 { - // Find the lowest unused FD, starting from min_fd. If the first such unused FD is in - // between used FDs, the find_map combinator will return it. If the first such unused FD - // is after all other used FDs, the find_map combinator will return None, and we will use - // the FD following the greatest FD thus far. - let candidate_new_fd = - self.handles.range(min_fd..).zip(min_fd..).find_map(|((fd, _fh), counter)| { - if *fd != counter { - // There was a gap in the fds stored, return the first unused one - // (note that this relies on BTreeMap iterating in key order) - Some(counter) - } else { - // This fd is used, keep going - None - } - }); - let new_fd = candidate_new_fd.unwrap_or_else(|| { - // find_map ran out of BTreeMap entries before finding a free fd, use one plus the - // maximum fd in the map - self.handles - .last_key_value() - .map(|(fd, _)| fd.checked_add(1).unwrap()) - .unwrap_or(min_fd) - }); - - self.handles.try_insert(new_fd, file_handle).unwrap(); - new_fd - } -} - impl<'mir, 'tcx: 'mir> EvalContextExtPrivate<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn macos_stat_write_buf( @@ -411,10 +191,11 @@ trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx /// An open directory, tracked by DirHandler. #[derive(Debug)] -pub struct OpenDir { +struct OpenDir { /// The directory reader on the host. read_dir: ReadDir, - /// The most recent entry returned by readdir() + /// The most recent entry returned by readdir(). + /// Will be freed by the next call. entry: Pointer>, } @@ -425,8 +206,11 @@ impl OpenDir { } } +/// The table of open directories. +/// Curiously, Unix/POSIX does not unify this into the "file descriptor" concept... everything +/// is a file, except a directory is not? #[derive(Debug)] -pub struct DirHandler { +pub struct DirTable { /// Directory iterators used to emulate libc "directory streams", as used in opendir, readdir, /// and closedir. /// @@ -441,7 +225,7 @@ pub struct DirHandler { next_id: u64, } -impl DirHandler { +impl DirTable { #[allow(clippy::arithmetic_side_effects)] fn insert_new(&mut self, read_dir: ReadDir) -> u64 { let id = self.next_id; @@ -451,9 +235,9 @@ impl DirHandler { } } -impl Default for DirHandler { - fn default() -> DirHandler { - DirHandler { +impl Default for DirTable { + fn default() -> DirTable { + DirTable { streams: FxHashMap::default(), // Skip 0 as an ID, because it looks like a null pointer to libc next_id: 1, @@ -461,9 +245,9 @@ impl Default for DirHandler { } } -impl VisitProvenance for DirHandler { +impl VisitProvenance for DirTable { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let DirHandler { streams, next_id: _ } = self; + let DirTable { streams, next_id: _ } = self; for dir in streams.values() { dir.entry.visit_provenance(visit); @@ -615,200 +399,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } let fd = options.open(path).map(|file| { - let fh = &mut this.machine.file_handler; + let fh = &mut this.machine.fds; fh.insert_fd(Box::new(FileHandle { file, writable })) }); this.try_unwrap_io_result(fd) } - fn fcntl(&mut self, args: &[OpTy<'tcx, Provenance>]) -> InterpResult<'tcx, i32> { - let this = self.eval_context_mut(); - - if args.len() < 2 { - throw_ub_format!( - "incorrect number of arguments for fcntl: got {}, expected at least 2", - args.len() - ); - } - let fd = this.read_scalar(&args[0])?.to_i32()?; - let cmd = this.read_scalar(&args[1])?.to_i32()?; - - // We only support getting the flags for a descriptor. - if cmd == this.eval_libc_i32("F_GETFD") { - // Currently this is the only flag that `F_GETFD` returns. It is OK to just return the - // `FD_CLOEXEC` value without checking if the flag is set for the file because `std` - // always sets this flag when opening a file. However we still need to check that the - // file itself is open. - if this.machine.file_handler.handles.contains_key(&fd) { - Ok(this.eval_libc_i32("FD_CLOEXEC")) - } else { - this.handle_not_found() - } - } else if cmd == this.eval_libc_i32("F_DUPFD") - || cmd == this.eval_libc_i32("F_DUPFD_CLOEXEC") - { - // Note that we always assume the FD_CLOEXEC flag is set for every open file, in part - // because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only - // differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor, - // thus they can share the same implementation here. - if args.len() < 3 { - throw_ub_format!( - "incorrect number of arguments for fcntl with cmd=`F_DUPFD`/`F_DUPFD_CLOEXEC`: got {}, expected at least 3", - args.len() - ); - } - let start = this.read_scalar(&args[2])?.to_i32()?; - - let fh = &mut this.machine.file_handler; - - match fh.handles.get_mut(&fd) { - Some(file_descriptor) => { - let dup_result = file_descriptor.dup(); - match dup_result { - Ok(dup_fd) => Ok(fh.insert_fd_with_min_fd(dup_fd, start)), - Err(e) => { - this.set_last_error_from_io_error(e.kind())?; - Ok(-1) - } - } - } - None => this.handle_not_found(), - } - } else if this.tcx.sess.target.os == "macos" && cmd == this.eval_libc_i32("F_FULLFSYNC") { - // Reject if isolation is enabled. - if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { - this.reject_in_isolation("`fcntl`", reject_with)?; - this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?; - return Ok(-1); - } - - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support fullfsync for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`F_FULLFSYNC` is only supported on file-backed file descriptors" - ) - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_all); - this.try_unwrap_io_result(io_result) - } else { - this.handle_not_found() - } - } else { - throw_unsup_format!("the {:#x} command is not supported for `fcntl`)", cmd); - } - } - - fn close(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let fd = this.read_scalar(fd_op)?.to_i32()?; - - Ok(Scalar::from_i32( - if let Some(file_descriptor) = this.machine.file_handler.handles.remove(&fd) { - let result = file_descriptor.close(this.machine.communicate())?; - this.try_unwrap_io_result(result)? - } else { - this.handle_not_found()? - }, - )) - } - - /// Function used when a handle is not found inside `FileHandler`. It returns `Ok(-1)`and sets - /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses - /// `T: From` instead of `i32` directly because some fs functions return different integer - /// types (like `read`, that returns an `i64`). - fn handle_not_found>(&mut self) -> InterpResult<'tcx, T> { - let this = self.eval_context_mut(); - let ebadf = this.eval_libc("EBADF"); - this.set_last_error(ebadf)?; - Ok((-1).into()) - } - - fn read( - &mut self, - fd: i32, - buf: Pointer>, - count: u64, - ) -> InterpResult<'tcx, i64> { - let this = self.eval_context_mut(); - - // Isolation check is done via `FileDescriptor` trait. - - trace!("Reading from FD {}, size {}", fd, count); - - // Check that the *entire* buffer is actually valid memory. - this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; - - // We cap the number of read bytes to the largest value that we are able to fit in both the - // host's and target's `isize`. This saves us from having to handle overflows later. - let count = count - .min(u64::try_from(this.target_isize_max()).unwrap()) - .min(u64::try_from(isize::MAX).unwrap()); - let communicate = this.machine.communicate(); - - if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) { - trace!("read: FD mapped to {:?}", file_descriptor); - // We want to read at most `count` bytes. We are sure that `count` is not negative - // because it was a target's `usize`. Also we are sure that its smaller than - // `usize::MAX` because it is bounded by the host's `isize`. - let mut bytes = vec![0; usize::try_from(count).unwrap()]; - // `File::read` never returns a value larger than `count`, - // so this cannot fail. - let result = file_descriptor - .read(communicate, &mut bytes, *this.tcx)? - .map(|c| i64::try_from(c).unwrap()); - - match result { - Ok(read_bytes) => { - // If reading to `bytes` did not fail, we write those bytes to the buffer. - this.write_bytes_ptr(buf, bytes)?; - Ok(read_bytes) - } - Err(e) => { - this.set_last_error_from_io_error(e.kind())?; - Ok(-1) - } - } - } else { - trace!("read: FD not found"); - this.handle_not_found() - } - } - - fn write( - &mut self, - fd: i32, - buf: Pointer>, - count: u64, - ) -> InterpResult<'tcx, i64> { - let this = self.eval_context_mut(); - - // Isolation check is done via `FileDescriptor` trait. - - // Check that the *entire* buffer is actually valid memory. - this.check_ptr_access(buf, Size::from_bytes(count), CheckInAllocMsg::MemoryAccessTest)?; - - // We cap the number of written bytes to the largest value that we are able to fit in both the - // host's and target's `isize`. This saves us from having to handle overflows later. - let count = count - .min(u64::try_from(this.target_isize_max()).unwrap()) - .min(u64::try_from(isize::MAX).unwrap()); - let communicate = this.machine.communicate(); - - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?; - let result = file_descriptor - .write(communicate, bytes, *this.tcx)? - .map(|c| i64::try_from(c).unwrap()); - this.try_unwrap_io_result(result) - } else { - this.handle_not_found() - } - } - fn lseek64( &mut self, fd: i32, @@ -832,16 +429,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; let communicate = this.machine.communicate(); - Ok(Scalar::from_i64( - if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) { - let result = file_descriptor - .seek(communicate, seek_from)? - .map(|offset| i64::try_from(offset).unwrap()); - this.try_unwrap_io_result(result)? - } else { - this.handle_not_found()? - }, - )) + Ok(Scalar::from_i64(if let Some(file_descriptor) = this.machine.fds.get_mut(fd) { + let result = file_descriptor + .seek(communicate, seek_from)? + .map(|offset| i64::try_from(offset).unwrap()); + this.try_unwrap_io_result(result)? + } else { + this.fd_not_found()? + })) } fn unlink(&mut self, path_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { @@ -970,7 +565,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fstat`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } let metadata = match FileMetadata::from_fd(this, fd)? { @@ -1269,7 +864,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { match result { Ok(dir_iter) => { - let id = this.machine.dir_handler.insert_new(dir_iter); + let id = this.machine.dirs.insert_new(dir_iter); // The libc API for opendir says that this method returns a pointer to an opaque // structure, but we are returning an ID number. Thus, pass it as a scalar of @@ -1301,7 +896,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(Scalar::null_ptr(this)); } - let open_dir = this.machine.dir_handler.streams.get_mut(&dirp).ok_or_else(|| { + let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { err_unsup_format!("the DIR pointer passed to readdir64 did not come from opendir") })?; @@ -1366,7 +961,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } }; - let open_dir = this.machine.dir_handler.streams.get_mut(&dirp).unwrap(); + let open_dir = this.machine.dirs.streams.get_mut(&dirp).unwrap(); let old_entry = std::mem::replace(&mut open_dir.entry, entry); this.free(old_entry, MiriMemoryKind::Runtime)?; @@ -1391,10 +986,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`readdir_r`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } - let open_dir = this.machine.dir_handler.streams.get_mut(&dirp).ok_or_else(|| { + let open_dir = this.machine.dirs.streams.get_mut(&dirp).ok_or_else(|| { err_unsup_format!("the DIR pointer passed to readdir_r did not come from opendir") })?; Ok(Scalar::from_i32(match open_dir.read_dir.next() { @@ -1507,15 +1102,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`closedir`", reject_with)?; // Set error code as "EBADF" (bad fd) - return this.handle_not_found(); + return this.fd_not_found(); } - if let Some(open_dir) = this.machine.dir_handler.streams.remove(&dirp) { + if let Some(open_dir) = this.machine.dirs.streams.remove(&dirp) { this.free(open_dir.entry, MiriMemoryKind::Runtime)?; drop(open_dir); Ok(0) } else { - this.handle_not_found() + this.fd_not_found() } } @@ -1526,37 +1121,35 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`ftruncate64`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } - Ok(Scalar::from_i32( - if let Some(file_descriptor) = this.machine.file_handler.handles.get_mut(&fd) { - // FIXME: Support ftruncate64 for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`ftruncate64` is only supported on file-backed file descriptors" - ) - })?; - if *writable { - if let Ok(length) = length.try_into() { - let result = file.set_len(length); - this.try_unwrap_io_result(result.map(|_| 0i32))? - } else { - let einval = this.eval_libc("EINVAL"); - this.set_last_error(einval)?; - -1 - } + Ok(Scalar::from_i32(if let Some(file_descriptor) = this.machine.fds.get_mut(fd) { + // FIXME: Support ftruncate64 for all FDs + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!( + "`ftruncate64` is only supported on file-backed file descriptors" + ) + })?; + if *writable { + if let Ok(length) = length.try_into() { + let result = file.set_len(length); + this.try_unwrap_io_result(result.map(|_| 0i32))? } else { - // The file is not writable let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; -1 } } else { - this.handle_not_found()? - }, - )) + // The file is not writable + let einval = this.eval_libc("EINVAL"); + this.set_last_error(einval)?; + -1 + } + } else { + this.fd_not_found()? + })) } fn fsync(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { @@ -1573,20 +1166,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fsync`", reject_with)?; // Set error code as "EBADF" (bad fd) - return this.handle_not_found(); + return this.fd_not_found(); } - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support fsync for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!("`fsync` is only supported on file-backed file descriptors") - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_all); - this.try_unwrap_io_result(io_result) - } else { - this.handle_not_found() - } + return self.ffullsync_fd(fd); + } + + fn ffullsync_fd(&mut self, fd: i32) -> InterpResult<'tcx, i32> { + let this = self.eval_context_mut(); + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(this.fd_not_found()?); + }; + // Only regular files support synchronization. + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!("`fsync` is only supported on file-backed file descriptors") + })?; + let io_result = maybe_sync_file(file, *writable, File::sync_all); + this.try_unwrap_io_result(io_result) } fn fdatasync(&mut self, fd_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> { @@ -1598,22 +1195,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`fdatasync`", reject_with)?; // Set error code as "EBADF" (bad fd) - return this.handle_not_found(); + return this.fd_not_found(); } - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support fdatasync for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`fdatasync` is only supported on file-backed file descriptors" - ) - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_data); - this.try_unwrap_io_result(io_result) - } else { - this.handle_not_found() - } + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(this.fd_not_found()?); + }; + // Only regular files support synchronization. + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!("`fdatasync` is only supported on file-backed file descriptors") + })?; + let io_result = maybe_sync_file(file, *writable, File::sync_data); + this.try_unwrap_io_result(io_result) } fn sync_file_range( @@ -1648,22 +1242,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let IsolatedOp::Reject(reject_with) = this.machine.isolated_op { this.reject_in_isolation("`sync_file_range`", reject_with)?; // Set error code as "EBADF" (bad fd) - return Ok(Scalar::from_i32(this.handle_not_found()?)); + return Ok(Scalar::from_i32(this.fd_not_found()?)); } - if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { - // FIXME: Support sync_data_range for all FDs - let FileHandle { file, writable } = - file_descriptor.downcast_ref::().ok_or_else(|| { - err_unsup_format!( - "`sync_data_range` is only supported on file-backed file descriptors" - ) - })?; - let io_result = maybe_sync_file(file, *writable, File::sync_data); - Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) - } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) - } + let Some(file_descriptor) = this.machine.fds.get(fd) else { + return Ok(Scalar::from_i32(this.fd_not_found()?)); + }; + // Only regular files support synchronization. + let FileHandle { file, writable } = + file_descriptor.downcast_ref::().ok_or_else(|| { + err_unsup_format!( + "`sync_data_range` is only supported on file-backed file descriptors" + ) + })?; + let io_result = maybe_sync_file(file, *writable, File::sync_data); + Ok(Scalar::from_i32(this.try_unwrap_io_result(io_result)?)) } fn readlink( @@ -1720,7 +1313,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // "returns 1 if fd is an open file descriptor referring to a terminal; // otherwise 0 is returned, and errno is set to indicate the error" let fd = this.read_scalar(miri_fd)?.to_i32()?; - let error = if let Some(fd) = this.machine.file_handler.handles.get(&fd) { + let error = if let Some(fd) = this.machine.fds.get(fd) { if fd.is_tty(this.machine.communicate()) { return Ok(Scalar::from_i32(1)); } else { @@ -1897,7 +1490,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { match file { Ok(f) => { - let fh = &mut this.machine.file_handler; + let fh = &mut this.machine.fds; let fd = fh.insert_fd(Box::new(FileHandle { file: f, writable: true })); return Ok(fd); } @@ -1963,7 +1556,7 @@ impl FileMetadata { ecx: &mut MiriInterpCx<'_, 'tcx>, fd: i32, ) -> InterpResult<'tcx, Option> { - let option = ecx.machine.file_handler.handles.get(&fd); + let option = ecx.machine.fds.get(fd); let file = match option { Some(file_descriptor) => &file_descriptor @@ -1974,7 +1567,7 @@ impl FileMetadata { ) })? .file, - None => return ecx.handle_not_found().map(|_: i32| None), + None => return ecx.fd_not_found().map(|_: i32| None), }; let metadata = file.metadata(); diff --git a/src/tools/miri/src/shims/unix/linux/fd.rs b/src/tools/miri/src/shims/unix/linux/fd.rs index 22fbb6da95a49..7d5177e5c424f 100644 --- a/src/tools/miri/src/shims/unix/linux/fd.rs +++ b/src/tools/miri/src/shims/unix/linux/fd.rs @@ -1,17 +1,12 @@ use std::cell::Cell; -use rustc_middle::ty::ScalarInt; - +use crate::shims::unix::*; use crate::*; use epoll::{Epoll, EpollEvent}; use event::Event; -use socketpair::SocketPair; - -use shims::unix::fs::EvalContextExt as _; pub mod epoll; pub mod event; -pub mod socketpair; impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { @@ -35,7 +30,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { throw_unsup_format!("epoll_create1 flags {flags} are not implemented"); } - let fd = this.machine.file_handler.insert_fd(Box::new(Epoll::default())); + let fd = this.machine.fds.insert_fd(Box::new(Epoll::default())); Ok(Scalar::from_i32(fd)) } @@ -79,7 +74,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let data = this.read_scalar(&data)?; let event = EpollEvent { events, data }; - if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { + if let Some(epfd) = this.machine.fds.get_mut(epfd) { let epfd = epfd .downcast_mut::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; @@ -87,10 +82,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { epfd.file_descriptors.insert(fd, event); Ok(Scalar::from_i32(0)) } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) + Ok(Scalar::from_i32(this.fd_not_found()?)) } } else if op == epoll_ctl_del { - if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { + if let Some(epfd) = this.machine.fds.get_mut(epfd) { let epfd = epfd .downcast_mut::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_ctl`"))?; @@ -98,7 +93,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { epfd.file_descriptors.remove(&fd); Ok(Scalar::from_i32(0)) } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) + Ok(Scalar::from_i32(this.fd_not_found()?)) } } else { let einval = this.eval_libc("EINVAL"); @@ -150,7 +145,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let _maxevents = this.read_scalar(maxevents)?.to_i32()?; let _timeout = this.read_scalar(timeout)?.to_i32()?; - if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { + if let Some(epfd) = this.machine.fds.get_mut(epfd) { let _epfd = epfd .downcast_mut::() .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; @@ -158,7 +153,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // FIXME return number of events ready when scheme for marking events ready exists throw_unsup_format!("returning ready events from epoll_wait is not yet implemented"); } else { - Ok(Scalar::from_i32(this.handle_not_found()?)) + Ok(Scalar::from_i32(this.fd_not_found()?)) } } @@ -203,51 +198,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { throw_unsup_format!("EFD_SEMAPHORE is unsupported"); } - let fh = &mut this.machine.file_handler; - let fd = fh.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); + let fd = this.machine.fds.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); Ok(Scalar::from_i32(fd)) } - - /// Currently this function creates new `SocketPair`s without specifying the domain, type, or - /// protocol of the new socket and these are stored in the socket values `sv` argument. - /// - /// This function creates an unnamed pair of connected sockets in the specified domain, of the - /// specified type, and using the optionally specified protocol. - /// - /// The `domain` argument specified a communication domain; this selects the protocol family - /// used for communication. The socket `type` specifies the communication semantics. - /// The `protocol` specifies a particular protocol to use with the socket. Normally there's - /// only a single protocol supported for a particular socket type within a given protocol - /// family, in which case `protocol` can be specified as 0. It is possible that many protocols - /// exist and in that case, a particular protocol must be specified. - /// - /// For more information on the arguments see the socket manpage: - /// - /// - /// - fn socketpair( - &mut self, - domain: &OpTy<'tcx, Provenance>, - type_: &OpTy<'tcx, Provenance>, - protocol: &OpTy<'tcx, Provenance>, - sv: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let _domain = this.read_scalar(domain)?.to_i32()?; - let _type_ = this.read_scalar(type_)?.to_i32()?; - let _protocol = this.read_scalar(protocol)?.to_i32()?; - let sv = this.deref_pointer(sv)?; - - let fh = &mut this.machine.file_handler; - let sv0 = fh.insert_fd(Box::new(SocketPair)); - let sv0 = ScalarInt::try_from_int(sv0, sv.layout.size).unwrap(); - let sv1 = fh.insert_fd(Box::new(SocketPair)); - let sv1 = ScalarInt::try_from_int(sv1, sv.layout.size).unwrap(); - - this.write_scalar(sv0, &sv)?; - this.write_scalar(sv1, &sv.offset(sv.layout.size, sv.layout, this)?)?; - - Ok(Scalar::from_i32(0)) - } } diff --git a/src/tools/miri/src/shims/unix/linux/fd/epoll.rs b/src/tools/miri/src/shims/unix/linux/fd/epoll.rs index 8c5aed6def64d..f2da76ca98d7d 100644 --- a/src/tools/miri/src/shims/unix/linux/fd/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/fd/epoll.rs @@ -1,6 +1,6 @@ use crate::*; -use crate::shims::unix::fs::FileDescriptor; +use crate::shims::unix::FileDescriptor; use rustc_data_structures::fx::FxHashMap; use std::io; diff --git a/src/tools/miri/src/shims/unix/linux/fd/event.rs b/src/tools/miri/src/shims/unix/linux/fd/event.rs index 49408fda3ae31..0eb4befd52fb8 100644 --- a/src/tools/miri/src/shims/unix/linux/fd/event.rs +++ b/src/tools/miri/src/shims/unix/linux/fd/event.rs @@ -1,4 +1,4 @@ -use crate::shims::unix::fs::FileDescriptor; +use crate::shims::unix::FileDescriptor; use rustc_const_eval::interpret::InterpResult; use rustc_middle::ty::TyCtxt; diff --git a/src/tools/miri/src/shims/unix/linux/fd/socketpair.rs b/src/tools/miri/src/shims/unix/linux/fd/socketpair.rs deleted file mode 100644 index 6adae88235f3d..0000000000000 --- a/src/tools/miri/src/shims/unix/linux/fd/socketpair.rs +++ /dev/null @@ -1,28 +0,0 @@ -use crate::*; - -use crate::shims::unix::fs::FileDescriptor; - -use std::io; - -/// Pair of connected sockets. -/// -/// We currently don't allow sending any data through this pair, so this can be just a dummy. -#[derive(Debug)] -pub struct SocketPair; - -impl FileDescriptor for SocketPair { - fn name(&self) -> &'static str { - "socketpair" - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(SocketPair)) - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - Ok(Ok(0)) - } -} diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index d13ada0f4cf00..7e600f4c54bbd 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -3,15 +3,12 @@ use rustc_target::spec::abi::Abi; use crate::machine::SIGRTMAX; use crate::machine::SIGRTMIN; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; use shims::unix::linux::fd::EvalContextExt as _; use shims::unix::linux::mem::EvalContextExt as _; use shims::unix::linux::sync::futex; -use shims::unix::mem::EvalContextExt as _; -use shims::unix::sync::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; pub fn is_dyn_sym(name: &str) -> bool { matches!(name, "getrandom") diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index 3af01eb44d8a4..53a02bf5e0b5a 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -1,10 +1,9 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; pub fn is_dyn_sym(_name: &str) -> bool { false diff --git a/src/tools/miri/src/shims/unix/mod.rs b/src/tools/miri/src/shims/unix/mod.rs index 638473da02b77..2bc41e1a62d62 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -1,7 +1,9 @@ pub mod foreign_items; +mod fd; mod fs; mod mem; +mod socket; mod sync; mod thread; @@ -9,7 +11,15 @@ mod freebsd; mod linux; mod macos; -pub use fs::{DirHandler, FileHandler}; +pub use fd::{FdTable, FileDescriptor}; +pub use fs::DirTable; +// All the unix-specific extension traits +pub use fd::EvalContextExt as _; +pub use fs::EvalContextExt as _; +pub use mem::EvalContextExt as _; +pub use socket::EvalContextExt as _; +pub use sync::EvalContextExt as _; +pub use thread::EvalContextExt as _; // Make up some constants. const UID: u32 = 1000; diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs new file mode 100644 index 0000000000000..aa06425ffa1a9 --- /dev/null +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -0,0 +1,66 @@ +use std::io; + +use crate::shims::unix::*; +use crate::*; + +/// Pair of connected sockets. +/// +/// We currently don't allow sending any data through this pair, so this can be just a dummy. +/// FIXME: show proper errors when trying to send/receive +#[derive(Debug)] +struct SocketPair; + +impl FileDescriptor for SocketPair { + fn name(&self) -> &'static str { + "socketpair" + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(SocketPair)) + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + Ok(Ok(0)) + } +} + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// Currently this function this function is a stub. Eventually we need to + /// properly implement an FD type for sockets and have this function create + /// two sockets and associated FDs such that writing to one will produce + /// data that can be read from the other. + /// + /// For more information on the arguments see the socketpair manpage: + /// + fn socketpair( + &mut self, + domain: &OpTy<'tcx, Provenance>, + type_: &OpTy<'tcx, Provenance>, + protocol: &OpTy<'tcx, Provenance>, + sv: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let _domain = this.read_scalar(domain)?.to_i32()?; + let _type_ = this.read_scalar(type_)?.to_i32()?; + let _protocol = this.read_scalar(protocol)?.to_i32()?; + let sv = this.deref_pointer(sv)?; + + // FIXME: fail on unsupported inputs + + let fds = &mut this.machine.fds; + let sv0 = fds.insert_fd(Box::new(SocketPair)); + let sv0 = Scalar::try_from_int(sv0, sv.layout.size).unwrap(); + let sv1 = fds.insert_fd(Box::new(SocketPair)); + let sv1 = Scalar::try_from_int(sv1, sv.layout.size).unwrap(); + + this.write_scalar(sv0, &sv)?; + this.write_scalar(sv1, &sv.offset(sv.layout.size, sv.layout, this)?)?; + + Ok(Scalar::from_i32(0)) + } +} From 2cfb3a6750b730bc15dac6e0c319e10ec6045636 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Apr 2024 11:31:38 +0200 Subject: [PATCH 15/28] shims/linux: move epoll and eventfd into their own files, together with their FD types --- src/tools/miri/src/shims/unix/fd.rs | 2 + .../src/shims/unix/linux/{fd.rs => epoll.rs} | 93 +++++++++---------- .../unix/linux/{fd/event.rs => eventfd.rs} | 62 +++++++++++-- .../miri/src/shims/unix/linux/fd/epoll.rs | 47 ---------- .../src/shims/unix/linux/foreign_items.rs | 3 +- src/tools/miri/src/shims/unix/linux/mod.rs | 3 +- src/tools/miri/src/shims/unix/socket.rs | 1 - 7 files changed, 105 insertions(+), 106 deletions(-) rename src/tools/miri/src/shims/unix/linux/{fd.rs => epoll.rs} (75%) rename src/tools/miri/src/shims/unix/linux/{fd/event.rs => eventfd.rs} (50%) delete mode 100644 src/tools/miri/src/shims/unix/linux/fd/epoll.rs diff --git a/src/tools/miri/src/shims/unix/fd.rs b/src/tools/miri/src/shims/unix/fd.rs index bc9348ee0e8c5..a5fe38b902de8 100644 --- a/src/tools/miri/src/shims/unix/fd.rs +++ b/src/tools/miri/src/shims/unix/fd.rs @@ -52,6 +52,8 @@ pub trait FileDescriptor: std::fmt::Debug + Any { fn dup(&mut self) -> io::Result>; fn is_tty(&self, _communicate_allowed: bool) -> bool { + // Most FDs are not tty's and the consequence of a wrong `false` are minor, + // so we use a default impl here. false } } diff --git a/src/tools/miri/src/shims/unix/linux/fd.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs similarity index 75% rename from src/tools/miri/src/shims/unix/linux/fd.rs rename to src/tools/miri/src/shims/unix/linux/epoll.rs index 7d5177e5c424f..82e0bffff77d1 100644 --- a/src/tools/miri/src/shims/unix/linux/fd.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -1,12 +1,50 @@ -use std::cell::Cell; +use std::io; + +use rustc_data_structures::fx::FxHashMap; use crate::shims::unix::*; use crate::*; -use epoll::{Epoll, EpollEvent}; -use event::Event; -pub mod epoll; -pub mod event; +/// An `Epoll` file descriptor connects file handles and epoll events +#[derive(Clone, Debug, Default)] +struct Epoll { + /// The file descriptors we are watching, and what we are watching for. + file_descriptors: FxHashMap, +} + +/// Epoll Events associate events with data. +/// These fields are currently unused by miri. +/// This matches the `epoll_event` struct defined +/// by the epoll_ctl man page. For more information +/// see the man page: +/// +/// +#[derive(Clone, Debug)] +struct EpollEvent { + #[allow(dead_code)] + events: u32, + /// `Scalar` is used to represent the + /// `epoll_data` type union. + #[allow(dead_code)] + data: Scalar, +} + +impl FileDescriptor for Epoll { + fn name(&self) -> &'static str { + "epoll" + } + + fn dup(&mut self) -> io::Result> { + Ok(Box::new(self.clone())) + } + + fn close<'tcx>( + self: Box, + _communicate_allowed: bool, + ) -> InterpResult<'tcx, io::Result> { + Ok(Ok(0)) + } +} impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { @@ -156,49 +194,4 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(Scalar::from_i32(this.fd_not_found()?)) } } - - /// This function creates an `Event` that is used as an event wait/notify mechanism by - /// user-space applications, and by the kernel to notify user-space applications of events. - /// The `Event` contains an `u64` counter maintained by the kernel. The counter is initialized - /// with the value specified in the `initval` argument. - /// - /// A new file descriptor referring to the `Event` is returned. The `read`, `write`, `poll`, - /// `select`, and `close` operations can be performed on the file descriptor. For more - /// information on these operations, see the man page linked below. - /// - /// The `flags` are not currently implemented for eventfd. - /// The `flags` may be bitwise ORed to change the behavior of `eventfd`: - /// `EFD_CLOEXEC` - Set the close-on-exec (`FD_CLOEXEC`) flag on the new file descriptor. - /// `EFD_NONBLOCK` - Set the `O_NONBLOCK` file status flag on the new open file description. - /// `EFD_SEMAPHORE` - miri does not support semaphore-like semantics. - /// - /// - #[expect(clippy::needless_if)] - fn eventfd( - &mut self, - val: &OpTy<'tcx, Provenance>, - flags: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let val = this.read_scalar(val)?.to_u32()?; - let flags = this.read_scalar(flags)?.to_i32()?; - - let efd_cloexec = this.eval_libc_i32("EFD_CLOEXEC"); - let efd_nonblock = this.eval_libc_i32("EFD_NONBLOCK"); - let efd_semaphore = this.eval_libc_i32("EFD_SEMAPHORE"); - - if flags & (efd_cloexec | efd_nonblock | efd_semaphore) == 0 { - throw_unsup_format!("{flags} is unsupported"); - } - // FIXME handle the cloexec and nonblock flags - if flags & efd_cloexec == efd_cloexec {} - if flags & efd_nonblock == efd_nonblock {} - if flags & efd_semaphore == efd_semaphore { - throw_unsup_format!("EFD_SEMAPHORE is unsupported"); - } - - let fd = this.machine.fds.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); - Ok(Scalar::from_i32(fd)) - } } diff --git a/src/tools/miri/src/shims/unix/linux/fd/event.rs b/src/tools/miri/src/shims/unix/linux/eventfd.rs similarity index 50% rename from src/tools/miri/src/shims/unix/linux/fd/event.rs rename to src/tools/miri/src/shims/unix/linux/eventfd.rs index 0eb4befd52fb8..4e066493d2743 100644 --- a/src/tools/miri/src/shims/unix/linux/fd/event.rs +++ b/src/tools/miri/src/shims/unix/linux/eventfd.rs @@ -1,11 +1,13 @@ -use crate::shims::unix::FileDescriptor; +//! Linux `eventfd` implementation. +//! Currently just a stub. +use std::cell::Cell; +use std::io; -use rustc_const_eval::interpret::InterpResult; use rustc_middle::ty::TyCtxt; use rustc_target::abi::Endian; -use std::cell::Cell; -use std::io; +use crate::shims::unix::*; +use crate::*; /// A kind of file descriptor created by `eventfd`. /// The `Event` type isn't currently written to by `eventfd`. @@ -15,10 +17,10 @@ use std::io; /// /// #[derive(Debug)] -pub struct Event { +struct Event { /// The object contains an unsigned 64-bit integer (uint64_t) counter that is maintained by the /// kernel. This counter is initialized with the value specified in the argument initval. - pub val: Cell, + val: Cell, } impl FileDescriptor for Event { @@ -70,3 +72,51 @@ impl FileDescriptor for Event { Ok(Ok(8)) } } + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// This function creates an `Event` that is used as an event wait/notify mechanism by + /// user-space applications, and by the kernel to notify user-space applications of events. + /// The `Event` contains an `u64` counter maintained by the kernel. The counter is initialized + /// with the value specified in the `initval` argument. + /// + /// A new file descriptor referring to the `Event` is returned. The `read`, `write`, `poll`, + /// `select`, and `close` operations can be performed on the file descriptor. For more + /// information on these operations, see the man page linked below. + /// + /// The `flags` are not currently implemented for eventfd. + /// The `flags` may be bitwise ORed to change the behavior of `eventfd`: + /// `EFD_CLOEXEC` - Set the close-on-exec (`FD_CLOEXEC`) flag on the new file descriptor. + /// `EFD_NONBLOCK` - Set the `O_NONBLOCK` file status flag on the new open file description. + /// `EFD_SEMAPHORE` - miri does not support semaphore-like semantics. + /// + /// + #[expect(clippy::needless_if)] + fn eventfd( + &mut self, + val: &OpTy<'tcx, Provenance>, + flags: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let val = this.read_scalar(val)?.to_u32()?; + let flags = this.read_scalar(flags)?.to_i32()?; + + let efd_cloexec = this.eval_libc_i32("EFD_CLOEXEC"); + let efd_nonblock = this.eval_libc_i32("EFD_NONBLOCK"); + let efd_semaphore = this.eval_libc_i32("EFD_SEMAPHORE"); + + if flags & (efd_cloexec | efd_nonblock | efd_semaphore) == 0 { + throw_unsup_format!("{flags} is unsupported"); + } + // FIXME handle the cloexec and nonblock flags + if flags & efd_cloexec == efd_cloexec {} + if flags & efd_nonblock == efd_nonblock {} + if flags & efd_semaphore == efd_semaphore { + throw_unsup_format!("EFD_SEMAPHORE is unsupported"); + } + + let fd = this.machine.fds.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); + Ok(Scalar::from_i32(fd)) + } +} diff --git a/src/tools/miri/src/shims/unix/linux/fd/epoll.rs b/src/tools/miri/src/shims/unix/linux/fd/epoll.rs deleted file mode 100644 index f2da76ca98d7d..0000000000000 --- a/src/tools/miri/src/shims/unix/linux/fd/epoll.rs +++ /dev/null @@ -1,47 +0,0 @@ -use crate::*; - -use crate::shims::unix::FileDescriptor; - -use rustc_data_structures::fx::FxHashMap; -use std::io; - -/// An `Epoll` file descriptor connects file handles and epoll events -#[derive(Clone, Debug, Default)] -pub struct Epoll { - /// The file descriptors we are watching, and what we are watching for. - pub file_descriptors: FxHashMap, -} - -/// Epoll Events associate events with data. -/// These fields are currently unused by miri. -/// This matches the `epoll_event` struct defined -/// by the epoll_ctl man page. For more information -/// see the man page: -/// -/// -#[derive(Clone, Debug)] -pub struct EpollEvent { - #[allow(dead_code)] - pub events: u32, - /// `Scalar` is used to represent the - /// `epoll_data` type union. - #[allow(dead_code)] - pub data: Scalar, -} - -impl FileDescriptor for Epoll { - fn name(&self) -> &'static str { - "epoll" - } - - fn dup(&mut self) -> io::Result> { - Ok(Box::new(self.clone())) - } - - fn close<'tcx>( - self: Box, - _communicate_allowed: bool, - ) -> InterpResult<'tcx, io::Result> { - Ok(Ok(0)) - } -} diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 7e600f4c54bbd..2aaec3f5a0b44 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -6,7 +6,8 @@ use crate::machine::SIGRTMIN; use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::linux::fd::EvalContextExt as _; +use shims::unix::linux::epoll::EvalContextExt as _; +use shims::unix::linux::eventfd::EvalContextExt as _; use shims::unix::linux::mem::EvalContextExt as _; use shims::unix::linux::sync::futex; diff --git a/src/tools/miri/src/shims/unix/linux/mod.rs b/src/tools/miri/src/shims/unix/linux/mod.rs index fe18f1a32fd42..84b604eb9b82e 100644 --- a/src/tools/miri/src/shims/unix/linux/mod.rs +++ b/src/tools/miri/src/shims/unix/linux/mod.rs @@ -1,4 +1,5 @@ -pub mod fd; +pub mod epoll; +pub mod eventfd; pub mod foreign_items; pub mod mem; pub mod sync; diff --git a/src/tools/miri/src/shims/unix/socket.rs b/src/tools/miri/src/shims/unix/socket.rs index aa06425ffa1a9..84ddd746fb584 100644 --- a/src/tools/miri/src/shims/unix/socket.rs +++ b/src/tools/miri/src/shims/unix/socket.rs @@ -6,7 +6,6 @@ use crate::*; /// Pair of connected sockets. /// /// We currently don't allow sending any data through this pair, so this can be just a dummy. -/// FIXME: show proper errors when trying to send/receive #[derive(Debug)] struct SocketPair; From 23fba86c3b1e3c1d42de5a8f35c206c766059dc2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Apr 2024 15:16:36 +0200 Subject: [PATCH 16/28] eventfd: fix flag check and note a FIXME --- src/tools/miri/src/shims/unix/linux/eventfd.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/src/shims/unix/linux/eventfd.rs b/src/tools/miri/src/shims/unix/linux/eventfd.rs index 4e066493d2743..0f28b69ac4a8e 100644 --- a/src/tools/miri/src/shims/unix/linux/eventfd.rs +++ b/src/tools/miri/src/shims/unix/linux/eventfd.rs @@ -29,6 +29,7 @@ impl FileDescriptor for Event { } fn dup(&mut self) -> io::Result> { + // FIXME: this is wrong, the new and old FD should refer to the same event object! Ok(Box::new(Event { val: self.val.clone() })) } @@ -91,7 +92,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// `EFD_SEMAPHORE` - miri does not support semaphore-like semantics. /// /// - #[expect(clippy::needless_if)] fn eventfd( &mut self, val: &OpTy<'tcx, Provenance>, @@ -106,14 +106,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let efd_nonblock = this.eval_libc_i32("EFD_NONBLOCK"); let efd_semaphore = this.eval_libc_i32("EFD_SEMAPHORE"); - if flags & (efd_cloexec | efd_nonblock | efd_semaphore) == 0 { - throw_unsup_format!("{flags} is unsupported"); + if flags & (efd_cloexec | efd_nonblock | efd_semaphore) != flags { + throw_unsup_format!("eventfd: flag {flags:#x} is unsupported"); + } + if flags & efd_cloexec == efd_cloexec { + // cloexec does nothing as we don't support `exec` + } + if flags & efd_nonblock == efd_nonblock { + // FIXME remember the nonblock flag } - // FIXME handle the cloexec and nonblock flags - if flags & efd_cloexec == efd_cloexec {} - if flags & efd_nonblock == efd_nonblock {} if flags & efd_semaphore == efd_semaphore { - throw_unsup_format!("EFD_SEMAPHORE is unsupported"); + throw_unsup_format!("eventfd: EFD_SEMAPHORE is unsupported"); } let fd = this.machine.fds.insert_fd(Box::new(Event { val: Cell::new(val.into()) })); From f2120893c696886317e1ef66e4f1e6419a147989 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Apr 2024 15:19:33 +0200 Subject: [PATCH 17/28] epoll: note a FIXME --- src/tools/miri/src/shims/unix/linux/epoll.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/miri/src/shims/unix/linux/epoll.rs b/src/tools/miri/src/shims/unix/linux/epoll.rs index 82e0bffff77d1..5161d91ca3639 100644 --- a/src/tools/miri/src/shims/unix/linux/epoll.rs +++ b/src/tools/miri/src/shims/unix/linux/epoll.rs @@ -35,6 +35,8 @@ impl FileDescriptor for Epoll { } fn dup(&mut self) -> io::Result> { + // FIXME: this is probably wrong -- check if the `dup`ed descriptor truly uses an + // independent event set. Ok(Box::new(self.clone())) } From 20807c0175450bf5bfab0dcd4ab0238cab8a3eac Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Thu, 4 Apr 2024 04:55:34 +0000 Subject: [PATCH 18/28] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 90194ec603864..02e2e6459fbff 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -b688d53a1736c17e49328a706a90829a9937a91a +0accf4ec4c07d23aa86f6a97aeb8797941abc30e From 219226fea9d16998852b0c3cf87b94ab296fdd24 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Apr 2024 13:15:16 +0200 Subject: [PATCH 19/28] linux/foreign_items: reorder things to make more sense, remove duplicate socketpair --- .../miri/src/shims/unix/foreign_items.rs | 7 +- .../src/shims/unix/freebsd/foreign_items.rs | 3 +- .../src/shims/unix/linux/foreign_items.rs | 69 ++++++++----------- 3 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index b1e1aec5880a9..3a56aa9138b45 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -6,14 +6,9 @@ use rustc_span::Symbol; use rustc_target::abi::{Align, Size}; use rustc_target::spec::abi::Abi; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fd::EvalContextExt as _; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::mem::EvalContextExt as _; -use shims::unix::socket::EvalContextExt as _; -use shims::unix::sync::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; use shims::unix::freebsd::foreign_items as freebsd; use shims::unix::linux::foreign_items as linux; diff --git a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs index 6814e0d428325..ffb583123d41b 100644 --- a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs @@ -1,10 +1,9 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; +use crate::shims::unix::*; use crate::*; use shims::foreign_items::EmulateForeignItemResult; -use shims::unix::fs::EvalContextExt as _; -use shims::unix::thread::EvalContextExt as _; pub fn is_dyn_sym(_name: &str) -> bool { false diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 2aaec3f5a0b44..497e8bee70ec9 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -29,34 +29,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // See `fn emulate_foreign_item_inner` in `shims/foreign_items.rs` for the general pattern. match link_name.as_str() { - // errno - "__errno_location" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let errno_place = this.last_error_place()?; - this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?; - } - // File related shims (but also see "syscall" below for statx) "readdir64" => { let [dirp] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.linux_readdir64(dirp)?; this.write_scalar(result, dest)?; } - "mmap64" => { - let [addr, length, prot, flags, fd, offset] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let offset = this.read_scalar(offset)?.to_i64()?; - let ptr = this.mmap(addr, length, prot, flags, fd, offset.into())?; - this.write_scalar(ptr, dest)?; - } - - // Linux-only "sync_file_range" => { let [fd, offset, nbytes, flags] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.sync_file_range(fd, offset, nbytes, flags)?; this.write_scalar(result, dest)?; } + + // epoll, eventfd "epoll_create1" => { let [flag] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; let result = this.epoll_create1(flag)?; @@ -80,29 +66,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.eventfd(val, flag)?; this.write_scalar(result, dest)?; } - "mremap" => { - let [old_address, old_size, new_size, flags] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let ptr = this.mremap(old_address, old_size, new_size, flags)?; - this.write_scalar(ptr, dest)?; - } - "socketpair" => { - let [domain, type_, protocol, sv] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - let result = this.socketpair(domain, type_, protocol, sv)?; - this.write_scalar(result, dest)?; - } - "__libc_current_sigrtmin" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - this.write_scalar(Scalar::from_i32(SIGRTMIN), dest)?; - } - "__libc_current_sigrtmax" => { - let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - - this.write_scalar(Scalar::from_i32(SIGRTMAX), dest)?; - } // Threading "pthread_condattr_setclock" => { @@ -204,6 +167,34 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; getrandom(this, ptr, len, flags, dest)?; } + "mmap64" => { + let [addr, length, prot, flags, fd, offset] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let offset = this.read_scalar(offset)?.to_i64()?; + let ptr = this.mmap(addr, length, prot, flags, fd, offset.into())?; + this.write_scalar(ptr, dest)?; + } + "mremap" => { + let [old_address, old_size, new_size, flags] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let ptr = this.mremap(old_address, old_size, new_size, flags)?; + this.write_scalar(ptr, dest)?; + } + "__errno_location" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let errno_place = this.last_error_place()?; + this.write_scalar(errno_place.to_ref(this).to_scalar(), dest)?; + } + "__libc_current_sigrtmin" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + this.write_scalar(Scalar::from_i32(SIGRTMIN), dest)?; + } + "__libc_current_sigrtmax" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + this.write_scalar(Scalar::from_i32(SIGRTMAX), dest)?; + } // Incomplete shims that we "stub out" just to get pre-main initialization code to work. // These shims are enabled only when the caller is in the standard library. From 8f68946e9899b63ef8f3cefb3e7c40faefb5fa94 Mon Sep 17 00:00:00 2001 From: findseat Date: Sat, 6 Apr 2024 10:35:24 +0800 Subject: [PATCH 20/28] chore: fix some typos Signed-off-by: findseat --- .../miri/src/borrow_tracker/tree_borrows/diagnostics.rs | 4 ++-- src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs | 2 +- .../miri/src/borrow_tracker/tree_borrows/tree/tests.rs | 6 +++--- src/tools/miri/tests/fail/tree_borrows/spurious_read.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 2690bc026a120..b9f0b5bc17a2c 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -48,7 +48,7 @@ impl AccessCause { /// Complete data for an event: #[derive(Clone, Debug)] pub struct Event { - /// Transformation of permissions that occured because of this event. + /// Transformation of permissions that occurred because of this event. pub transition: PermTransition, /// Kind of the access that triggered this event. pub access_cause: AccessCause, @@ -58,7 +58,7 @@ pub struct Event { /// `None` means that this is an implicit access to the entire allocation /// (used for the implicit read on protector release). pub access_range: Option, - /// The transition recorded by this event only occured on a subrange of + /// The transition recorded by this event only occurred on a subrange of /// `access_range`: a single access on `access_range` triggers several events, /// each with their own mutually disjoint `transition_range`. No-op transitions /// should not be recorded as events, so the union of all `transition_range` is not diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 0fea78daa8838..2470624181e74 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -473,7 +473,7 @@ impl Tree { let rperms = { let mut perms = UniValMap::default(); // We manually set it to `Active` on all in-bounds positions. - // We also ensure that it is initalized, so that no `Active` but + // We also ensure that it is initialized, so that no `Active` but // not yet initialized nodes exist. Essentially, we pretend there // was a write that initialized these to `Active`. perms.insert(root_idx, LocationState::new_init(Permission::new_active())); diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs index f568850d8dba0..6777f41ac2d0d 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs @@ -75,7 +75,7 @@ fn protected_enforces_noalias() { } } -/// We are going to exhaustively test the possibily of inserting +/// We are going to exhaustively test the possibility of inserting /// a spurious read in some code. /// /// We choose some pointer `x` through which we want a spurious read to be inserted. @@ -270,7 +270,7 @@ mod spurious_read { match self { TestEvent::Access(acc) => write!(f, "{acc}"), // The fields of the `Ret` variants just serve to make them - // impossible to instanciate via the `RetX = NoRet` type; we can + // impossible to instantiate via the `RetX = NoRet` type; we can // always ignore their value. TestEvent::RetX(_) => write!(f, "ret x"), TestEvent::RetY(_) => write!(f, "ret y"), @@ -395,7 +395,7 @@ mod spurious_read { match evt { TestEvent::Access(acc) => self.perform_test_access(acc), // The fields of the `Ret` variants just serve to make them - // impossible to instanciate via the `RetX = NoRet` type; we can + // impossible to instantiate via the `RetX = NoRet` type; we can // always ignore their value. TestEvent::RetX(_) => self.end_protector_x(), TestEvent::RetY(_) => self.end_protector_y(), diff --git a/src/tools/miri/tests/fail/tree_borrows/spurious_read.rs b/src/tools/miri/tests/fail/tree_borrows/spurious_read.rs index 3f39dcb4b76ad..50ef0ceef91e7 100644 --- a/src/tools/miri/tests/fail/tree_borrows/spurious_read.rs +++ b/src/tools/miri/tests/fail/tree_borrows/spurious_read.rs @@ -89,7 +89,7 @@ fn retagx_retagy_retx_writey_rety() { // - retag `y` protected // - (wait for the other thread to return so that there is no foreign protector when we write) // - attempt a write through `y`. - // - (UB should have occured by now, but the next step would be to + // - (UB should have occurred by now, but the next step would be to // remove `y`'s protector) let thread_y = thread::spawn(move || { let b = (2, by); From f797a14379fa6eab7e50a927f2ca7ef8b2084cfe Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Apr 2024 08:06:48 +0200 Subject: [PATCH 21/28] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 02e2e6459fbff..6ad8fba723c89 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -0accf4ec4c07d23aa86f6a97aeb8797941abc30e +23d47dba319331d4418827cfbb8c1af283497d3c From a2799ef869ce5f0a25f29db983c20d0dcccd6527 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Apr 2024 09:03:19 +0200 Subject: [PATCH 22/28] remove miri-test-libstd hacks that are no longer needed --- library/alloc/src/lib.rs | 6 ------ library/core/src/lib.rs | 6 +----- library/std/src/lib.rs | 8 +------- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index cafd59cb0d954..e21479450c231 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -56,11 +56,6 @@ //! [`Rc`]: rc //! [`RefCell`]: core::cell -// To run alloc tests without x.py without ending up with two copies of alloc, Miri needs to be -// able to "empty" this crate. See . -// rustc itself never sets the feature, so this line has no effect there. -#![cfg(any(not(feature = "miri-test-libstd"), test, doctest))] -// #![allow(unused_attributes)] #![stable(feature = "alloc", since = "1.36.0")] #![doc( @@ -71,7 +66,6 @@ #![doc(cfg_hide( not(test), not(any(test, bootstrap)), - any(not(feature = "miri-test-libstd"), test, doctest), no_global_oom_handling, not(no_global_oom_handling), not(no_rc), diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index ba19ca1f45d7c..956b85d42ac64 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -57,10 +57,7 @@ // // This cfg won't affect doc tests. #![cfg(not(test))] -// To run core tests without x.py without ending up with two copies of core, Miri needs to be -// able to "empty" this crate. See . -// rustc itself never sets the feature, so this line has no effect there. -#![cfg(any(not(feature = "miri-test-libstd"), test, doctest))] +// #![stable(feature = "core", since = "1.6.0")] #![doc( html_playground_url = "https://play.rust-lang.org/", @@ -71,7 +68,6 @@ #![doc(rust_logo)] #![doc(cfg_hide( not(test), - any(not(feature = "miri-test-libstd"), test, doctest), no_fp_fmt_parse, target_pointer_width = "16", target_pointer_width = "32", diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 31a8711e0eb97..b509503ce4dce 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -212,13 +212,7 @@ //! [rust-discord]: https://discord.gg/rust-lang //! [array]: prim@array //! [slice]: prim@slice -// To run std tests without x.py without ending up with two copies of std, Miri needs to be -// able to "empty" this crate. See . -// rustc itself never sets the feature, so this line has no effect there. -#![cfg(any(not(feature = "miri-test-libstd"), test, doctest))] -// miri-test-libstd also prefers to make std use the sysroot versions of the dependencies. -#![cfg_attr(feature = "miri-test-libstd", feature(rustc_private))] -// + #![cfg_attr(not(feature = "restricted-std"), stable(feature = "rust1", since = "1.0.0"))] #![cfg_attr(feature = "restricted-std", unstable(feature = "restricted_std", issue = "none"))] #![cfg_attr(not(bootstrap), rustc_preserve_ub_checks)] From 9989653269fafb2dd35a4ff94fbb6e0c93555c17 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 6 Apr 2024 14:08:20 +0200 Subject: [PATCH 23/28] make 'missing extern static' error consistent with missing shim --- src/tools/miri/src/machine.rs | 8 ++------ src/tools/miri/tests/fail/extern_static.stderr | 4 ++-- src/tools/miri/tests/fail/extern_static_in_const.stderr | 4 ++-- src/tools/miri/tests/fail/extern_static_wrong_size.rs | 2 +- src/tools/miri/tests/fail/extern_static_wrong_size.stderr | 4 ++-- ...-miri-3288-ice-symbolic-alignment-extern-static.stderr | 4 ++-- 6 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 14b7afcc971fd..ff081328a7223 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1066,7 +1066,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let extern_decl_layout = ecx.tcx.layout_of(ty::ParamEnv::empty().and(def_ty)).unwrap(); if extern_decl_layout.size != shim_size || extern_decl_layout.align.abi != shim_align { throw_unsup_format!( - "`extern` static `{name}` from crate `{krate}` has been declared \ + "extern static `{link_name}` has been declared as `{krate}::{name}` \ with a size of {decl_size} bytes and alignment of {decl_align} bytes, \ but Miri emulates it via an extern static shim \ with a size of {shim_size} bytes and alignment of {shim_align} bytes", @@ -1080,11 +1080,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } Ok(ptr) } else { - throw_unsup_format!( - "`extern` static `{name}` from crate `{krate}` is not supported by Miri", - name = ecx.tcx.def_path_str(def_id), - krate = ecx.tcx.crate_name(def_id.krate), - ) + throw_unsup_format!("extern static `{link_name}` is not supported by Miri",) } } diff --git a/src/tools/miri/tests/fail/extern_static.stderr b/src/tools/miri/tests/fail/extern_static.stderr index c3de4dadb0a30..34bb273f04be0 100644 --- a/src/tools/miri/tests/fail/extern_static.stderr +++ b/src/tools/miri/tests/fail/extern_static.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `FOO` from crate `extern_static` is not supported by Miri +error: unsupported operation: extern static `FOO` is not supported by Miri --> $DIR/extern_static.rs:LL:CC | LL | let _val = unsafe { std::ptr::addr_of!(FOO) }; - | ^^^ `extern` static `FOO` from crate `extern_static` is not supported by Miri + | ^^^ extern static `FOO` is not supported by Miri | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/src/tools/miri/tests/fail/extern_static_in_const.stderr b/src/tools/miri/tests/fail/extern_static_in_const.stderr index 23f775f25888a..45d1632cce2ee 100644 --- a/src/tools/miri/tests/fail/extern_static_in_const.stderr +++ b/src/tools/miri/tests/fail/extern_static_in_const.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `E` from crate `extern_static_in_const` is not supported by Miri +error: unsupported operation: extern static `E` is not supported by Miri --> $DIR/extern_static_in_const.rs:LL:CC | LL | let _val = X; - | ^ `extern` static `E` from crate `extern_static_in_const` is not supported by Miri + | ^ extern static `E` is not supported by Miri | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/src/tools/miri/tests/fail/extern_static_wrong_size.rs b/src/tools/miri/tests/fail/extern_static_wrong_size.rs index 17061f0e5c81c..fee3c38c25e5e 100644 --- a/src/tools/miri/tests/fail/extern_static_wrong_size.rs +++ b/src/tools/miri/tests/fail/extern_static_wrong_size.rs @@ -6,5 +6,5 @@ extern "C" { } fn main() { - let _val = unsafe { environ }; //~ ERROR: /has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of [48] bytes and alignment of [48] bytes/ + let _val = unsafe { environ }; //~ ERROR: /with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of [48] bytes and alignment of [48] bytes/ } diff --git a/src/tools/miri/tests/fail/extern_static_wrong_size.stderr b/src/tools/miri/tests/fail/extern_static_wrong_size.stderr index c935a548f806d..27699d780c228 100644 --- a/src/tools/miri/tests/fail/extern_static_wrong_size.stderr +++ b/src/tools/miri/tests/fail/extern_static_wrong_size.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `environ` from crate `extern_static_wrong_size` has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes +error: unsupported operation: extern static `environ` has been declared as `extern_static_wrong_size::environ` with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes --> $DIR/extern_static_wrong_size.rs:LL:CC | LL | let _val = unsafe { environ }; - | ^^^^^^^ `extern` static `environ` from crate `extern_static_wrong_size` has been declared with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes + | ^^^^^^^ extern static `environ` has been declared as `extern_static_wrong_size::environ` with a size of 1 bytes and alignment of 1 bytes, but Miri emulates it via an extern static shim with a size of N bytes and alignment of N bytes | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr b/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr index a4249d2e8816d..e5dbe749884cc 100644 --- a/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr +++ b/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr @@ -1,8 +1,8 @@ -error: unsupported operation: `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri +error: unsupported operation: extern static `_dispatch_queue_attr_concurrent` is not supported by Miri --> $DIR/issue-miri-3288-ice-symbolic-alignment-extern-static.rs:LL:CC | LL | let _val = *DISPATCH_QUEUE_CONCURRENT; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri + | ^^^^^^^^^^^^^^^^^^^^^^^^^ extern static `_dispatch_queue_attr_concurrent` is not supported by Miri | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: From 4967895e558870303a38b5b92b9d7ebc018eba65 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 14:53:30 +0200 Subject: [PATCH 24/28] Move `search-result-color.goml` common parts into a function --- tests/rustdoc-gui/search-result-color.goml | 121 +++++++++------------ 1 file changed, 52 insertions(+), 69 deletions(-) diff --git a/tests/rustdoc-gui/search-result-color.goml b/tests/rustdoc-gui/search-result-color.goml index d4da23fa156d9..c26d562803398 100644 --- a/tests/rustdoc-gui/search-result-color.goml +++ b/tests/rustdoc-gui/search-result-color.goml @@ -1,6 +1,37 @@ // The goal of this test is to ensure the color of the text is the one expected. include: "utils.goml" + +define-function: ( + "check-search-color", + [theme, count_color, desc_color, path_color, bottom_border_color], + block { + call-function: ("switch-theme", {"theme": |theme|}) + + // Waiting for the search results to appear... + wait-for: "#search-tabs" + assert-css: ( + "#search-tabs > button > .count", + {"color": |count_color|}, + ALL, + ) + assert-css: ( + "//*[@class='desc'][text()='Just a normal struct.']", + {"color": |desc_color|}, + ) + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']", + {"color": |path_color|}, + ) + + // Checking the color of the bottom border. + assert-css: ( + ".search-results > a", + {"border-bottom-color": |bottom_border_color|} + ) + } +) + define-function: ( "check-result-color", [result_kind, color, hover_color], @@ -44,29 +75,13 @@ go-to: "file://" + |DOC_PATH| + "/test_docs/index.html?search=coo" show-text: true // Ayu theme -call-function: ("switch-theme", {"theme": "ayu"}) - -// Waiting for the search results to appear... -wait-for: "#search-tabs" -assert-css: ( - "#search-tabs > button > .count", - {"color": "#888"}, - ALL, -) -assert-css: ( - "//*[@class='desc'][text()='Just a normal struct.']", - {"color": "#c5c5c5"}, -) -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']", - {"color": "#0096cf"}, -) - -// Checking the color of the bottom border. -assert-css: ( - ".search-results > a", - {"border-bottom-color": "#aaa3"} -) +call-function: ("check-search-color", { + "theme": "ayu", + "count_color": "#888", + "desc_color": "#c5c5c5", + "path_color": "#0096cf", + "bottom_border_color": "#aaa3", +}) store-value: (entry_color, "#0096cf") // color of the search entry store-value: (hover_entry_color, "#fff") // color of the hovered/focused search entry @@ -152,29 +167,13 @@ assert-css: ( ) // Dark theme -call-function: ("switch-theme", {"theme": "dark"}) - -// Waiting for the search results to appear... -wait-for: "#search-tabs" -assert-css: ( - "#search-tabs > button > .count", - {"color": "#888"}, - ALL, -) -assert-css: ( - "//*[@class='desc'][text()='Just a normal struct.']", - {"color": "#ddd"}, -) -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']", - {"color": "#ddd"}, -) - -// Checking the color of the bottom border. -assert-css: ( - ".search-results > a", - {"border-bottom-color": "#aaa3"} -) +call-function: ("check-search-color", { + "theme": "dark", + "count_color": "#888", + "desc_color": "#ddd", + "path_color": "#ddd", + "bottom_border_color": "#aaa3", +}) store-value: (entry_color, "#ddd") // color of the search entry store-value: (hover_entry_color, "#ddd") // color of the hovered/focused search entry @@ -248,29 +247,13 @@ assert-css: ( ) // Light theme -call-function: ("switch-theme", {"theme": "light"}) - -// Waiting for the search results to appear... -wait-for: "#search-tabs" -assert-css: ( - "#search-tabs > button > .count", - {"color": "#888"}, - ALL, -) -assert-css: ( - "//*[@class='desc'][text()='Just a normal struct.']", - {"color": "#000"}, -) -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']", - {"color": "#000"}, -) - -// Checking the color of the bottom border. -assert-css: ( - ".search-results > a", - {"border-bottom-color": "#aaa3"} -) +call-function: ("check-search-color", { + "theme": "light", + "count_color": "#888", + "desc_color": "#000", + "path_color": "#000", + "bottom_border_color": "#aaa3", +}) store-value: (entry_color, "#000") // color of the search entry store-value: (hover_entry_color, "#000") // color of the hovered/focused search entry From f14e4db1f92bcdde9ffa1488bbb1024712df06e5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 15:24:09 +0200 Subject: [PATCH 25/28] Move more common code into a function in `tests/rustdoc-gui/search-result-color.goml` --- tests/rustdoc-gui/search-result-color.goml | 74 +++++++++++++--------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/tests/rustdoc-gui/search-result-color.goml b/tests/rustdoc-gui/search-result-color.goml index c26d562803398..33f45d0d39360 100644 --- a/tests/rustdoc-gui/search-result-color.goml +++ b/tests/rustdoc-gui/search-result-color.goml @@ -69,6 +69,32 @@ define-function: ( }, ) +define-function: ( + "check-container-color", + [link_color, path_color, hover_background], + block { + // Checking the `` container. + move-cursor-to: ".search-input" + focus: ".search-input" // To ensure the `` container isn't focused or hovered. + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", + {"color": |link_color|, "background-color": "transparent"}, + ALL, + ) + + // Checking color and background on hover. + move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']" + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']", + {"color": |path_color|}, + ) + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", + {"color": |path_color|, "background-color": |hover_background|}, + ) + } +) + go-to: "file://" + |DOC_PATH| + "/test_docs/index.html?search=coo" // This is needed so that the text color is computed. @@ -146,24 +172,12 @@ call-function: ( }, ) -// Checking the `` container. -move-cursor-to: ".search-input" -focus: ".search-input" // To ensure the `` container isnt focus or hover. -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": "#0096cf", "background-color": "transparent"}, - ALL, -) - -// Checking color and background on hover. -move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']" -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']", - {"color": "#fff"}, -) -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": "#fff", "background-color": "#3c3c3c"}, +call-function: ( + "check-container-color", { + "link_color": "#0096cf", + "path_color": "#fff", + "hover_background": "#3c3c3c", + }, ) // Dark theme @@ -238,12 +252,12 @@ call-function: ( }, ) -// Checking the `` container. -move-cursor-to: ".search-input" -focus: ".search-input" // To ensure the `` container isnt focus or hover. -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": "#ddd", "background-color": "transparent"}, +call-function: ( + "check-container-color", { + "link_color": "#ddd", + "path_color": "#ddd", + "hover_background": "#616161", + }, ) // Light theme @@ -318,12 +332,12 @@ call-function: ( }, ) -// Checking the `` container. -move-cursor-to: ".search-input" -focus: ".search-input" // To ensure the `` container isnt focus or hover. -assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": "#000", "background-color": "transparent"}, +call-function: ( + "check-container-color", { + "link_color": "#000", + "path_color": "#000", + "hover_background": "#ccc", + }, ) // Check the alias. From 809e0c7453aef3afbbf0856e5d88549811ece154 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 6 Apr 2024 13:35:54 +0000 Subject: [PATCH 26/28] Add missing -Zquery-dep-graph to the spike-neg incr comp tests This ensures that the tests actually test what they are meant to test rather than exitting immediately with an error that -Zquery-dep-graph has to be passed. --- tests/incremental/spike-neg1.rs | 1 + tests/incremental/spike-neg2.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/incremental/spike-neg1.rs b/tests/incremental/spike-neg1.rs index c5fe31862b477..356c61759f93d 100644 --- a/tests/incremental/spike-neg1.rs +++ b/tests/incremental/spike-neg1.rs @@ -7,6 +7,7 @@ //@ revisions:rpass1 rpass2 //@ should-fail +//@ compile-flags: -Z query-dep-graph #![feature(rustc_attrs)] diff --git a/tests/incremental/spike-neg2.rs b/tests/incremental/spike-neg2.rs index 140aa4684a778..9f69d7a757a52 100644 --- a/tests/incremental/spike-neg2.rs +++ b/tests/incremental/spike-neg2.rs @@ -7,6 +7,7 @@ //@ revisions:rpass1 rpass2 //@ should-fail +//@ compile-flags: -Z query-dep-graph #![feature(rustc_attrs)] From 2b1c799636ca9253280d7082cb6aa770c7571428 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 15:47:12 +0200 Subject: [PATCH 27/28] Move duplicated code into `check-search-color` function --- tests/rustdoc-gui/search-result-color.goml | 316 +++++++-------------- 1 file changed, 98 insertions(+), 218 deletions(-) diff --git a/tests/rustdoc-gui/search-result-color.goml b/tests/rustdoc-gui/search-result-color.goml index 33f45d0d39360..5a31afc6e6104 100644 --- a/tests/rustdoc-gui/search-result-color.goml +++ b/tests/rustdoc-gui/search-result-color.goml @@ -4,7 +4,11 @@ include: "utils.goml" define-function: ( "check-search-color", - [theme, count_color, desc_color, path_color, bottom_border_color], + [ + theme, count_color, desc_color, path_color, bottom_border_color, keyword_color, + struct_color, associatedtype_color, tymethod_color, method_color, structfield_color, + structfield_hover_color, macro_color, fn_color, hover_path_color, hover_background, grey + ], block { call-function: ("switch-theme", {"theme": |theme|}) @@ -29,6 +33,59 @@ define-function: ( ".search-results > a", {"border-bottom-color": |bottom_border_color|} ) + + store-value: (entry_color, |path_color|) // color of the search entry + store-value: (hover_entry_color, |hover_path_color|) // color of the hovered/focused search entry + store-value: (background_color, "transparent") + store-value: (hover_background_color, |hover_background|) + store-value: (grey, |grey|) + + call-function: ("check-result-color", { + "result_kind": "keyword", + "color": |keyword_color|, + "hover_color": |keyword_color|, + }) + call-function: ("check-result-color", { + "result_kind": "struct", + "color": |struct_color|, + "hover_color": |struct_color|, + }) + call-function: ("check-result-color", { + "result_kind": "associatedtype", + "color": |associatedtype_color|, + "hover_color": |associatedtype_color|, + }) + call-function: ("check-result-color", { + "result_kind": "tymethod", + "color": |tymethod_color|, + "hover_color": |tymethod_color|, + }) + call-function: ("check-result-color", { + "result_kind": "method", + "color": |method_color|, + "hover_color": |method_color|, + }) + call-function: ("check-result-color", { + "result_kind": "structfield", + "color": |structfield_color|, + "hover_color": |structfield_hover_color|, + }) + call-function: ("check-result-color", { + "result_kind": "macro", + "color": |macro_color|, + "hover_color": |macro_color|, + }) + call-function: ("check-result-color", { + "result_kind": "fn", + "color": |fn_color|, + "hover_color": |fn_color|, + }) + + call-function: ("check-container-color", { + "path_color": |path_color|, + "hover_path_color": |hover_path_color|, + "hover_background": |hover_background|, + }) } ) @@ -71,14 +128,14 @@ define-function: ( define-function: ( "check-container-color", - [link_color, path_color, hover_background], + [path_color, hover_path_color, hover_background], block { // Checking the `` container. move-cursor-to: ".search-input" focus: ".search-input" // To ensure the `` container isn't focused or hovered. assert-css: ( "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": |link_color|, "background-color": "transparent"}, + {"color": |path_color|, "background-color": "transparent"}, ALL, ) @@ -86,11 +143,11 @@ define-function: ( move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']" assert-css: ( "//*[@class='result-name']//*[text()='test_docs::']", - {"color": |path_color|}, + {"color": |hover_path_color|}, ) assert-css: ( "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": |path_color|, "background-color": |hover_background|}, + {"color": |hover_path_color|, "background-color": |hover_background|}, ) } ) @@ -107,79 +164,20 @@ call-function: ("check-search-color", { "desc_color": "#c5c5c5", "path_color": "#0096cf", "bottom_border_color": "#aaa3", + "keyword_color": "#39afd7", + "struct_color": "#ffa0a5", + "associatedtype_color": "#39afd7", + "tymethod_color": "#fdd687", + "method_color": "#fdd687", + "structfield_color": "#0096cf", + "structfield_hover_color": "#fff", + "macro_color": "#a37acc", + "fn_color": "#fdd687", + "hover_path_color": "#fff", + "hover_background": "#3c3c3c", + "grey": "#999", }) -store-value: (entry_color, "#0096cf") // color of the search entry -store-value: (hover_entry_color, "#fff") // color of the hovered/focused search entry -store-value: (background_color, "transparent") // background color -store-value: (hover_background_color, "#3c3c3c") // hover background color -store-value: (grey, "#999") - -call-function: ( - "check-result-color", { - "result_kind": "keyword", - "color": "#39afd7", - "hover_color": "#39afd7", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "struct", - "color": "#ffa0a5", - "hover_color": "#ffa0a5", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "associatedtype", - "color": "#39afd7", - "hover_color": "#39afd7", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "tymethod", - "color": "#fdd687", - "hover_color": "#fdd687", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "method", - "color": "#fdd687", - "hover_color": "#fdd687", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "structfield", - "color": "#0096cf", - "hover_color": "#fff", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "macro", - "color": "#a37acc", - "hover_color": "#a37acc", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "fn", - "color": "#fdd687", - "hover_color": "#fdd687", - }, -) - -call-function: ( - "check-container-color", { - "link_color": "#0096cf", - "path_color": "#fff", - "hover_background": "#3c3c3c", - }, -) - // Dark theme call-function: ("check-search-color", { "theme": "dark", @@ -187,79 +185,20 @@ call-function: ("check-search-color", { "desc_color": "#ddd", "path_color": "#ddd", "bottom_border_color": "#aaa3", + "keyword_color": "#d2991d", + "struct_color": "#2dbfb8", + "associatedtype_color": "#d2991d", + "tymethod_color": "#2bab63", + "method_color": "#2bab63", + "structfield_color": "#ddd", + "structfield_hover_color": "#ddd", + "macro_color": "#09bd00", + "fn_color": "#2bab63", + "hover_path_color": "#ddd", + "hover_background": "#616161", + "grey": "#ccc", }) -store-value: (entry_color, "#ddd") // color of the search entry -store-value: (hover_entry_color, "#ddd") // color of the hovered/focused search entry -store-value: (background_color, "transparent") // background color -store-value: (hover_background_color, "#616161") // hover background color -store-value: (grey, "#ccc") - -call-function: ( - "check-result-color", { - "result_kind": "keyword", - "color": "#d2991d", - "hover_color": "#d2991d", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "struct", - "color": "#2dbfb8", - "hover_color": "#2dbfb8", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "associatedtype", - "color": "#d2991d", - "hover_color": "#d2991d", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "tymethod", - "color": "#2bab63", - "hover_color": "#2bab63", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "method", - "color": "#2bab63", - "hover_color": "#2bab63", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "structfield", - "color": "#ddd", - "hover_color": "#ddd", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "macro", - "color": "#09bd00", - "hover_color": "#09bd00", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "fn", - "color": "#2bab63", - "hover_color": "#2bab63", - }, -) - -call-function: ( - "check-container-color", { - "link_color": "#ddd", - "path_color": "#ddd", - "hover_background": "#616161", - }, -) - // Light theme call-function: ("check-search-color", { "theme": "light", @@ -267,79 +206,20 @@ call-function: ("check-search-color", { "desc_color": "#000", "path_color": "#000", "bottom_border_color": "#aaa3", + "keyword_color": "#3873ad", + "struct_color": "#ad378a", + "associatedtype_color": "#3873ad", + "tymethod_color": "#ad7c37", + "method_color": "#ad7c37", + "structfield_color": "#000", + "structfield_hover_color": "#000", + "macro_color": "#068000", + "fn_color": "#ad7c37", + "hover_path_color": "#000", + "hover_background": "#ccc", + "grey": "#999", }) -store-value: (entry_color, "#000") // color of the search entry -store-value: (hover_entry_color, "#000") // color of the hovered/focused search entry -store-value: (background_color, "transparent") // background color -store-value: (hover_background_color, "#ccc") // hover background color -store-value: (grey, "#999") - -call-function: ( - "check-result-color", { - "result_kind": "keyword", - "color": "#3873ad", - "hover_color": "#3873ad", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "struct", - "color": "#ad378a", - "hover_color": "#ad378a", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "associatedtype", - "color": "#3873ad", - "hover_color": "#3873ad", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "tymethod", - "color": "#ad7c37", - "hover_color": "#ad7c37", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "method", - "color": "#ad7c37", - "hover_color": "#ad7c37", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "structfield", - "color": "#000", - "hover_color": "#000", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "macro", - "color": "#068000", - "hover_color": "#068000", - }, -) -call-function: ( - "check-result-color", { - "result_kind": "fn", - "color": "#ad7c37", - "hover_color": "#ad7c37", - }, -) - -call-function: ( - "check-container-color", { - "link_color": "#000", - "path_color": "#000", - "hover_background": "#ccc", - }, -) - // Check the alias. go-to: "file://" + |DOC_PATH| + "/test_docs/index.html" // If the text isn't displayed, the browser doesn't compute color style correctly... From 53c5a69dfdd01145fc508053adadbed17d2b315c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 15:52:00 +0200 Subject: [PATCH 28/28] Move `check-container-color`'s code into `check-search-color` function --- tests/rustdoc-gui/search-result-color.goml | 50 ++++++++-------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/tests/rustdoc-gui/search-result-color.goml b/tests/rustdoc-gui/search-result-color.goml index 5a31afc6e6104..fd0b86af3ea6b 100644 --- a/tests/rustdoc-gui/search-result-color.goml +++ b/tests/rustdoc-gui/search-result-color.goml @@ -81,11 +81,25 @@ define-function: ( "hover_color": |fn_color|, }) - call-function: ("check-container-color", { - "path_color": |path_color|, - "hover_path_color": |hover_path_color|, - "hover_background": |hover_background|, - }) + // Checking the `` container. + move-cursor-to: ".search-input" + focus: ".search-input" // To ensure the `` container isn't focused or hovered. + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", + {"color": |path_color|, "background-color": "transparent"}, + ALL, + ) + + // Checking color and background on hover. + move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']" + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']", + {"color": |hover_path_color|}, + ) + assert-css: ( + "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", + {"color": |hover_path_color|, "background-color": |hover_background|}, + ) } ) @@ -126,32 +140,6 @@ define-function: ( }, ) -define-function: ( - "check-container-color", - [path_color, hover_path_color, hover_background], - block { - // Checking the `` container. - move-cursor-to: ".search-input" - focus: ".search-input" // To ensure the `` container isn't focused or hovered. - assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": |path_color|, "background-color": "transparent"}, - ALL, - ) - - // Checking color and background on hover. - move-cursor-to: "//*[@class='desc'][text()='Just a normal struct.']" - assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']", - {"color": |hover_path_color|}, - ) - assert-css: ( - "//*[@class='result-name']//*[text()='test_docs::']/ancestor::a", - {"color": |hover_path_color|, "background-color": |hover_background|}, - ) - } -) - go-to: "file://" + |DOC_PATH| + "/test_docs/index.html?search=coo" // This is needed so that the text color is computed.