From 6e05d61e388b799cd97a4751652816891029fc78 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Fri, 27 Jun 2025 23:42:58 +0300 Subject: [PATCH 01/35] Synced rust-analyzer configs --- src/tools/miri/etc/rust_analyzer_helix.toml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/etc/rust_analyzer_helix.toml b/src/tools/miri/etc/rust_analyzer_helix.toml index 62db463a1918c..9bfb09120d8ac 100644 --- a/src/tools/miri/etc/rust_analyzer_helix.toml +++ b/src/tools/miri/etc/rust_analyzer_helix.toml @@ -9,23 +9,21 @@ linkedProjects = [ ] [language-server.rust-analyzer.config.check] -invocationLocation = "root" invocationStrategy = "once" overrideCommand = [ - "env", - "MIRI_AUTO_OPS=no", "./miri", "clippy", # make this `check` when working with a locally built rustc "--message-format=json", ] +[language-server.rust-analyzer.config.cargo.extraEnv] +MIRI_AUTO_OPS = "no" +MIRI_IN_RA = "1" + # Contrary to what the name suggests, this also affects proc macros. -[language-server.rust-analyzer.config.buildScripts] -invocationLocation = "root" +[language-server.rust-analyzer.config.cargo.buildScripts] invocationStrategy = "once" overrideCommand = [ - "env", - "MIRI_AUTO_OPS=no", "./miri", "check", "--message-format=json", From 75e22d5d2fcf6482705c20649f3c77273d42276a Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Sun, 29 Jun 2025 04:58:58 +0000 Subject: [PATCH 02/35] 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 b2ab17fd76ad0..c9462a0cd4a35 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -d41e12f1f4e4884c356f319b881921aa37040de5 +8141c2265f5f2b26d89abe8df5fa67286f2425d4 From 7be1bf7dfe5fb8e846b7776c815e6daea9419e62 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 12:11:19 +0200 Subject: [PATCH 03/35] make ./miri work on stable again --- .../miri/.github/workflows/setup/action.yml | 4 --- src/tools/miri/miri | 28 +++++++++++-------- src/tools/miri/miri-script/src/commands.rs | 16 +++++++---- src/tools/miri/miri.bat | 4 +-- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml index 146b432171e1d..f374897bc783c 100644 --- a/src/tools/miri/.github/workflows/setup/action.yml +++ b/src/tools/miri/.github/workflows/setup/action.yml @@ -39,10 +39,6 @@ runs: run: cargo install -f rustup-toolchain-install-master hyperfine shell: bash - - name: Install nightly toolchain - run: rustup toolchain install nightly --profile minimal - shell: bash - - name: Install "master" toolchain run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then diff --git a/src/tools/miri/miri b/src/tools/miri/miri index b1b146d79905b..549998ae44a31 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -2,20 +2,24 @@ set -e # We want to call the binary directly, so we need to know where it ends up. ROOT_DIR="$(dirname "$0")" -MIRI_SCRIPT_TARGET_DIR="$ROOT_DIR"/miri-script/target -TOOLCHAIN="+nightly" +TARGET_DIR="$ROOT_DIR"/miri-script/target +# Prepare cargo invocation. +# We have to overwrite the toolchain since `+miri` might be activated and not installed. +TOOLCHAIN="+stable" +CARGO_FLAGS=("-q") # If we are being invoked for RA, use JSON output and the default toolchain (to make proc-macros # work in RA). This needs a different target dir to avoid mixing up the builds. +# Also set `-Zroot-dir` so that RA can identify where the error occurred. if [ -n "$MIRI_IN_RA" ]; then - MESSAGE_FORMAT="--message-format=json" TOOLCHAIN="" - MIRI_SCRIPT_TARGET_DIR="$MIRI_SCRIPT_TARGET_DIR"/ra + CARGO_FLAGS+=("--message-format=json" "-Zroot-dir=$ROOT_DIR") + TARGET_DIR="$ROOT_DIR"/target fi -# We need a nightly toolchain, for `-Zroot-dir`. -cargo $TOOLCHAIN build $CARGO_EXTRA_FLAGS --manifest-path "$ROOT_DIR"/miri-script/Cargo.toml \ - -Zroot-dir="$ROOT_DIR" \ - -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" $MESSAGE_FORMAT || \ - ( echo "Failed to build miri-script. Is the 'nightly' toolchain installed?"; exit 1 ) -# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through -# rustup (that sets it's own environmental variables), which is undesirable. -"$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" +# Run cargo. +cargo $TOOLCHAIN build --manifest-path "$ROOT_DIR"/miri-script/Cargo.toml \ + --target-dir "$TARGET_DIR" "${CARGO_FLAGS[@]}" || \ + ( echo "Failed to build miri-script. Is the 'stable' toolchain installed?"; exit 1 ) +# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. +# Invoking `cargo run` goes through rustup (that sets it's own environmental variables), which is +# undesirable. +"$TARGET_DIR"/debug/miri-script "$@" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index d18e9c7791fc2..a3778215019f7 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -707,9 +707,12 @@ impl Command { let mut early_flags = Vec::::new(); // In `dep` mode, the target is already passed via `MIRI_TEST_TARGET` - if !dep && let Some(target) = &target { - early_flags.push("--target".into()); - early_flags.push(target.into()); + #[expect(clippy::collapsible_if)] // we need to wait until this is stable + if !dep { + if let Some(target) = &target { + early_flags.push("--target".into()); + early_flags.push(target.into()); + } } early_flags.push("--edition".into()); early_flags.push(edition.as_deref().unwrap_or("2021").into()); @@ -737,8 +740,11 @@ impl Command { // Add Miri flags let mut cmd = cmd.args(&miri_flags).args(&early_flags).args(&flags); // For `--dep` we also need to set the target in the env var. - if dep && let Some(target) = &target { - cmd = cmd.env("MIRI_TEST_TARGET", target); + #[expect(clippy::collapsible_if)] // we need to wait until this is stable + if dep { + if let Some(target) = &target { + cmd = cmd.env("MIRI_TEST_TARGET", target); + } } // Finally, run the thing. Ok(cmd.run()?) diff --git a/src/tools/miri/miri.bat b/src/tools/miri/miri.bat index b046b6310ddad..6f9a8f38d6559 100644 --- a/src/tools/miri/miri.bat +++ b/src/tools/miri/miri.bat @@ -5,8 +5,8 @@ set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target :: If any other steps are added, the "|| exit /b" must be appended to early :: return from the script. If not, it will continue execution. -cargo +nightly build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ - || (echo Failed to build miri-script. Is the 'nightly' toolchain installed? & exit /b) +cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ + || (echo Failed to build miri-script. Is the 'stable' toolchain installed? & exit /b) :: Forwards all arguments to this file to the executable. :: We invoke the binary directly to avoid going through rustup, which would set some extra From ea690818bdd904d94f40375ab5e6bd996e008ba0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 10:43:15 +0200 Subject: [PATCH 04/35] also test on arm-64 linux hosts --- src/tools/miri/.github/workflows/ci.yml | 48 +++++++++++++------ .../miri/.github/workflows/setup/action.yml | 6 ++- src/tools/miri/ci/ci.sh | 12 +++-- 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 9dbf51e9796a7..50a7cff0fa715 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -13,35 +13,55 @@ defaults: shell: bash jobs: - build: + test: + name: build and test on ${{ matrix.host_target }} strategy: fail-fast: false matrix: include: - - os: ubuntu-latest - host_target: x86_64-unknown-linux-gnu - - os: macos-14 - host_target: aarch64-apple-darwin - - os: windows-latest - host_target: i686-pc-windows-msvc + - host_target: x86_64-unknown-linux-gnu + os: ubuntu-latest + # Needs a libffi patch: + # - host_target: i686-unknown-linux-gnu + # os: ubuntu-latest + - host_target: aarch64-unknown-linux-gnu + os: ubuntu-24.04-arm + # Disabled due to . + # - host_target: armv7-unknown-linux-gnueabihf + # os: ubuntu-24.04-arm + - host_target: aarch64-apple-darwin + os: macos-latest + - host_target: i686-pc-windows-msvc + os: windows-latest runs-on: ${{ matrix.os }} env: HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v4 + - name: Install multilib dependencies + if: ${{ matrix.host_target == 'i686-unknown-linux-gnu' }} + run: | + sudo dpkg --add-architecture i386 + sudo apt update + sudo apt install gcc-multilib zlib1g-dev:i386 libffi-dev:i386 + - name: Install multilib dependencies + if: ${{ matrix.host_target == 'armv7-unknown-linux-gnueabihf' }} + run: | + sudo dpkg --add-architecture armhf + sudo apt update + sudo apt install gcc-arm-linux-gnueabihf zlib1g-dev:armhf libffi-dev:armhf - uses: ./.github/workflows/setup with: toolchain_flags: "--host ${{ matrix.host_target }}" - # The `style` job only runs on Linux; this makes sure the Windows-host-specific + - name: Test Miri + run: ./ci/ci.sh + + # The `style` job only runs on Linux; this makes sure the host-specific # code is also covered by clippy. - name: Check clippy - if: ${{ matrix.os == 'windows-latest' }} run: ./miri clippy -- -D warnings - - name: Test Miri - run: ./ci/ci.sh - style: name: style checks runs-on: ubuntu-latest @@ -73,7 +93,7 @@ jobs: # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! # And they should be added below in `cron-fail-notify` as well. conclusion: - needs: [build, style, coverage] + needs: [test, style, coverage] # We need to ensure this job does *not* get skipped if its dependencies fail, # because a skipped job is considered a success by GitHub. So we have to # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run @@ -135,7 +155,7 @@ jobs: cron-fail-notify: name: cronjob failure notification runs-on: ubuntu-latest - needs: [build, style, coverage] + needs: [test, style, coverage] if: ${{ github.event_name == 'schedule' && failure() }} steps: # Send a Zulip notification diff --git a/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml index 146b432171e1d..ad86e8ab5d652 100644 --- a/src/tools/miri/.github/workflows/setup/action.yml +++ b/src/tools/miri/.github/workflows/setup/action.yml @@ -2,6 +2,7 @@ name: "Miri CI setup" description: "Sets up Miri CI" inputs: toolchain_flags: + description: extra flags to pass to rustup-toolchain-install-master required: false default: '' runs: @@ -31,8 +32,9 @@ runs: ~/.cargo/bin ~/.cargo/.crates.toml ~/.cargo/.crates2.json - key: cargo-${{ runner.os }}-${{ hashFiles('**/Cargo.lock', '.github/workflows/**/*.yml') }} - restore-keys: cargo-${{ runner.os }} + # Bump the version when something here changes that needs a cache reset. + key: cargo-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('**/Cargo.lock') }}-v1 + restore-keys: cargo-${{ runner.os }}-${{ runner.arch }} - name: Install rustup-toolchain-install-master if: steps.cache.outputs.cache-hit != 'true' diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 8941af681a478..35a1cb4d6506a 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -143,11 +143,16 @@ case $HOST_TARGET in GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 MANY_SEEDS=64 TEST_TARGET=i686-unknown-linux-gnu run_tests - MANY_SEEDS=64 TEST_TARGET=aarch64-unknown-linux-gnu run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-apple-darwin run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-pc-windows-gnu run_tests + ;; + aarch64-unknown-linux-gnu) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 candidate MANY_SEEDS=64 TEST_TARGET=aarch64-pc-windows-msvc run_tests + # Custom target JSON file + TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std ;; aarch64-apple-darwin) # Host @@ -172,13 +177,10 @@ case $HOST_TARGET in TEST_TARGET=wasm32-wasip2 run_tests_minimal $BASIC wasm TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std - # Custom target JSON file - TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std ;; i686-pc-windows-msvc) # Host - # Without GC_STRESS and with reduced many-seeds count as this is the slowest runner. - # (The macOS runner checks windows-msvc with full many-seeds count.) + # Without GC_STRESS as this is the slowest runner. 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, From d539e66d90008f7c95e4448361d3c6ba48a62987 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 21:29:57 +0200 Subject: [PATCH 05/35] 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 c9462a0cd4a35..3869fd3912098 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -8141c2265f5f2b26d89abe8df5fa67286f2425d4 +ed2d759783dc9de134bbb3f01085b1e6dbf539f3 From ab2443cbad0d0e2b40f9c862756e4233a17174bb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 14:47:44 +0200 Subject: [PATCH 06/35] fix type mismatches in native-lib/scalar_arguments test --- src/tools/miri/tests/native-lib/scalar_arguments.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/tests/native-lib/scalar_arguments.c b/src/tools/miri/tests/native-lib/scalar_arguments.c index acccf06f3df2b..8cf38f74413c9 100644 --- a/src/tools/miri/tests/native-lib/scalar_arguments.c +++ b/src/tools/miri/tests/native-lib/scalar_arguments.c @@ -22,11 +22,11 @@ EXPORT uint32_t get_unsigned_int(void) { return -10; } -EXPORT short add_int16(int16_t x) { +EXPORT int16_t add_int16(int16_t x) { return x + 3; } -EXPORT long add_short_to_long(int16_t x, int64_t y) { +EXPORT int64_t add_short_to_long(int16_t x, int64_t y) { return x + y; } From 6aaaff88638573e7bb512484d3749c80fd50e8b0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 14:51:47 +0200 Subject: [PATCH 07/35] test on x86-32 and arm-32 --- src/tools/miri/.github/workflows/ci.yml | 41 +++++++++++-------- .../miri/.github/workflows/setup/action.yml | 2 +- src/tools/miri/ci/ci.sh | 23 +++++++---- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 50a7cff0fa715..adcaeb53beba0 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -14,21 +14,23 @@ defaults: jobs: test: - name: build and test on ${{ matrix.host_target }} + name: test (${{ matrix.host_target }}) strategy: fail-fast: false matrix: include: - host_target: x86_64-unknown-linux-gnu os: ubuntu-latest - # Needs a libffi patch: - # - host_target: i686-unknown-linux-gnu - # os: ubuntu-latest + - host_target: i686-unknown-linux-gnu + os: ubuntu-latest + multiarch: i386 + gcc_cross: i686-linux-gnu - host_target: aarch64-unknown-linux-gnu os: ubuntu-24.04-arm - # Disabled due to . - # - host_target: armv7-unknown-linux-gnueabihf - # os: ubuntu-24.04-arm + - host_target: armv7-unknown-linux-gnueabihf + os: ubuntu-24.04-arm + multiarch: armhf + gcc_cross: arm-linux-gnueabihf - host_target: aarch64-apple-darwin os: macos-latest - host_target: i686-pc-windows-msvc @@ -38,22 +40,27 @@ jobs: HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v4 - - name: Install multilib dependencies - if: ${{ matrix.host_target == 'i686-unknown-linux-gnu' }} - run: | - sudo dpkg --add-architecture i386 - sudo apt update - sudo apt install gcc-multilib zlib1g-dev:i386 libffi-dev:i386 - - name: Install multilib dependencies - if: ${{ matrix.host_target == 'armv7-unknown-linux-gnueabihf' }} + - name: install multiarch + if: ${{ matrix.multiarch != '' }} run: | - sudo dpkg --add-architecture armhf + sudo dpkg --add-architecture ${{ matrix.multiarch }} sudo apt update - sudo apt install gcc-arm-linux-gnueabihf zlib1g-dev:armhf libffi-dev:armhf + sudo apt install $(echo "libatomic1: zlib1g-dev:" | sed 's/:/:${{ matrix.multiarch }}/g') - uses: ./.github/workflows/setup with: toolchain_flags: "--host ${{ matrix.host_target }}" + # We set up the cross-compiler *after* the basic setup as setting CC would otherwise + # cause confusion. + - name: install gcc-cross + if: ${{ matrix.gcc_cross != '' }} + run: | + sudo apt install gcc-${{ matrix.gcc_cross }} + echo "Setting environment variables:" + echo "CC=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV + TARGET_UPPERCASE=$(echo ${{ matrix.host_target }} | tr '[:lower:]-' '[:upper:]_') + echo "CARGO_TARGET_${TARGET_UPPERCASE}_LINKER=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV + - name: Test Miri run: ./ci/ci.sh diff --git a/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml index 582f8450cd9cb..9110e6947f490 100644 --- a/src/tools/miri/.github/workflows/setup/action.yml +++ b/src/tools/miri/.github/workflows/setup/action.yml @@ -36,7 +36,7 @@ runs: key: cargo-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('**/Cargo.lock') }}-v1 restore-keys: cargo-${{ runner.os }}-${{ runner.arch }} - - name: Install rustup-toolchain-install-master + - name: Install the tools we need if: steps.cache.outputs.cache-hit != 'true' run: cargo install -f rustup-toolchain-install-master hyperfine shell: bash diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 35a1cb4d6506a..67c18d3b4019f 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -146,6 +146,18 @@ case $HOST_TARGET in MANY_SEEDS=64 TEST_TARGET=x86_64-apple-darwin run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-pc-windows-gnu run_tests ;; + i686-unknown-linux-gnu) + # Host + # Without GC_STRESS as this is a slow runner. + MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests + # Partially supported targets (tier 2) + BASIC="empty_main integer heap_alloc libc-mem vec string btreemap" # ensures we have the basics: pre-main code, system allocator + UNIX="hello panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there + TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX time hashmap random thread sync concurrency epoll eventfd + TEST_TARGET=wasm32-wasip2 run_tests_minimal $BASIC wasm + TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std + TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std + ;; aarch64-unknown-linux-gnu) # Host GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests @@ -154,6 +166,10 @@ case $HOST_TARGET in # Custom target JSON file TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std ;; + armv7-unknown-linux-gnueabihf) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests + ;; aarch64-apple-darwin) # Host GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests @@ -170,13 +186,6 @@ case $HOST_TARGET in MANY_SEEDS=16 TEST_TARGET=x86_64-pc-solaris run_tests MANY_SEEDS=16 TEST_TARGET=x86_64-unknown-freebsd run_tests MANY_SEEDS=16 TEST_TARGET=i686-unknown-freebsd run_tests - # Partially supported targets (tier 2) - BASIC="empty_main integer heap_alloc libc-mem vec string btreemap" # ensures we have the basics: pre-main code, system allocator - UNIX="hello panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there - TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX time hashmap random thread sync concurrency epoll eventfd - TEST_TARGET=wasm32-wasip2 run_tests_minimal $BASIC wasm - TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std - TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std ;; i686-pc-windows-msvc) # Host From e788b3c79ec839593d37d770709829efedc500ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 16:39:29 +0200 Subject: [PATCH 08/35] also test on s390x via qemu --- src/tools/miri/.github/workflows/ci.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index adcaeb53beba0..d494ba43b85f2 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -31,6 +31,11 @@ jobs: os: ubuntu-24.04-arm multiarch: armhf gcc_cross: arm-linux-gnueabihf + - host_target: s390x-unknown-linux-gnu + os: ubuntu-latest + multiarch: s390x + gcc_cross: s390x-linux-gnu + qemu: true - host_target: aarch64-apple-darwin os: macos-latest - host_target: i686-pc-windows-msvc @@ -40,11 +45,18 @@ jobs: HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v4 + - name: install qemu + if: ${{ matrix.qemu }} + run: sudo apt install qemu-user qemu-user-binfmt - name: install multiarch if: ${{ matrix.multiarch != '' }} run: | + # s390x, ppc64el need Ubuntu Ports to be in the mirror list + sudo bash -c "echo 'https://ports.ubuntu.com/ priority:4' >> /etc/apt/apt-mirrors.txt" + # Add architecture sudo dpkg --add-architecture ${{ matrix.multiarch }} sudo apt update + # Install needed packages sudo apt install $(echo "libatomic1: zlib1g-dev:" | sed 's/:/:${{ matrix.multiarch }}/g') - uses: ./.github/workflows/setup with: @@ -61,7 +73,10 @@ jobs: TARGET_UPPERCASE=$(echo ${{ matrix.host_target }} | tr '[:lower:]-' '[:upper:]_') echo "CARGO_TARGET_${TARGET_UPPERCASE}_LINKER=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV + # The main test job! We don't run this in qemu as that is quite slow, + # so those targets only get the clippy check below. - name: Test Miri + if: ${{ !matrix.qemu }} run: ./ci/ci.sh # The `style` job only runs on Linux; this makes sure the host-specific From d5590734c9b72f7fb2296c3043f526794e1db6f3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 16:49:58 +0200 Subject: [PATCH 09/35] remove duplicate clippy check --- src/tools/miri/.github/workflows/ci.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index d494ba43b85f2..151d170defc01 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -75,13 +75,13 @@ jobs: # The main test job! We don't run this in qemu as that is quite slow, # so those targets only get the clippy check below. - - name: Test Miri + - name: test Miri if: ${{ !matrix.qemu }} run: ./ci/ci.sh # The `style` job only runs on Linux; this makes sure the host-specific # code is also covered by clippy. - - name: Check clippy + - name: clippy run: ./miri clippy -- -D warnings style: @@ -93,8 +93,6 @@ jobs: - name: rustfmt 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) From ec85b705453a015cbd560fa741c01fcf94045f15 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 29 Jun 2025 16:52:50 +0200 Subject: [PATCH 10/35] add a riscv64 test job --- src/tools/miri/.github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 151d170defc01..ed4bcc0dd3b93 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -31,6 +31,11 @@ jobs: os: ubuntu-24.04-arm multiarch: armhf gcc_cross: arm-linux-gnueabihf + - host_target: riscv64gc-unknown-linux-gnu + os: ubuntu-latest + multiarch: riscv64 + gcc_cross: riscv64-linux-gnu + qemu: true - host_target: s390x-unknown-linux-gnu os: ubuntu-latest multiarch: s390x From d69b8e2b48cd5e3ac4f8ee7af995241ea7c3a010 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 13 Jun 2025 00:08:58 +0000 Subject: [PATCH 11/35] Add shims for `gettid`-esque functions Various platforms provide a function to return the current OS thread ID, but they all use a slightly different name. Add shims for these functions for Apple, FreeBSD, and Windows, with tests to account for those and a few more platforms that are not yet supported by Miri. The syscall and extern symbol is included as well on Linux. These should be useful in general but will also help support printing the OS thread ID in panic messages [1]. [1]: https://github.com/rust-lang/rust/pull/115746 Squashed commit from Ralf: try_from_scalar: extend comment --- src/tools/miri/src/helpers.rs | 1 - src/tools/miri/src/shims/env.rs | 22 +++ src/tools/miri/src/shims/extern_static.rs | 2 +- src/tools/miri/src/shims/unix/env.rs | 49 ++++- .../src/shims/unix/freebsd/foreign_items.rs | 5 + .../src/shims/unix/linux/foreign_items.rs | 4 +- .../miri/src/shims/unix/linux_like/syscall.rs | 6 + .../src/shims/unix/macos/foreign_items.rs | 5 + .../miri/src/shims/windows/foreign_items.rs | 17 ++ src/tools/miri/src/shims/windows/handle.rs | 4 + .../concurrency/windows_thread_invalid.rs | 9 + .../concurrency/windows_thread_invalid.stderr | 13 ++ src/tools/miri/tests/pass-dep/shims/gettid.rs | 183 ++++++++++++++++++ .../miri/tests/pass/alloc-access-tracking.rs | 4 +- 14 files changed, 312 insertions(+), 12 deletions(-) create mode 100644 src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs create mode 100644 src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr create mode 100644 src/tools/miri/tests/pass-dep/shims/gettid.rs diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index fb34600fa37dd..4821edb0942a0 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1337,7 +1337,6 @@ where /// Check that the number of varargs is at least the minimum what we expect. /// Fixed args should not be included. -/// Use `check_vararg_fixed_arg_count` to extract the varargs slice from full function arguments. pub fn check_min_vararg_count<'a, 'tcx, const N: usize>( name: &'a str, args: &'a [OpTy<'tcx>], diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs index e99a8fd6e8c09..9dfb1ebb90a82 100644 --- a/src/tools/miri/src/shims/env.rs +++ b/src/tools/miri/src/shims/env.rs @@ -110,8 +110,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } + /// Get the process identifier. fn get_pid(&self) -> u32 { let this = self.eval_context_ref(); if this.machine.communicate() { std::process::id() } else { 1000 } } + + /// Get an "OS" thread ID for the current thread. + fn get_current_tid(&self) -> u32 { + let this = self.eval_context_ref(); + self.get_tid(this.machine.threads.active_thread()) + } + + /// Get an "OS" thread ID for any thread. + fn get_tid(&self, thread: ThreadId) -> u32 { + let this = self.eval_context_ref(); + let index = thread.to_u32(); + let target_os = &this.tcx.sess.target.os; + if target_os == "linux" || target_os == "netbsd" { + // On Linux, the main thread has PID == TID so we uphold this. NetBSD also appears + // to exhibit the same behavior, though I can't find a citation. + this.get_pid().strict_add(index) + } else { + // Other platforms do not display any relationship between PID and TID. + index + } + } } diff --git a/src/tools/miri/src/shims/extern_static.rs b/src/tools/miri/src/shims/extern_static.rs index a2ea3dbd88b11..5cf9265c7040f 100644 --- a/src/tools/miri/src/shims/extern_static.rs +++ b/src/tools/miri/src/shims/extern_static.rs @@ -66,7 +66,7 @@ impl<'tcx> MiriMachine<'tcx> { ecx, &["__cxa_thread_atexit_impl", "__clock_gettime64"], )?; - Self::weak_symbol_extern_statics(ecx, &["getrandom", "statx"])?; + Self::weak_symbol_extern_statics(ecx, &["getrandom", "gettid", "statx"])?; } "freebsd" => { Self::null_ptr_extern_statics(ecx, &["__cxa_thread_atexit_impl"])?; diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index 604fb0974d29a..a0e5d3f0127ea 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -274,15 +274,52 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(Scalar::from_u32(this.get_pid())) } - fn linux_gettid(&mut self) -> InterpResult<'tcx, Scalar> { + /// The `gettid`-like function for Unix platforms that take no parameters and return a 32-bit + /// integer. It is not always named "gettid". + fn unix_gettid(&mut self, link_name: &str) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_ref(); - this.assert_target_os("linux", "gettid"); + this.assert_target_os_is_unix(link_name); - let index = this.machine.threads.active_thread().to_u32(); + // For most platforms the return type is an `i32`, but some are unsigned. The TID + // will always be positive so we don't need to differentiate. + interp_ok(Scalar::from_u32(this.get_current_tid())) + } + + /// The Apple-specific `int pthread_threadid_np(pthread_t thread, uint64_t *thread_id)`, which + /// allows querying the ID for arbitrary threads, identified by their pthread_t. + /// + /// API documentation: . + fn apple_pthread_threadip_np( + &mut self, + thread_op: &OpTy<'tcx>, + tid_op: &OpTy<'tcx>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + this.assert_target_os("macos", "pthread_threadip_np"); + + let tid_dest = this.read_pointer(tid_op)?; + if this.ptr_is_null(tid_dest)? { + // If NULL is passed, an error is immediately returned + return interp_ok(this.eval_libc("EINVAL")); + } + + let thread = this.read_scalar(thread_op)?.to_int(this.libc_ty_layout("pthread_t").size)?; + let thread = if thread == 0 { + // Null thread ID indicates that we are querying the active thread. + this.machine.threads.active_thread() + } else { + // Our pthread_t is just the raw ThreadId. + let Ok(thread) = this.thread_id_try_from(thread) else { + return interp_ok(this.eval_libc("ESRCH")); + }; + thread + }; - // Compute a TID for this thread, ensuring that the main thread has PID == TID. - let tid = this.get_pid().strict_add(index); + let tid = this.get_tid(thread); + let tid_dest = this.deref_pointer_as(tid_op, this.machine.layouts.u64)?; + this.write_int(tid, &tid_dest)?; - interp_ok(Scalar::from_u32(tid)) + // Possible errors have been handled, return success. + interp_ok(Scalar::from_u32(0)) } } 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 42502d5bf09af..33564a2f84c78 100644 --- a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs @@ -56,6 +56,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; this.write_scalar(res, dest)?; } + "pthread_getthreadid_np" => { + let [] = this.check_shim(abi, CanonAbi::C, link_name, args)?; + let result = this.unix_gettid(link_name.as_str())?; + this.write_scalar(result, dest)?; + } "cpuset_getaffinity" => { // The "same" kind of api as `sched_getaffinity` but more fine grained control for FreeBSD specifically. 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 aeaff1cb13a53..b3e99e6cc68f7 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -18,7 +18,7 @@ use crate::*; const TASK_COMM_LEN: u64 = 16; pub fn is_dyn_sym(name: &str) -> bool { - matches!(name, "statx") + matches!(name, "gettid" | "statx") } impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -117,7 +117,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } "gettid" => { let [] = this.check_shim(abi, CanonAbi::C, link_name, args)?; - let result = this.linux_gettid()?; + let result = this.unix_gettid(link_name.as_str())?; this.write_scalar(result, dest)?; } diff --git a/src/tools/miri/src/shims/unix/linux_like/syscall.rs b/src/tools/miri/src/shims/unix/linux_like/syscall.rs index d42d6b9073ecf..d3534e6e1bc6e 100644 --- a/src/tools/miri/src/shims/unix/linux_like/syscall.rs +++ b/src/tools/miri/src/shims/unix/linux_like/syscall.rs @@ -4,6 +4,7 @@ use rustc_span::Symbol; use rustc_target::callconv::FnAbi; use crate::helpers::check_min_vararg_count; +use crate::shims::unix::env::EvalContextExt; use crate::shims::unix::linux_like::eventfd::EvalContextExt as _; use crate::shims::unix::linux_like::sync::futex; use crate::*; @@ -24,6 +25,7 @@ pub fn syscall<'tcx>( let sys_getrandom = ecx.eval_libc("SYS_getrandom").to_target_usize(ecx)?; let sys_futex = ecx.eval_libc("SYS_futex").to_target_usize(ecx)?; let sys_eventfd2 = ecx.eval_libc("SYS_eventfd2").to_target_usize(ecx)?; + let sys_gettid = ecx.eval_libc("SYS_gettid").to_target_usize(ecx)?; match ecx.read_target_usize(op)? { // `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)` @@ -53,6 +55,10 @@ pub fn syscall<'tcx>( let result = ecx.eventfd(initval, flags)?; ecx.write_int(result.to_i32()?, dest)?; } + num if num == sys_gettid => { + let result = ecx.unix_gettid("SYS_gettid")?; + ecx.write_int(result.to_u32()?, dest)?; + } num => { throw_unsup_format!("syscall: unsupported syscall number {num}"); } 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 ae921a013a40e..2330371809104 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -222,6 +222,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; this.write_scalar(res, dest)?; } + "pthread_threadid_np" => { + let [thread, tid_ptr] = this.check_shim(abi, CanonAbi::C, link_name, args)?; + let res = this.apple_pthread_threadip_np(thread, tid_ptr)?; + this.write_scalar(res, dest)?; + } // Synchronization primitives "os_sync_wait_on_address" => { diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index de10357f5faee..959abc0bacaa6 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -629,6 +629,23 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(name, &name_ptr)?; this.write_scalar(res, dest)?; } + "GetThreadId" => { + let [handle] = this.check_shim(abi, sys_conv, link_name, args)?; + let handle = this.read_handle(handle, "GetThreadId")?; + let thread = match handle { + Handle::Thread(thread) => thread, + Handle::Pseudo(PseudoHandle::CurrentThread) => this.active_thread(), + _ => this.invalid_handle("GetThreadDescription")?, + }; + let tid = this.get_tid(thread); + this.write_scalar(Scalar::from_u32(tid), dest)?; + } + "GetCurrentThreadId" => { + let [] = this.check_shim(abi, sys_conv, link_name, args)?; + let thread = this.active_thread(); + let tid = this.get_tid(thread); + this.write_scalar(Scalar::from_u32(tid), dest)?; + } // Miscellaneous "ExitProcess" => { diff --git a/src/tools/miri/src/shims/windows/handle.rs b/src/tools/miri/src/shims/windows/handle.rs index 1e30bf25ed92e..8a965ea316d72 100644 --- a/src/tools/miri/src/shims/windows/handle.rs +++ b/src/tools/miri/src/shims/windows/handle.rs @@ -166,6 +166,10 @@ impl Handle { /// Structurally invalid handles return [`HandleError::InvalidHandle`]. /// If the handle is structurally valid but semantically invalid, e.g. a for non-existent thread /// ID, returns [`HandleError::ThreadNotFound`]. + /// + /// This function is deliberately private; shims should always use `read_handle`. + /// That enforces handle validity even when Windows does not: for now, we argue invalid + /// handles are always a bug and programmers likely want to know about them. fn try_from_scalar<'tcx>( handle: Scalar, cx: &MiriInterpCx<'tcx>, diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs new file mode 100644 index 0000000000000..2e0729c9b4965 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs @@ -0,0 +1,9 @@ +//! Ensure we error if thread functions are called with invalid handles +//@only-target: windows # testing Windows API + +use windows_sys::Win32::System::Threading::GetThreadId; + +fn main() { + let _tid = unsafe { GetThreadId(std::ptr::dangling_mut()) }; + //~^ ERROR: invalid handle +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr new file mode 100644 index 0000000000000..8d4b049b7402e --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: invalid handle 1 passed to GetThreadId + --> tests/fail-dep/concurrency/windows_thread_invalid.rs:LL:CC + | +LL | let _tid = unsafe { GetThreadId(std::ptr::dangling_mut()) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ abnormal termination occurred here + | + = note: BACKTRACE: + = note: inside `main` at tests/fail-dep/concurrency/windows_thread_invalid.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass-dep/shims/gettid.rs b/src/tools/miri/tests/pass-dep/shims/gettid.rs new file mode 100644 index 0000000000000..b7a2fa49ef862 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/shims/gettid.rs @@ -0,0 +1,183 @@ +//! Test for `gettid` and similar functions for retrieving an OS thread ID. +//@ revisions: with_isolation without_isolation +//@ [without_isolation] compile-flags: -Zmiri-disable-isolation + +#![feature(linkage)] + +fn gettid() -> u64 { + cfg_if::cfg_if! { + if #[cfg(any(target_os = "android", target_os = "linux"))] { + gettid_linux_like() + } else if #[cfg(target_os = "nto")] { + unsafe { libc::gettid() as u64 } + } else if #[cfg(target_os = "openbsd")] { + unsafe { libc::getthrid() as u64 } + } else if #[cfg(target_os = "freebsd")] { + unsafe { libc::pthread_getthreadid_np() as u64 } + } else if #[cfg(target_os = "netbsd")] { + unsafe { libc::_lwp_self() as u64 } + } else if #[cfg(any(target_os = "solaris", target_os = "illumos"))] { + // On Solaris and Illumos, the `pthread_t` is the OS TID. + unsafe { libc::pthread_self() as u64 } + } else if #[cfg(target_vendor = "apple")] { + let mut id = 0u64; + let status: libc::c_int = unsafe { libc::pthread_threadid_np(0, &mut id) }; + assert_eq!(status, 0); + id + } else if #[cfg(windows)] { + use windows_sys::Win32::System::Threading::GetCurrentThreadId; + unsafe { GetCurrentThreadId() as u64 } + } else { + compile_error!("platform has no gettid") + } + } +} + +/// Test the libc function, the syscall, and the extern symbol. +#[cfg(any(target_os = "android", target_os = "linux"))] +fn gettid_linux_like() -> u64 { + unsafe extern "C" { + #[linkage = "extern_weak"] + static gettid: Option libc::pid_t>; + } + + let from_libc = unsafe { libc::gettid() as u64 }; + let from_sys = unsafe { libc::syscall(libc::SYS_gettid) as u64 }; + let from_static = unsafe { gettid.unwrap()() as u64 }; + + assert_eq!(from_libc, from_sys); + assert_eq!(from_libc, from_static); + + from_libc +} + +/// Specific platforms can query the tid of arbitrary threads from a `pthread_t` / `HANDLE` +#[cfg(any(target_vendor = "apple", windows))] +mod queried { + use std::ffi::c_void; + use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + use std::{ptr, thread}; + + use super::*; + + static SPAWNED_TID: AtomicU64 = AtomicU64::new(0); + static CAN_JOIN: AtomicBool = AtomicBool::new(false); + + /// Save this thread's TID, give the spawning thread a chance to query it separately before + /// being joined. + fn thread_body() { + SPAWNED_TID.store(gettid(), Ordering::Relaxed); + + // Spin until the main thread has a chance to read this thread's ID + while !CAN_JOIN.load(Ordering::Relaxed) { + thread::yield_now(); + } + } + + /// Spawn a thread, query then return its TID. + #[cfg(target_vendor = "apple")] + fn spawn_update_join() -> u64 { + extern "C" fn thread_start(_data: *mut c_void) -> *mut c_void { + thread_body(); + ptr::null_mut() + } + + let mut t: libc::pthread_t = 0; + let mut spawned_tid_from_handle = 0u64; + + unsafe { + let res = libc::pthread_create(&mut t, ptr::null(), thread_start, ptr::null_mut()); + assert_eq!(res, 0, "failed to spawn thread"); + + let res = libc::pthread_threadid_np(t, &mut spawned_tid_from_handle); + assert_eq!(res, 0, "failed to query thread ID"); + CAN_JOIN.store(true, Ordering::Relaxed); + + let res = libc::pthread_join(t, ptr::null_mut()); + assert_eq!(res, 0, "failed to join thread"); + + // Apple also has two documented return values for invalid threads and null pointers + let res = libc::pthread_threadid_np(libc::pthread_t::MAX, &mut 0); + assert_eq!(res, libc::ESRCH, "expected ESRCH for invalid TID"); + let res = libc::pthread_threadid_np(0, ptr::null_mut()); + assert_eq!(res, libc::EINVAL, "invalid EINVAL for a null pointer"); + } + + spawned_tid_from_handle + } + + /// Spawn a thread, query then return its TID. + #[cfg(windows)] + fn spawn_update_join() -> u64 { + use windows_sys::Win32::Foundation::WAIT_FAILED; + use windows_sys::Win32::System::Threading::{ + CreateThread, GetThreadId, INFINITE, WaitForSingleObject, + }; + + extern "system" fn thread_start(_data: *mut c_void) -> u32 { + thread_body(); + 0 + } + + let spawned_tid_from_handle; + let mut tid_at_spawn = 0u32; + + unsafe { + let handle = + CreateThread(ptr::null(), 0, Some(thread_start), ptr::null(), 0, &mut tid_at_spawn); + assert!(!handle.is_null(), "failed to spawn thread"); + + spawned_tid_from_handle = GetThreadId(handle); + assert_ne!(spawned_tid_from_handle, 0, "failed to query thread ID"); + CAN_JOIN.store(true, Ordering::Relaxed); + + let res = WaitForSingleObject(handle, INFINITE); + assert_ne!(res, WAIT_FAILED, "failed to join thread"); + } + + // Windows also indirectly returns the TID from `CreateThread`, ensure that matches up. + assert_eq!(spawned_tid_from_handle, tid_at_spawn); + + spawned_tid_from_handle.into() + } + + pub fn check() { + let spawned_tid_from_handle = spawn_update_join(); + let spawned_tid_from_self = SPAWNED_TID.load(Ordering::Relaxed); + let current_tid = gettid(); + + // Ensure that we got a different thread ID. + assert_ne!(spawned_tid_from_handle, current_tid); + + // Ensure that we get the same result from `gettid` and from querying a thread's handle + assert_eq!(spawned_tid_from_handle, spawned_tid_from_self); + } +} + +fn main() { + let tid = gettid(); + + std::thread::spawn(move || { + assert_ne!(gettid(), tid); + }); + + // Test that in isolation mode a deterministic value will be returned. + // The value is not important, we only care that whatever the value is, + // won't change from execution to execution. + if cfg!(with_isolation) { + if cfg!(target_os = "linux") { + // Linux starts the TID at the PID, which is 1000. + assert_eq!(tid, 1000); + } else { + // Other platforms start counting from 0. + assert_eq!(tid, 0); + } + } + + // On Linux and NetBSD, the first TID is the PID. + #[cfg(any(target_os = "linux", target_os = "netbsd"))] + assert_eq!(tid, unsafe { libc::getpid() } as u64); + + #[cfg(any(target_vendor = "apple", windows))] + queried::check(); +} diff --git a/src/tools/miri/tests/pass/alloc-access-tracking.rs b/src/tools/miri/tests/pass/alloc-access-tracking.rs index 9eba0ca171bc6..c47063bef03c7 100644 --- a/src/tools/miri/tests/pass/alloc-access-tracking.rs +++ b/src/tools/miri/tests/pass/alloc-access-tracking.rs @@ -1,7 +1,7 @@ #![no_std] #![no_main] -//@compile-flags: -Zmiri-track-alloc-id=19 -Zmiri-track-alloc-accesses -Cpanic=abort -//@normalize-stderr-test: "id 19" -> "id $$ALLOC" +//@compile-flags: -Zmiri-track-alloc-id=21 -Zmiri-track-alloc-accesses -Cpanic=abort +//@normalize-stderr-test: "id 21" -> "id $$ALLOC" //@only-target: linux # alloc IDs differ between OSes (due to extern static allocations) extern "Rust" { From cd971c6e1e969996a92889e26714cde45556ec87 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 Jun 2025 08:22:51 +0200 Subject: [PATCH 12/35] linux futex: fix for val > i32::MAX --- src/tools/miri/src/shims/unix/linux_like/sync.rs | 7 ++++--- src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs | 5 +++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/tools/miri/src/shims/unix/linux_like/sync.rs b/src/tools/miri/src/shims/unix/linux_like/sync.rs index 86e8b57824c27..9fad74c0241e4 100644 --- a/src/tools/miri/src/shims/unix/linux_like/sync.rs +++ b/src/tools/miri/src/shims/unix/linux_like/sync.rs @@ -15,12 +15,13 @@ pub fn futex<'tcx>( ) -> InterpResult<'tcx> { let [addr, op, val] = check_min_vararg_count("`syscall(SYS_futex, ...)`", varargs)?; + // See for docs. // The first three arguments (after the syscall number itself) are the same to all futex operations: - // (int *addr, int op, int val). + // (uint32_t *addr, int op, uint32_t val). // We checked above that these definitely exist. let addr = ecx.read_pointer(addr)?; let op = ecx.read_scalar(op)?.to_i32()?; - let val = ecx.read_scalar(val)?.to_i32()?; + let val = ecx.read_scalar(val)?.to_u32()?; // This is a vararg function so we have to bring our own type for this pointer. let addr = ecx.ptr_to_mplace(addr, ecx.machine.layouts.i32); @@ -138,7 +139,7 @@ pub fn futex<'tcx>( // It's not uncommon for `addr` to be passed as another type than `*mut i32`, such as `*const AtomicI32`. // We do an acquire read -- it only seems reasonable that if we observe a value here, we // actually establish an ordering with that value. - let futex_val = ecx.read_scalar_atomic(&addr, AtomicReadOrd::Acquire)?.to_i32()?; + let futex_val = ecx.read_scalar_atomic(&addr, AtomicReadOrd::Acquire)?.to_u32()?; if val == futex_val { // The value still matches, so we block the thread and make it wait for FUTEX_WAKE. diff --git a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs index f8f1c554f0d71..19d86f09595d5 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs +++ b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs @@ -33,6 +33,11 @@ fn wake_nobody() { 0, ); } + + // Wake u32::MAX waiters. + unsafe { + assert_eq!(libc::syscall(libc::SYS_futex, addr_of!(futex), libc::FUTEX_WAKE, u32::MAX), 0); + } } fn wake_dangling() { From 0c3c9e4438379bb49d45961122e4deaacdb0fa4b Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 1 Jul 2025 04:58:24 +0000 Subject: [PATCH 13/35] 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 3869fd3912098..8d09db11f2d18 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -ed2d759783dc9de134bbb3f01085b1e6dbf539f3 +fdad98d7463eebcdca94716ec3036c38a8d66f50 From 8958967fd498ad2e4fbaf08085e6ba3872776089 Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Tue, 1 Jul 2025 05:07:00 +0000 Subject: [PATCH 14/35] fmt --- src/tools/miri/tests/pass/float.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tools/miri/tests/pass/float.rs b/src/tools/miri/tests/pass/float.rs index 0fec1fb15eb86..fe7316c6680df 100644 --- a/src/tools/miri/tests/pass/float.rs +++ b/src/tools/miri/tests/pass/float.rs @@ -1066,7 +1066,6 @@ pub fn libm() { assert_eq!((-1f32).powf(f32::NEG_INFINITY), 1.0); assert_eq!((-1f64).powf(f64::NEG_INFINITY), 1.0); - assert_eq!(0f32.powi(10), 0.0); assert_eq!(0f64.powi(100), 0.0); assert_eq!(0f32.powi(9), 0.0); @@ -1490,7 +1489,6 @@ fn test_non_determinism() { test_operations_f64(19., 11.); test_operations_f128(25., 18.); - // SNaN^0 = (1 | NaN) ensure_nondet(|| f32::powf(SNAN_F32, 0.0).is_nan()); ensure_nondet(|| f64::powf(SNAN_F64, 0.0).is_nan()); From c52d5222981879f44e2cb28a1a62ae659ea2e19b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 1 Jul 2025 07:24:40 +0200 Subject: [PATCH 15/35] ./miri toolchain: no need to run 'cargo metadata' --- src/tools/miri/miri-script/src/commands.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index a3778215019f7..e948beef00447 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -231,11 +231,6 @@ impl Command { cmd!(sh, "rustup override set miri").run()?; // Cleanup. cmd!(sh, "cargo clean").run()?; - // Call `cargo metadata` on the sources in case that changes the lockfile - // (which fails under some setups when it is done from inside vscode). - let sysroot = cmd!(sh, "rustc --print sysroot").read()?; - let sysroot = sysroot.trim(); - cmd!(sh, "cargo metadata --format-version 1 --manifest-path {sysroot}/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml").ignore_stdout().run()?; Ok(()) } From 828f5944f8aa24803de2c0909e16afaed6a7e33d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 1 Jul 2025 07:48:10 +0200 Subject: [PATCH 16/35] re-balance CI jobs --- src/tools/miri/ci/ci.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 67c18d3b4019f..5767d17827952 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -163,6 +163,10 @@ case $HOST_TARGET in GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 candidate MANY_SEEDS=64 TEST_TARGET=aarch64-pc-windows-msvc run_tests + # Extra tier 2 + MANY_SEEDS=16 TEST_TARGET=arm-unknown-linux-gnueabi run_tests # 32bit ARM + MANY_SEEDS=16 TEST_TARGET=aarch64-pc-windows-gnullvm run_tests # gnullvm ABI + MANY_SEEDS=16 TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Custom target JSON file TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std ;; @@ -176,10 +180,6 @@ case $HOST_TARGET in # Extra tier 1 MANY_SEEDS=64 TEST_TARGET=i686-pc-windows-gnu run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests - # Extra tier 2 - MANY_SEEDS=16 TEST_TARGET=arm-unknown-linux-gnueabi run_tests # 32bit ARM - MANY_SEEDS=16 TEST_TARGET=aarch64-pc-windows-gnullvm run_tests # gnullvm ABI - MANY_SEEDS=16 TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Not officially supported tier 2 MANY_SEEDS=16 TEST_TARGET=mips-unknown-linux-gnu run_tests # a 32bit big-endian target, and also a target without 64bit atomics MANY_SEEDS=16 TEST_TARGET=x86_64-unknown-illumos run_tests From 4606afb125e325a5f3ad6d1e3aafb4edee79fafb Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Thu, 12 Jun 2025 17:03:33 +0200 Subject: [PATCH 17/35] Remove leaky synchronisation objects. --- src/tools/miri/src/concurrency/init_once.rs | 98 +++++++++----- src/tools/miri/src/concurrency/sync.rs | 136 +++++++------------- src/tools/miri/src/concurrency/thread.rs | 4 +- src/tools/miri/src/lib.rs | 6 +- src/tools/miri/src/machine.rs | 5 - src/tools/miri/src/shims/unix/macos/sync.rs | 5 +- src/tools/miri/src/shims/unix/sync.rs | 41 +++--- src/tools/miri/src/shims/windows/sync.rs | 37 +++--- 8 files changed, 148 insertions(+), 184 deletions(-) diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index c26384f65f6cc..165215f9270ce 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -1,13 +1,11 @@ +use std::cell::RefCell; use std::collections::VecDeque; - -use rustc_index::Idx; +use std::rc::Rc; use super::thread::DynUnblockCallback; use super::vector_clock::VClock; use crate::*; -super::sync::declare_id!(InitOnceId); - #[derive(Default, Debug, Copy, Clone, PartialEq, Eq)] /// The current status of a one time initialization. pub enum InitOnceStatus { @@ -25,44 +23,70 @@ pub(super) struct InitOnce { clock: VClock, } -impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} -pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { +impl InitOnce { #[inline] - fn init_once_status(&mut self, id: InitOnceId) -> InitOnceStatus { - let this = self.eval_context_ref(); - this.machine.sync.init_onces[id].status - } - - /// Put the thread into the queue waiting for the initialization. - #[inline] - fn init_once_enqueue_and_block(&mut self, id: InitOnceId, callback: DynUnblockCallback<'tcx>) { - let this = self.eval_context_mut(); - let thread = this.active_thread(); - let init_once = &mut this.machine.sync.init_onces[id]; - assert_ne!(init_once.status, InitOnceStatus::Complete, "queueing on complete init once"); - init_once.waiters.push_back(thread); - this.block_thread(BlockReason::InitOnce(id), None, callback); + pub fn status(&self) -> InitOnceStatus { + self.status } /// Begin initializing this InitOnce. Must only be called after checking that it is currently /// uninitialized. #[inline] - fn init_once_begin(&mut self, id: InitOnceId) { - let this = self.eval_context_mut(); - let init_once = &mut this.machine.sync.init_onces[id]; + pub fn begin(&mut self) { assert_eq!( - init_once.status, + self.status(), InitOnceStatus::Uninitialized, "beginning already begun or complete init once" ); - init_once.status = InitOnceStatus::Begun; + self.status = InitOnceStatus::Begun; } +} +#[derive(Default, Clone, Debug)] +pub struct InitOnceRef(Rc>); + +impl InitOnceRef { + pub fn new() -> Self { + Self(Default::default()) + } + + pub fn status(&self) -> InitOnceStatus { + self.0.borrow().status() + } + + pub fn begin(&self) { + self.0.borrow_mut().begin(); + } +} + +impl VisitProvenance for InitOnceRef { + // InitOnce contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} +} + +impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} +pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + /// Put the thread into the queue waiting for the initialization. #[inline] - fn init_once_complete(&mut self, id: InitOnceId) -> InterpResult<'tcx> { + fn init_once_enqueue_and_block( + &mut self, + init_once_ref: InitOnceRef, + callback: DynUnblockCallback<'tcx>, + ) { let this = self.eval_context_mut(); - let init_once = &mut this.machine.sync.init_onces[id]; + let thread = this.active_thread(); + let mut init_once = init_once_ref.0.borrow_mut(); + assert_ne!(init_once.status, InitOnceStatus::Complete, "queueing on complete init once"); + + init_once.waiters.push_back(thread); + this.block_thread(BlockReason::InitOnce, None, callback); + } + #[inline] + fn init_once_complete(&mut self, init_once_ref: &InitOnceRef) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + + let mut init_once = init_once_ref.0.borrow_mut(); assert_eq!( init_once.status, InitOnceStatus::Begun, @@ -79,17 +103,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Wake up everyone. // need to take the queue to avoid having `this` be borrowed multiple times - for waiter in std::mem::take(&mut init_once.waiters) { - this.unblock_thread(waiter, BlockReason::InitOnce(id))?; + let waiters = std::mem::take(&mut init_once.waiters); + drop(init_once); + for waiter in waiters { + this.unblock_thread(waiter, BlockReason::InitOnce)?; } interp_ok(()) } #[inline] - fn init_once_fail(&mut self, id: InitOnceId) -> InterpResult<'tcx> { + fn init_once_fail(&mut self, init_once_ref: &InitOnceRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let init_once = &mut this.machine.sync.init_onces[id]; + let mut init_once = init_once_ref.0.borrow_mut(); assert_eq!( init_once.status, InitOnceStatus::Begun, @@ -106,7 +132,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Wake up one waiting thread, so they can go ahead and try to init this. if let Some(waiter) = init_once.waiters.pop_front() { - this.unblock_thread(waiter, BlockReason::InitOnce(id))?; + drop(init_once); + this.unblock_thread(waiter, BlockReason::InitOnce)?; } interp_ok(()) @@ -115,15 +142,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Synchronize with the previous completion of an InitOnce. /// Must only be called after checking that it is complete. #[inline] - fn init_once_observe_completed(&mut self, id: InitOnceId) { + fn init_once_observe_completed(&mut self, init_once_ref: &InitOnceRef) { let this = self.eval_context_mut(); + let init_once = init_once_ref.0.borrow(); assert_eq!( - this.init_once_status(id), + init_once.status, InitOnceStatus::Complete, "observing the completion of incomplete init once" ); - this.acquire_clock(&this.machine.sync.init_onces[id].clock); + this.acquire_clock(&init_once.clock); } } diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 74379d6438d62..179094db0febd 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -8,45 +8,10 @@ use std::time::Duration; use rustc_abi::Size; use rustc_data_structures::fx::FxHashMap; -use rustc_index::{Idx, IndexVec}; -use super::init_once::InitOnce; use super::vector_clock::VClock; use crate::*; -/// We cannot use the `newtype_index!` macro because we have to use 0 as a -/// sentinel value meaning that the identifier is not assigned. This is because -/// the pthreads static initializers initialize memory with zeros (see the -/// `src/shims/sync.rs` file). -macro_rules! declare_id { - ($name: ident) => { - /// 0 is used to indicate that the id was not yet assigned and, - /// therefore, is not a valid identifier. - #[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] - pub struct $name(std::num::NonZero); - - impl $crate::VisitProvenance for $name { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} - } - - impl Idx for $name { - fn new(idx: usize) -> Self { - // We use 0 as a sentinel value (see the comment above) and, - // therefore, need to shift by one when converting from an index - // into a vector. - let shifted_idx = u32::try_from(idx).unwrap().strict_add(1); - $name(std::num::NonZero::new(shifted_idx).unwrap()) - } - fn index(self) -> usize { - // See the comment in `Self::new`. - // (This cannot underflow because `self.0` is `NonZero`.) - usize::try_from(self.0.get() - 1).unwrap() - } - } - }; -} -pub(super) use declare_id; - /// The mutex state. #[derive(Default, Debug)] struct Mutex { @@ -64,8 +29,8 @@ struct Mutex { pub struct MutexRef(Rc>); impl MutexRef { - fn new() -> Self { - MutexRef(Rc::new(RefCell::new(Mutex::default()))) + pub fn new() -> Self { + Self(Default::default()) } /// Get the id of the thread that currently owns this lock, or `None` if it is not locked. @@ -75,9 +40,8 @@ impl MutexRef { } impl VisitProvenance for MutexRef { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // Mutex contains no provenance. - } + // Mutex contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} } /// The read-write lock state. @@ -138,8 +102,8 @@ impl RwLock { pub struct RwLockRef(Rc>); impl RwLockRef { - fn new() -> Self { - RwLockRef(Rc::new(RefCell::new(RwLock::default()))) + pub fn new() -> Self { + Self(Default::default()) } pub fn is_locked(&self) -> bool { @@ -152,13 +116,10 @@ impl RwLockRef { } impl VisitProvenance for RwLockRef { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // RwLockRef contains no provenance. - } + // RwLock contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} } -declare_id!(CondvarId); - /// The conditional variable state. #[derive(Default, Debug)] struct Condvar { @@ -171,6 +132,24 @@ struct Condvar { clock: VClock, } +#[derive(Default, Clone, Debug)] +pub struct CondvarRef(Rc>); + +impl CondvarRef { + pub fn new() -> Self { + Self(Default::default()) + } + + pub fn is_awaited(&self) -> bool { + !self.0.borrow().waiters.is_empty() + } +} + +impl VisitProvenance for CondvarRef { + // Condvar contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} +} + /// The futex state. #[derive(Default, Debug)] struct Futex { @@ -183,19 +162,22 @@ struct Futex { clock: VClock, } -#[derive(Default, Clone)] +#[derive(Default, Clone, Debug)] pub struct FutexRef(Rc>); impl FutexRef { + pub fn new() -> Self { + Self(Default::default()) + } + pub fn waiters(&self) -> usize { self.0.borrow().waiters.len() } } impl VisitProvenance for FutexRef { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // No provenance in `Futex`. - } + // Futex contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} } /// A thread waiting on a futex. @@ -207,13 +189,6 @@ struct FutexWaiter { bitset: u32, } -/// The state of all synchronization objects. -#[derive(Default, Debug)] -pub struct SynchronizationObjects { - condvars: IndexVec, - pub(super) init_onces: IndexVec, -} - // Private extension trait for local helper methods impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { @@ -237,23 +212,6 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } } -impl SynchronizationObjects { - pub fn mutex_create(&mut self) -> MutexRef { - MutexRef::new() - } - pub fn rwlock_create(&mut self) -> RwLockRef { - RwLockRef::new() - } - - pub fn condvar_create(&mut self) -> CondvarId { - self.condvars.push(Default::default()) - } - - pub fn init_once_create(&mut self) -> InitOnceId { - self.init_onces.push(Default::default()) - } -} - impl<'tcx> AllocExtra<'tcx> { fn get_sync(&self, offset: Size) -> Option<&T> { self.sync.get(&offset).and_then(|s| s.downcast_ref::()) @@ -663,19 +621,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } - /// Is the conditional variable awaited? - #[inline] - fn condvar_is_awaited(&mut self, id: CondvarId) -> bool { - let this = self.eval_context_mut(); - !this.machine.sync.condvars[id].waiters.is_empty() - } - /// Release the mutex and let the current thread wait on the given condition variable. /// Once it is signaled, the mutex will be acquired and `retval_succ` will be written to `dest`. /// If the timeout happens first, `retval_timeout` will be written to `dest`. fn condvar_wait( &mut self, - condvar: CondvarId, + condvar_ref: CondvarRef, mutex_ref: MutexRef, timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>, retval_succ: Scalar, @@ -695,14 +646,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } let thread = this.active_thread(); - let waiters = &mut this.machine.sync.condvars[condvar].waiters; - waiters.push_back(thread); + + condvar_ref.0.borrow_mut().waiters.push_back(thread); this.block_thread( - BlockReason::Condvar(condvar), + BlockReason::Condvar, timeout, callback!( @capture<'tcx> { - condvar: CondvarId, + condvar_ref: CondvarRef, mutex_ref: MutexRef, retval_succ: Scalar, retval_timeout: Scalar, @@ -714,7 +665,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // The condvar was signaled. Make sure we get the clock for that. if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { data_race.acquire_clock( - &this.machine.sync.condvars[condvar].clock, + &condvar_ref.0.borrow().clock, &this.machine.threads, ); } @@ -725,7 +676,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { UnblockKind::TimedOut => { // We have to remove the waiter from the queue again. let thread = this.active_thread(); - let waiters = &mut this.machine.sync.condvars[condvar].waiters; + let waiters = &mut condvar_ref.0.borrow_mut().waiters; waiters.retain(|waiter| *waiter != thread); // Now get back the lock. this.condvar_reacquire_mutex(mutex_ref, retval_timeout, dest) @@ -739,9 +690,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Wake up some thread (if there is any) sleeping on the conditional /// variable. Returns `true` iff any thread was woken up. - fn condvar_signal(&mut self, id: CondvarId) -> InterpResult<'tcx, bool> { + fn condvar_signal(&mut self, condvar_ref: &CondvarRef) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); - let condvar = &mut this.machine.sync.condvars[id]; + let mut condvar = condvar_ref.0.borrow_mut(); // Each condvar signal happens-before the end of the condvar wake if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { @@ -750,7 +701,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let Some(waiter) = condvar.waiters.pop_front() else { return interp_ok(false); }; - this.unblock_thread(waiter, BlockReason::Condvar(id))?; + drop(condvar); + this.unblock_thread(waiter, BlockReason::Condvar)?; interp_ok(true) } diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index c8a408fd8ace8..56c197948546c 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -97,13 +97,13 @@ pub enum BlockReason { /// Blocked on a mutex. Mutex, /// Blocked on a condition variable. - Condvar(CondvarId), + Condvar, /// Blocked on a reader-writer lock. RwLock, /// Blocked on a Futex variable. Futex, /// Blocked on an InitOnce. - InitOnce(InitOnceId), + InitOnce, /// Blocked on epoll. Epoll, /// Blocked on eventfd. diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index edbc004d058ef..494e0f3fe85f4 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -127,10 +127,8 @@ pub use crate::concurrency::cpu_affinity::MAX_CPUS; pub use crate::concurrency::data_race::{ AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _, }; -pub use crate::concurrency::init_once::{EvalContextExt as _, InitOnceId}; -pub use crate::concurrency::sync::{ - CondvarId, EvalContextExt as _, MutexRef, RwLockRef, SynchronizationObjects, -}; +pub use crate::concurrency::init_once::{EvalContextExt as _, InitOnceRef}; +pub use crate::concurrency::sync::{CondvarRef, EvalContextExt as _, MutexRef, RwLockRef}; pub use crate::concurrency::thread::{ BlockReason, DynUnblockCallback, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager, TimeoutAnchor, TimeoutClock, UnblockKind, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 3a748c4c68726..088de965e2422 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -499,9 +499,6 @@ pub struct MiriMachine<'tcx> { /// in `sched_getaffinity` pub(crate) thread_cpu_affinity: FxHashMap, - /// The state of the primitive synchronization objects. - pub(crate) sync: SynchronizationObjects, - /// Precomputed `TyLayout`s for primitive data types that are commonly used inside Miri. pub(crate) layouts: PrimitiveLayouts<'tcx>, @@ -713,7 +710,6 @@ impl<'tcx> MiriMachine<'tcx> { layouts, threads, thread_cpu_affinity, - sync: SynchronizationObjects::default(), static_roots: Vec::new(), profiler, string_cache: Default::default(), @@ -903,7 +899,6 @@ impl VisitProvenance for MiriMachine<'_> { let MiriMachine { threads, thread_cpu_affinity: _, - sync: _, tls, env_vars, main_fn_ret_place, diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index 19f55e6c91782..05616dd5a4287 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -68,10 +68,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // LAZY_INIT_COOKIE). This can't be hit via `std::sync::Mutex`. interp_ok(MacOsUnfairLock::Poisoned) }, - |ecx| { - let mutex_ref = ecx.machine.sync.mutex_create(); - interp_ok(MacOsUnfairLock::Active { mutex_ref }) - }, + |_| interp_ok(MacOsUnfairLock::Active { mutex_ref: MutexRef::new() }), ) } } diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 50eb4d922891b..e20e3b79c3bec 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -171,8 +171,7 @@ fn mutex_create<'tcx>( kind: MutexKind, ) -> InterpResult<'tcx, PthreadMutex> { let mutex = ecx.deref_pointer_as(mutex_ptr, ecx.libc_ty_layout("pthread_mutex_t"))?; - let id = ecx.machine.sync.mutex_create(); - let data = PthreadMutex { mutex_ref: id, kind }; + let data = PthreadMutex { mutex_ref: MutexRef::new(), kind }; ecx.lazy_sync_init(&mutex, mutex_init_offset(ecx)?, data.clone())?; interp_ok(data) } @@ -193,8 +192,7 @@ where || throw_ub_format!("`pthread_mutex_t` can't be moved after first use"), |ecx| { let kind = mutex_kind_from_static_initializer(ecx, &mutex)?; - let id = ecx.machine.sync.mutex_create(); - interp_ok(PthreadMutex { mutex_ref: id, kind }) + interp_ok(PthreadMutex { mutex_ref: MutexRef::new(), kind }) }, ) } @@ -278,8 +276,7 @@ where )? { throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`"); } - let rwlock_ref = ecx.machine.sync.rwlock_create(); - interp_ok(PthreadRwLock { rwlock_ref }) + interp_ok(PthreadRwLock { rwlock_ref: RwLockRef::new() }) }, ) } @@ -372,9 +369,9 @@ enum ClockId { Monotonic, } -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Clone)] struct PthreadCondvar { - id: CondvarId, + condvar_ref: CondvarRef, clock: ClockId, } @@ -384,9 +381,8 @@ fn cond_create<'tcx>( clock: ClockId, ) -> InterpResult<'tcx, PthreadCondvar> { let cond = ecx.deref_pointer_as(cond_ptr, ecx.libc_ty_layout("pthread_cond_t"))?; - let id = ecx.machine.sync.condvar_create(); - let data = PthreadCondvar { id, clock }; - ecx.lazy_sync_init(&cond, cond_init_offset(ecx)?, data)?; + let data = PthreadCondvar { condvar_ref: CondvarRef::new(), clock }; + ecx.lazy_sync_init(&cond, cond_init_offset(ecx)?, data.clone())?; interp_ok(data) } @@ -411,8 +407,7 @@ where throw_unsup_format!("unsupported static initializer used for `pthread_cond_t`"); } // This used the static initializer. The clock there is always CLOCK_REALTIME. - let id = ecx.machine.sync.condvar_create(); - interp_ok(PthreadCondvar { id, clock: ClockId::Realtime }) + interp_ok(PthreadCondvar { condvar_ref: CondvarRef::new(), clock: ClockId::Realtime }) }, ) } @@ -817,15 +812,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_cond_signal(&mut self, cond_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); - let id = cond_get_data(this, cond_op)?.id; - this.condvar_signal(id)?; + let condvar = cond_get_data(this, cond_op)?.condvar_ref.clone(); + this.condvar_signal(&condvar)?; interp_ok(()) } fn pthread_cond_broadcast(&mut self, cond_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); - let id = cond_get_data(this, cond_op)?.id; - while this.condvar_signal(id)? {} + let condvar = cond_get_data(this, cond_op)?.condvar_ref.clone(); + while this.condvar_signal(&condvar)? {} interp_ok(()) } @@ -837,11 +832,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let data = *cond_get_data(this, cond_op)?; + let data = cond_get_data(this, cond_op)?.clone(); let mutex_ref = mutex_get_data(this, mutex_op)?.mutex_ref.clone(); this.condvar_wait( - data.id, + data.condvar_ref, mutex_ref, None, // no timeout Scalar::from_i32(0), @@ -861,7 +856,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let data = *cond_get_data(this, cond_op)?; + let data = cond_get_data(this, cond_op)?.clone(); let mutex_ref = mutex_get_data(this, mutex_op)?.mutex_ref.clone(); // Extract the timeout. @@ -884,7 +879,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; this.condvar_wait( - data.id, + data.condvar_ref, mutex_ref, Some((timeout_clock, TimeoutAnchor::Absolute, duration)), Scalar::from_i32(0), @@ -900,8 +895,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Reading the field also has the side-effect that we detect double-`destroy` // since we make the field uninit below. - let id = cond_get_data(this, cond_op)?.id; - if this.condvar_is_awaited(id) { + let condvar = &cond_get_data(this, cond_op)?.condvar_ref; + if condvar.is_awaited() { throw_ub_format!("destroying an awaited conditional variable"); } diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 8d5ea7db9e496..9165e76b63d14 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -2,13 +2,13 @@ use std::time::Duration; use rustc_abi::Size; -use crate::concurrency::init_once::InitOnceStatus; +use crate::concurrency::init_once::{EvalContextExt as _, InitOnceStatus}; use crate::concurrency::sync::FutexRef; use crate::*; -#[derive(Copy, Clone)] +#[derive(Clone)] struct WindowsInitOnce { - id: InitOnceId, + init_once: InitOnceRef, } struct WindowsFutex { @@ -37,10 +37,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { &init_once, init_offset, || throw_ub_format!("`INIT_ONCE` can't be moved after first use"), - |this| { + |_| { // TODO: check that this is still all-zero. - let id = this.machine.sync.init_once_create(); - interp_ok(WindowsInitOnce { id }) + interp_ok(WindowsInitOnce { init_once: InitOnceRef::new() }) }, ) } @@ -48,20 +47,20 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Returns `true` if we were succssful, `false` if we would block. fn init_once_try_begin( &mut self, - id: InitOnceId, + init_once_ref: &InitOnceRef, pending_place: &MPlaceTy<'tcx>, dest: &MPlaceTy<'tcx>, ) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); - interp_ok(match this.init_once_status(id) { + interp_ok(match init_once_ref.status() { InitOnceStatus::Uninitialized => { - this.init_once_begin(id); + init_once_ref.begin(); this.write_scalar(this.eval_windows("c", "TRUE"), pending_place)?; this.write_scalar(this.eval_windows("c", "TRUE"), dest)?; true } InitOnceStatus::Complete => { - this.init_once_observe_completed(id); + this.init_once_observe_completed(init_once_ref); this.write_scalar(this.eval_windows("c", "FALSE"), pending_place)?; this.write_scalar(this.eval_windows("c", "TRUE"), dest)?; true @@ -84,7 +83,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let id = this.init_once_get_data(init_once_op)?.id; + let init_once = this.init_once_get_data(init_once_op)?.init_once.clone(); let flags = this.read_scalar(flags_op)?.to_u32()?; // PBOOL is int* let pending_place = this.deref_pointer_as(pending_op, this.machine.layouts.i32)?; @@ -98,7 +97,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("non-null `lpContext` in `InitOnceBeginInitialize`"); } - if this.init_once_try_begin(id, &pending_place, dest)? { + if this.init_once_try_begin(&init_once, &pending_place, dest)? { // Done! return interp_ok(()); } @@ -106,16 +105,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // We have to block, and then try again when we are woken up. let dest = dest.clone(); this.init_once_enqueue_and_block( - id, + init_once.clone(), callback!( @capture<'tcx> { - id: InitOnceId, + init_once: InitOnceRef, pending_place: MPlaceTy<'tcx>, dest: MPlaceTy<'tcx>, } |this, unblock: UnblockKind| { assert_eq!(unblock, UnblockKind::Ready); - let ret = this.init_once_try_begin(id, &pending_place, &dest)?; + let ret = this.init_once_try_begin(&init_once, &pending_place, &dest)?; assert!(ret, "we were woken up but init_once_try_begin still failed"); interp_ok(()) } @@ -132,7 +131,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let id = this.init_once_get_data(init_once_op)?.id; + let init_once = this.init_once_get_data(init_once_op)?.init_once.clone(); let flags = this.read_scalar(flags_op)?.to_u32()?; let context = this.read_pointer(context_op)?; @@ -148,7 +147,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("non-null `lpContext` in `InitOnceBeginInitialize`"); } - if this.init_once_status(id) != InitOnceStatus::Begun { + if init_once.status() != InitOnceStatus::Begun { // The docs do not say anything about this case, but it seems better to not allow it. throw_ub_format!( "calling InitOnceComplete on a one time initialization that has not begun or is already completed" @@ -156,9 +155,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } if success { - this.init_once_complete(id)?; + this.init_once_complete(&init_once)?; } else { - this.init_once_fail(id)?; + this.init_once_fail(&init_once)?; } interp_ok(this.eval_windows("c", "TRUE")) From 9d9f0e045548797767c27591ac56909e68a3988a Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Wed, 2 Jul 2025 04:57:05 +0000 Subject: [PATCH 18/35] 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 8d09db11f2d18..18ebea643010c 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -fdad98d7463eebcdca94716ec3036c38a8d66f50 +f51c9870bab634afb9e7a262b6ca7816bb9e940d From 28900b0ee2eb1a7a364b7dc33b57b87344074078 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 18 Jun 2025 11:55:11 +0200 Subject: [PATCH 19/35] Add tracing feature to enable tracing_chrome support --- src/tools/miri/CONTRIBUTING.md | 9 + src/tools/miri/Cargo.lock | 1 + src/tools/miri/Cargo.toml | 2 + src/tools/miri/rustfmt.toml | 5 + src/tools/miri/src/bin/log/mod.rs | 2 + src/tools/miri/src/bin/log/setup.rs | 126 ++++ src/tools/miri/src/bin/log/tracing_chrome.rs | 625 +++++++++++++++++++ src/tools/miri/src/bin/miri.rs | 100 +-- src/tools/miri/src/machine.rs | 2 + 9 files changed, 799 insertions(+), 73 deletions(-) create mode 100644 src/tools/miri/src/bin/log/mod.rs create mode 100644 src/tools/miri/src/bin/log/setup.rs create mode 100644 src/tools/miri/src/bin/log/tracing_chrome.rs diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 739f0702252ab..fef7f807e9360 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -158,6 +158,15 @@ compiler that has `debug=true` set in `bootstrap.toml`. You can set `MIRI_BACKTRACE=1` to get a backtrace of where an evaluation error was originally raised. +#### Tracing + +You can generate a Chrome trace file from a Miri execution by passing `--features=tracing` during the +build and then setting `MIRI_TRACING=1` when running Miri. This will generate a `.json` file that +you can visualize in [Perfetto](https://ui.perfetto.dev/). For example: + +```sh +MIRI_TRACING=1 ./miri run --features=tracing tests/pass/hello.rs +``` ### UI testing diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index d3123caaa4788..aa6f059cec2c2 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -627,6 +627,7 @@ dependencies = [ "regex", "rustc_version", "serde", + "serde_json", "smallvec", "tempfile", "tikv-jemalloc-sys", diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 0b4b8e26bb8d0..7afca0cd61600 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -27,6 +27,7 @@ chrono = { version = "0.4.38", default-features = false } chrono-tz = "0.10" directories = "6" bitflags = "2.6" +serde_json = { version = "1.0", optional = true } # Copied from `compiler/rustc/Cargo.toml`. # But only for some targets, it fails for others. Rustc configures this in its CI, but we can't @@ -67,6 +68,7 @@ default = ["stack-cache"] genmc = [] stack-cache = [] stack-cache-consistency-check = ["stack-cache"] +tracing = ["serde_json"] [lints.rust.unexpected_cfgs] level = "warn" diff --git a/src/tools/miri/rustfmt.toml b/src/tools/miri/rustfmt.toml index 49650d8486c40..92fd22b7f5540 100644 --- a/src/tools/miri/rustfmt.toml +++ b/src/tools/miri/rustfmt.toml @@ -8,3 +8,8 @@ imports_granularity = "Module" force_multiline_blocks = true match_arm_blocks = false match_arm_leading_pipes = "Preserve" + +ignore = [ + # This file is copy-pasted from the tracing_chrome crate and should remain like the original. + "src/bin/log/tracing_chrome.rs" +] diff --git a/src/tools/miri/src/bin/log/mod.rs b/src/tools/miri/src/bin/log/mod.rs new file mode 100644 index 0000000000000..f3b2fdb5348e0 --- /dev/null +++ b/src/tools/miri/src/bin/log/mod.rs @@ -0,0 +1,2 @@ +pub mod setup; +mod tracing_chrome; diff --git a/src/tools/miri/src/bin/log/setup.rs b/src/tools/miri/src/bin/log/setup.rs new file mode 100644 index 0000000000000..a4ad70c847c79 --- /dev/null +++ b/src/tools/miri/src/bin/log/setup.rs @@ -0,0 +1,126 @@ +use std::env::{self, VarError}; +use std::str::FromStr; +use std::sync::{Mutex, OnceLock}; + +use rustc_middle::ty::TyCtxt; +use rustc_session::{CtfeBacktrace, EarlyDiagCtxt}; + +/// The tracing layer from `tracing-chrome` starts a thread in the background that saves data to +/// file and closes the file when stopped. If the thread is not stopped properly, the file will be +/// missing end terminators (`]` for JSON arrays) and other data may also not be flushed. Therefore +/// we need to keep a guard that, when [Drop]ped, will send a signal to stop the thread. Make sure +/// to manually drop this guard using [deinit_loggers], if you are exiting the program with +/// [std::process::exit]! +#[must_use] +struct TracingGuard { + #[cfg(feature = "tracing")] + _chrome: super::tracing_chrome::FlushGuard, + _no_construct: (), +} + +// This ensures TracingGuard is always a drop-type, even when the `_chrome` field is disabled. +impl Drop for TracingGuard { + fn drop(&mut self) {} +} + +fn rustc_logger_config() -> rustc_log::LoggerConfig { + // Start with the usual env vars. + let mut cfg = rustc_log::LoggerConfig::from_env("RUSTC_LOG"); + + // Overwrite if MIRI_LOG is set. + if let Ok(var) = env::var("MIRI_LOG") { + // MIRI_LOG serves as default for RUSTC_LOG, if that is not set. + if matches!(cfg.filter, Err(VarError::NotPresent)) { + // We try to be a bit clever here: if `MIRI_LOG` is just a single level + // used for everything, we only apply it to the parts of rustc that are + // CTFE-related. Otherwise, we use it verbatim for `RUSTC_LOG`. + // This way, if you set `MIRI_LOG=trace`, you get only the right parts of + // rustc traced, but you can also do `MIRI_LOG=miri=trace,rustc_const_eval::interpret=debug`. + if tracing::Level::from_str(&var).is_ok() { + cfg.filter = Ok(format!( + "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var},miri={var}" + )); + } else { + cfg.filter = Ok(var); + } + } + } + + cfg +} + +/// The global logger can only be set once per process, so track whether that already happened and +/// keep a [TracingGuard] so it can be [Drop]ped later using [deinit_loggers]. +static LOGGER_INITED: OnceLock>> = OnceLock::new(); + +fn init_logger_once(early_dcx: &EarlyDiagCtxt) { + // If the logger is not yet initialized, initialize it. + LOGGER_INITED.get_or_init(|| { + let guard = if env::var_os("MIRI_TRACING").is_some() { + #[cfg(not(feature = "tracing"))] + { + crate::show_error!( + "fatal error: cannot enable MIRI_TRACING since Miri was not built with the \"tracing\" feature" + ); + } + + #[cfg(feature = "tracing")] + { + let (chrome_layer, chrome_guard) = + super::tracing_chrome::ChromeLayerBuilder::new().include_args(true).build(); + rustc_driver::init_logger_with_additional_layer( + early_dcx, + rustc_logger_config(), + || { + tracing_subscriber::layer::SubscriberExt::with( + tracing_subscriber::Registry::default(), + chrome_layer, + ) + }, + ); + + Some(TracingGuard { _chrome: chrome_guard, _no_construct: () }) + } + } else { + // initialize the logger without any tracing enabled + rustc_driver::init_logger(early_dcx, rustc_logger_config()); + None + }; + Mutex::new(guard) + }); +} + +pub fn init_early_loggers(early_dcx: &EarlyDiagCtxt) { + // We only initialize `rustc` if the env var is set (so the user asked for it). + // If it is not set, we avoid initializing now so that we can initialize later with our custom + // settings, and *not* log anything for what happens before `miri` starts interpreting. + if env::var_os("RUSTC_LOG").is_some() { + init_logger_once(early_dcx); + } +} + +pub fn init_late_loggers(early_dcx: &EarlyDiagCtxt, tcx: TyCtxt<'_>) { + // If the logger is not yet initialized, initialize it. + init_logger_once(early_dcx); + + // If `MIRI_BACKTRACE` is set and `RUSTC_CTFE_BACKTRACE` is not, set `RUSTC_CTFE_BACKTRACE`. + // Do this late, so we ideally only apply this to Miri's errors. + if let Some(val) = env::var_os("MIRI_BACKTRACE") { + let ctfe_backtrace = match &*val.to_string_lossy() { + "immediate" => CtfeBacktrace::Immediate, + "0" => CtfeBacktrace::Disabled, + _ => CtfeBacktrace::Capture, + }; + *tcx.sess.ctfe_backtrace.borrow_mut() = ctfe_backtrace; + } +} + +/// Must be called before the program terminates to ensure the trace file is closed correctly. Not +/// doing so will result in invalid trace files. Also see [TracingGuard]. +pub fn deinit_loggers() { + if let Some(guard) = LOGGER_INITED.get() + && let Ok(mut guard) = guard.lock() + { + std::mem::drop(guard.take()); + } +} diff --git a/src/tools/miri/src/bin/log/tracing_chrome.rs b/src/tools/miri/src/bin/log/tracing_chrome.rs new file mode 100644 index 0000000000000..459acea6f0bf7 --- /dev/null +++ b/src/tools/miri/src/bin/log/tracing_chrome.rs @@ -0,0 +1,625 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: Copyright (c) 2020 Thoren Paulson +//! This file is taken unmodified from the following link, except for file attributes and +//! `extern crate` at the top. +//! https://github.com/thoren-d/tracing-chrome/blob/7e2625ab4aeeef2f0ef9bde9d6258dd181c04472/src/lib.rs +//! Depending on the tracing-chrome crate from crates.io is unfortunately not possible, since it +//! depends on `tracing_core` which conflicts with rustc_private's `tracing_core` (meaning it would +//! not be possible to use the [ChromeLayer] in a context that expects a [Layer] from +//! rustc_private's `tracing_core` version). +#![allow(warnings)] +#![cfg(feature = "tracing")] + +// This is here and not in src/lib.rs since it is a direct dependency of tracing_chrome.rs and +// should not be included if the "tracing" feature is disabled. +extern crate tracing_core; + +use tracing_core::{field::Field, span, Event, Subscriber}; +use tracing_subscriber::{ + layer::Context, + registry::{LookupSpan, SpanRef}, + Layer, +}; + +use serde_json::{json, Value as JsonValue}; +use std::{ + marker::PhantomData, + path::Path, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, Mutex, + }, +}; + +use std::io::{BufWriter, Write}; +use std::sync::mpsc; +use std::sync::mpsc::Sender; +use std::{ + cell::{Cell, RefCell}, + thread::JoinHandle, +}; + +thread_local! { + static OUT: RefCell>> = const { RefCell::new(None) }; + static TID: RefCell> = const { RefCell::new(None) }; +} + +type NameFn = Box) -> String + Send + Sync>; +type Object = serde_json::Map; + +/// A [`Layer`](tracing_subscriber::Layer) that writes a Chrome trace file. +pub struct ChromeLayer +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + out: Arc>>, + start: std::time::Instant, + max_tid: AtomicUsize, + include_args: bool, + include_locations: bool, + trace_style: TraceStyle, + name_fn: Option>, + cat_fn: Option>, + _inner: PhantomData, +} + +/// A builder for [`ChromeLayer`](crate::ChromeLayer). +#[derive(Default)] +pub struct ChromeLayerBuilder +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + out_writer: Option>, + name_fn: Option>, + cat_fn: Option>, + include_args: bool, + include_locations: bool, + trace_style: TraceStyle, + _inner: PhantomData, +} + +/// Decides how traces will be recorded. +#[derive(Default)] +pub enum TraceStyle { + /// Traces will be recorded as a group of threads. + /// In this style, spans should be entered and exited on the same thread. + #[default] + Threaded, + + /// Traces will recorded as a group of asynchronous operations. + Async, +} + +impl ChromeLayerBuilder +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + pub fn new() -> Self { + ChromeLayerBuilder { + out_writer: None, + name_fn: None, + cat_fn: None, + include_args: false, + include_locations: true, + trace_style: TraceStyle::Threaded, + _inner: PhantomData, + } + } + + /// Set the file to which to output the trace. + /// + /// Defaults to `./trace-{unix epoch in micros}.json`. + /// + /// # Panics + /// + /// If `file` could not be opened/created. To handle errors, + /// open a file and pass it to [`writer`](crate::ChromeLayerBuilder::writer) instead. + pub fn file>(self, file: P) -> Self { + self.writer(std::fs::File::create(file).expect("Failed to create trace file.")) + } + + /// Supply an arbitrary writer to which to write trace contents. + /// + /// # Examples + /// + /// ```rust + /// # use tracing_chrome::ChromeLayerBuilder; + /// # use tracing_subscriber::prelude::*; + /// let (layer, guard) = ChromeLayerBuilder::new().writer(std::io::sink()).build(); + /// # tracing_subscriber::registry().with(layer).init(); + /// ``` + pub fn writer(mut self, writer: W) -> Self { + self.out_writer = Some(Box::new(writer)); + self + } + + /// Include arguments in each trace entry. + /// + /// Defaults to `false`. + /// + /// Includes the arguments used when creating a span/event + /// in the "args" section of the trace entry. + pub fn include_args(mut self, include: bool) -> Self { + self.include_args = include; + self + } + + /// Include file+line with each trace entry. + /// + /// Defaults to `true`. + /// + /// This can add quite a bit of data to the output so turning + /// it off might be helpful when collecting larger traces. + pub fn include_locations(mut self, include: bool) -> Self { + self.include_locations = include; + self + } + + /// Sets the style used when recording trace events. + /// + /// See [`TraceStyle`](crate::TraceStyle) for details. + pub fn trace_style(mut self, style: TraceStyle) -> Self { + self.trace_style = style; + self + } + + /// Allows supplying a function that derives a name from + /// an Event or Span. The result is used as the "name" field + /// on trace entries. + /// + /// # Example + /// ``` + /// use tracing_chrome::{ChromeLayerBuilder, EventOrSpan}; + /// use tracing_subscriber::{registry::Registry, prelude::*}; + /// + /// let (chrome_layer, _guard) = ChromeLayerBuilder::new().name_fn(Box::new(|event_or_span| { + /// match event_or_span { + /// EventOrSpan::Event(ev) => { ev.metadata().name().into() }, + /// EventOrSpan::Span(_s) => { "span".into() }, + /// } + /// })).build(); + /// tracing_subscriber::registry().with(chrome_layer).init() + /// ``` + pub fn name_fn(mut self, name_fn: NameFn) -> Self { + self.name_fn = Some(name_fn); + self + } + + /// Allows supplying a function that derives a category from + /// an Event or Span. The result is used as the "cat" field on + /// trace entries. + /// + /// # Example + /// ``` + /// use tracing_chrome::{ChromeLayerBuilder, EventOrSpan}; + /// use tracing_subscriber::{registry::Registry, prelude::*}; + /// + /// let (chrome_layer, _guard) = ChromeLayerBuilder::new().category_fn(Box::new(|_| { + /// "my_module".into() + /// })).build(); + /// tracing_subscriber::registry().with(chrome_layer).init() + /// ``` + pub fn category_fn(mut self, cat_fn: NameFn) -> Self { + self.cat_fn = Some(cat_fn); + self + } + + /// Creates a [`ChromeLayer`](crate::ChromeLayer) and associated [`FlushGuard`](crate::FlushGuard). + /// + /// # Panics + /// + /// If no file or writer was specified and the default trace file could not be opened/created. + pub fn build(self) -> (ChromeLayer, FlushGuard) { + ChromeLayer::new(self) + } +} + +/// This guard will signal the thread writing the trace file to stop and join it when dropped. +pub struct FlushGuard { + sender: Sender, + handle: Cell>>, +} + +impl FlushGuard { + /// Signals the trace writing thread to flush to disk. + pub fn flush(&self) { + if let Some(handle) = self.handle.take() { + let _ignored = self.sender.send(Message::Flush); + self.handle.set(Some(handle)); + } + } + + /// Finishes the current trace and starts a new one. + /// + /// If a [`Write`](std::io::Write) implementation is supplied, + /// the new trace is written to it. Otherwise, the new trace + /// goes to `./trace-{unix epoc in micros}.json`. + pub fn start_new(&self, writer: Option>) { + if let Some(handle) = self.handle.take() { + let _ignored = self.sender.send(Message::StartNew(writer)); + self.handle.set(Some(handle)); + } + } +} + +impl Drop for FlushGuard { + fn drop(&mut self) { + if let Some(handle) = self.handle.take() { + let _ignored = self.sender.send(Message::Drop); + if handle.join().is_err() { + eprintln!("tracing_chrome: Trace writing thread panicked."); + } + } + } +} + +struct Callsite { + tid: usize, + name: String, + target: String, + file: Option<&'static str>, + line: Option, + args: Option>, +} + +enum Message { + Enter(f64, Callsite, Option), + Event(f64, Callsite), + Exit(f64, Callsite, Option), + NewThread(usize, String), + Flush, + Drop, + StartNew(Option>), +} + +/// Represents either an [`Event`](tracing_core::Event) or [`SpanRef`](tracing_subscriber::registry::SpanRef). +pub enum EventOrSpan<'a, 'b, S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + Event(&'a Event<'b>), + Span(&'a SpanRef<'b, S>), +} + +fn create_default_writer() -> Box { + Box::new( + std::fs::File::create(format!( + "./trace-{}.json", + std::time::SystemTime::UNIX_EPOCH + .elapsed() + .unwrap() + .as_micros() + )) + .expect("Failed to create trace file."), + ) +} + +impl ChromeLayer +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + fn new(mut builder: ChromeLayerBuilder) -> (ChromeLayer, FlushGuard) { + let (tx, rx) = mpsc::channel(); + OUT.with(|val| val.replace(Some(tx.clone()))); + + let out_writer = builder + .out_writer + .unwrap_or_else(|| create_default_writer()); + + let handle = std::thread::spawn(move || { + let mut write = BufWriter::new(out_writer); + write.write_all(b"[\n").unwrap(); + + let mut has_started = false; + let mut thread_names: Vec<(usize, String)> = Vec::new(); + for msg in rx { + if let Message::Flush = &msg { + write.flush().unwrap(); + continue; + } else if let Message::Drop = &msg { + break; + } else if let Message::StartNew(writer) = msg { + // Finish off current file + write.write_all(b"\n]").unwrap(); + write.flush().unwrap(); + + // Get or create new writer + let out_writer = writer.unwrap_or_else(|| create_default_writer()); + write = BufWriter::new(out_writer); + write.write_all(b"[\n").unwrap(); + has_started = false; + + // Write saved thread names + for (tid, name) in thread_names.iter() { + let entry = json!({ + "ph": "M", + "pid": 1, + "name": "thread_name", + "tid": *tid, + "args": { + "name": name, + }, + }); + + if has_started { + write.write_all(b",\n").unwrap(); + } + serde_json::to_writer(&mut write, &entry).unwrap(); + has_started = true; + } + continue; + } + + let (ph, ts, callsite, id) = match &msg { + Message::Enter(ts, callsite, None) => ("B", Some(ts), Some(callsite), None), + Message::Enter(ts, callsite, Some(root_id)) => { + ("b", Some(ts), Some(callsite), Some(root_id)) + } + Message::Event(ts, callsite) => ("i", Some(ts), Some(callsite), None), + Message::Exit(ts, callsite, None) => ("E", Some(ts), Some(callsite), None), + Message::Exit(ts, callsite, Some(root_id)) => { + ("e", Some(ts), Some(callsite), Some(root_id)) + } + Message::NewThread(_tid, _name) => ("M", None, None, None), + Message::Flush | Message::Drop | Message::StartNew(_) => { + panic!("Was supposed to break by now.") + } + }; + let mut entry = json!({ + "ph": ph, + "pid": 1, + }); + + if let Message::NewThread(tid, name) = msg { + thread_names.push((tid, name.clone())); + entry["name"] = "thread_name".into(); + entry["tid"] = tid.into(); + entry["args"] = json!({ "name": name }); + } else { + let ts = ts.unwrap(); + let callsite = callsite.unwrap(); + entry["ts"] = (*ts).into(); + entry["name"] = callsite.name.clone().into(); + entry["cat"] = callsite.target.clone().into(); + entry["tid"] = callsite.tid.into(); + + if let Some(&id) = id { + entry["id"] = id.into(); + } + + if ph == "i" { + entry["s"] = "t".into(); + } + + if let (Some(file), Some(line)) = (callsite.file, callsite.line) { + entry[".file"] = file.into(); + entry[".line"] = line.into(); + } + + if let Some(call_args) = &callsite.args { + if !call_args.is_empty() { + entry["args"] = (**call_args).clone().into(); + } + } + } + + if has_started { + write.write_all(b",\n").unwrap(); + } + serde_json::to_writer(&mut write, &entry).unwrap(); + has_started = true; + } + + write.write_all(b"\n]").unwrap(); + write.flush().unwrap(); + }); + + let guard = FlushGuard { + sender: tx.clone(), + handle: Cell::new(Some(handle)), + }; + let layer = ChromeLayer { + out: Arc::new(Mutex::new(tx)), + start: std::time::Instant::now(), + max_tid: AtomicUsize::new(0), + name_fn: builder.name_fn.take(), + cat_fn: builder.cat_fn.take(), + include_args: builder.include_args, + include_locations: builder.include_locations, + trace_style: builder.trace_style, + _inner: PhantomData, + }; + + (layer, guard) + } + + fn get_tid(&self) -> (usize, bool) { + TID.with(|value| { + let tid = *value.borrow(); + match tid { + Some(tid) => (tid, false), + None => { + let tid = self.max_tid.fetch_add(1, Ordering::SeqCst); + value.replace(Some(tid)); + (tid, true) + } + } + }) + } + + fn get_callsite(&self, data: EventOrSpan) -> Callsite { + let (tid, new_thread) = self.get_tid(); + let name = self.name_fn.as_ref().map(|name_fn| name_fn(&data)); + let target = self.cat_fn.as_ref().map(|cat_fn| cat_fn(&data)); + let meta = match data { + EventOrSpan::Event(e) => e.metadata(), + EventOrSpan::Span(s) => s.metadata(), + }; + let args = match data { + EventOrSpan::Event(e) => { + if self.include_args { + let mut args = Object::new(); + e.record(&mut JsonVisitor { object: &mut args }); + Some(Arc::new(args)) + } else { + None + } + } + EventOrSpan::Span(s) => s + .extensions() + .get::() + .map(|e| &e.args) + .cloned(), + }; + let name = name.unwrap_or_else(|| meta.name().into()); + let target = target.unwrap_or_else(|| meta.target().into()); + let (file, line) = if self.include_locations { + (meta.file(), meta.line()) + } else { + (None, None) + }; + + if new_thread { + let name = match std::thread::current().name() { + Some(name) => name.to_owned(), + None => tid.to_string(), + }; + self.send_message(Message::NewThread(tid, name)); + } + + Callsite { + tid, + name, + target, + file, + line, + args, + } + } + + fn get_root_id(span: SpanRef) -> u64 { + span.scope() + .from_root() + .take(1) + .next() + .unwrap_or(span) + .id() + .into_u64() + } + + fn enter_span(&self, span: SpanRef, ts: f64) { + let callsite = self.get_callsite(EventOrSpan::Span(&span)); + let root_id = match self.trace_style { + TraceStyle::Async => Some(ChromeLayer::get_root_id(span)), + _ => None, + }; + self.send_message(Message::Enter(ts, callsite, root_id)); + } + + fn exit_span(&self, span: SpanRef, ts: f64) { + let callsite = self.get_callsite(EventOrSpan::Span(&span)); + let root_id = match self.trace_style { + TraceStyle::Async => Some(ChromeLayer::get_root_id(span)), + _ => None, + }; + self.send_message(Message::Exit(ts, callsite, root_id)); + } + + fn get_ts(&self) -> f64 { + self.start.elapsed().as_nanos() as f64 / 1000.0 + } + + fn send_message(&self, message: Message) { + OUT.with(move |val| { + if val.borrow().is_some() { + let _ignored = val.borrow().as_ref().unwrap().send(message); + } else { + let out = self.out.lock().unwrap().clone(); + let _ignored = out.send(message); + val.replace(Some(out)); + } + }); + } +} + +impl Layer for ChromeLayer +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) { + if let TraceStyle::Async = self.trace_style { + return; + } + + let ts = self.get_ts(); + self.enter_span(ctx.span(id).expect("Span not found."), ts); + } + + fn on_record(&self, id: &span::Id, values: &span::Record<'_>, ctx: Context<'_, S>) { + if self.include_args { + let span = ctx.span(id).unwrap(); + let mut exts = span.extensions_mut(); + + let args = exts.get_mut::(); + + if let Some(args) = args { + let args = Arc::make_mut(&mut args.args); + values.record(&mut JsonVisitor { object: args }); + } + } + } + + fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { + let ts = self.get_ts(); + let callsite = self.get_callsite(EventOrSpan::Event(event)); + self.send_message(Message::Event(ts, callsite)); + } + + fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { + if let TraceStyle::Async = self.trace_style { + return; + } + let ts = self.get_ts(); + self.exit_span(ctx.span(id).expect("Span not found."), ts); + } + + fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, ctx: Context<'_, S>) { + if self.include_args { + let mut args = Object::new(); + attrs.record(&mut JsonVisitor { object: &mut args }); + ctx.span(id).unwrap().extensions_mut().insert(ArgsWrapper { + args: Arc::new(args), + }); + } + if let TraceStyle::Threaded = self.trace_style { + return; + } + + let ts = self.get_ts(); + self.enter_span(ctx.span(id).expect("Span not found."), ts); + } + + fn on_close(&self, id: span::Id, ctx: Context<'_, S>) { + if let TraceStyle::Threaded = self.trace_style { + return; + } + + let ts = self.get_ts(); + self.exit_span(ctx.span(&id).expect("Span not found."), ts); + } +} + +struct JsonVisitor<'a> { + object: &'a mut Object, +} + +impl<'a> tracing_subscriber::field::Visit for JsonVisitor<'a> { + fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { + self.object + .insert(field.name().to_owned(), format!("{value:?}").into()); + } +} + +struct ArgsWrapper { + args: Arc, +} diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index d410d7bcc8757..e5ba204013647 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -10,6 +10,8 @@ // Some "regular" crates we want to share with rustc extern crate tracing; +#[cfg(feature = "tracing")] +extern crate tracing_subscriber; // The rustc crates we need extern crate rustc_abi; @@ -24,14 +26,16 @@ extern crate rustc_middle; extern crate rustc_session; extern crate rustc_span; -use std::env::{self, VarError}; +mod log; + +use std::env; use std::num::NonZero; use std::ops::Range; use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; +use std::sync::Arc; use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; -use std::sync::{Arc, Once}; use miri::{ BacktraceStyle, BorrowTrackerMethod, GenmcConfig, GenmcCtx, MiriConfig, MiriEntryFnType, @@ -52,12 +56,14 @@ use rustc_middle::query::LocalCrate; use rustc_middle::traits::{ObligationCause, ObligationCauseCode}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::util::Providers; +use rustc_session::EarlyDiagCtxt; use rustc_session::config::{CrateType, ErrorOutputType, OptLevel}; use rustc_session::search_paths::PathKind; -use rustc_session::{CtfeBacktrace, EarlyDiagCtxt}; use rustc_span::def_id::DefId; use tracing::debug; +use crate::log::setup::{deinit_loggers, init_early_loggers, init_late_loggers}; + struct MiriCompilerCalls { miri_config: Option, many_seeds: Option, @@ -154,13 +160,13 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { if tcx.sess.dcx().has_errors_or_delayed_bugs().is_some() { tcx.dcx().fatal("miri cannot be run on programs that fail compilation"); } - - let early_dcx = EarlyDiagCtxt::new(tcx.sess.opts.error_format); - init_late_loggers(&early_dcx, tcx); if !tcx.crate_types().contains(&CrateType::Executable) { tcx.dcx().fatal("miri only makes sense on bin crates"); } + let early_dcx = EarlyDiagCtxt::new(tcx.sess.opts.error_format); + init_late_loggers(&early_dcx, tcx); + let (entry_def_id, entry_type) = entry_fn(tcx); let mut config = self.miri_config.take().expect("after_analysis must only be called once"); @@ -213,7 +219,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { if !many_seeds.keep_going { // `abort_if_errors` would actually not stop, since `par_for_each` waits for the // rest of the to finish, so we just exit immediately. - std::process::exit(return_code); + exit(return_code); } exit_code.store(return_code, Ordering::Relaxed); num_failed.fetch_add(1, Ordering::Relaxed); @@ -223,7 +229,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { if num_failed > 0 { eprintln!("{num_failed}/{total} SEEDS FAILED", total = many_seeds.seeds.count()); } - std::process::exit(exit_code.0.into_inner()); + exit(exit_code.0.into_inner()); } else { let return_code = miri::eval_entry(tcx, entry_def_id, entry_type, &config, None) .unwrap_or_else(|| { @@ -232,7 +238,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { tcx.dcx().abort_if_errors(); rustc_driver::EXIT_FAILURE }); - std::process::exit(return_code); + exit(return_code); } // Unreachable. @@ -327,83 +333,31 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { } } -fn show_error(msg: &impl std::fmt::Display) -> ! { - eprintln!("fatal error: {msg}"); - std::process::exit(1) +fn exit(exit_code: i32) -> ! { + // Drop the tracing guard before exiting, so tracing calls are flushed correctly. + deinit_loggers(); + std::process::exit(exit_code); } -macro_rules! show_error { - ($($tt:tt)*) => { show_error(&format_args!($($tt)*)) }; -} - -fn rustc_logger_config() -> rustc_log::LoggerConfig { - // Start with the usual env vars. - let mut cfg = rustc_log::LoggerConfig::from_env("RUSTC_LOG"); - - // Overwrite if MIRI_LOG is set. - if let Ok(var) = env::var("MIRI_LOG") { - // MIRI_LOG serves as default for RUSTC_LOG, if that is not set. - if matches!(cfg.filter, Err(VarError::NotPresent)) { - // We try to be a bit clever here: if `MIRI_LOG` is just a single level - // used for everything, we only apply it to the parts of rustc that are - // CTFE-related. Otherwise, we use it verbatim for `RUSTC_LOG`. - // This way, if you set `MIRI_LOG=trace`, you get only the right parts of - // rustc traced, but you can also do `MIRI_LOG=miri=trace,rustc_const_eval::interpret=debug`. - if tracing::Level::from_str(&var).is_ok() { - cfg.filter = Ok(format!( - "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var},miri={var}" - )); - } else { - cfg.filter = Ok(var); - } - } - } - - cfg -} - -/// The global logger can only be set once per process, so track -/// whether that already happened. -static LOGGER_INITED: Once = Once::new(); - -fn init_early_loggers(early_dcx: &EarlyDiagCtxt) { - // We only initialize `rustc` if the env var is set (so the user asked for it). - // If it is not set, we avoid initializing now so that we can initialize later with our custom - // settings, and *not* log anything for what happens before `miri` starts interpreting. - if env::var_os("RUSTC_LOG").is_some() { - LOGGER_INITED.call_once(|| { - rustc_driver::init_logger(early_dcx, rustc_logger_config()); - }); - } +fn show_error_(msg: &impl std::fmt::Display) -> ! { + eprintln!("fatal error: {msg}"); + exit(1) } -fn init_late_loggers(early_dcx: &EarlyDiagCtxt, tcx: TyCtxt<'_>) { - // If the logger is not yet initialized, initialize it. - LOGGER_INITED.call_once(|| { - rustc_driver::init_logger(early_dcx, rustc_logger_config()); - }); - - // If `MIRI_BACKTRACE` is set and `RUSTC_CTFE_BACKTRACE` is not, set `RUSTC_CTFE_BACKTRACE`. - // Do this late, so we ideally only apply this to Miri's errors. - if let Some(val) = env::var_os("MIRI_BACKTRACE") { - let ctfe_backtrace = match &*val.to_string_lossy() { - "immediate" => CtfeBacktrace::Immediate, - "0" => CtfeBacktrace::Disabled, - _ => CtfeBacktrace::Capture, - }; - *tcx.sess.ctfe_backtrace.borrow_mut() = ctfe_backtrace; - } +macro_rules! show_error { + ($($tt:tt)*) => { $crate::show_error_(&format_args!($($tt)*)) }; } +use show_error; /// Execute a compiler with the given CLI arguments and callbacks. fn run_compiler_and_exit( args: &[String], callbacks: &mut (dyn rustc_driver::Callbacks + Send), ) -> ! { - // Invoke compiler, and handle return code. + // Invoke compiler, catch any unwinding panics and handle return code. let exit_code = rustc_driver::catch_with_exit_code(move || rustc_driver::run_compiler(args, callbacks)); - std::process::exit(exit_code) + exit(exit_code) } /// Parses a comma separated list of `T` from the given string: diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index b4d7db34efa7b..7ef3ee82b3182 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1019,6 +1019,8 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { const PANIC_ON_ALLOC_FAIL: bool = false; + const TRACING_ENABLED: bool = cfg!(feature = "tracing"); + #[inline(always)] fn enforce_alignment(ecx: &MiriInterpCx<'tcx>) -> bool { ecx.machine.check_alignment != AlignmentCheck::None From d2f2271ccde38f591e17874569eefb79fdfa61d5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Jul 2025 08:30:53 +0200 Subject: [PATCH 20/35] rename show_error -> fatal_error --- src/tools/miri/src/bin/log/setup.rs | 2 +- src/tools/miri/src/bin/miri.rs | 66 ++++++++++++++--------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/tools/miri/src/bin/log/setup.rs b/src/tools/miri/src/bin/log/setup.rs index a4ad70c847c79..da0ba528b2c4d 100644 --- a/src/tools/miri/src/bin/log/setup.rs +++ b/src/tools/miri/src/bin/log/setup.rs @@ -59,7 +59,7 @@ fn init_logger_once(early_dcx: &EarlyDiagCtxt) { let guard = if env::var_os("MIRI_TRACING").is_some() { #[cfg(not(feature = "tracing"))] { - crate::show_error!( + crate::fatal_error!( "fatal error: cannot enable MIRI_TRACING since Miri was not built with the \"tracing\" feature" ); } diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index e5ba204013647..a2e893dd91d98 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -339,15 +339,15 @@ fn exit(exit_code: i32) -> ! { std::process::exit(exit_code); } -fn show_error_(msg: &impl std::fmt::Display) -> ! { +fn fatal_error_(msg: &impl std::fmt::Display) -> ! { eprintln!("fatal error: {msg}"); exit(1) } -macro_rules! show_error { - ($($tt:tt)*) => { $crate::show_error_(&format_args!($($tt)*)) }; +macro_rules! fatal_error { + ($($tt:tt)*) => { $crate::fatal_error_(&format_args!($($tt)*)) }; } -use show_error; +use fatal_error; /// Execute a compiler with the given CLI arguments and callbacks. fn run_compiler_and_exit( @@ -520,7 +520,7 @@ fn main() { params.precise_interior_mut = false; } _ => - show_error!( + fatal_error!( "`-Zmiri-tree-borrows` is required before `-Zmiri-tree-borrows-no-precise-interior-mut`" ), }; @@ -547,7 +547,7 @@ fn main() { "warn-nobacktrace" => miri::IsolatedOp::Reject(miri::RejectOpWith::WarningWithoutBacktrace), _ => - show_error!( + fatal_error!( "-Zmiri-isolation-error must be `abort`, `hide`, `warn`, or `warn-nobacktrace`" ), }; @@ -578,16 +578,16 @@ fn main() { "all" => RetagFields::Yes, "none" => RetagFields::No, "scalar" => RetagFields::OnlyScalar, - _ => show_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), + _ => fatal_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), }; } else if let Some(param) = arg.strip_prefix("-Zmiri-seed=") { let seed = param.parse::().unwrap_or_else(|_| { - show_error!("-Zmiri-seed must be an integer that fits into u64") + fatal_error!("-Zmiri-seed must be an integer that fits into u64") }); miri_config.seed = Some(seed); } else if let Some(param) = arg.strip_prefix("-Zmiri-many-seeds=") { let range = parse_range(param).unwrap_or_else(|err| { - show_error!( + fatal_error!( "-Zmiri-many-seeds requires a range in the form `from..to` or `..to`: {err}" ) }); @@ -604,51 +604,51 @@ fn main() { miri_config.forwarded_env_vars.push(param.to_owned()); } else if let Some(param) = arg.strip_prefix("-Zmiri-env-set=") { let Some((name, value)) = param.split_once('=') else { - show_error!("-Zmiri-env-set requires an argument of the form ="); + fatal_error!("-Zmiri-env-set requires an argument of the form ="); }; miri_config.set_env_vars.insert(name.to_owned(), value.to_owned()); } else if let Some(param) = arg.strip_prefix("-Zmiri-track-pointer-tag=") { let ids: Vec = parse_comma_list(param).unwrap_or_else(|err| { - show_error!("-Zmiri-track-pointer-tag requires a comma separated list of valid `u64` arguments: {err}") + fatal_error!("-Zmiri-track-pointer-tag requires a comma separated list of valid `u64` arguments: {err}") }); for id in ids.into_iter().map(miri::BorTag::new) { if let Some(id) = id { miri_config.tracked_pointer_tags.insert(id); } else { - show_error!("-Zmiri-track-pointer-tag requires nonzero arguments"); + fatal_error!("-Zmiri-track-pointer-tag requires nonzero arguments"); } } } else if let Some(param) = arg.strip_prefix("-Zmiri-track-alloc-id=") { let ids = parse_comma_list::>(param).unwrap_or_else(|err| { - show_error!("-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {err}") + fatal_error!("-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {err}") }); miri_config.tracked_alloc_ids.extend(ids.into_iter().map(miri::AllocId)); } else if arg == "-Zmiri-track-alloc-accesses" { miri_config.track_alloc_accesses = true; } else if let Some(param) = arg.strip_prefix("-Zmiri-address-reuse-rate=") { miri_config.address_reuse_rate = parse_rate(param) - .unwrap_or_else(|err| show_error!("-Zmiri-address-reuse-rate {err}")); + .unwrap_or_else(|err| fatal_error!("-Zmiri-address-reuse-rate {err}")); } else if let Some(param) = arg.strip_prefix("-Zmiri-address-reuse-cross-thread-rate=") { miri_config.address_reuse_cross_thread_rate = parse_rate(param) - .unwrap_or_else(|err| show_error!("-Zmiri-address-reuse-cross-thread-rate {err}")); + .unwrap_or_else(|err| fatal_error!("-Zmiri-address-reuse-cross-thread-rate {err}")); } else if let Some(param) = arg.strip_prefix("-Zmiri-compare-exchange-weak-failure-rate=") { miri_config.cmpxchg_weak_failure_rate = parse_rate(param).unwrap_or_else(|err| { - show_error!("-Zmiri-compare-exchange-weak-failure-rate {err}") + fatal_error!("-Zmiri-compare-exchange-weak-failure-rate {err}") }); } else if let Some(param) = arg.strip_prefix("-Zmiri-preemption-rate=") { - miri_config.preemption_rate = - parse_rate(param).unwrap_or_else(|err| show_error!("-Zmiri-preemption-rate {err}")); + miri_config.preemption_rate = parse_rate(param) + .unwrap_or_else(|err| fatal_error!("-Zmiri-preemption-rate {err}")); } else if arg == "-Zmiri-report-progress" { // This makes it take a few seconds between progress reports on my laptop. miri_config.report_progress = Some(1_000_000); } else if let Some(param) = arg.strip_prefix("-Zmiri-report-progress=") { let interval = param.parse::().unwrap_or_else(|err| { - show_error!("-Zmiri-report-progress requires a `u32`: {}", err) + fatal_error!("-Zmiri-report-progress requires a `u32`: {}", err) }); miri_config.report_progress = Some(interval); } else if let Some(param) = arg.strip_prefix("-Zmiri-provenance-gc=") { let interval = param.parse::().unwrap_or_else(|err| { - show_error!("-Zmiri-provenance-gc requires a `u32`: {}", err) + fatal_error!("-Zmiri-provenance-gc requires a `u32`: {}", err) }); miri_config.gc_interval = interval; } else if let Some(param) = arg.strip_prefix("-Zmiri-measureme=") { @@ -658,7 +658,7 @@ fn main() { "0" => BacktraceStyle::Off, "1" => BacktraceStyle::Short, "full" => BacktraceStyle::Full, - _ => show_error!("-Zmiri-backtrace may only be 0, 1, or full"), + _ => fatal_error!("-Zmiri-backtrace may only be 0, 1, or full"), }; } else if let Some(param) = arg.strip_prefix("-Zmiri-native-lib=") { let filename = param.to_string(); @@ -675,27 +675,27 @@ fn main() { miri_config.native_lib.push(filename.into()); } } else { - show_error!("-Zmiri-native-lib `{}` does not exist", filename); + fatal_error!("-Zmiri-native-lib `{}` does not exist", filename); } } else if arg == "-Zmiri-force-old-native-lib-mode" { miri_config.force_old_native_lib = true; } else if let Some(param) = arg.strip_prefix("-Zmiri-num-cpus=") { let num_cpus = param .parse::() - .unwrap_or_else(|err| show_error!("-Zmiri-num-cpus requires a `u32`: {}", err)); + .unwrap_or_else(|err| fatal_error!("-Zmiri-num-cpus requires a `u32`: {}", err)); if !(1..=miri::MAX_CPUS).contains(&usize::try_from(num_cpus).unwrap()) { - show_error!("-Zmiri-num-cpus must be in the range 1..={}", miri::MAX_CPUS); + fatal_error!("-Zmiri-num-cpus must be in the range 1..={}", miri::MAX_CPUS); } miri_config.num_cpus = num_cpus; } else if let Some(param) = arg.strip_prefix("-Zmiri-force-page-size=") { let page_size = param.parse::().unwrap_or_else(|err| { - show_error!("-Zmiri-force-page-size requires a `u64`: {}", err) + fatal_error!("-Zmiri-force-page-size requires a `u64`: {}", err) }); // Convert from kilobytes to bytes. let page_size = if page_size.is_power_of_two() { page_size * 1024 } else { - show_error!("-Zmiri-force-page-size requires a power of 2: {page_size}"); + fatal_error!("-Zmiri-force-page-size requires a power of 2: {page_size}"); }; miri_config.page_size = Some(page_size); } else { @@ -706,22 +706,22 @@ fn main() { // Tree Borrows implies strict provenance, and is not compatible with native calls. if matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows { .. })) { if miri_config.provenance_mode != ProvenanceMode::Strict { - show_error!( + fatal_error!( "Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance" ); } if !miri_config.native_lib.is_empty() { - show_error!("Tree Borrows is not compatible with calling native functions"); + fatal_error!("Tree Borrows is not compatible with calling native functions"); } } // Native calls and strict provenance are not compatible. if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict { - show_error!("strict provenance is not compatible with calling native functions"); + fatal_error!("strict provenance is not compatible with calling native functions"); } // You can set either one seed or many. if many_seeds.is_some() && miri_config.seed.is_some() { - show_error!("Only one of `-Zmiri-seed` and `-Zmiri-many-seeds can be set"); + fatal_error!("Only one of `-Zmiri-seed` and `-Zmiri-many-seeds can be set"); } // Ensure we have parallelism for many-seeds mode. @@ -737,12 +737,12 @@ fn main() { assert_eq!(genmc_config.is_some(), miri_config.genmc_mode); if genmc_config.is_some() { if !miri_config.data_race_detector { - show_error!("Cannot disable data race detection in GenMC mode (currently)"); + fatal_error!("Cannot disable data race detection in GenMC mode (currently)"); } else if !miri_config.weak_memory_emulation { - show_error!("Cannot disable weak memory emulation in GenMC mode"); + fatal_error!("Cannot disable weak memory emulation in GenMC mode"); } } else if miri_config.weak_memory_emulation && !miri_config.data_race_detector { - show_error!( + fatal_error!( "Weak memory emulation cannot be enabled when the data race detector is disabled" ); }; From 510040fb4447fdb2c40082222e1b8128018a123b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Jul 2025 12:12:24 +0200 Subject: [PATCH 21/35] skip env var memory for leak check --- src/tools/miri/src/concurrency/data_race.rs | 8 ------- src/tools/miri/src/eval.rs | 10 -------- src/tools/miri/src/machine.rs | 6 ++--- src/tools/miri/src/shims/env.rs | 9 ------- src/tools/miri/src/shims/unix/env.rs | 26 +++++---------------- 5 files changed, 9 insertions(+), 50 deletions(-) diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 714eb1fba91c5..b5e7e9d0ac0d8 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -971,14 +971,6 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { } } - /// After all threads are done running, this allows data races to occur for subsequent - /// 'administrative' machine accesses (that logically happen outside of the Abstract Machine). - fn allow_data_races_all_threads_done(&mut self) { - let this = self.eval_context_ref(); - assert!(this.have_all_terminated()); - this.machine.data_race.set_ongoing_action_data_race_free(true); - } - /// Calls the callback with the "release" clock of the current thread. /// Other threads can acquire this clock in the future to establish synchronization /// with this program point. diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 44612dae1ee54..63578912c2422 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -283,16 +283,6 @@ impl<'tcx> MainThreadState<'tcx> { // to be like a global `static`, so that all memory reached by it is considered to "not leak". this.terminate_active_thread(TlsAllocAction::Leak)?; - // Machine cleanup. Only do this if all threads have terminated; threads that are still running - // might cause Stacked Borrows errors (https://github.com/rust-lang/miri/issues/2396). - if this.have_all_terminated() { - // Even if all threads have terminated, we have to beware of data races since some threads - // might not have joined the main thread (https://github.com/rust-lang/miri/issues/2020, - // https://github.com/rust-lang/miri/issues/2508). - this.allow_data_races_all_threads_done(); - EnvVars::cleanup(this).expect("error during env var cleanup"); - } - // Stop interpreter loop. throw_machine_stop!(TerminationInfo::Exit { code: exit_code, leak_check: true }); } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index ebf04c4f04953..4f3dc4853259d 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -130,11 +130,11 @@ pub enum MiriMemoryKind { WinHeap, /// Windows "local" memory (to be freed with `LocalFree`) WinLocal, - /// Memory for args, errno, and other parts of the machine-managed environment. + /// Memory for args, errno, env vars, and other parts of the machine-managed environment. /// This memory may leak. Machine, - /// Memory allocated by the runtime (e.g. env vars). Separate from `Machine` - /// because we clean it up and leak-check it. + /// Memory allocated by the runtime, e.g. for readdir. Separate from `Machine` because we clean + /// it up (or expect the user to invoke operations that clean it up) and leak-check it. Runtime, /// Globals copied from `tcx`. /// This memory may leak. diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs index 9dfb1ebb90a82..689cd3a7269cc 100644 --- a/src/tools/miri/src/shims/env.rs +++ b/src/tools/miri/src/shims/env.rs @@ -59,15 +59,6 @@ impl<'tcx> EnvVars<'tcx> { interp_ok(()) } - pub(crate) fn cleanup(ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>) -> InterpResult<'tcx> { - let this = ecx.eval_context_mut(); - match this.machine.env_vars { - EnvVars::Unix(_) => UnixEnvVars::cleanup(this), - EnvVars::Windows(_) => interp_ok(()), // no cleanup needed - EnvVars::Uninit => interp_ok(()), - } - } - pub(crate) fn unix(&self) -> &UnixEnvVars<'tcx> { match self { EnvVars::Unix(env) => env, diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index a0e5d3f0127ea..eb4365e20042c 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -1,6 +1,6 @@ +use std::env; use std::ffi::{OsStr, OsString}; use std::io::ErrorKind; -use std::{env, mem}; use rustc_abi::{FieldIdx, Size}; use rustc_data_structures::fx::FxHashMap; @@ -50,20 +50,6 @@ impl<'tcx> UnixEnvVars<'tcx> { interp_ok(UnixEnvVars { map: env_vars_machine, environ }) } - pub(crate) fn cleanup(ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>) -> InterpResult<'tcx> { - // Deallocate individual env vars. - let env_vars = mem::take(&mut ecx.machine.env_vars.unix_mut().map); - for (_name, ptr) in env_vars { - ecx.deallocate_ptr(ptr, None, MiriMemoryKind::Runtime.into())?; - } - // Deallocate environ var list. - let environ = &ecx.machine.env_vars.unix().environ; - let old_vars_ptr = ecx.read_pointer(environ)?; - ecx.deallocate_ptr(old_vars_ptr, None, MiriMemoryKind::Runtime.into())?; - - interp_ok(()) - } - pub(crate) fn environ(&self) -> Pointer { self.environ.ptr() } @@ -112,7 +98,7 @@ fn alloc_env_var<'tcx>( let mut name_osstring = name.to_os_string(); name_osstring.push("="); name_osstring.push(value); - ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Runtime.into()) + ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()) } /// Allocates an `environ` block with the given list of pointers. @@ -128,7 +114,7 @@ fn alloc_environ_block<'tcx>( ecx.machine.layouts.mut_raw_ptr.ty, u64::try_from(vars.len()).unwrap(), ))?; - let vars_place = ecx.allocate(vars_layout, MiriMemoryKind::Runtime.into())?; + let vars_place = ecx.allocate(vars_layout, MiriMemoryKind::Machine.into())?; for (idx, var) in vars.into_iter_enumerated() { let place = ecx.project_field(&vars_place, idx)?; ecx.write_pointer(var, &place)?; @@ -171,7 +157,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let Some((name, value)) = new { let var_ptr = alloc_env_var(this, &name, &value)?; if let Some(var) = this.machine.env_vars.unix_mut().map.insert(name, var_ptr) { - this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?; + this.deallocate_ptr(var, None, MiriMemoryKind::Machine.into())?; } this.update_environ()?; interp_ok(Scalar::from_i32(0)) // return zero on success @@ -195,7 +181,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } if let Some(old) = success { if let Some(var) = old { - this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?; + this.deallocate_ptr(var, None, MiriMemoryKind::Machine.into())?; } this.update_environ()?; interp_ok(Scalar::from_i32(0)) @@ -253,7 +239,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Deallocate the old environ list. let environ = this.machine.env_vars.unix().environ.clone(); let old_vars_ptr = this.read_pointer(&environ)?; - this.deallocate_ptr(old_vars_ptr, None, MiriMemoryKind::Runtime.into())?; + this.deallocate_ptr(old_vars_ptr, None, MiriMemoryKind::Machine.into())?; // Write the new list. let vals = this.machine.env_vars.unix().map.values().copied().collect(); From 8fc0fd5a8016997b3d4e47efebb7f16893e9fbd1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Jul 2025 15:16:03 +0200 Subject: [PATCH 22/35] use more clever approach for genmc conditional import --- src/tools/miri/src/concurrency/mod.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/tools/miri/src/concurrency/mod.rs b/src/tools/miri/src/concurrency/mod.rs index 17d0f3f5ff67e..49bcc0d30b506 100644 --- a/src/tools/miri/src/concurrency/mod.rs +++ b/src/tools/miri/src/concurrency/mod.rs @@ -9,18 +9,9 @@ mod vector_clock; pub mod weak_memory; // Import either the real genmc adapter or a dummy module. -cfg_select! { - feature = "genmc" => { - mod genmc; - pub use self::genmc::{GenmcCtx, GenmcConfig}; - } - _ => { - #[path = "genmc/dummy.rs"] - mod genmc_dummy; - use self::genmc_dummy as genmc; - pub use self::genmc::{GenmcCtx, GenmcConfig}; - } -} +#[cfg_attr(not(feature = "genmc"), path = "genmc/dummy.rs")] +mod genmc; pub use self::data_race_handler::{AllocDataRaceHandler, GlobalDataRaceHandler}; +pub use self::genmc::{GenmcConfig, GenmcCtx}; pub use self::vector_clock::VClock; From db617afe8b188e3c08a26f40a3662833a38e8c3c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 3 Jul 2025 08:40:04 +0200 Subject: [PATCH 23/35] only set host-specific CC; improve and de-duplicate native libs testing logic --- src/tools/miri/.github/workflows/ci.yml | 2 +- .../tests/native-lib/pass/ptr_read_access.rs | 4 --- .../tests/native-lib/pass/ptr_write_access.rs | 3 -- .../tests/native-lib/pass/scalar_arguments.rs | 4 --- src/tools/miri/tests/ui.rs | 30 +++++++++++-------- 5 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index ed4bcc0dd3b93..11c0f08debe6f 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -74,7 +74,7 @@ jobs: run: | sudo apt install gcc-${{ matrix.gcc_cross }} echo "Setting environment variables:" - echo "CC=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV + echo "CC_${{ matrix.host_target }}=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV TARGET_UPPERCASE=$(echo ${{ matrix.host_target }} | tr '[:lower:]-' '[:upper:]_') echo "CARGO_TARGET_${TARGET_UPPERCASE}_LINKER=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs index 3ccfecc6fb379..4c85213536785 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs @@ -1,7 +1,3 @@ -// Only works on Unix targets -//@ignore-target: windows wasm -//@only-on-host - fn main() { test_access_pointer(); test_access_simple(); diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index bd4e0b23601f1..86a9c97f4cecc 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -1,6 +1,3 @@ -// Only works on Unix targets -//@ignore-target: windows wasm -//@only-on-host //@compile-flags: -Zmiri-permissive-provenance #![feature(box_as_ptr)] diff --git a/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs b/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs index c896bd8dd345f..9e99977a692a7 100644 --- a/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs +++ b/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs @@ -1,7 +1,3 @@ -// Only works on Unix targets -//@ignore-target: windows wasm -//@only-on-host - extern "C" { fn add_one_int(x: i32) -> i32; fn add_int16(x: i16) -> i16; diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 46472b51f9cd3..5239f8338ee1a 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -29,20 +29,17 @@ fn miri_path() -> PathBuf { PathBuf::from(env::var("MIRI").unwrap_or_else(|_| env!("CARGO_BIN_EXE_miri").into())) } -fn get_host() -> String { - rustc_version::VersionMeta::for_command(std::process::Command::new(miri_path())) - .expect("failed to parse rustc version info") - .host -} - pub fn flagsplit(flags: &str) -> Vec { // This code is taken from `RUSTFLAGS` handling in cargo. flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() } // Build the shared object file for testing native function calls. -fn build_native_lib() -> PathBuf { - let cc = env::var("CC").unwrap_or_else(|_| "cc".into()); +fn build_native_lib(target: &str) -> PathBuf { + // Loosely follow the logic of the `cc` crate for finding the compiler. + let cc = env::var(format!("CC_{target}")) + .or_else(|_| env::var("CC")) + .unwrap_or_else(|_| "cc".into()); // Target directory that we can write to. let so_target_dir = Path::new(env!("CARGO_TARGET_TMPDIR")).join("miri-native-lib"); // Create the directory if it does not already exist. @@ -201,7 +198,7 @@ fn run_tests( // If we're testing the native-lib functionality, then build the shared object file for testing // external C function calls and push the relevant compiler flag. if path.starts_with("tests/native-lib/") { - let native_lib = build_native_lib(); + let native_lib = build_native_lib(target); let mut flag = std::ffi::OsString::from("-Zmiri-native-lib="); flag.push(native_lib.into_os_string()); config.program.args.push(flag); @@ -305,14 +302,21 @@ fn ui( .with_context(|| format!("ui tests in {path} for {target} failed")) } -fn get_target() -> String { - env::var("MIRI_TEST_TARGET").ok().unwrap_or_else(get_host) +fn get_host() -> String { + rustc_version::VersionMeta::for_command(std::process::Command::new(miri_path())) + .expect("failed to parse rustc version info") + .host +} + +fn get_target(host: &str) -> String { + env::var("MIRI_TEST_TARGET").ok().unwrap_or_else(|| host.to_owned()) } fn main() -> Result<()> { ui_test::color_eyre::install()?; - let target = get_target(); + let host = get_host(); + let target = get_target(&host); let tmpdir = tempfile::Builder::new().prefix("miri-uitest-").tempdir()?; let mut args = std::env::args_os(); @@ -329,7 +333,7 @@ fn main() -> Result<()> { ui(Mode::Panic, "tests/panic", &target, WithDependencies, tmpdir.path())?; ui(Mode::Fail, "tests/fail", &target, WithoutDependencies, tmpdir.path())?; ui(Mode::Fail, "tests/fail-dep", &target, WithDependencies, tmpdir.path())?; - if cfg!(unix) { + if cfg!(unix) && target == host { ui(Mode::Pass, "tests/native-lib/pass", &target, WithoutDependencies, tmpdir.path())?; ui(Mode::Fail, "tests/native-lib/fail", &target, WithoutDependencies, tmpdir.path())?; } From 2cbbd6c9f555ffec7bd48d74f5c71996de9945c6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 3 Jul 2025 10:28:05 +0200 Subject: [PATCH 24/35] 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 18ebea643010c..38f228aa8f67a 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -f51c9870bab634afb9e7a262b6ca7816bb9e940d +6268d0aa34b46981533b09827c1454b8cf27e032 From 7ea812fd54f04070d16a5f69ff3b034814bf72b8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 3 Jul 2025 17:47:34 +0200 Subject: [PATCH 25/35] nanosleep: fix argument name and add a missing argument read --- src/tools/miri/src/shims/time.rs | 11 ++++------- src/tools/miri/src/shims/unix/foreign_items.rs | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index 28f4ca5bb1b76..2fc42c13edd34 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -330,18 +330,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(Scalar::from_i32(0)) // KERN_SUCCESS } - fn nanosleep( - &mut self, - req_op: &OpTy<'tcx>, - _rem: &OpTy<'tcx>, // Signal handlers are not supported, so rem will never be written to. - ) -> InterpResult<'tcx, Scalar> { + fn nanosleep(&mut self, duration: &OpTy<'tcx>, rem: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); this.assert_target_os_is_unix("nanosleep"); - let req = this.deref_pointer_as(req_op, this.libc_ty_layout("timespec"))?; + let duration = this.deref_pointer_as(duration, this.libc_ty_layout("timespec"))?; + let _rem = this.read_pointer(rem)?; // Signal handlers are not supported, so rem will never be written to. - let duration = match this.read_timespec(&req)? { + let duration = match this.read_timespec(&duration)? { Some(duration) => duration, None => { return this.set_last_error_and_return_i32(LibcError("EINVAL")); diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index b3c58397a02bd..438a9b420be6b 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -963,8 +963,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_null(dest)?; } "nanosleep" => { - let [req, rem] = this.check_shim(abi, CanonAbi::C, link_name, args)?; - let result = this.nanosleep(req, rem)?; + let [duration, rem] = this.check_shim(abi, CanonAbi::C, link_name, args)?; + let result = this.nanosleep(duration, rem)?; this.write_scalar(result, dest)?; } "sched_getaffinity" => { From 04522bb89c7cc55b475a2b5216d8b461524147ec Mon Sep 17 00:00:00 2001 From: The Miri Cronjob Bot Date: Fri, 4 Jul 2025 04:57:31 +0000 Subject: [PATCH 26/35] 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 38f228aa8f67a..5a4378e78f91b 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -6268d0aa34b46981533b09827c1454b8cf27e032 +837c5dd7de03aa97190593aef4e70d53e1bb574b From 0d656e0de5414889f71270b3e14512ea9c189e5f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 4 Jul 2025 15:05:57 +0200 Subject: [PATCH 27/35] declare data race and weak memory support as non-experimental --- src/tools/miri/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index e609fb9b6f9e4..b05acff72b50a 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -11,12 +11,12 @@ instance: * Not sufficiently aligned memory accesses and references * Violation of basic type invariants (a `bool` that is not 0 or 1, for example, or an invalid enum discriminant) +* Data races and emulation of *some* weak memory effects, i.e., + atomic reads can return outdated values * **Experimental**: Violations of the [Stacked Borrows] rules governing aliasing for reference types * **Experimental**: Violations of the [Tree Borrows] aliasing rules, as an optional alternative to [Stacked Borrows] -* **Experimental**: Data races and emulation of weak memory effects, i.e., - atomic reads can return outdated values. On top of that, Miri will also tell you about memory leaks: when there is memory still allocated at the end of the execution, and that memory is not reachable From 1620117f94914c62e09b47ce25fe3cd15c4e132c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Jul 2025 09:01:43 +0200 Subject: [PATCH 28/35] 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 5a4378e78f91b..0130cf13a45f7 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -837c5dd7de03aa97190593aef4e70d53e1bb574b +733b47ea4b1b86216f14ef56e49440c33933f230 From 72043e3181a4050f02cdb1d6e2574c54c1b47013 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Jul 2025 09:12:36 +0200 Subject: [PATCH 29/35] fmt --- src/tools/miri/src/shims/foreign_items.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 1b66735ddb18a..9ddba8c2b48d8 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -615,7 +615,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // This is a no-op shim that only exists to prevent making the allocator shims instantly stable. let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?; } - name if name == this.mangle_internal_symbol("__rust_alloc_error_handler_should_panic_v2") => { + name if name + == this.mangle_internal_symbol("__rust_alloc_error_handler_should_panic_v2") => + { // Gets the value of the `oom` option. let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?; let val = this.tcx.sess.opts.unstable_opts.oom.should_panic(); From a1f4172afa99af14782478c9fa5068141ef4033b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Jul 2025 10:39:03 +0200 Subject: [PATCH 30/35] update lockfile --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 34fc0860a4b75..8135f56e8deef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2394,6 +2394,7 @@ dependencies = [ "regex", "rustc_version", "serde", + "serde_json", "smallvec", "tempfile", "tikv-jemalloc-sys", From 44d7238d86d7c58203e522334511d83a5f7626ff Mon Sep 17 00:00:00 2001 From: Adrian Friedli Date: Sat, 5 Jul 2025 13:24:24 +0200 Subject: [PATCH 31/35] remove armv5te-unknown-linux-gnueabi target maintainer --- .../rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md b/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md index 1baf1049994b1..0aebbc34d400b 100644 --- a/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md +++ b/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md @@ -7,7 +7,7 @@ floating-point units. ## Target maintainers -[@koalatux](https://github.com/koalatux) +There are currently no formally documented target maintainers. ## Requirements From 90762c26b67f36a7d93a07fd90b2c354dd2be2a0 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 5 Jul 2025 12:37:17 +0000 Subject: [PATCH 32/35] Complete mut_visit. --- compiler/rustc_ast/src/visit.rs | 90 +++++++++++++++++++++++++++++---- 1 file changed, 79 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 5dd6882b02568..f8ecff69a7635 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -1202,9 +1202,10 @@ macro_rules! common_visitor_and_walkers { let TyPat { id, kind, span, tokens: _ } = tp; try_visit!(visit_id(vis, id)); match kind { - TyPatKind::Range(start, end, _include_end) => { + TyPatKind::Range(start, end, Spanned { span, node: _include_end }) => { visit_opt!(vis, visit_anon_const, start); visit_opt!(vis, visit_anon_const, end); + try_visit!(visit_span(vis, span)); } TyPatKind::Or(variants) => walk_list!(vis, visit_ty_pat, variants), TyPatKind::Err(_) => {} @@ -1523,16 +1524,26 @@ macro_rules! common_visitor_and_walkers { } pub fn walk_inline_asm<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, asm: &$($lt)? $($mut)? InlineAsm) -> V::Result { - // FIXME: Visit spans inside all this currently ignored stuff. let InlineAsm { asm_macro: _, - template: _, - template_strs: _, + template, + template_strs, operands, - clobber_abis: _, + clobber_abis, options: _, - line_spans: _, + line_spans, } = asm; + for piece in template { + match piece { + InlineAsmTemplatePiece::String(_str) => {} + InlineAsmTemplatePiece::Placeholder { operand_idx: _, modifier: _, span } => { + try_visit!(visit_span(vis, span)); + } + } + } + for (_s1, _s2, span) in template_strs { + try_visit!(visit_span(vis, span)); + } for (op, span) in operands { match op { InlineAsmOperand::In { expr, reg: _ } @@ -1553,6 +1564,12 @@ macro_rules! common_visitor_and_walkers { } try_visit!(visit_span(vis, span)); } + for (_s1, span) in clobber_abis { + try_visit!(visit_span(vis, span)) + } + for span in line_spans { + try_visit!(visit_span(vis, span)) + } V::Result::output() } @@ -1565,9 +1582,9 @@ macro_rules! common_visitor_and_walkers { vis.visit_path(path) } - // FIXME: visit the template exhaustively. pub fn walk_format_args<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, fmt: &$($lt)? $($mut)? FormatArgs) -> V::Result { - let FormatArgs { span, template: _, arguments, uncooked_fmt_str: _, is_source_literal: _ } = fmt; + let FormatArgs { span, template, arguments, uncooked_fmt_str: _, is_source_literal: _ } = fmt; + let args = $(${ignore($mut)} arguments.all_args_mut())? $(${ignore($lt)} arguments.all_args())? ; for FormatArgument { kind, expr } in args { match kind { @@ -1578,9 +1595,58 @@ macro_rules! common_visitor_and_walkers { } try_visit!(vis.visit_expr(expr)); } + for piece in template { + match piece { + FormatArgsPiece::Literal(_symbol) => {} + FormatArgsPiece::Placeholder(placeholder) => try_visit!(walk_format_placeholder(vis, placeholder)), + } + } visit_span(vis, span) } + fn walk_format_placeholder<$($lt,)? V: $Visitor$(<$lt>)?>( + vis: &mut V, + placeholder: &$($lt)? $($mut)? FormatPlaceholder, + ) -> V::Result { + let FormatPlaceholder { argument, span, format_options, format_trait: _ } = placeholder; + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + let FormatArgPosition { span, index: _, kind: _ } = argument; + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + let FormatOptions { + width, + precision, + alignment: _, + fill: _, + sign: _, + alternate: _, + zero_pad: _, + debug_hex: _, + } = format_options; + match width { + None => {} + Some(FormatCount::Literal(_)) => {} + Some(FormatCount::Argument(FormatArgPosition { span, index: _, kind: _ })) => { + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + } + } + match precision { + None => {} + Some(FormatCount::Literal(_)) => {} + Some(FormatCount::Argument(FormatArgPosition { span, index: _, kind: _ })) => { + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + } + } + V::Result::output() + } + pub fn walk_expr<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, expression: &$($lt)? $($mut)? Expr) -> V::Result { let Expr { id, kind, span, attrs, tokens: _ } = expression; try_visit!(visit_id(vis, id)); @@ -1601,7 +1667,7 @@ macro_rules! common_visitor_and_walkers { try_visit!(visit_expr_fields(vis, fields)); match rest { StructRest::Base(expr) => try_visit!(vis.visit_expr(expr)), - StructRest::Rest(_span) => {} + StructRest::Rest(span) => try_visit!(visit_span(vis, span)), StructRest::None => {} } } @@ -1688,7 +1754,8 @@ macro_rules! common_visitor_and_walkers { visit_opt!(vis, visit_label, opt_label); try_visit!(vis.visit_block(block)); } - ExprKind::Gen(_capt, body, _kind, decl_span) => { + ExprKind::Gen(capture_clause, body, _kind, decl_span) => { + try_visit!(vis.visit_capture_by(capture_clause)); try_visit!(vis.visit_block(body)); try_visit!(visit_span(vis, decl_span)); } @@ -1705,9 +1772,10 @@ macro_rules! common_visitor_and_walkers { try_visit!(vis.visit_expr(rhs)); try_visit!(visit_span(vis, span)); } - ExprKind::AssignOp(_op, left_expression, right_expression) => { + ExprKind::AssignOp(Spanned { span, node: _ }, left_expression, right_expression) => { try_visit!(vis.visit_expr(left_expression)); try_visit!(vis.visit_expr(right_expression)); + try_visit!(visit_span(vis, span)); } ExprKind::Field(subexpression, ident) => { try_visit!(vis.visit_expr(subexpression)); From 39ee1b2d774d4be6959e3b045052abef54a29e0b Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 5 Jul 2025 15:11:09 +0000 Subject: [PATCH 33/35] Remove yields_in_scope from the scope tree. --- .../rustc_hir_analysis/src/check/region.rs | 205 ++++-------------- compiler/rustc_middle/src/middle/region.rs | 92 -------- .../src/builder/expr/as_rvalue.rs | 3 - 3 files changed, 37 insertions(+), 263 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index c458878da15b5..288105dfba714 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -15,7 +15,6 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{Arm, Block, Expr, LetStmt, Pat, PatKind, Stmt}; use rustc_index::Idx; -use rustc_middle::bug; use rustc_middle::middle::region::*; use rustc_middle::ty::TyCtxt; use rustc_session::lint; @@ -34,14 +33,6 @@ struct Context { struct ScopeResolutionVisitor<'tcx> { tcx: TyCtxt<'tcx>, - // The number of expressions and patterns visited in the current body. - expr_and_pat_count: usize, - // When this is `true`, we record the `Scopes` we encounter - // when processing a Yield expression. This allows us to fix - // up their indices. - pessimistic_yield: bool, - // Stores scopes when `pessimistic_yield` is `true`. - fixup_scopes: Vec, // The generated scope tree. scope_tree: ScopeTree, @@ -199,19 +190,14 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir: visitor.cx = prev_cx; } +#[tracing::instrument(level = "debug", skip(visitor))] fn resolve_pat<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, pat: &'tcx hir::Pat<'tcx>) { // If this is a binding then record the lifetime of that binding. if let PatKind::Binding(..) = pat.kind { record_var_lifetime(visitor, pat.hir_id.local_id); } - debug!("resolve_pat - pre-increment {} pat = {:?}", visitor.expr_and_pat_count, pat); - intravisit::walk_pat(visitor, pat); - - visitor.expr_and_pat_count += 1; - - debug!("resolve_pat - post-increment {} pat = {:?}", visitor.expr_and_pat_count, pat); } fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hir::Stmt<'tcx>) { @@ -243,68 +229,15 @@ fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hi } } +#[tracing::instrument(level = "debug", skip(visitor))] fn resolve_expr<'tcx>( visitor: &mut ScopeResolutionVisitor<'tcx>, expr: &'tcx hir::Expr<'tcx>, terminating: bool, ) { - debug!("resolve_expr - pre-increment {} expr = {:?}", visitor.expr_and_pat_count, expr); - let prev_cx = visitor.cx; visitor.enter_node_scope_with_dtor(expr.hir_id.local_id, terminating); - let prev_pessimistic = visitor.pessimistic_yield; - - // Ordinarily, we can rely on the visit order of HIR intravisit - // to correspond to the actual execution order of statements. - // However, there's a weird corner case with compound assignment - // operators (e.g. `a += b`). The evaluation order depends on whether - // or not the operator is overloaded (e.g. whether or not a trait - // like AddAssign is implemented). - - // For primitive types (which, despite having a trait impl, don't actually - // end up calling it), the evaluation order is right-to-left. For example, - // the following code snippet: - // - // let y = &mut 0; - // *{println!("LHS!"); y} += {println!("RHS!"); 1}; - // - // will print: - // - // RHS! - // LHS! - // - // However, if the operator is used on a non-primitive type, - // the evaluation order will be left-to-right, since the operator - // actually get desugared to a method call. For example, this - // nearly identical code snippet: - // - // let y = &mut String::new(); - // *{println!("LHS String"); y} += {println!("RHS String"); "hi"}; - // - // will print: - // LHS String - // RHS String - // - // To determine the actual execution order, we need to perform - // trait resolution. Unfortunately, we need to be able to compute - // yield_in_scope before type checking is even done, as it gets - // used by AST borrowcheck. - // - // Fortunately, we don't need to know the actual execution order. - // It suffices to know the 'worst case' order with respect to yields. - // Specifically, we need to know the highest 'expr_and_pat_count' - // that we could assign to the yield expression. To do this, - // we pick the greater of the two values from the left-hand - // and right-hand expressions. This makes us overly conservative - // about what types could possibly live across yield points, - // but we will never fail to detect that a type does actually - // live across a yield point. The latter part is critical - - // we're already overly conservative about what types will live - // across yield points, as the generated MIR will determine - // when things are actually live. However, for typecheck to work - // properly, we can't miss any types. - match expr.kind { // Conditional or repeating scopes are always terminating // scopes, meaning that temporaries cannot outlive them. @@ -360,55 +293,42 @@ fn resolve_expr<'tcx>( let body = visitor.tcx.hir_body(body); visitor.visit_body(body); } + // Ordinarily, we can rely on the visit order of HIR intravisit + // to correspond to the actual execution order of statements. + // However, there's a weird corner case with compound assignment + // operators (e.g. `a += b`). The evaluation order depends on whether + // or not the operator is overloaded (e.g. whether or not a trait + // like AddAssign is implemented). + // + // For primitive types (which, despite having a trait impl, don't actually + // end up calling it), the evaluation order is right-to-left. For example, + // the following code snippet: + // + // let y = &mut 0; + // *{println!("LHS!"); y} += {println!("RHS!"); 1}; + // + // will print: + // + // RHS! + // LHS! + // + // However, if the operator is used on a non-primitive type, + // the evaluation order will be left-to-right, since the operator + // actually get desugared to a method call. For example, this + // nearly identical code snippet: + // + // let y = &mut String::new(); + // *{println!("LHS String"); y} += {println!("RHS String"); "hi"}; + // + // will print: + // LHS String + // RHS String + // + // To determine the actual execution order, we need to perform + // trait resolution. Fortunately, we don't need to know the actual execution order. hir::ExprKind::AssignOp(_, left_expr, right_expr) => { - debug!( - "resolve_expr - enabling pessimistic_yield, was previously {}", - prev_pessimistic - ); - - let start_point = visitor.fixup_scopes.len(); - visitor.pessimistic_yield = true; - - // If the actual execution order turns out to be right-to-left, - // then we're fine. However, if the actual execution order is left-to-right, - // then we'll assign too low a count to any `yield` expressions - // we encounter in 'right_expression' - they should really occur after all of the - // expressions in 'left_expression'. visitor.visit_expr(right_expr); - visitor.pessimistic_yield = prev_pessimistic; - - debug!("resolve_expr - restoring pessimistic_yield to {}", prev_pessimistic); visitor.visit_expr(left_expr); - debug!("resolve_expr - fixing up counts to {}", visitor.expr_and_pat_count); - - // Remove and process any scopes pushed by the visitor - let target_scopes = visitor.fixup_scopes.drain(start_point..); - - for scope in target_scopes { - let yield_data = - visitor.scope_tree.yield_in_scope.get_mut(&scope).unwrap().last_mut().unwrap(); - let count = yield_data.expr_and_pat_count; - let span = yield_data.span; - - // expr_and_pat_count never decreases. Since we recorded counts in yield_in_scope - // before walking the left-hand side, it should be impossible for the recorded - // count to be greater than the left-hand side count. - if count > visitor.expr_and_pat_count { - bug!( - "Encountered greater count {} at span {:?} - expected no greater than {}", - count, - span, - visitor.expr_and_pat_count - ); - } - let new_count = visitor.expr_and_pat_count; - debug!( - "resolve_expr - increasing count for scope {:?} from {} to {} at span {:?}", - scope, count, new_count, span - ); - - yield_data.expr_and_pat_count = new_count; - } } hir::ExprKind::If(cond, then, Some(otherwise)) => { @@ -453,43 +373,6 @@ fn resolve_expr<'tcx>( _ => intravisit::walk_expr(visitor, expr), } - visitor.expr_and_pat_count += 1; - - debug!("resolve_expr post-increment {}, expr = {:?}", visitor.expr_and_pat_count, expr); - - if let hir::ExprKind::Yield(_, source) = &expr.kind { - // Mark this expr's scope and all parent scopes as containing `yield`. - let mut scope = Scope { local_id: expr.hir_id.local_id, data: ScopeData::Node }; - loop { - let data = YieldData { - span: expr.span, - expr_and_pat_count: visitor.expr_and_pat_count, - source: *source, - }; - match visitor.scope_tree.yield_in_scope.get_mut(&scope) { - Some(yields) => yields.push(data), - None => { - visitor.scope_tree.yield_in_scope.insert(scope, vec![data]); - } - } - - if visitor.pessimistic_yield { - debug!("resolve_expr in pessimistic_yield - marking scope {:?} for fixup", scope); - visitor.fixup_scopes.push(scope); - } - - // Keep traversing up while we can. - match visitor.scope_tree.parent_map.get(&scope) { - // Don't cross from closure bodies to their parent. - Some(&superscope) => match superscope.data { - ScopeData::CallSite => break, - _ => scope = superscope, - }, - None => break, - } - } - } - visitor.cx = prev_cx; } @@ -612,8 +495,8 @@ fn resolve_local<'tcx>( } } - // Make sure we visit the initializer first, so expr_and_pat_count remains correct. - // The correct order, as shared between coroutine_interior, drop_ranges and intravisitor, + // Make sure we visit the initializer first. + // The correct order, as shared between drop_ranges and intravisitor, // is to walk initializer, followed by pattern bindings, finally followed by the `else` block. if let Some(expr) = init { visitor.visit_expr(expr); @@ -798,16 +681,7 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> { } fn enter_body(&mut self, hir_id: hir::HirId, f: impl FnOnce(&mut Self)) { - // Save all state that is specific to the outer function - // body. These will be restored once down below, once we've - // visited the body. - let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0); let outer_cx = self.cx; - // The 'pessimistic yield' flag is set to true when we are - // processing a `+=` statement and have to make pessimistic - // control flow assumptions. This doesn't apply to nested - // bodies within the `+=` statements. See #69307. - let outer_pessimistic_yield = mem::replace(&mut self.pessimistic_yield, false); self.enter_scope(Scope { local_id: hir_id.local_id, data: ScopeData::CallSite }); self.enter_scope(Scope { local_id: hir_id.local_id, data: ScopeData::Arguments }); @@ -815,9 +689,7 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> { f(self); // Restore context we had at the start. - self.expr_and_pat_count = outer_ec; self.cx = outer_cx; - self.pessimistic_yield = outer_pessimistic_yield; } } @@ -919,10 +791,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree { let mut visitor = ScopeResolutionVisitor { tcx, scope_tree: ScopeTree::default(), - expr_and_pat_count: 0, cx: Context { parent: None, var_parent: None }, - pessimistic_yield: false, - fixup_scopes: vec![], extended_super_lets: Default::default(), }; diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 92eab59dd0274..0f5b63f5c1d24 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -7,7 +7,6 @@ //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/borrow_check.html use std::fmt; -use std::ops::Deref; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::unord::UnordMap; @@ -228,82 +227,6 @@ pub struct ScopeTree { /// This information is used later for linting to identify locals and /// temporary values that will receive backwards-incompatible drop orders. pub backwards_incompatible_scope: UnordMap, - - /// If there are any `yield` nested within a scope, this map - /// stores the `Span` of the last one and its index in the - /// postorder of the Visitor traversal on the HIR. - /// - /// HIR Visitor postorder indexes might seem like a peculiar - /// thing to care about. but it turns out that HIR bindings - /// and the temporary results of HIR expressions are never - /// storage-live at the end of HIR nodes with postorder indexes - /// lower than theirs, and therefore don't need to be suspended - /// at yield-points at these indexes. - /// - /// For an example, suppose we have some code such as: - /// ```rust,ignore (example) - /// foo(f(), yield y, bar(g())) - /// ``` - /// - /// With the HIR tree (calls numbered for expository purposes) - /// - /// ```text - /// Call#0(foo, [Call#1(f), Yield(y), Call#2(bar, Call#3(g))]) - /// ``` - /// - /// Obviously, the result of `f()` was created before the yield - /// (and therefore needs to be kept valid over the yield) while - /// the result of `g()` occurs after the yield (and therefore - /// doesn't). If we want to infer that, we can look at the - /// postorder traversal: - /// ```plain,ignore - /// `foo` `f` Call#1 `y` Yield `bar` `g` Call#3 Call#2 Call#0 - /// ``` - /// - /// In which we can easily see that `Call#1` occurs before the yield, - /// and `Call#3` after it. - /// - /// To see that this method works, consider: - /// - /// Let `D` be our binding/temporary and `U` be our other HIR node, with - /// `HIR-postorder(U) < HIR-postorder(D)`. Suppose, as in our example, - /// U is the yield and D is one of the calls. - /// Let's show that `D` is storage-dead at `U`. - /// - /// Remember that storage-live/storage-dead refers to the state of - /// the *storage*, and does not consider moves/drop flags. - /// - /// Then: - /// - /// 1. From the ordering guarantee of HIR visitors (see - /// `rustc_hir::intravisit`), `D` does not dominate `U`. - /// - /// 2. Therefore, `D` is *potentially* storage-dead at `U` (because - /// we might visit `U` without ever getting to `D`). - /// - /// 3. However, we guarantee that at each HIR point, each - /// binding/temporary is always either always storage-live - /// or always storage-dead. This is what is being guaranteed - /// by `terminating_scopes` including all blocks where the - /// count of executions is not guaranteed. - /// - /// 4. By `2.` and `3.`, `D` is *statically* storage-dead at `U`, - /// QED. - /// - /// This property ought to not on (3) in an essential way -- it - /// is probably still correct even if we have "unrestricted" terminating - /// scopes. However, why use the complicated proof when a simple one - /// works? - /// - /// A subtle thing: `box` expressions, such as `box (&x, yield 2, &y)`. It - /// might seem that a `box` expression creates a `Box` temporary - /// when it *starts* executing, at `HIR-preorder(BOX-EXPR)`. That might - /// be true in the MIR desugaring, but it is not important in the semantics. - /// - /// The reason is that semantically, until the `box` expression returns, - /// the values are still owned by their containing expressions. So - /// we'll see that `&x`. - pub yield_in_scope: UnordMap>, } /// See the `rvalue_candidates` field for more information on rvalue @@ -316,15 +239,6 @@ pub struct RvalueCandidate { pub lifetime: Option, } -#[derive(Debug, Copy, Clone, HashStable)] -pub struct YieldData { - /// The `Span` of the yield. - pub span: Span, - /// The number of expressions and patterns appearing before the `yield` in the body, plus one. - pub expr_and_pat_count: usize, - pub source: hir::YieldSource, -} - impl ScopeTree { pub fn record_scope_parent(&mut self, child: Scope, parent: Option) { debug!("{:?}.parent = {:?}", child, parent); @@ -380,10 +294,4 @@ impl ScopeTree { true } - - /// Checks whether the given scope contains a `yield`. If so, - /// returns `Some(YieldData)`. If not, returns `None`. - pub fn yield_in_scope(&self, scope: Scope) -> Option<&[YieldData]> { - self.yield_in_scope.get(&scope).map(Deref::deref) - } } diff --git a/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs index 975226bb642a7..daf8fa5f19eee 100644 --- a/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs @@ -171,9 +171,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.diverge_from(block); block = success; - // The `Box` temporary created here is not a part of the HIR, - // and therefore is not considered during coroutine auto-trait - // determination. See the comment about `box` at `yield_in_scope`. let result = this.local_decls.push(LocalDecl::new(expr.ty, expr_span)); this.cfg .push(block, Statement::new(source_info, StatementKind::StorageLive(result))); From 8eb9f709793405b03a28e827bfe83146230473b6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 5 Jul 2025 18:37:08 +0000 Subject: [PATCH 34/35] Stop using Key trait randomly --- .../rustc_const_eval/src/const_eval/mod.rs | 5 ++-- compiler/rustc_lint/src/map_unit_fn.rs | 5 ++-- tests/ui/lint/lint_map_unit_fn.stderr | 25 +++++++++---------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index d95d552d7d54e..0082f90f3b8cb 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -1,9 +1,9 @@ // Not in interpret to make sure we do not use private implementation details use rustc_abi::{FieldIdx, VariantIdx}; -use rustc_middle::query::Key; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::{bug, mir}; +use rustc_span::DUMMY_SP; use tracing::instrument; use crate::interpret::InterpCx; @@ -71,8 +71,7 @@ pub fn tag_for_variant_provider<'tcx>( let (ty, variant_index) = key.value; assert!(ty.is_enum()); - let ecx = - InterpCx::new(tcx, ty.default_span(tcx), key.typing_env, crate::const_eval::DummyMachine); + let ecx = InterpCx::new(tcx, DUMMY_SP, key.typing_env, crate::const_eval::DummyMachine); let layout = ecx.layout_of(ty).unwrap(); ecx.tag_for_variant(layout, variant_index).unwrap().map(|(tag, _tag_field)| tag) diff --git a/compiler/rustc_lint/src/map_unit_fn.rs b/compiler/rustc_lint/src/map_unit_fn.rs index 3b27e45613690..af509cb786db9 100644 --- a/compiler/rustc_lint/src/map_unit_fn.rs +++ b/compiler/rustc_lint/src/map_unit_fn.rs @@ -1,5 +1,4 @@ use rustc_hir::{Expr, ExprKind, HirId, Stmt, StmtKind}; -use rustc_middle::query::Key; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint, declare_lint_pass}; @@ -69,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for MapUnitFn { .span_of_impl(*id) .unwrap_or(default_span), argument_label: args[0].span, - map_label: arg_ty.default_span(cx.tcx), + map_label: span, suggestion: path.ident.span, replace: "for_each".to_string(), }, @@ -88,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for MapUnitFn { .span_of_impl(*id) .unwrap_or(default_span), argument_label: args[0].span, - map_label: arg_ty.default_span(cx.tcx), + map_label: span, suggestion: path.ident.span, replace: "for_each".to_string(), }, diff --git a/tests/ui/lint/lint_map_unit_fn.stderr b/tests/ui/lint/lint_map_unit_fn.stderr index 91542af0f6df8..930ecd30d1d1b 100644 --- a/tests/ui/lint/lint_map_unit_fn.stderr +++ b/tests/ui/lint/lint_map_unit_fn.stderr @@ -25,19 +25,18 @@ LL + x.iter_mut().for_each(foo); error: `Iterator::map` call that discard the iterator's values --> $DIR/lint_map_unit_fn.rs:11:18 | -LL | x.iter_mut().map(|items| { - | ^ ------- - | | | - | ____________________|___this function returns `()`, which is likely not what you wanted - | | __________________| - | | | -LL | | | -LL | | | items.sort(); -LL | | | }); - | | | -^ after this call to map, the resulting iterator is `impl Iterator`, which means the only information carried by the iterator is the number of items - | | |_____|| - | |_______| - | called `Iterator::map` with callable that returns `()` +LL | x.iter_mut().map(|items| { + | ^ ------- + | | | + | ___________________|___this function returns `()`, which is likely not what you wanted + | | __________________| + | || +LL | || +LL | || items.sort(); +LL | || }); + | ||_____-^ after this call to map, the resulting iterator is `impl Iterator`, which means the only information carried by the iterator is the number of items + | |______| + | called `Iterator::map` with callable that returns `()` | = note: `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated help: you might have meant to use `Iterator::for_each` From 2d8ffff10ae22cbe0effb7fb6f5561c8a859279e Mon Sep 17 00:00:00 2001 From: Jonathan Brouwer Date: Mon, 30 Jun 2025 16:09:01 +0200 Subject: [PATCH 35/35] Port `#[ignore]` to the new attribute parsing infrastructure Signed-off-by: Jonathan Brouwer --- .../src/attributes.rs | 7 +++ .../src/encode_cross_crate.rs | 1 + .../rustc_attr_parsing/src/attributes/mod.rs | 1 + .../src/attributes/test_attrs.rs | 46 +++++++++++++++++++ compiler/rustc_attr_parsing/src/context.rs | 2 + compiler/rustc_hir/src/hir.rs | 1 + compiler/rustc_parse/src/validate_attr.rs | 4 +- compiler/rustc_passes/src/check_attr.rs | 5 +- tests/ui/attributes/malformed-attrs.rs | 7 +++ tests/ui/attributes/malformed-attrs.stderr | 29 ++++++++---- ...issue-43106-gating-of-builtin-attrs.stderr | 12 ++--- .../lint/unused/unused-attr-duplicate.stderr | 24 +++++----- .../ui/malformed/malformed-regressions.stderr | 18 ++++---- 13 files changed, 117 insertions(+), 40 deletions(-) create mode 100644 compiler/rustc_attr_parsing/src/attributes/test_attrs.rs diff --git a/compiler/rustc_attr_data_structures/src/attributes.rs b/compiler/rustc_attr_data_structures/src/attributes.rs index b5934f4e36e89..6af15da7d08cb 100644 --- a/compiler/rustc_attr_data_structures/src/attributes.rs +++ b/compiler/rustc_attr_data_structures/src/attributes.rs @@ -250,6 +250,13 @@ pub enum AttributeKind { span: Span, }, + /// Represents `#[ignore]` + Ignore { + span: Span, + /// ignore can optionally have a reason: `#[ignore = "reason this is ignored"]` + reason: Option, + }, + /// Represents `#[inline]` and `#[rustc_force_inline]`. Inline(InlineAttr, Span), diff --git a/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs b/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs index 02e95ddcb6fdb..8ebd38a6ba7e3 100644 --- a/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs +++ b/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs @@ -26,6 +26,7 @@ impl AttributeKind { Deprecation { .. } => Yes, DocComment { .. } => Yes, ExportName { .. } => Yes, + Ignore { .. } => No, Inline(..) => No, LinkName { .. } => Yes, LinkSection { .. } => No, diff --git a/compiler/rustc_attr_parsing/src/attributes/mod.rs b/compiler/rustc_attr_parsing/src/attributes/mod.rs index f5ac3890a46a7..55fbb82546632 100644 --- a/compiler/rustc_attr_parsing/src/attributes/mod.rs +++ b/compiler/rustc_attr_parsing/src/attributes/mod.rs @@ -41,6 +41,7 @@ pub(crate) mod repr; pub(crate) mod rustc_internal; pub(crate) mod semantics; pub(crate) mod stability; +pub(crate) mod test_attrs; pub(crate) mod traits; pub(crate) mod transparency; pub(crate) mod util; diff --git a/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs b/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs new file mode 100644 index 0000000000000..cea3ee52ff43d --- /dev/null +++ b/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs @@ -0,0 +1,46 @@ +use rustc_attr_data_structures::AttributeKind; +use rustc_attr_data_structures::lints::AttributeLintKind; +use rustc_feature::{AttributeTemplate, template}; +use rustc_span::{Symbol, sym}; + +use crate::attributes::{AttributeOrder, OnDuplicate, SingleAttributeParser}; +use crate::context::{AcceptContext, Stage}; +use crate::parser::ArgParser; + +pub(crate) struct IgnoreParser; + +impl SingleAttributeParser for IgnoreParser { + const PATH: &[Symbol] = &[sym::ignore]; + const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepLast; + const ON_DUPLICATE: OnDuplicate = OnDuplicate::Warn; + const TEMPLATE: AttributeTemplate = template!(Word, NameValueStr: "reason"); + + fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option { + Some(AttributeKind::Ignore { + span: cx.attr_span, + reason: match args { + ArgParser::NoArgs => None, + ArgParser::NameValue(name_value) => { + let Some(str_value) = name_value.value_as_str() else { + let suggestions = >::TEMPLATE + .suggestions(false, "ignore"); + let span = cx.attr_span; + cx.emit_lint( + AttributeLintKind::IllFormedAttributeInput { suggestions }, + span, + ); + return None; + }; + Some(str_value) + } + ArgParser::List(_) => { + let suggestions = + >::TEMPLATE.suggestions(false, "ignore"); + let span = cx.attr_span; + cx.emit_lint(AttributeLintKind::IllFormedAttributeInput { suggestions }, span); + return None; + } + }, + }) + } +} diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 265e1bb6a8cba..2a01ee58493f6 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -37,6 +37,7 @@ use crate::attributes::semantics::MayDangleParser; use crate::attributes::stability::{ BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser, }; +use crate::attributes::test_attrs::IgnoreParser; use crate::attributes::traits::SkipDuringMethodDispatchParser; use crate::attributes::transparency::TransparencyParser; use crate::attributes::{AttributeParser as _, Combine, Single, WithoutArgs}; @@ -126,6 +127,7 @@ attribute_parsers!( // tidy-alphabetical-start Single, Single, + Single, Single, Single, Single, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index ca6405ea20944..559a771931e9c 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1303,6 +1303,7 @@ impl AttributeExt for Attribute { Attribute::Parsed(AttributeKind::Deprecation { span, .. }) => *span, Attribute::Parsed(AttributeKind::DocComment { span, .. }) => *span, Attribute::Parsed(AttributeKind::MayDangle(span)) => *span, + Attribute::Parsed(AttributeKind::Ignore { span, .. }) => *span, a => panic!("can't get the span of an arbitrary parsed attribute: {a:?}"), } } diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index 27355a422d1fe..8fdc06ee463ad 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/compiler/rustc_parse/src/validate_attr.rs @@ -304,6 +304,7 @@ fn emit_malformed_attribute( | sym::naked | sym::no_mangle | sym::non_exhaustive + | sym::ignore | sym::must_use | sym::track_caller | sym::link_name @@ -319,8 +320,7 @@ fn emit_malformed_attribute( // Some of previously accepted forms were used in practice, // report them as warnings for now. - let should_warn = - |name| matches!(name, sym::doc | sym::ignore | sym::link | sym::test | sym::bench); + let should_warn = |name| matches!(name, sym::doc | sym::link | sym::test | sym::bench); let error_msg = format!("malformed `{name}` attribute input"); let mut suggestions = vec![]; diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index c5ced4064140e..18b3ab12e2d36 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -215,6 +215,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { Attribute::Parsed(AttributeKind::MayDangle(attr_span)) => { self.check_may_dangle(hir_id, *attr_span) } + Attribute::Parsed(AttributeKind::Ignore { span, .. }) => { + self.check_generic_attr(hir_id, sym::ignore, *span, target, Target::Fn) + } Attribute::Parsed(AttributeKind::MustUse { span, .. }) => { self.check_must_use(hir_id, *span, target) } @@ -303,7 +306,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } [sym::path, ..] => self.check_generic_attr_unparsed(hir_id, attr, target, Target::Mod), [sym::macro_export, ..] => self.check_macro_export(hir_id, attr, target), - [sym::ignore, ..] | [sym::should_panic, ..] => { + [sym::should_panic, ..] => { self.check_generic_attr_unparsed(hir_id, attr, target, Target::Fn) } [sym::automatically_derived, ..] => { diff --git a/tests/ui/attributes/malformed-attrs.rs b/tests/ui/attributes/malformed-attrs.rs index dbe9c35b0a4c4..a09fe86557d21 100644 --- a/tests/ui/attributes/malformed-attrs.rs +++ b/tests/ui/attributes/malformed-attrs.rs @@ -219,4 +219,11 @@ macro_rules! slump { () => {} } +#[ignore = 1] +//~^ ERROR valid forms for the attribute are +//~| WARN this was previously accepted by the compiler +fn thing() { + +} + fn main() {} diff --git a/tests/ui/attributes/malformed-attrs.stderr b/tests/ui/attributes/malformed-attrs.stderr index 32b0ddf87ba4f..9fe4f45b3ef74 100644 --- a/tests/ui/attributes/malformed-attrs.stderr +++ b/tests/ui/attributes/malformed-attrs.stderr @@ -309,15 +309,6 @@ LL | #[link] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #57571 -error: valid forms for the attribute are `#[ignore]` and `#[ignore = "reason"]` - --> $DIR/malformed-attrs.rs:93:1 - | -LL | #[ignore()] - | ^^^^^^^^^^^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #57571 - error: invalid argument --> $DIR/malformed-attrs.rs:187:1 | @@ -600,6 +591,24 @@ LL | #[inline = 5] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #57571 +error: valid forms for the attribute are `#[ignore = "reason"]` and `#[ignore]` + --> $DIR/malformed-attrs.rs:93:1 + | +LL | #[ignore()] + | ^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57571 + +error: valid forms for the attribute are `#[ignore = "reason"]` and `#[ignore]` + --> $DIR/malformed-attrs.rs:222:1 + | +LL | #[ignore = 1] + | ^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57571 + error[E0308]: mismatched types --> $DIR/malformed-attrs.rs:110:23 | @@ -611,7 +620,7 @@ LL | #[coroutine = 63] || {} = note: expected unit type `()` found coroutine `{coroutine@$DIR/malformed-attrs.rs:110:23: 110:25}` -error: aborting due to 73 previous errors; 3 warnings emitted +error: aborting due to 74 previous errors; 3 warnings emitted Some errors have detailed explanations: E0308, E0463, E0539, E0565, E0658, E0805. For more information about an error, try `rustc --explain E0308`. diff --git a/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr b/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr index 9280dfdf92e5b..5d7d1caeeab0b 100644 --- a/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr +++ b/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr @@ -367,12 +367,6 @@ warning: `#[should_panic]` only has an effect on functions LL | #![should_panic] | ^^^^^^^^^^^^^^^^ -warning: `#[ignore]` only has an effect on functions - --> $DIR/issue-43106-gating-of-builtin-attrs.rs:54:1 - | -LL | #![ignore] - | ^^^^^^^^^^ - warning: `#[proc_macro_derive]` only has an effect on functions --> $DIR/issue-43106-gating-of-builtin-attrs.rs:60:1 | @@ -387,6 +381,12 @@ LL | #![link()] | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! +warning: `#[ignore]` only has an effect on functions + --> $DIR/issue-43106-gating-of-builtin-attrs.rs:54:1 + | +LL | #![ignore] + | ^^^^^^^^^^ + warning: attribute should be applied to a foreign function or static --> $DIR/issue-43106-gating-of-builtin-attrs.rs:66:1 | diff --git a/tests/ui/lint/unused/unused-attr-duplicate.stderr b/tests/ui/lint/unused/unused-attr-duplicate.stderr index eff478d04aee8..275eb05630520 100644 --- a/tests/ui/lint/unused/unused-attr-duplicate.stderr +++ b/tests/ui/lint/unused/unused-attr-duplicate.stderr @@ -40,18 +40,6 @@ LL | #[path = "auxiliary/lint_unused_extern_crate.rs"] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! -error: unused attribute - --> $DIR/unused-attr-duplicate.rs:53:1 - | -LL | #[ignore = "some text"] - | ^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute - | -note: attribute also specified here - --> $DIR/unused-attr-duplicate.rs:52:1 - | -LL | #[ignore] - | ^^^^^^^^^ - error: unused attribute --> $DIR/unused-attr-duplicate.rs:55:1 | @@ -165,6 +153,18 @@ note: attribute also specified here LL | #[macro_export] | ^^^^^^^^^^^^^^^ +error: unused attribute + --> $DIR/unused-attr-duplicate.rs:53:1 + | +LL | #[ignore = "some text"] + | ^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute + | +note: attribute also specified here + --> $DIR/unused-attr-duplicate.rs:52:1 + | +LL | #[ignore] + | ^^^^^^^^^ + error: unused attribute --> $DIR/unused-attr-duplicate.rs:60:1 | diff --git a/tests/ui/malformed/malformed-regressions.stderr b/tests/ui/malformed/malformed-regressions.stderr index 535db55a13d61..8c22919a1c2f1 100644 --- a/tests/ui/malformed/malformed-regressions.stderr +++ b/tests/ui/malformed/malformed-regressions.stderr @@ -8,15 +8,6 @@ LL | #[doc] = note: for more information, see issue #57571 = note: `#[deny(ill_formed_attribute_input)]` on by default -error: valid forms for the attribute are `#[ignore]` and `#[ignore = "reason"]` - --> $DIR/malformed-regressions.rs:3:1 - | -LL | #[ignore()] - | ^^^^^^^^^^^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #57571 - error: attribute must be of the form `#[link(name = "...", /*opt*/ kind = "dylib|static|...", /*opt*/ wasm_import_module = "...", /*opt*/ import_name_type = "decorated|noprefix|undecorated")]` --> $DIR/malformed-regressions.rs:7:1 | @@ -35,6 +26,15 @@ LL | #[link = ""] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #57571 +error: valid forms for the attribute are `#[ignore = "reason"]` and `#[ignore]` + --> $DIR/malformed-regressions.rs:3:1 + | +LL | #[ignore()] + | ^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57571 + error: valid forms for the attribute are `#[inline(always|never)]` and `#[inline]` --> $DIR/malformed-regressions.rs:5:1 |