diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6d0782a..d1d112e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -82,6 +82,130 @@ jobs: cargo test --target i686-pc-windows-msvc --test windows_spawn pseudoconsole_regular_stdio_stays_off_parent_pipes -- --exact + stress: + name: Spawn cleanup stress / ${{ matrix.target }} + strategy: + fail-fast: false + matrix: + target: [x86_64-pc-windows-msvc, i686-pc-windows-msvc] + runs-on: windows-latest + timeout-minutes: 30 + env: + CARGO_BUILD_TARGET: ${{ matrix.target }} + WINDOWS_SPAWN_STRESS_CASES: "1300" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: stable + targets: ${{ matrix.target }} + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + key: ${{ matrix.target }} + - run: cargo test --locked --test windows_spawn spawn_cleanup_stress -- --ignored --exact + + careful: + name: Windows tests under cargo-careful + runs-on: windows-latest + timeout-minutes: 45 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: nightly-2026-10-13 + components: rust-src + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - uses: taiki-e/install-action@1168f6810a3cb30a3514bf5756a3b7f0422c6df8 # v2.87.28 + with: + tool: cargo-careful@0.4.10 + - run: >- + cargo +nightly-2026-10-13 careful test --workspace --all-targets --locked + -- --test-threads=1 + + arm64: + name: Windows 11 ARM64 + runs-on: windows-11-arm + timeout-minutes: 30 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: stable + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - run: cargo xtask test + + miri: + name: Miri over the platform-neutral decisions + runs-on: ubuntu-latest + timeout-minutes: 45 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: nightly-2026-10-13 + components: miri, rust-src + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - run: cargo +nightly-2026-10-13 miri test --package windows-spawn --lib --locked + + sanitizer: + name: AddressSanitizer over the platform-neutral decisions + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + RUSTFLAGS: -Zsanitizer=address + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: nightly-2026-10-13 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - run: >- + cargo +nightly-2026-10-13 test --package windows-spawn --lib --locked + --target x86_64-unknown-linux-gnu + + fuzz: + name: Fuzz / ${{ matrix.target }} + strategy: + fail-fast: false + matrix: + target: [command-line, environment-block] + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + FUZZ_TARGET: ${{ matrix.target }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: nightly-2026-10-13 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + workspaces: fuzz + - uses: taiki-e/install-action@1168f6810a3cb30a3514bf5756a3b7f0422c6df8 # v2.87.28 + with: + tool: cargo-fuzz@0.13.2 + - run: >- + cargo +nightly-2026-10-13 fuzz run "$FUZZ_TARGET" --target x86_64-unknown-linux-gnu + -- -max_total_time=600 -max_len=16384 + - if: failure() + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: fuzz-${{ matrix.target }} + path: fuzz/artifacts/${{ matrix.target }} + if-no-files-found: ignore + verifier: name: Application Verifier runs-on: windows-latest @@ -192,7 +316,21 @@ jobs: ci-required: name: CI required if: always() - needs: [hygiene, test, i686-smoke, verifier, linux, proofs, coverage, package] + needs: + - hygiene + - test + - i686-smoke + - stress + - careful + - arm64 + - miri + - sanitizer + - fuzz + - verifier + - linux + - proofs + - coverage + - package runs-on: ubuntu-latest steps: - if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') diff --git a/.github/workflows/deep-verification.yml b/.github/workflows/deep-verification.yml new file mode 100644 index 0000000..f2a12bc --- /dev/null +++ b/.github/workflows/deep-verification.yml @@ -0,0 +1,93 @@ +name: Deep verification + +"on": + schedule: + - cron: "17 3 * * *" + workflow_dispatch: + +permissions: + contents: read + +# One deep run at a time; a dispatch that meets a scheduled run waits for it. +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +jobs: + stress: + name: Spawn cleanup stress / ${{ matrix.target }} + strategy: + fail-fast: false + matrix: + include: + - target: x86_64-pc-windows-msvc + cases: "100000" + - target: i686-pc-windows-msvc + cases: "20000" + runs-on: windows-latest + timeout-minutes: 180 + env: + CARGO_BUILD_TARGET: ${{ matrix.target }} + WINDOWS_SPAWN_STRESS_CASES: ${{ matrix.cases }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: stable + targets: ${{ matrix.target }} + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + key: ${{ matrix.target }} + - run: cargo test --locked --test windows_spawn spawn_cleanup_stress -- --ignored --exact + + random: + name: Random command lines and environments at fifty times their size + runs-on: windows-latest + timeout-minutes: 300 + env: + RUSTFLAGS: --cfg windows_spawn_vectors + WINDOWS_SPAWN_RANDOM_SCALE: "50" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: stable + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - run: cargo test --locked --test argv_roundtrip --test command_line_parsers + + fuzz: + name: Fuzz / ${{ matrix.target }} + strategy: + fail-fast: false + matrix: + target: [command-line, environment-block] + runs-on: ubuntu-latest + timeout-minutes: 330 + env: + FUZZ_TARGET: ${{ matrix.target }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 + with: + toolchain: nightly-2026-10-13 + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + workspaces: fuzz + - uses: taiki-e/install-action@1168f6810a3cb30a3514bf5756a3b7f0422c6df8 # v2.87.28 + with: + tool: cargo-fuzz@0.13.2 + - run: >- + cargo +nightly-2026-10-13 fuzz run "$FUZZ_TARGET" --target x86_64-unknown-linux-gnu + -- -max_total_time=18000 -max_len=16384 + - if: failure() + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: fuzz-${{ matrix.target }} + path: fuzz/artifacts/${{ matrix.target }} + if-no-files-found: ignore diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 80bb2b6..b184df9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,6 +46,22 @@ On Windows, `just verifier` runs every test under `appverif.exe`, which stops a It needs an administrator and the `appverif.exe` of the Windows SDK, which the hosted Windows runners of CI also have. Three tests misuse a handle on purpose, and each must stop, which shows the verifier watches the run. +Deeper checks need no change to the code, and ADR 0017 says what each covers: + +```powershell +$env:WINDOWS_SPAWN_STRESS_CASES = "1300" +cargo test --test windows_spawn spawn_cleanup_stress -- --ignored --exact +cargo +nightly careful test --workspace --all-targets -- --test-threads=1 +``` + +```sh +cargo +nightly miri test --package windows-spawn --lib +cargo +nightly fuzz run command-line -- -max_total_time=600 -max_len=16384 +cargo +nightly fuzz run environment-block -- -max_total_time=600 -max_len=16384 +``` + +`WINDOWS_SPAWN_RANDOM_SCALE` multiplies the number of random command lines and environments, which the nightly workflow raises to fifty times. + ## Expectations - Keep the documented ownership and cleanup behavior, including on errors. diff --git a/Cargo.toml b/Cargo.toml index 1b2b1a6..582bb43 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,7 +82,7 @@ unsafe_op_in_unsafe_fn = "deny" unused_lifetimes = "deny" unused_qualifications = "deny" variant_size_differences = "deny" -unexpected_cfgs = { level = "deny", check-cfg = ["cfg(kani)", "cfg(windows_spawn_vectors)"] } +unexpected_cfgs = { level = "deny", check-cfg = ["cfg(fuzzing)", "cfg(kani)", "cfg(windows_spawn_vectors)"] } [workspace.lints.clippy] all = { level = "deny", priority = -1 } diff --git a/REUSE.toml b/REUSE.toml index b1219c8..b198b54 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -18,6 +18,7 @@ path = [ "deny.toml", "docs/**", "examples/**", + "fuzz/**", "justfile", "proofs/**", "public-api/**", @@ -30,7 +31,7 @@ SPDX-FileCopyrightText = "2026 Yasunobu Sakashita" SPDX-License-Identifier = "MIT OR Apache-2.0" [[annotations]] -path = ["Cargo.lock", "proofs/quint/package-lock.json"] +path = ["Cargo.lock", "fuzz/Cargo.lock", "proofs/quint/package-lock.json"] precedence = "override" SPDX-FileCopyrightText = "NONE" SPDX-License-Identifier = "CC0-1.0" diff --git a/docs/adr/0017-long-careful-and-fuzzed-runs.md b/docs/adr/0017-long-careful-and-fuzzed-runs.md new file mode 100644 index 0000000..73c4ca6 --- /dev/null +++ b/docs/adr/0017-long-careful-and-fuzzed-runs.md @@ -0,0 +1,39 @@ +# 0017 — Long, careful, and fuzzed runs check what bounded tests can't + +Status: accepted (2026-10-13) + +## Context + +The tests run each behavior a few times, in one configuration, on x64 and i686. +A handle that leaks once in many spawns, an unsafe precondition that holds by luck, a fault on ARM64, and an encoder fault beyond the vectors and the bounds of Kani all pass them. +PR #14 ran a stress test, `cargo careful`, an ARM64 smoke test, Miri, AddressSanitizer, and two fuzz targets, and main dropped them with #14. + +## Decision + +- An ignored test, `spawn_cleanup_stress`, spawns children in thirteen modes. + The modes cover both commits, a dropped suspended child, collected output, written input, a killed tree, transferred handles, a Job, an alternate parent, and two failed spawns. + It requires the process's handle count to end where it started, after a warm-up that runs each mode twice. + `WINDOWS_SPAWN_STRESS_CASES` sets the number of spawns: 1300 on every PR on x64 and i686, and 100000 on x64 and 20000 on i686 every night. +- CI runs the Windows tests under `cargo careful`, whose standard library checks the preconditions of the unsafe operations in `sys`, such as those of `slice::from_raw_parts`. +- CI runs `cargo xtask test` on a Windows 11 ARM64 runner. +- A `fuzz` crate compiles the encoder and the block builder from `src/domain`, and requires the parser models to read back what they wrote. + The parser models and the block scan compile under `cfg(fuzzing)` for it. + Each target runs for ten minutes on every PR, and for five hours every night. + The fuzz crate is its own workspace, outside the package, the MSRV, and the workspace lints, and the repository gate checks its targets. +- Miri and AddressSanitizer run the platform-neutral decisions on Linux. + Those decisions hold no unsafe code, so these runs guard against a change that brings some in, at the cost of minutes. +- `WINDOWS_SPAWN_RANDOM_SCALE` multiplies the random command lines and environments, which keep their seeds, and the nightly workflow runs them at fifty times their size. + +Rejected: + +- The fuzz targets of #14, which checked only that the code doesn't panic. +- A stress test that measures time, which ADR 0010 bans. + A handle count decides by state. +- ARM64 through emulation, which would test the emulator. + +## Consequences + +- CI gains six jobs, and `required` waits for each of them. +- A leak in every spawn fails a PR, and a rarer leak fails the nightly run, whose 100000 spawns let it add up. +- A failed fuzz run uploads the input that failed. +- The nightly workflow runs on its schedule or on demand, and it gates no PR. diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..b7fa0a7 --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,4 @@ +/target +/corpus +/artifacts +/coverage diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock new file mode 100644 index 0000000..702a6e5 --- /dev/null +++ b/fuzz/Cargo.lock @@ -0,0 +1,89 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "arbitrary" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3bc62ac97cc33321f50863d514c3bc38a453947a8f9e781137e47c7401020aed" + +[[package]] +name = "cc" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6651c9ed80effdc7db0ff72512157f901af5e3549e341e24b1dd4887d836d838" +dependencies = [ + "find-msvc-tools", + "jobserver", + "libc", + "shlex", +] + +[[package]] +name = "cfg-if" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600" + +[[package]] +name = "find-msvc-tools" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aedcfb3409746eddb02b9e19ebda1c3394f759a152e48ee875a0844d1b955484" + +[[package]] +name = "getrandom" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" +dependencies = [ + "cfg-if", + "libc", + "r-efi", +] + +[[package]] +name = "jobserver" +version = "0.1.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c00acbd29eabad4a2392fa0e921c874934dbbf4194312ad20f04a0ed67a3cb3" +dependencies = [ + "getrandom", + "libc", +] + +[[package]] +name = "libc" +version = "0.2.190" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce5d3ddc6d3fa000eb1536d85e147bfe31aacaba692ed6a876f95cb7c855be78" + +[[package]] +name = "libfuzzer-sys" +version = "0.4.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9fd2f41a1cba099f79a0b6b6c35656cf7c03351a7bae8ff0f28f25270f929d2" +dependencies = [ + "arbitrary", + "cc", +] + +[[package]] +name = "r-efi" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" + +[[package]] +name = "shlex" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" + +[[package]] +name = "windows-spawn-fuzz" +version = "0.0.0" +dependencies = [ + "libfuzzer-sys", +] diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..b73ed1d --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,31 @@ +[package] +name = "windows-spawn-fuzz" +version = "0.0.0" +edition = "2021" +publish = false + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4.13" + +[lints.rust] +unexpected_cfgs = { level = "deny", check-cfg = ["cfg(fuzzing)", "cfg(kani)"] } + +[[bin]] +name = "command-line" +path = "fuzz_targets/command_line.rs" +test = false +doc = false +bench = false + +[[bin]] +name = "environment-block" +path = "fuzz_targets/environment_block.rs" +test = false +doc = false +bench = false + +# Its own workspace, so the crate's workspace, MSRV, and lints stay as they are. +[workspace] diff --git a/fuzz/fuzz_targets/command_line.rs b/fuzz/fuzz_targets/command_line.rs new file mode 100644 index 0000000..6f8515c --- /dev/null +++ b/fuzz/fuzz_targets/command_line.rs @@ -0,0 +1,61 @@ +//! Encodes a program name and arguments the fuzzer chooses, and requires both C runtime parser models to read every one back. +//! +//! It compiles `src/domain/command_line_rules.rs` itself, so it fuzzes the encoder the crate runs. +//! NUL ends a command line, so it separates the texts the input holds, and the first text is the program name. +//! Validation keeps a double quote out of a program name, so an input with one tests nothing. + +#![no_main] + +#[path = "../../src/domain/command_line_rules.rs"] +mod command_line_rules; + +use command_line_rules::parsers::{split, Dialect}; +use command_line_rules::{append_argument, append_program, NUL, QUOTE, SPACE}; +use libfuzzer_sys::fuzz_target; + +/// The part of the `UnitSink` of `src/domain/mod.rs` that the encoder calls. +trait UnitSink { + fn put(&mut self, unit: u16); +} + +impl UnitSink for Vec { + fn put(&mut self, unit: u16) { + self.push(unit); + } +} + +/// The longest command line `CreateProcessW` accepts, its terminator included, which sizes the models' storage. +const LINE: usize = 32_767; + +fuzz_target!(|data: &[u8]| { + let units: Vec = data + .chunks_exact(size_of::()) + .map(|pair| u16::from_le_bytes([pair[0], pair[1]])) + .collect(); + let mut texts = units.split(|unit| *unit == NUL); + let Some(program) = texts.next() else { + return; + }; + if program.contains("E) { + return; + } + let arguments: Vec<&[u16]> = texts.collect(); + + let mut line = Vec::new(); + append_program(&mut line, program); + for argument in &arguments { + line.put(SPACE); + append_argument(&mut line, argument); + } + + for dialect in [Dialect::Ucrt, Dialect::Msvcrt] { + let Some(read) = split::(&line, dialect) else { + return; + }; + let expected = std::iter::once(program).chain(arguments.iter().copied()); + assert_eq!(read.len(), arguments.len() + 1, "{dialect:?} split {line:?}"); + for (index, text) in expected.enumerate() { + assert_eq!(read.get(index), Some(text), "{dialect:?} split {line:?}"); + } + } +}); diff --git a/fuzz/fuzz_targets/environment_block.rs b/fuzz/fuzz_targets/environment_block.rs new file mode 100644 index 0000000..4ba284f --- /dev/null +++ b/fuzz/fuzz_targets/environment_block.rs @@ -0,0 +1,58 @@ +//! Builds an environment block from variables the fuzzer chooses, and requires the block to end where `scan` finds its end and to read back as those variables, in order. +//! +//! It compiles `src/domain/environment_rules.rs` itself, so it fuzzes the builder the crate runs. +//! NUL ends an entry, so it separates the texts the input holds, which alternate between a key and its value. +//! Validation keeps a key nonempty and keeps `=` out of it after its first unit, where a hidden key such as `=C:` holds one, so other keys test nothing. + +#![no_main] + +#[path = "../../src/domain/environment_rules.rs"] +mod environment_rules; + +use environment_rules::{append_entry, entries, finish, scan, NUL, SEPARATOR}; +use libfuzzer_sys::fuzz_target; + +/// The `UnitSink` of `src/domain/mod.rs`, which the builder writes into. +trait UnitSink { + fn put(&mut self, unit: u16); + + fn is_empty(&self) -> bool; +} + +impl UnitSink for Vec { + fn put(&mut self, unit: u16) { + self.push(unit); + } + + fn is_empty(&self) -> bool { + Self::is_empty(self) + } +} + +/// The texts of one variable: its key and its value. +const KEY_AND_VALUE: usize = 2; + +fuzz_target!(|data: &[u8]| { + let units: Vec = data + .chunks_exact(size_of::()) + .map(|pair| u16::from_le_bytes([pair[0], pair[1]])) + .collect(); + let texts: Vec<&[u16]> = units.split(|unit| *unit == NUL).collect(); + let variables: Vec<(&[u16], &[u16])> = texts + .chunks_exact(KEY_AND_VALUE) + .map(|pair| (pair[0], pair[1])) + .filter(|(key, _)| key.split_first().is_some_and(|(_, rest)| !rest.contains(&SEPARATOR))) + .collect(); + + let mut block = Vec::new(); + for (key, value) in &variables { + append_entry(&mut block, key, value); + } + finish(&mut block); + + let end = scan(&block).expect("a built block ends"); + let last = block.len() - 1; + assert_eq!(end, if variables.is_empty() { 0 } else { last }); + let read: Vec<(&[u16], &[u16])> = entries(&block[..end]).collect(); + assert_eq!(read, variables); +}); diff --git a/src/command.rs b/src/command.rs index 49c72cf..231929f 100644 --- a/src/command.rs +++ b/src/command.rs @@ -210,6 +210,9 @@ impl Command { } /// Clears the inherited environment and prior recorded modifications. + /// + /// The child receives only the variables set after this call. + /// Windows on ARM64 still adds `PROCESSOR_ARCHITECTURE` to every process it starts. pub fn env_clear(&mut self) -> &mut Self { self.env_clear = true; self.env_ops.clear(); diff --git a/src/domain/command_line_rules.rs b/src/domain/command_line_rules.rs index 6fe7416..1ebe4ed 100644 --- a/src/domain/command_line_rules.rs +++ b/src/domain/command_line_rules.rs @@ -71,11 +71,14 @@ fn put_backslashes(line: &mut impl UnitSink, count: usize) { } } -/// Executable models of the Windows command-line parsers, for proofs and tests. -#[cfg(any(test, kani))] +/// Executable models of the Windows command-line parsers, for proofs, tests, and the fuzzer. +#[cfg(any(test, kani, fuzzing))] pub(crate) mod parsers { use super::{BACKSLASH, NUL, QUOTE, SPACE, TAB}; + /// The backslashes before a double quote that stand for one backslash. + const BACKSLASH_PAIR: usize = 2; + /// How a parser reads two double quotes inside a quoted region. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Dialect { @@ -194,8 +197,8 @@ pub(crate) mod parsers { continue; } if unit == QUOTE { - let escaped = backslashes % 2 == 1; - let dropped = backslashes.saturating_sub(backslashes / 2); + let escaped = backslashes % BACKSLASH_PAIR == 1; + let dropped = backslashes.saturating_sub(backslashes / BACKSLASH_PAIR); arguments.length = arguments.length.saturating_sub(dropped); backslashes = 0; if escaped { diff --git a/src/domain/environment_rules.rs b/src/domain/environment_rules.rs index 529851f..9012a7f 100644 --- a/src/domain/environment_rules.rs +++ b/src/domain/environment_rules.rs @@ -51,7 +51,7 @@ pub(crate) fn finish(block: &mut impl UnitSink) { } /// Returns how many units of `block` precede the NUL that ends it, reading no unit past that NUL. -#[cfg(any(test, kani))] +#[cfg(any(test, kani, fuzzing))] pub(crate) fn scan(block: &[u16]) -> Option { let mut previous = None; for (index, unit) in block.iter().enumerate() { diff --git a/tests/argv_roundtrip.rs b/tests/argv_roundtrip.rs index b502f94..7166159 100644 --- a/tests/argv_roundtrip.rs +++ b/tests/argv_roundtrip.rs @@ -2,7 +2,6 @@ //! //! Seeded random environments then pass through the crate's block builder into children, //! which must hold exactly the variables the commands set, merged and ordered as Windows compares names. -#![allow(clippy::as_conversions, clippy::expect_used, clippy::unwrap_in_result)] #[cfg(windows)] mod random; @@ -15,7 +14,7 @@ mod windows { use std::io; use std::mem::size_of; use std::os::windows::ffi::{OsStrExt, OsStringExt}; - use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle, RawHandle}; + use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; use std::ptr; use windows_spawn::{Command, Stdio}; @@ -31,60 +30,67 @@ mod windows { STARTUPINFOW, }; - use crate::random::Generator; + use crate::random::{scaled, Generator}; const OUTPUT: &str = "WINDOWS_SPAWN_ROUNDTRIP_OUTPUT"; const VALUE: &str = "WINDOWS_SPAWN_ROUNDTRIP_VALUE"; const MIXED: &str = "WINDOWS_SPAWN_ROUNDTRIP_MIXED"; const REMOVE: &str = "WINDOWS_SPAWN_ROUNDTRIP_REMOVE"; - fn append_os(buffer: &mut Vec, value: &OsStr) { + /// Writes `count` as the little-endian `DWORD` that precedes what it counts. + fn append_count(buffer: &mut Vec, count: usize) -> io::Result<()> { + let count = u32::try_from(count).map_err(io::Error::other)?; + buffer.extend_from_slice(&count.to_le_bytes()); + Ok(()) + } + + fn append_os(buffer: &mut Vec, value: &OsStr) -> io::Result<()> { let wide: Vec = value.encode_wide().collect(); - let length = u32::try_from(wide.len()).expect("test values fit in a DWORD"); - buffer.extend_from_slice(&length.to_le_bytes()); + append_count(buffer, wide.len())?; for unit in wide { buffer.extend_from_slice(&unit.to_le_bytes()); } + Ok(()) + } + + /// Reads a variable the parent sets for the child. + fn parent_variable(name: &str) -> io::Result { + std::env::var_os(name).ok_or_else(|| io::Error::other(format!("the parent sets {name}"))) } fn child() -> io::Result<()> { - let output = std::env::var_os(OUTPUT).expect("parent supplies output path"); + let output = parent_variable(OUTPUT)?; let arguments: Vec = std::env::args_os().skip(2).collect(); - let mut encoded = encode_arguments(&arguments); - append_os( - &mut encoded, - &std::env::var_os(VALUE).expect("parent supplies environment value"), - ); - append_os( - &mut encoded, - &std::env::var_os(MIXED).expect("case-insensitive replacement survives"), - ); + let mut encoded = encode_arguments(&arguments)?; + append_os(&mut encoded, &parent_variable(VALUE)?)?; + append_os(&mut encoded, &parent_variable(MIXED)?)?; encoded.push(u8::from(std::env::var_os(REMOVE).is_some())); fs::write(output, encoded) } - fn encode_arguments(arguments: &[OsString]) -> Vec { + fn encode_arguments(arguments: &[OsString]) -> io::Result> { let mut encoded = Vec::new(); - let count = u32::try_from(arguments.len()).expect("test corpus fits in a DWORD"); - encoded.extend_from_slice(&count.to_le_bytes()); + append_count(&mut encoded, arguments.len())?; for argument in arguments { - append_os(&mut encoded, argument); + append_os(&mut encoded, argument)?; } - encoded + Ok(encoded) } fn broker_child() -> io::Result<()> { let mut arguments = std::env::args_os().skip(2); - let output = arguments.next().expect("parent supplies output path"); + let output = arguments + .next() + .ok_or_else(|| io::Error::other("the parent passes the output path"))?; let arguments: Vec = arguments.collect(); - fs::write(output, encode_arguments(&arguments)) + fs::write(output, encode_arguments(&arguments)?) } /// Starts `line` with a null application name, as a launch broker does. fn launch_like_a_broker(line: &OsStr) -> io::Result { let mut wide: Vec = line.encode_wide().chain(Some(0)).collect(); let startup = STARTUPINFOW { - cb: u32::try_from(size_of::()).expect("STARTUPINFOW fits in a DWORD"), + cb: u32::try_from(size_of::()).map_err(io::Error::other)?, ..STARTUPINFOW::default() }; let mut information = PROCESS_INFORMATION::default(); @@ -109,8 +115,8 @@ mod windows { // SAFETY: CreateProcessW succeeded, so both handles are new and owned here. let (process, _thread) = unsafe { ( - OwnedHandle::from_raw_handle(information.hProcess as RawHandle), - OwnedHandle::from_raw_handle(information.hThread as RawHandle), + OwnedHandle::from_raw_handle(information.hProcess), + OwnedHandle::from_raw_handle(information.hThread), ) }; // SAFETY: the owned process handle stays open for the wait. @@ -143,7 +149,7 @@ mod windows { let actual = fs::read(&output)?; fs::remove_file(output)?; - if actual != encode_arguments(&arguments) { + if actual != encode_arguments(&arguments)? { return Err(io::Error::other( "a broker-launched command line decoded to different arguments", )); @@ -205,9 +211,9 @@ mod windows { ))); } - let mut expected = encode_arguments(&arguments); - append_os(&mut expected, &environment); - append_os(&mut expected, &mixed); + let mut expected = encode_arguments(&arguments)?; + append_os(&mut expected, &environment)?; + append_os(&mut expected, &mixed)?; expected.push(0); let actual = fs::read(&output)?; @@ -234,9 +240,14 @@ mod windows { /// The unit a generated name gets in place of the separator. const UNDERSCORE: u16 = 0x5f; - /// The variables a tool sets inside every process it watches, which no generated name can match: + /// The variables a tool or Windows sets inside every process, which no generated name can match: /// Application Verifier's, so `cargo xtask verifier` can run this test, and the LLVM profile runtime's, so coverage can. - const TOOL_VARIABLES: [&str; 2] = ["OANOCACHE", "__LLVM_PROFILE_RT_INIT_ONCE"]; + /// Windows on ARM64 also gives every process its architecture, even a process whose environment holds nothing else. + const INJECTED_VARIABLES: [&str; 3] = [ + "OANOCACHE", + "__LLVM_PROFILE_RT_INIT_ONCE", + "PROCESSOR_ARCHITECTURE", + ]; /// Units whose case Windows folds and units that look alike: /// `A`, `a`, `Z`, `z`, `_`, `0`, `É`, `é`, `ß`, `İ`, `ı`, `DŽ`, `Dž`, `dž`, `Σ`, `σ`, `ς`, Cyrillic capital and small zhe (U+0416 and U+0436), fullwidth `Ａ` and `ａ`, @@ -260,9 +271,14 @@ mod windows { } /// Orders two names as the crate's environment map does, with the comparison Windows uses for names. + /// + /// A name too long for the comparison, which no generated name reaches, falls back to the order of its units, as an unknown result does. fn by_name(left: &[u16], right: &[u16]) -> Ordering { - let left_length = i32::try_from(left.len()).expect("names fit in an i32"); - let right_length = i32::try_from(right.len()).expect("names fit in an i32"); + let (Ok(left_length), Ok(right_length)) = + (i32::try_from(left.len()), i32::try_from(right.len())) + else { + return left.cmp(right); + }; // SAFETY: both pointers are readable for their lengths. match unsafe { CompareStringOrdinal(left.as_ptr(), left_length, right.as_ptr(), right_length, 1) @@ -314,7 +330,8 @@ mod windows { let mut generator = Generator::new(ENVIRONMENT_SEED); let profile = std::env::var_os("LLVM_PROFILE_FILE"); let mut mismatches = Vec::new(); - for _ in 0..RANDOM_ENVIRONMENTS { + let environments = scaled(RANDOM_ENVIRONMENTS)?; + for _ in 0..environments { let mut variables: Vec<(Vec, Vec)> = Vec::new(); if let Some(profile) = &profile { variables.push(( @@ -375,7 +392,7 @@ mod windows { .map(|(name, value)| format!("std\t{}\t{}", hex(name), hex(value))) .collect(); let report = String::from_utf8_lossy(&output.stdout); - let prefixes: Vec = TOOL_VARIABLES + let prefixes: Vec = INJECTED_VARIABLES .iter() .flat_map(|name| { let name = hex(&OsStr::new(name).encode_wide().collect::>()); @@ -402,7 +419,7 @@ mod windows { shown.join("\n") ))); } - println!("{RANDOM_ENVIRONMENTS} random environments through the block builder: every child holds exactly its variables"); + println!("{environments} random environments through the block builder: every child holds exactly its variables"); Ok(()) } diff --git a/tests/command_line_parsers.rs b/tests/command_line_parsers.rs index c9c1136..0bc121d 100644 --- a/tests/command_line_parsers.rs +++ b/tests/command_line_parsers.rs @@ -21,7 +21,7 @@ mod windows { use windows_spawn::{Command, Stdio}; use windows_sys::Win32::Foundation::LocalFree; - use crate::random::Generator; + use crate::random::{scaled, Generator}; use windows_sys::Win32::System::Environment::GetCommandLineW; use windows_sys::Win32::System::LibraryLoader::{GetProcAddress, LoadLibraryW}; use windows_sys::Win32::UI::Shell::CommandLineToArgvW; @@ -360,7 +360,8 @@ mod windows { fn random_lines() -> io::Result<()> { let mut generator = Generator::new(SEED); let mut mismatches = Vec::new(); - for _ in 0..RANDOM_LINES { + let lines = scaled(RANDOM_LINES)?; + for _ in 0..lines { let count = generator.below(MOST_ARGUMENTS).saturating_add(1); let arguments: Vec> = (0..count) .map(|_| generator.text(LONGEST_ARGUMENT, &INTERESTING)) @@ -394,9 +395,7 @@ mod windows { shown.join("\n") ))); } - println!( - "{RANDOM_LINES} random command lines through four parsers: every argument read back" - ); + println!("{lines} random command lines through four parsers: every argument read back"); Ok(()) } diff --git a/tests/random/mod.rs b/tests/random/mod.rs index b256fb7..b2223bf 100644 --- a/tests/random/mod.rs +++ b/tests/random/mod.rs @@ -7,6 +7,26 @@ const MULTIPLIER: u64 = 0x2545_f491_4f6c_dd1d; /// The units after NUL, which no argument or variable may hold. const NONZERO_UNITS: u64 = 0xffff; +/// The variable that multiplies the number of random cases, which the nightly workflow raises. +const SCALE: &str = "WINDOWS_SPAWN_RANDOM_SCALE"; + +/// Returns `cases` times `WINDOWS_SPAWN_RANDOM_SCALE`, or `cases` when the variable is unset. +/// +/// The seed stays the same, so a larger run checks the cases of a smaller one first. +pub(crate) fn scaled(cases: usize) -> std::io::Result { + let invalid = |error| std::io::Error::new(std::io::ErrorKind::InvalidInput, error); + let scale = match std::env::var(SCALE) { + Ok(text) => text + .parse::() + .map_err(|error| invalid(error.to_string()))?, + Err(std::env::VarError::NotPresent) => 1, + Err(error) => return Err(invalid(error.to_string())), + }; + cases + .checked_mul(scale) + .ok_or_else(|| invalid(format!("{SCALE} times {cases} overflows"))) +} + pub(crate) struct Generator(u64); impl Generator { diff --git a/tests/windows_spawn.rs b/tests/windows_spawn.rs index 23c2789..c1f5c51 100644 --- a/tests/windows_spawn.rs +++ b/tests/windows_spawn.rs @@ -341,18 +341,20 @@ fn requested_mitigation_is_visible_on_the_real_process() -> io::Result<()> { Ok(()) } +/// Returns the number of handles this process holds. +fn handle_count() -> io::Result { + let mut count = 0; + // SAFETY: the pseudo-handle is valid and `count` is writable DWORD storage. + let success = unsafe { GetProcessHandleCount(GetCurrentProcess(), &mut count) }; + if success == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(count) + } +} + #[test] fn failed_transactions_do_not_leak_handles() -> io::Result<()> { - fn handle_count() -> io::Result { - let mut count = 0; - // SAFETY: the pseudo-handle is valid and `count` is writable DWORD storage. - let success = unsafe { GetProcessHandleCount(GetCurrentProcess(), &mut count) }; - if success == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(count) - } - } fn fail_after_acquisition(missing_directory: &Path) { let mut command = cmd("exit /b 0"); command @@ -1123,6 +1125,211 @@ fn reusable_command_environment_and_accessors_work() -> io::Result<()> { Ok(()) } +/// A way a spawn can go, which [`spawn_cleanup_stress`] takes in turn. +#[derive(Clone, Copy, Debug)] +enum StressMode { + Wait, + Resume, + DropSuspended, + Status, + Output, + Input, + KillTree, + TreeOutput, + Handles, + Job, + Parent, + FailedCreation, + FailedSearch, +} + +impl StressMode { + const ALL: [Self; 13] = [ + Self::Wait, + Self::Resume, + Self::DropSuspended, + Self::Status, + Self::Output, + Self::Input, + Self::KillTree, + Self::TreeOutput, + Self::Handles, + Self::Job, + Self::Parent, + Self::FailedCreation, + Self::FailedSearch, + ]; +} + +/// The exit code of the stress children, which no termination gives. +const STRESS_EXIT: i32 = 3; + +/// Requires `status` to carry the stress children's exit code. +fn stress_exit(status: std::process::ExitStatus) -> io::Result<()> { + if status.code() == Some(STRESS_EXIT) { + Ok(()) + } else { + Err(io::Error::other(format!( + "exited with {status:?}, not {STRESS_EXIT}" + ))) + } +} + +/// Requires `spawned` to fail in `operation`. +fn stress_failure(spawned: windows_spawn::Result, operation: Operation) -> io::Result<()> { + match spawned { + Err(SpawnError::Windows(failure)) if failure.operation() == operation => Ok(()), + Err(error) => Err(io::Error::other(format!( + "failed with {error}, not in {operation}" + ))), + Ok(_) => Err(io::Error::other(format!( + "succeeded, not failed in {operation}" + ))), + } +} + +/// What the stress spawns borrow, created before the count starts. +struct Stress { + nul: File, + job: Job, + parent: ParentProcess, + directory: TempDir, +} + +impl Stress { + fn new() -> io::Result { + Ok(Self { + nul: File::options().read(true).write(true).open("NUL")?, + job: Job::create()?, + parent: ParentProcess::open(std::process::id())?, + directory: TempDir::new("stress")?, + }) + } + + fn run(&self, mode: StressMode) -> io::Result<()> { + let mut command = cmd("exit /b 3"); + match mode { + StressMode::Wait => stress_exit(command.spawn()?.wait()?), + StressMode::Resume => stress_exit(command.spawn_suspended()?.resume()?.wait()?), + StressMode::DropSuspended => { + drop(command.spawn_suspended()?); + Ok(()) + } + StressMode::Status => stress_exit(command.status()?), + StressMode::Output => { + let output = cmd("echo out& echo err 1>&2").output()?; + if output.stdout.starts_with(b"out") && output.stderr.starts_with(b"err") { + Ok(()) + } else { + Err(io::Error::other(format!("read {output:?}"))) + } + } + StressMode::Input => { + let mut sort = cmd("sort"); + let mut child = sort.stdin(Stdio::piped()).stdout(Stdio::piped()).spawn()?; + let mut input = child + .stdin + .take() + .ok_or_else(|| io::Error::other("stdin is piped"))?; + input.write_all(b"typed\r\n")?; + drop(input); + let output = child.wait_with_output()?; + if output.stdout.starts_with(b"typed") { + Ok(()) + } else { + Err(io::Error::other(format!("read {output:?}"))) + } + } + StressMode::KillTree => { + let mut waiting = cmd("set /p x="); + let mut child = waiting.stdin(Stdio::piped()).own_process_tree().spawn()?; + child.kill()?; + match child.wait_tree()?.code() { + Some(1) => Ok(()), + code => Err(io::Error::other(format!( + "a killed tree exited with {code:?}" + ))), + } + } + StressMode::TreeOutput => { + let output = cmd("echo tree").own_process_tree().output()?; + if output.stdout.starts_with(b"tree") { + Ok(()) + } else { + Err(io::Error::other(format!("read {output:?}"))) + } + } + StressMode::Handles => { + command + .arg_handle(&self.nul)? + .env_handle("WINDOWS_SPAWN_HANDLE", &self.nul)?; + stress_exit(command.status()?) + } + StressMode::Job => { + stress_exit(command.status_with(SpawnOptions::new().job(&self.job))?) + } + StressMode::Parent => { + command + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + stress_exit(command.status_with(SpawnOptions::new().parent_process(&self.parent))?) + } + StressMode::FailedCreation => { + command + .current_dir(self.directory.join("missing")) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .own_process_tree(); + stress_failure(command.spawn(), Operation::CreateProcess) + } + StressMode::FailedSearch => { + let mut missing = Command::new("windows-spawn-missing-program"); + missing + .env_clear() + .stdout(Stdio::piped()) + .own_process_tree(); + stress_failure(missing.spawn(), Operation::ResolveExecutable) + } + } + } +} + +/// Spawns children in every [`StressMode`], `WINDOWS_SPAWN_STRESS_CASES` times in all, and requires this process's handle count not to grow. +/// +/// Each mode runs twice before the count starts, so what the system creates once, such as a thread-pool worker, is already there. +/// It runs only on request, and alone, because the count belongs to the process: +/// `cargo test --test windows_spawn spawn_cleanup_stress -- --ignored --exact`. +#[test] +#[ignore = "set WINDOWS_SPAWN_STRESS_CASES, and run it alone"] +fn spawn_cleanup_stress() -> io::Result<()> { + let cases: usize = std::env::var("WINDOWS_SPAWN_STRESS_CASES") + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))? + .parse() + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; + let stress = Stress::new()?; + for mode in StressMode::ALL.into_iter().chain(StressMode::ALL) { + stress.run(mode)?; + } + let before = handle_count()?; + for (case, mode) in StressMode::ALL.into_iter().cycle().take(cases).enumerate() { + stress + .run(mode) + .map_err(|error| io::Error::other(format!("case {case}, {mode:?}: {error}")))?; + } + let after = handle_count()?; + assert!( + after <= before, + "the handle count grew from {before} to {after} over {cases} spawns" + ); + println!( + "{cases} spawns in {} modes: the handle count went from {before} to {after}", + StressMode::ALL.len() + ); + Ok(()) +} + #[test] fn dropping_suspended_child_prevents_execution() -> io::Result<()> { let (probe, _gate, report) = ProbeCommand::new(Role::Ran)?; diff --git a/xtask/src/gates.rs b/xtask/src/gates.rs index b18bf7d..5ffea55 100644 --- a/xtask/src/gates.rs +++ b/xtask/src/gates.rs @@ -63,24 +63,6 @@ const EXCEPTIONS: &[Exception] = &[ count: 1, reason: "tests assert by panicking", }, - Exception { - file: "tests/argv_roundtrip.rs", - lint: "clippy::as_conversions", - count: 1, - reason: "tests call Win32 directly with handle values", - }, - Exception { - file: "tests/argv_roundtrip.rs", - lint: "clippy::expect_used", - count: 1, - reason: "tests assert by panicking", - }, - Exception { - file: "tests/argv_roundtrip.rs", - lint: "clippy::unwrap_in_result", - count: 1, - reason: "tests assert by panicking", - }, ]; /// Identifiers that measure or wait on time (ADR 0010). @@ -178,7 +160,7 @@ impl fmt::Display for Finding { /// Checks every Rust file under the repository's source directories. pub(crate) fn check(root: &Path) -> io::Result> { let mut files = Vec::new(); - for directory in ["src", "tests", "examples", "xtask/src"] { + for directory in ["src", "tests", "examples", "fuzz/fuzz_targets", "xtask/src"] { collect_rust_files(&root.join(directory), &mut files)?; } files.sort();