diff --git a/Cargo.lock b/Cargo.lock index d4ea00b29b..8c90a30780 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "cc", "libc", @@ -5622,13 +5622,14 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "inkwell", "llvm-sys", "log", "perry-api-manifest", + "perry-codegen", "perry-dispatch", "perry-hir", "serde", @@ -5638,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-hir", @@ -5646,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-hir", @@ -5654,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-dispatch", @@ -5663,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-hir", @@ -5671,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "base64", @@ -5683,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-hir", @@ -5691,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "async-trait", @@ -5720,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "serde", "serde_json", @@ -5735,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1290" +version = "0.5.1291" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5746,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "clap", @@ -5761,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "block2", "objc2", @@ -5771,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "argon2", "perry-ffi", @@ -5779,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "reqwest", @@ -5788,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "bcrypt", "perry-ffi", @@ -5796,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "rusqlite", @@ -5804,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "scraper", @@ -5812,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "perry-runtime", @@ -5820,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "chrono", "cron", @@ -5830,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "chrono", "perry-ffi", @@ -5838,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "rust_decimal", @@ -5846,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "serde_json", @@ -5854,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5862,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "perry-runtime", @@ -5870,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "bytes", "http-body-util", @@ -5895,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "bytes", "lazy_static", @@ -5908,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "bytes", "h2", @@ -5932,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "lazy_static", "perry-ffi", @@ -5942,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "jsonwebtoken", @@ -5953,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "lru", "perry-ffi", @@ -5962,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "chrono", "perry-ffi", @@ -5970,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "bson", "futures-util", @@ -5982,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "chrono", "perry-ffi", @@ -5992,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "nanoid", "perry-ffi", @@ -6001,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "bytes", "perry-ffi", @@ -6014,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6033,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "lettre", "perry-ffi", @@ -6043,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "printpdf", @@ -6051,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "sqlx", @@ -6060,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "governor", "perry-ffi", @@ -6068,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "fast_image_resize", "image", @@ -6078,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "lazy_static", "perry-ffi", @@ -6094,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "perry-runtime", @@ -6103,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "uuid", @@ -6111,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ffi", "regex", @@ -6121,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "futures-util", "lazy_static", @@ -6134,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "brotli", "flate2", @@ -6144,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "dashmap", "once_cell", @@ -6153,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-api-manifest", @@ -6171,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-diagnostics", @@ -6183,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "base64", @@ -6225,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6327,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "perry-hir", @@ -6343,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "itoa", @@ -6367,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "rand 0.10.1", "serde", @@ -6377,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6400,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "block2", @@ -6416,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "block2", @@ -6431,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1290" +version = "0.5.1291" [[package]] name = "perry-ui-test" @@ -6442,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1290" +version = "0.5.1291" [[package]] name = "perry-ui-tvos" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "block2", @@ -6462,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "block2", @@ -6478,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "block2", "libc", @@ -6491,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "base64", "libc", @@ -6508,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "anyhow", "base64", @@ -6531,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1290" +version = "0.5.1291" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 3083936d28..1e9d7e0c6c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1290" +version = "0.5.1291" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7509-integration-suites-native-roots.md b/changelog.d/7509-integration-suites-native-roots.md new file mode 100644 index 0000000000..d0aecdde9f --- /dev/null +++ b/changelog.d/7509-integration-suites-native-roots.md @@ -0,0 +1,28 @@ +### Fixed + +**`perry-codegen` integration suites can declare which root lowering they assert (#7493).** + +#7370 made native roots (RS4GC statepoints) the default lowering, and `NativeRootsPin` was added so a test asserting on shadow-stack IR could say so. The in-crate unit tests were repaired with it; the five integration suites that assert the same mechanics were not, because `NativeRootsPin` is `#[cfg(test)]` and `tests/*.rs` link the crate as an ordinary external consumer — the item does not exist for them. Those suites run nightly/at-tag only, so nothing turned red at merge time and `shadow_slot_hygiene` sat at 0/12 on `main`. + +The pin is now reachable as `perry_codegen::testing::NativeRootsPin`, behind a `testing` cargo feature that only `perry-codegen`'s own `[dev-dependencies]` entry on itself enables. It is a feature rather than a `#[doc(hidden)] pub` because with the feature off the pin, its thread-local and the branch it adds to `rs4gc_enabled()` are `#[cfg]`-ed out of the artifact — absent, not private — and cargo resolves dev-dependency features only for test/bench targets, so no production profile can reach it (`cargo tree -e features,no-dev` shows `default` + `llvm-inprocess` only). `NativeRootsPin::native()` joins `shadow()`: a pin also outranks `PERRY_RS4GC`, which is what keeps a pinned assertion meaning the same thing during a `PERRY_RS4GC=0` sweep. + +Pins are per test, not per file — the files disagree internally: + +| suite | shadow | native | unpinned | before → after | +|---|---|---|---|---| +| `shadow_slot_hygiene` | 12 | – | – | 0/12 → 11/12 | +| `scalar_replaced_slot_roots` | 11 | – | – | 2/11 → 5/11 | +| `temp_root_operand_temporaries` | 2 | – | 17 | 12/19 → 13/19 | +| `temp_root_argument_temporaries` | – | – | 7 | 3/7 → 3/7 | +| `native_proof_regressions` | 2 | 15 | 238 | 249/253 → 253/255 | +| `native_proof_buffer_views` | – | 1 | 31 | 28/30 → 30/32 | + +Three tests were pinned although they were **passing**: `numeric_only_scalar_replaced_{object,array}_emits_no_rooting` and `a_collection_free_construction_emits_no_this_slot_root` assert `bind_calls(&ir) == 0` / `!contains("@js_shadow_slot_bind")`, which under the native default is true of every program. They were green without their subject running. Two of the three now fail for a real reason (#7504) — a red test that measures something beats a green one that does not. + +The fifteen `invalidation` pins are `native()` for a different reason: `assert_buffer_store_uses_dynamic_fallback` proves the absence of a native buffer GEP with a module-wide `!ir.contains("getelementptr inbounds i8")`, and the shadow lowering's inline slot addressing emits that instruction for unrelated reasons, so `PERRY_RS4GC=0` made them report a stale proof that was never there (#7505). + +**A poisoned `ARTIFACT_ENV_LOCK` turned 4 failures into 55.** `native_proof_regressions` reported 55 failures at default parallelism and 4 under `--test-threads=1`; 51 of the 55 were `PoisonError` — #7490's shape again. `PERRY_NATIVE_REPS*` are process-global and the restore was hand-written after the compile, so a panic inside `compile_module` left them installed and every later unlocked compile wrote artifact JSON into a directory another test was reading; the torn read panicked inside the lock. Both suites' copy-pasted harnesses are replaced by one `tests/native_proof_support/mod.rs` with a poison-tolerant accessor, an RAII env guard and an artifact reader that treats a foreign or half-written neighbour as noise. Two sabotage tests plant each failure shape and fail against the pre-fix code. Default-parallelism result: 198/253 → 253/255. + +**Tripwire, in the required tier.** `codegen::testing_feature_gate_tests::host_target_lowering_default_is_native_roots` lives in `src/`, so it runs in `cargo-test`: a future flip of the lowering default fails there, in the PR that makes it, naming the suites that then need re-pinning. It asserts its subject is live (both pins must give different answers; the `arm64_32-apple-watchos` arm must give the opposite default), so a constant-folded `rs4gc_enabled()` fails it rather than passing it. A sibling gate scans every workspace manifest and fails if a non-dev dependency edge ever enables the `testing` feature. + +Fifteen failures remain across these suites, none of them #7370's and none hidden: #7494 (three, pre-existing), #7503 (ten — the temp-root suites assert the pre-#7487 FFI spelling, and the eight that still pass are vacuous), #7504 (six plus `flat_const_row_aliases`), #7506 (one). No test was deleted, skipped or weakened. The per-PR source→suite mapping for these six suites is designed in #7507 and deliberately **not** landed here: they are not green, and a non-required job that is red on most PRs can never be promoted. The coverage question the pins raise — nine root-lowering mechanics with no assertion against the lowering that actually ships — is #7502. diff --git a/crates/perry-codegen/Cargo.toml b/crates/perry-codegen/Cargo.toml index e4bf5b4e52..11d6a676f3 100644 --- a/crates/perry-codegen/Cargo.toml +++ b/crates/perry-codegen/Cargo.toml @@ -28,6 +28,18 @@ workspace = true default = ["llvm-inprocess"] llvm-inprocess = ["dep:inkwell", "dep:llvm-sys"] +# Test-support only (#7493): exposes `perry_codegen::testing`, whose +# `NativeRootsPin` lets a test declare WHICH root lowering it asserts on. It is +# a feature rather than a `#[doc(hidden)] pub` so that with the feature off the +# pin, its thread-local and the branch it adds to `rs4gc_enabled()` are not in +# the artifact at all — see `src/testing.rs`. +# +# The ONLY edge that enables it is the `[dev-dependencies]` entry below, which +# cargo builds for test/bench targets only. `codegen::helpers:: +# testing_feature_gate_tests` fails the per-PR `cargo-test` job if any +# non-dev manifest section ever turns it on. +testing = [] + [dependencies] perry-hir.workspace = true perry-dispatch.workspace = true @@ -41,3 +53,12 @@ serde_json.workspace = true inkwell = { version = "0.9.0", features = ["llvm22-1"], optional = true } llvm-sys = { version = "221", optional = true } + +# Self dev-dependency (#7493). This is the whole mechanism by which the +# integration suites under `tests/` — which link this crate as an ordinary +# external consumer — can see `perry_codegen::testing`. Cargo supports +# dev-dependency cycles and resolves dev-dependency features ONLY when a +# test/bench target is being built, so `cargo build`/`--release`/`--profile +# dist` still compile the library with `testing` off. +[dev-dependencies] +perry-codegen = { path = ".", features = ["testing"] } diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index 858fa9fa4a..d8d5df03f2 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -114,7 +114,7 @@ pub(crate) fn precise_root_analysis_enabled() -> bool { /// explicit bridge's hand emission and its conservative CFG-union liveness. /// Requires an `opt` binary (`PERRY_LLVM_OPT`, Homebrew LLVM, or PATH). pub(crate) fn rs4gc_enabled() -> bool { - #[cfg(test)] + #[cfg(any(test, feature = "testing"))] if let Some(pinned) = NATIVE_ROOTS_OVERRIDE.with(|c| c.get()) { return pinned; } @@ -143,38 +143,61 @@ thread_local! { static NATIVE_ROOTS_TARGET_OK: std::cell::Cell = const { std::cell::Cell::new(false) }; } -/// Test-only RAII pin for the lowering under test. +// The pin's backing cell. Separate from `NATIVE_ROOTS_TARGET_OK` on purpose: +// `compile_module` calls `set_native_roots_for_target` per module, so a pin +// that wrote the target cell would be overwritten the moment the test invoked +// codegen. This is consulted FIRST and the per-module decision cannot clear it. +#[cfg(any(test, feature = "testing"))] +thread_local! { + static NATIVE_ROOTS_OVERRIDE: std::cell::Cell> = + const { std::cell::Cell::new(None) }; +} + +/// Test-support RAII pin for the root lowering under test. /// /// Now that native roots are the default, a test that asserts on shadow-stack /// IR has to SAY so — it used to get that lowering by accident, because there -/// was only one default. Eight tests broke on exactly this when the default -/// flipped, and every one of them was correct about what it asserted. +/// was only one default. Eight in-crate tests broke on exactly this when the +/// default flipped, and every one of them was correct about what it asserted; +/// five integration suites broke the same way and stayed red for weeks, because +/// this type was `#[cfg(test)]` and they could not reach it (#7493). /// /// Thread-local and restoring, so one test pinning a lowering cannot change /// another's — the same discipline `arena::quarantine`'s `ProtectionModeGuard` -/// already uses for the from-space instrument. -#[cfg(test)] -thread_local! { - /// Separate from `NATIVE_ROOTS_TARGET_OK` on purpose: `compile_module` - /// calls `set_native_roots_for_target` per module, so a pin that wrote the - /// target cell would be overwritten the moment the test invoked codegen. - /// This is consulted FIRST and the per-module decision cannot clear it. - static NATIVE_ROOTS_OVERRIDE: std::cell::Cell> = - const { std::cell::Cell::new(None) }; -} - -#[cfg(test)] -pub(crate) struct NativeRootsPin(Option); +/// already uses for the from-space instrument. Safe under `cargo test`'s +/// default parallelism. +/// +/// Reachable from `tests/*.rs` as `perry_codegen::testing::NativeRootsPin`; see +/// [`crate::testing`] for why that is behind a cargo feature rather than an +/// unconditional `pub`. +#[cfg(any(test, feature = "testing"))] +pub struct NativeRootsPin(Option); -#[cfg(test)] +#[cfg(any(test, feature = "testing"))] impl NativeRootsPin { - /// Pin this thread to the shadow-stack lowering for the guard's lifetime. - pub(crate) fn shadow() -> Self { + /// Pin this thread to the **shadow-stack** lowering for the guard's + /// lifetime — Perry's heap-backed shadow frame, `js_shadow_frame_enter` + + /// per-slot binds. + pub fn shadow() -> Self { NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(false)))) } + + /// Pin this thread to the **native-roots** (RS4GC statepoint) lowering: + /// `ptr addrspace(1)` root allocas, `gc "statepoint-example"`, relocations + /// inserted by LLVM. + /// + /// This is today's default on every target the runtime can walk, so a test + /// that wants it does not strictly *need* the pin — but a pin is not + /// redundant: it also overrides `PERRY_RS4GC` from the environment, so the + /// assertion means the same thing during a `PERRY_RS4GC=0` bisection run as + /// it does in CI. Without it, a whole-suite sweep under the process-global + /// env knob silently retargets every unpinned test at the other lowering. + pub fn native() -> Self { + NativeRootsPin(NATIVE_ROOTS_OVERRIDE.with(|c| c.replace(Some(true)))) + } } -#[cfg(test)] +#[cfg(any(test, feature = "testing"))] impl Drop for NativeRootsPin { fn drop(&mut self) { NATIVE_ROOTS_OVERRIDE.with(|c| c.set(self.0)); diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 768e9c5c56..5c1e7e5fe2 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -60,6 +60,8 @@ mod number_exactness_tests; mod opts; mod spec_abi; mod string_pool; +#[cfg(test)] +mod testing_feature_gate_tests; mod typed_abi; mod typed_abi_opt_report; diff --git a/crates/perry-codegen/src/codegen/testing_feature_gate_tests.rs b/crates/perry-codegen/src/codegen/testing_feature_gate_tests.rs new file mode 100644 index 0000000000..e25927f7d5 --- /dev/null +++ b/crates/perry-codegen/src/codegen/testing_feature_gate_tests.rs @@ -0,0 +1,328 @@ +//! #7493 — the `testing` cargo feature must never reach a production build. +//! +//! `perry_codegen::testing::NativeRootsPin` lets a test declare which root +//! lowering it asserts on. Its whole safety argument is that with the feature +//! off, the pin, its thread-local, and the branch it adds to the top of +//! `rs4gc_enabled()` are `#[cfg]`-ed out of the artifact — and that the only +//! manifest edge which turns the feature on is a `[dev-dependencies]` one, +//! which cargo resolves for test/bench targets only. +//! +//! The second half of that argument is a claim about manifests, and claims +//! about manifests rot. This is the gate for it. It lives in `src/`, so unlike +//! the integration suites it runs in the per-PR `cargo-test` job — which is +//! the tier #7493 exists because these suites are *not* in. +//! +//! Built so it can fail (CLAUDE.md, "four ways a gate can be unable to fail"): +//! the scan asserts its own subject was live. It must find at least one +//! manifest, must find the crate's own manifest among them, and must find the +//! dev-dependency edge that legitimately enables the feature — so "0 offenders +//! over 0 manifests" and "0 offenders over 40 manifests" cannot report the same +//! verdict. `feature_scan_flags_a_planted_production_edge` then plants the +//! exact offending shape in an in-memory manifest and asserts the scanner names +//! it. +//! +//! This file also holds the **root-lowering default tripwire**. #7370 flipped +//! the default from shadow stack to native roots, and the five integration +//! suites that assert on the losing lowering went red without anything in the +//! per-PR tier noticing — they run nightly/at-tag only. A default flip is a +//! deliberate act, so the fix is not to detect it after the fact but to make it +//! impossible to land silently: `host_target_lowering_default_is_native_roots` +//! fails in `cargo-test` (a REQUIRED context) the moment the default changes, +//! and its message names the suites that then need re-pinning. + +use std::path::{Path, PathBuf}; + +/// Sections whose entries cargo builds for NON-test targets. An edge here that +/// enabled `testing` would put the pin in shipped binaries. +const PRODUCTION_SECTIONS: [&str; 2] = ["dependencies", "build-dependencies"]; + +/// Workspace root, from this file's location. +fn workspace_root() -> PathBuf { + // /crates/perry-codegen/src/codegen/testing_feature_gate_tests.rs + Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(Path::parent) + .expect("crates/ should have two ancestors") + .to_path_buf() +} + +/// Every `Cargo.toml` in the workspace (root + one per crate). +fn workspace_manifests() -> Vec { + let root = workspace_root(); + let mut out = vec![root.join("Cargo.toml")]; + if let Ok(entries) = std::fs::read_dir(root.join("crates")) { + let mut crate_manifests: Vec = entries + .filter_map(Result::ok) + .map(|e| e.path().join("Cargo.toml")) + .filter(|p| p.is_file()) + .collect(); + crate_manifests.sort(); + out.extend(crate_manifests); + } + out +} + +/// A TOML section header line, e.g. `[dependencies.perry-codegen]` -> the +/// section path segments. Returns `None` for non-header lines. +fn section_path(line: &str) -> Option> { + let trimmed = line.trim(); + let inner = trimmed.strip_prefix('[')?.strip_suffix(']')?; + // `[[bin]]` and friends: strip the extra bracket pair. + let inner = inner.strip_prefix('[').unwrap_or(inner); + let inner = inner.strip_suffix(']').unwrap_or(inner); + Some( + inner + .split('.') + .map(|s| s.trim().trim_matches('"').to_string()) + .collect(), + ) +} + +/// Is this section one cargo builds for non-test targets? +/// +/// Handles both `[dependencies]` and target-specific +/// `[target.'cfg(unix)'.dependencies]`, and rejects every `dev-dependencies` +/// spelling of the same. +fn is_production_dependency_section(path: &[String]) -> bool { + // The section name that decides is the last one that names a dependency + // table — `[dependencies.foo]` and `[target.X.dependencies.foo]` both carry + // it, as does `[workspace.dependencies]`. + path.iter() + .any(|seg| PRODUCTION_SECTIONS.contains(&seg.as_str())) + && !path.iter().any(|seg| seg == "dev-dependencies") +} + +/// Scan one manifest's text. Returns `(offending lines, saw a dev edge that +/// enables the feature)`. +fn scan_manifest(label: &str, text: &str) -> (Vec, bool) { + let mut offenders = Vec::new(); + let mut saw_dev_edge = false; + let mut section: Vec = Vec::new(); + for (lineno, line) in text.lines().enumerate() { + if let Some(path) = section_path(line) { + section = path; + continue; + } + if !line.contains("testing") { + continue; + } + // Only dependency-table lines matter, and only ones naming this crate. + let names_this_crate = + line.contains("perry-codegen") || section.iter().any(|s| s == "perry-codegen"); + if !names_this_crate { + continue; + } + // `features = [..., "testing", ...]` — quoted, so a `testing = []` + // feature DEFINITION in `[features]` is not mistaken for an edge. + if !line.contains("\"testing\"") { + continue; + } + if is_production_dependency_section(§ion) { + offenders.push(format!( + "{label}:{}: [{}] enables perry-codegen's `testing` feature: {}", + lineno + 1, + section.join("."), + line.trim() + )); + } else if section.iter().any(|seg| seg == "dev-dependencies") { + saw_dev_edge = true; + } + } + (offenders, saw_dev_edge) +} + +#[test] +fn testing_feature_is_never_enabled_by_a_production_dependency_edge() { + let manifests = workspace_manifests(); + assert!( + manifests.len() > 10, + "the manifest scan found only {} files — the glob is broken and this \ + gate would be vacuously green", + manifests.len() + ); + + let mut offenders = Vec::new(); + let mut saw_dev_edge = false; + let mut saw_own_manifest = false; + for path in &manifests { + let Ok(text) = std::fs::read_to_string(path) else { + continue; + }; + let label = path + .strip_prefix(workspace_root()) + .unwrap_or(path) + .display() + .to_string(); + if label.contains("perry-codegen") { + saw_own_manifest = true; + } + let (mut found, dev) = scan_manifest(&label, &text); + offenders.append(&mut found); + saw_dev_edge |= dev; + } + + assert!( + saw_own_manifest, + "the scan never reached crates/perry-codegen/Cargo.toml" + ); + assert!( + saw_dev_edge, + "no [dev-dependencies] edge enables perry-codegen's `testing` feature — \ + either the mechanism was removed (in which case delete this gate and \ + the feature) or the scanner stopped recognising it, which would make \ + its clean verdict meaningless" + ); + assert!( + offenders.is_empty(), + "the `testing` feature is test-support only and must not be reachable \ + from a production build:\n {}", + offenders.join("\n ") + ); +} + +/// Sabotage: the scanner must actually name a production edge, not merely +/// return an empty list because it looks at nothing. +#[test] +fn feature_scan_flags_a_planted_production_edge() { + let planted = "\ +[package] +name = \"pretend\" + +[dependencies] +perry-codegen = { path = \"../perry-codegen\", features = [\"testing\"] } + +[dev-dependencies] +perry-codegen = { path = \"../perry-codegen\", features = [\"testing\"] } +"; + let (offenders, saw_dev_edge) = scan_manifest("planted/Cargo.toml", planted); + assert_eq!( + offenders.len(), + 1, + "the production edge should be the only offender: {offenders:?}" + ); + assert!( + offenders[0].contains("[dependencies]"), + "the offender should name the section it was found in: {}", + offenders[0] + ); + assert!( + saw_dev_edge, + "the dev edge should be recognised, not counted as an offender" + ); + + // …and a target-specific production table is the same offence. + let target_scoped = "\ +[target.'cfg(unix)'.dependencies] +perry-codegen = { path = \"../perry-codegen\", features = [\"testing\"] } +"; + let (offenders, _) = scan_manifest("planted2/Cargo.toml", target_scoped); + assert_eq!( + offenders.len(), + 1, + "a target-specific [dependencies] table is still a production edge: \ + {offenders:?}" + ); + + // …while the feature DEFINITION itself is not an edge. + let definition_only = "\ +[package] +name = \"perry-codegen\" + +[features] +testing = [] +"; + let (offenders, _) = scan_manifest("planted3/Cargo.toml", definition_only); + assert!( + offenders.is_empty(), + "declaring the feature is not enabling it: {offenders:?}" + ); +} + +/// Tripwire (#7493): the root-lowering default on a host the runtime can walk +/// must be NATIVE ROOTS, and the pin must be able to select either lowering. +/// +/// The suites listed in the failure message assert on one specific lowering and +/// say so with `perry_codegen::testing::NativeRootsPin`. WHICH pin each of them +/// needs is a function of this default. When #7370 flipped it, nothing in the +/// per-PR tier went red — those suites run nightly/at-tag only — and +/// `shadow_slot_hygiene` sat at 0/12 on `main` until someone ran the tier by +/// hand. This test lives in `src/`, so it runs in `cargo-test`, which IS a +/// required context: a future flip fails HERE, in the PR that makes it, with +/// the follow-on work named in the failure message. +/// +/// It asserts its subject was live rather than merely that nothing threw: the +/// two pins must produce DIFFERENT answers, and the unsupported-target arm must +/// produce the opposite default — so an `rs4gc_enabled()` wired to a constant +/// fails this test rather than passing it. +#[test] +fn host_target_lowering_default_is_native_roots() { + use super::helpers::{rs4gc_enabled, set_native_roots_for_target}; + use crate::testing::NativeRootsPin; + + // `PERRY_RS4GC` is an explicit, process-global override cached in a + // `OnceLock`; under it the DEFAULT is not what is being measured. + if std::env::var("PERRY_RS4GC").is_ok() { + return; + } + + for triple in [ + "aarch64-apple-darwin", + "arm64-apple-macosx15.0.0", + "x86_64-unknown-linux-gnu", + ] { + set_native_roots_for_target(triple); + assert!( + rs4gc_enabled(), + "the root-lowering default for {triple} is no longer native roots. \ + If that is intentional, re-pin the suites that assert on a specific \ + lowering BEFORE landing it — crates/perry-codegen/tests/ \ + shadow_slot_hygiene.rs, scalar_replaced_slot_roots.rs, \ + temp_root_operand_temporaries.rs, native_proof_regressions.rs (and \ + its invalidation module) and native_proof_buffer_views.rs — then \ + update this test. #7493 is what happens when that step is skipped: \ + those suites run nightly/at-tag only, so nothing goes red at merge \ + time." + ); + + // The pin must be able to say BOTH things, or a suite that declares its + // lowering is declaring nothing. + { + let _pin = NativeRootsPin::shadow(); + assert!( + !rs4gc_enabled(), + "NativeRootsPin::shadow() must select the shadow-stack lowering" + ); + } + assert!( + rs4gc_enabled(), + "the pin must restore the previous decision" + ); + { + let _pin = NativeRootsPin::native(); + assert!( + rs4gc_enabled(), + "NativeRootsPin::native() must select the native-roots lowering" + ); + } + } + + // A target whose frame bases the runtime cannot resolve must still fall + // back to the shadow stack — otherwise `gc_map` refuses and the compile + // fails outright. This arm is what makes the assertion above a statement + // about the DEFAULT rather than about a constant. + set_native_roots_for_target("arm64_32-apple-watchos"); + assert!( + !rs4gc_enabled(), + "watchOS ILP32 has no native-root map reader; the default must fall \ + back to the shadow stack there" + ); + // …and an explicit pin still outranks it, which is what lets a test assert + // native-roots IR while a `PERRY_RS4GC=0` sweep is in progress. + { + let _pin = NativeRootsPin::native(); + assert!( + rs4gc_enabled(), + "an explicit pin outranks the per-target default" + ); + } +} diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 9e38656578..39d885f8a4 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -40,6 +40,12 @@ pub(crate) mod stmt; pub mod strings; pub mod stubs; pub mod target_layout; +/// Test-support surface — compiled only under `cfg(test)` or the `testing` +/// cargo feature (which nothing but this crate's own `[dev-dependencies]` +/// enables). See the module docs for why it is a feature and not a +/// `#[doc(hidden)] pub`. +#[cfg(any(test, feature = "testing"))] +pub mod testing; pub(crate) mod type_analysis; pub(crate) mod type_analysis_class_fields; pub(crate) mod type_analysis_facts; diff --git a/crates/perry-codegen/src/testing.rs b/crates/perry-codegen/src/testing.rs new file mode 100644 index 0000000000..b8d0f3e1e6 --- /dev/null +++ b/crates/perry-codegen/src/testing.rs @@ -0,0 +1,68 @@ +//! Test-support surface for `perry-codegen`'s own integration suites. +//! +//! # Why this module exists +//! +//! `crates/perry-codegen/tests/*.rs` are *external consumers* of this crate: +//! cargo builds the library normally and links the suite against it, so +//! `#[cfg(test)]` items — which exist only in the library's own unit-test +//! build — are unreachable from them. +//! +//! That is how #7493 happened. #7370 made native roots (RS4GC statepoints) the +//! default lowering. `NativeRootsPin` was added so a test asserting on +//! shadow-stack IR could SAY so, the in-crate unit tests were repaired with it +//! — and five integration suites, which had no pin to reach for, were left red +//! (`shadow_slot_hygiene` at 0/12) for as long as it took someone to run the +//! nightly-only tier by hand. +//! +//! # Why a cargo feature and not `#[doc(hidden)] pub` +//! +//! Three shapes were available: +//! +//! 1. `#[doc(hidden)] pub` on the pin, unconditionally. Rejected: the pin would +//! then exist in every shipped `perry` binary, and — worse — the +//! thread-local read it adds to `rs4gc_enabled()` would be compiled into the +//! production decision path. A knob that is merely undocumented is still a +//! knob; this repo's own history (`PERRY_GC_FORCE_EVACUATE`, the `--pressure` +//! knob) is a list of modes that existed without anyone having decided they +//! should. +//! 2. Moving the suites' bodies into in-crate `#[cfg(test)]` modules. Correct in +//! principle (#5960: it would also put them in the per-PR `cargo-test` gate), +//! but `native_proof_regressions.rs` alone is 14k lines and the repo caps +//! files at 2000 (`scripts/check_file_size.sh`). Not a mechanical move. +//! 3. This: a `testing` cargo feature, enabled **only** by `perry-codegen`'s own +//! `[dev-dependencies]` entry on itself. +//! +//! Option 3 cannot leak into production behaviour, and the argument is +//! structural rather than a promise: +//! +//! * With `testing` off, `NativeRootsPin`, its backing thread-local, and the +//! `if let Some(pinned) = …` arm at the top of `rs4gc_enabled()` **do not +//! exist in the artifact**. They are `#[cfg]`-ed out, not merely private or +//! unreachable, so there is no symbol to call and no branch to take. +//! * The only manifest edge that turns the feature on is a `[dev-dependencies]` +//! one. Cargo builds dev-dependencies for test/bench targets only, so +//! `cargo build`, `cargo build --release` and `--profile dist` never see it. +//! * A production dependency edge that enabled it anyway would be a silent +//! regression, so it is *gated*, not trusted: +//! `codegen::helpers::testing_feature_gate_tests` scans every workspace +//! manifest and fails if any non-dev section enables `perry-codegen`'s +//! `testing` feature. That test lives in `src/`, so it runs in the per-PR +//! `cargo-test` job — the tier this whole issue is about not being in. +//! +//! # Using it +//! +//! ```ignore +//! // in crates/perry-codegen/tests/.rs +//! use perry_codegen::testing::NativeRootsPin; +//! +//! #[test] +//! fn asserts_on_shadow_stack_ir() { +//! let _pin = NativeRootsPin::shadow(); +//! // … assertions about js_shadow_frame_enter / slot binds … +//! } +//! ``` +//! +//! The pin is thread-local and restoring, so it is safe under `cargo test`'s +//! default parallelism: one test's pin cannot retarget another's compile. + +pub use crate::codegen::helpers::NativeRootsPin; diff --git a/crates/perry-codegen/tests/native_proof_buffer_views.rs b/crates/perry-codegen/tests/native_proof_buffer_views.rs index 3eae5e7f70..6602213127 100644 --- a/crates/perry-codegen/tests/native_proof_buffer_views.rs +++ b/crates/perry-codegen/tests/native_proof_buffer_views.rs @@ -2,8 +2,17 @@ // toolkit, and each file drives a different subset of it. Per-file pruning // would make the next test in this family re-add the builder it needs, so the // toolkit stays whole. +// +// LOWERING (#7493): one test here — `loop_length_bound_does_not_prove_ +// multibyte_buffer_read_inbounds` — pins `NativeRootsPin::native()`. It proves +// the absence of a native buffer GEP with a module-wide +// `!ir.contains("getelementptr inbounds i8")`, and the shadow-stack lowering's +// own inline slot addressing emits that instruction for unrelated reasons, so +// under `PERRY_RS4GC=0` it reports a proof leak that is not there. Same shape, +// same reasoning and the same durable fix as the `invalidation` module: #7505. #![allow(dead_code)] +use perry_codegen::testing::NativeRootsPin; use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::types::{FunctionType, ObjectType, PropertyInfo, Type}; use perry_hir::{ @@ -11,7 +20,9 @@ use perry_hir::{ UpdateOp, }; -static ARTIFACT_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); +#[path = "native_proof_support/mod.rs"] +mod native_proof_support; +use native_proof_support::{artifact_env_lock, artifact_for_module, NativeRepsEnv}; fn empty_opts() -> CompileOptions { CompileOptions { @@ -147,7 +158,7 @@ fn compile_artifact_json_for_module_with_opts( opts: CompileOptions, ) -> serde_json::Value { let name = module.name.clone(); - let _guard = ARTIFACT_ENV_LOCK.lock().unwrap(); + let _guard = artifact_env_lock(); let dir = std::env::temp_dir().join(format!( "perry_native_reps_test_{}_{}", std::process::id(), @@ -156,40 +167,15 @@ fn compile_artifact_json_for_module_with_opts( let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); - let old_reps = std::env::var_os("PERRY_NATIVE_REPS"); - let old_reps_dir = std::env::var_os("PERRY_NATIVE_REPS_DIR"); - std::env::set_var("PERRY_NATIVE_REPS", "1"); - std::env::set_var("PERRY_NATIVE_REPS_DIR", &dir); - - let compile_result = compile_module(&module, opts); - - match old_reps { - Some(value) => std::env::set_var("PERRY_NATIVE_REPS", value), - None => std::env::remove_var("PERRY_NATIVE_REPS"), - } - match old_reps_dir { - Some(value) => std::env::set_var("PERRY_NATIVE_REPS_DIR", value), - None => std::env::remove_var("PERRY_NATIVE_REPS_DIR"), - } + let compile_result = { + // Restored before any fallible step below, and on an unwind out of + // `compile_module` itself. + let _env = NativeRepsEnv::install(&dir, false); + compile_module(&module, opts) + }; compile_result.unwrap(); - let paths: Vec<_> = std::fs::read_dir(&dir) - .unwrap() - .map(|entry| entry.unwrap().path()) - .collect(); - let mut parsed = Vec::new(); - for path in paths { - if !path.extension().is_some_and(|ext| ext == "json") { - continue; - } - let text = std::fs::read_to_string(&path).unwrap(); - let value: serde_json::Value = serde_json::from_str(&text).unwrap(); - if value["module"] == name { - return value; - } - parsed.push(value["module"].clone()); - } - panic!("native reps artifact for {name} not found in {dir:?}; saw modules {parsed:?}"); + artifact_for_module(&dir, &name) } fn assert_typed_array_get_fallback_reason(artifact: &serde_json::Value, reason: &str) { @@ -660,6 +646,7 @@ fn artifact_records_buffer_read_u32_and_unsigned_materialization() { #[test] fn loop_length_bound_does_not_prove_multibyte_buffer_read_inbounds() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop( diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index 5e4c453b61..ad6240f48f 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -1,5 +1,14 @@ // See native_proof_buffer_views.rs — shared HIR builder toolkit, each file in // this family drives a different subset. +// +// LOWERING (#7493): this suite is overwhelmingly lowering-INDEPENDENT and +// deliberately unpinned. The exceptions are named at the tests themselves: +// three here pin `NativeRootsPin::shadow()` because they assert on the +// shadow-frame slot's own emission, and fifteen in `invalidation` pin +// `NativeRootsPin::native()` because their module-wide "no inbounds GEP" proxy +// collides with the shadow lowering's inline slot addressing (see that file's +// header). +use perry_codegen::testing::NativeRootsPin; use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::types::{ObjectType, PropertyInfo, Type, TypeParam}; use perry_hir::{ @@ -8,7 +17,9 @@ use perry_hir::{ ModuleInitKind, Param, Stmt, UnaryOp, UpdateOp, }; -static ARTIFACT_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); +#[path = "native_proof_support/mod.rs"] +mod native_proof_support; +use native_proof_support::{artifact_env_lock, artifact_for_module, NativeRepsEnv}; fn empty_opts() -> CompileOptions { CompileOptions { @@ -156,7 +167,7 @@ fn compile_artifact_json_for_module_with_opts_and_clone_rejections( all_typed_clone_rejections: bool, ) -> serde_json::Value { let name = module.name.clone(); - let _guard = ARTIFACT_ENV_LOCK.lock().unwrap(); + let _guard = artifact_env_lock(); let dir = std::env::temp_dir().join(format!( "perry_native_reps_test_{}_{}", std::process::id(), @@ -165,51 +176,15 @@ fn compile_artifact_json_for_module_with_opts_and_clone_rejections( let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); - let old_reps = std::env::var_os("PERRY_NATIVE_REPS"); - let old_reps_dir = std::env::var_os("PERRY_NATIVE_REPS_DIR"); - let old_all_typed_clone_rejections = - std::env::var_os("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS"); - std::env::set_var("PERRY_NATIVE_REPS", "1"); - std::env::set_var("PERRY_NATIVE_REPS_DIR", &dir); - if all_typed_clone_rejections { - std::env::set_var("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS", "1"); - } else { - std::env::remove_var("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS"); - } - - let compile_result = compile_module(&module, opts); - - match old_reps { - Some(value) => std::env::set_var("PERRY_NATIVE_REPS", value), - None => std::env::remove_var("PERRY_NATIVE_REPS"), - } - match old_reps_dir { - Some(value) => std::env::set_var("PERRY_NATIVE_REPS_DIR", value), - None => std::env::remove_var("PERRY_NATIVE_REPS_DIR"), - } - match old_all_typed_clone_rejections { - Some(value) => std::env::set_var("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS", value), - None => std::env::remove_var("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS"), - } + let compile_result = { + // Dropped — and so restored — before ANY fallible step below, and on + // an unwind out of `compile_module` itself. + let _env = NativeRepsEnv::install(&dir, all_typed_clone_rejections); + compile_module(&module, opts) + }; compile_result.unwrap(); - let paths: Vec<_> = std::fs::read_dir(&dir) - .unwrap() - .map(|entry| entry.unwrap().path()) - .collect(); - let mut parsed = Vec::new(); - for path in paths { - if !path.extension().is_some_and(|ext| ext == "json") { - continue; - } - let text = std::fs::read_to_string(&path).unwrap(); - let value: serde_json::Value = serde_json::from_str(&text).unwrap(); - if value["module"] == name { - return value; - } - parsed.push(value["module"].clone()); - } - panic!("native reps artifact for {name} not found in {dir:?}; saw modules {parsed:?}"); + artifact_for_module(&dir, &name) } fn param(id: u32, name: &str, ty: Type) -> Param { @@ -6688,6 +6663,11 @@ fn boxed_param_capture_module(name: &str) -> Module { #[test] fn boxed_local_slot_uses_i64_js_value_bits_until_helper_edges() { + // Asserts the SHADOW-STACK spelling of the box-pointer slot: a plain + // `store i64 , ptr %slot`. Under native roots the same slot is a + // `ptr addrspace(1)` alloca and the store is `store ptr addrspace(1) + // %rs4gc.sN` — the identical root, a different lowering (#7493). + let _pin = NativeRootsPin::shadow(); let module = boxed_local_capture_module("boxed_local_js_value_bits_ir.ts"); let ir = String::from_utf8(compile_module(&module, empty_opts()).unwrap()).unwrap(); let box_alloc = ir @@ -6797,6 +6777,11 @@ fn tdz_numeric_const_read_is_not_constant_folded() { #[test] fn boxed_param_slot_uses_i64_js_value_bits_until_helper_edges() { + // Asserts the SHADOW-STACK spelling of the box-pointer slot: a plain + // `store i64 , ptr %slot`. Under native roots the same slot is a + // `ptr addrspace(1)` alloca and the store is `store ptr addrspace(1) + // %rs4gc.sN` — the identical root, a different lowering (#7493). + let _pin = NativeRootsPin::shadow(); let module = boxed_param_capture_module("boxed_param_js_value_bits_ir.ts"); let ir = String::from_utf8(compile_module(&module, empty_opts()).unwrap()).unwrap(); let box_alloc = ir @@ -11691,6 +11676,11 @@ fn typed_f64_method_clone_rejects_this_default_rest_and_any() { #[test] fn typed_f64_receiver_method_clone_raw_loads_after_composed_guards() { + // NOT pinned, deliberately (#7493). #7493's sweep listed this among the + // failures `PERRY_RS4GC=0` heals; run alone it fails under BOTH lowerings, + // so that reading was an artifact of whole-suite ordering. It is a plain + // drifted assertion — the caller no longer emits a `$generic` call on guard + // failure — and is tracked in #7506, not here. let ir = String::from_utf8( compile_module(&typed_f64_receiver_method_positive_module(), empty_opts()).unwrap(), ) diff --git a/crates/perry-codegen/tests/native_proof_regressions/invalidation.rs b/crates/perry-codegen/tests/native_proof_regressions/invalidation.rs index 8dcda751af..b7fed43878 100644 --- a/crates/perry-codegen/tests/native_proof_regressions/invalidation.rs +++ b/crates/perry-codegen/tests/native_proof_regressions/invalidation.rs @@ -1,3 +1,19 @@ +//! LOWERING (#7493): fifteen tests here pin `NativeRootsPin::native()`. +//! +//! Their subject — buffer/length fact invalidation — is lowering-independent, +//! but the *assertion* is not: `assert_buffer_store_uses_dynamic_fallback` +//! proves the absence of a native buffer GEP with a MODULE-WIDE +//! `!ir.contains("getelementptr inbounds i8")`, and the shadow-stack lowering's +//! own inline slot addressing (#7088) emits exactly that instruction for +//! reasons that have nothing to do with a buffer store. Run the suite under +//! `PERRY_RS4GC=0` and these fifteen report a stale proof that was never there. +//! +//! Native roots are the default on every target the runtime can walk, so the +//! pin does not change what CI runs — it makes the assertion mean the same +//! thing during a `PERRY_RS4GC=0` bisection, which is the sweep a GC engineer +//! actually runs. The durable fix is to scope the search to the buffer-store +//! site instead of the whole module; #7505. + use super::*; fn block_between<'a>(ir: &'a str, start: &str, end: &str) -> &'a str { @@ -13,6 +29,7 @@ fn block_between<'a>(ir: &'a str, start: &str, end: &str) -> &'a str { #[test] fn localset_invalidates_native_i32_alias_facts() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop( @@ -33,6 +50,7 @@ fn localset_invalidates_native_i32_alias_facts() { #[test] fn update_invalidates_native_i32_alias_facts() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop( @@ -53,6 +71,7 @@ fn update_invalidates_native_i32_alias_facts() { #[test] fn localset_invalidates_min_length_facts() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "src", int(8)), buffer_let(2, "dst", int(8)), @@ -68,6 +87,7 @@ fn localset_invalidates_min_length_facts() { #[test] fn localset_invalidates_active_bounded_buffer_index_facts() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", false, int(8)), buffer_let(2, "buf", local(1)), @@ -88,6 +108,7 @@ fn localset_invalidates_active_bounded_buffer_index_facts() { #[test] fn inner_loop_bounded_buffer_fact_is_removed_after_outer_fact_invalidation() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", false, int(8)), buffer_let(2, "a", local(1)), @@ -113,6 +134,7 @@ fn inner_loop_bounded_buffer_fact_is_removed_after_outer_fact_invalidation() { #[test] fn localset_invalidates_buffer_view_local_length_sources() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", true, int(8)), buffer_let(2, "buf", local(1)), @@ -127,6 +149,7 @@ fn localset_invalidates_buffer_view_local_length_sources() { #[test] fn update_invalidates_buffer_view_local_length_sources() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", true, int(8)), buffer_let(2, "buf", local(1)), @@ -141,6 +164,7 @@ fn update_invalidates_buffer_view_local_length_sources() { #[test] fn negative_loop_counter_does_not_emit_inbounds_buffer_gep() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop_with_start_and_update( @@ -159,6 +183,7 @@ fn negative_loop_counter_does_not_emit_inbounds_buffer_gep() { #[test] fn decrementing_loop_update_does_not_emit_inbounds_buffer_gep() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop_with_start_and_update( @@ -177,6 +202,7 @@ fn decrementing_loop_update_does_not_emit_inbounds_buffer_gep() { #[test] fn body_counter_mutation_does_not_emit_inbounds_buffer_gep() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop( @@ -193,6 +219,7 @@ fn body_counter_mutation_does_not_emit_inbounds_buffer_gep() { #[test] fn inclusive_length_loop_does_not_emit_inbounds_buffer_gep() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "buf", int(8)), for_loop_with_op_start_and_update( @@ -1037,6 +1064,7 @@ fn loop_local_array_alias_push_blocks_packed_u32_loop_and_artifacts() { #[test] fn inclusive_local_length_bound_does_not_use_local_length_bound_fact() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", false, int(8)), buffer_let(2, "buf", local(1)), @@ -1057,6 +1085,7 @@ fn inclusive_local_length_bound_does_not_use_local_length_bound_fact() { #[test] fn negative_loop_counter_does_not_use_local_length_bound_fact() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", false, int(8)), buffer_let(2, "buf", local(1)), @@ -1076,6 +1105,7 @@ fn negative_loop_counter_does_not_use_local_length_bound_fact() { #[test] fn body_mutation_of_local_bound_does_not_use_local_length_bound_fact() { + let _pin = NativeRootsPin::native(); let body = vec![ number_let(1, "n", true, int(1)), buffer_let(2, "buf", local(1)), @@ -1096,6 +1126,7 @@ fn body_mutation_of_local_bound_does_not_use_local_length_bound_fact() { #[test] fn negative_loop_counter_does_not_use_min_length_bound_fact() { + let _pin = NativeRootsPin::native(); let body = vec![ buffer_let(1, "src", int(8)), buffer_let(2, "dst", int(8)), diff --git a/crates/perry-codegen/tests/native_proof_support/mod.rs b/crates/perry-codegen/tests/native_proof_support/mod.rs new file mode 100644 index 0000000000..5145d9e865 --- /dev/null +++ b/crates/perry-codegen/tests/native_proof_support/mod.rs @@ -0,0 +1,180 @@ +//! Shared harness for the `native_proof_*` suites' artifact-JSON tests +//! (#7493). +//! +//! `PERRY_NATIVE_REPS` / `PERRY_NATIVE_REPS_DIR` are read by `compile_module` +//! from the **process** environment, so they are global to a whole test binary +//! rather than to the test that set them. Both suites had a hand-rolled copy of +//! the same set-compile-restore dance, and both copies had the same two +//! defects. They are fixed once, here, and included by both binaries with +//! `#[path]` so a third copy cannot drift. +//! +//! Defect 1 — **poisoning cascade** (#7490's shape, fixed for +//! `typed_feedback.rs` in #7492). A test that unwinds while holding the lock +//! poisons it, and every later `lock().unwrap()` in the binary then dies with +//! `PoisonError` regardless of its own subject. On `main`, +//! `native_proof_regressions` reported **55 failures at default parallelism and +//! 4 under `--test-threads=1`** — 51 of the 55 were `PoisonError`, and *which* +//! tests they hit was decided by the scheduler. That reads as order-dependent +//! codegen state when it is only lock poisoning. +//! +//! Defect 2 — **env leak on unwind**. The restore was hand-written after the +//! compile, so a panic inside `compile_module` left `PERRY_NATIVE_REPS=1` and a +//! stale `PERRY_NATIVE_REPS_DIR` installed for the rest of the process. Every +//! later compile in the binary — including the many `compile_ir` ones that +//! never take this lock — then wrote artifact JSON into a directory another +//! test was reading, producing the torn `EOF while parsing a value` read that +//! poisoned the mutex in the first place. +//! +//! Each including binary gets its own `ARTIFACT_ENV_LOCK` static and its own +//! copy of the two tests below, which is what we want: the lock is per-process +//! and so is the property being asserted. + +#![allow(dead_code)] + +pub static ARTIFACT_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +/// Acquire [`ARTIFACT_ENV_LOCK`] tolerating a poisoned mutex. +/// +/// Recovery is sound because the protected state — the `PERRY_NATIVE_REPS*` +/// env vars — is restored by [`NativeRepsEnv`]'s `Drop` during the same unwind, +/// before the mutex is released. One test's failure must fail that test alone. +pub fn artifact_env_lock() -> std::sync::MutexGuard<'static, ()> { + ARTIFACT_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// RAII for the `PERRY_NATIVE_REPS*` env vars. +pub struct NativeRepsEnv { + reps: Option, + dir: Option, + all_typed_clone_rejections: Option, +} + +impl NativeRepsEnv { + /// Install the artifact-recording env for the lifetime of the guard. + /// Hold [`artifact_env_lock`] across this — the vars are process-global. + pub fn install(dir: &std::path::Path, all_typed_clone_rejections: bool) -> Self { + let saved = NativeRepsEnv { + reps: std::env::var_os("PERRY_NATIVE_REPS"), + dir: std::env::var_os("PERRY_NATIVE_REPS_DIR"), + all_typed_clone_rejections: std::env::var_os( + "PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS", + ), + }; + std::env::set_var("PERRY_NATIVE_REPS", "1"); + std::env::set_var("PERRY_NATIVE_REPS_DIR", dir); + if all_typed_clone_rejections { + std::env::set_var("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS", "1"); + } else { + std::env::remove_var("PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS"); + } + saved + } +} + +impl Drop for NativeRepsEnv { + fn drop(&mut self) { + fn restore(key: &str, value: Option<&std::ffi::OsString>) { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + restore("PERRY_NATIVE_REPS", self.reps.as_ref()); + restore("PERRY_NATIVE_REPS_DIR", self.dir.as_ref()); + restore( + "PERRY_NATIVE_REPS_ALL_TYPED_CLONE_REJECTIONS", + self.all_typed_clone_rejections.as_ref(), + ); + } +} + +/// Pick this test's own artifact out of `dir`, tolerating foreign or +/// half-written neighbours. +/// +/// `PERRY_NATIVE_REPS_DIR` is process-global while installed, so a test +/// compiling concurrently on another thread — none of which take the lock — +/// also drops its artifact here, possibly still being written when we read. +/// Treat that as noise (it cannot be the subject) instead of unwrapping a torn +/// read into a panic INSIDE the lock, which is what poisons it. If the subject +/// then turns out to be missing, the panic names everything that was skipped, +/// so a genuinely truncated target artifact stays diagnosable. +pub fn artifact_for_module(dir: &std::path::Path, name: &str) -> serde_json::Value { + let paths: Vec<_> = std::fs::read_dir(dir) + .unwrap() + .map(|entry| entry.unwrap().path()) + .collect(); + let mut skipped = Vec::new(); + for path in paths { + if !path.extension().is_some_and(|ext| ext == "json") { + continue; + } + let Ok(text) = std::fs::read_to_string(&path) else { + skipped.push(format!("{}: unreadable", path.display())); + continue; + }; + let value: serde_json::Value = match serde_json::from_str(&text) { + Ok(value) => value, + Err(err) => { + skipped.push(format!("{}: {err}", path.display())); + continue; + } + }; + if value["module"] == name { + return value; + } + skipped.push(format!("{}", value["module"])); + } + panic!("native reps artifact for {name} not found in {dir:?}; skipped {skipped:?}"); +} + +/// Sabotage test for [`artifact_env_lock`]: a test that panics while holding +/// the lock must fail *itself* and nothing else. +/// +/// This asserts its subject was live rather than merely that nothing threw — it +/// plants the exact #7490 shape, proves the mutex really is poisoned +/// afterwards, and only then demands the accessor still hand out a guard. It +/// fails against a plain `ARTIFACT_ENV_LOCK.lock().unwrap()`, so a green run +/// here is evidence. +/// +/// `a` sorts first, so under `--test-threads=1` every other test in the +/// including binary runs under a genuinely poisoned lock and the tolerance is +/// exercised suite-wide rather than in one isolated case. +#[test] +fn artifact_env_lock_is_poison_tolerant_so_one_failure_cannot_cascade() { + let sabotage = std::panic::catch_unwind(|| { + let _guard = artifact_env_lock(); + panic!("#7493 sabotage: unwinding while holding ARTIFACT_ENV_LOCK"); + }); + assert!(sabotage.is_err(), "the sabotage panic should have unwound"); + assert!( + ARTIFACT_ENV_LOCK.is_poisoned(), + "unwinding out of a lock-holding test should poison ARTIFACT_ENV_LOCK — \ + if it no longer does, this test is no longer exercising its subject" + ); + let _guard = artifact_env_lock(); +} + +/// Sabotage test for [`NativeRepsEnv`]: the env must survive an unwind out of +/// the compile it wraps. +#[test] +fn artifact_env_is_restored_even_when_the_compile_unwinds() { + let _guard = artifact_env_lock(); + let before = std::env::var_os("PERRY_NATIVE_REPS"); + let sabotage = std::panic::catch_unwind(|| { + let _env = NativeRepsEnv::install(std::path::Path::new("/nonexistent/perry7493"), false); + assert_eq!( + std::env::var("PERRY_NATIVE_REPS").ok().as_deref(), + Some("1"), + "the guard must actually install the var it claims to restore" + ); + panic!("#7493 sabotage: unwinding inside the artifact env window"); + }); + assert!(sabotage.is_err(), "the sabotage panic should have unwound"); + assert_eq!( + std::env::var_os("PERRY_NATIVE_REPS"), + before, + "PERRY_NATIVE_REPS leaked out of a panicking compile window" + ); +} diff --git a/crates/perry-codegen/tests/scalar_replaced_slot_roots.rs b/crates/perry-codegen/tests/scalar_replaced_slot_roots.rs index d45572a38d..1730449f31 100644 --- a/crates/perry-codegen/tests/scalar_replaced_slot_roots.rs +++ b/crates/perry-codegen/tests/scalar_replaced_slot_roots.rs @@ -18,7 +18,28 @@ //! Both directions are covered: the gate must stay silent for a //! literal whose fields are numbers, or every scalar-replaced `{x, y}` in a hot //! loop would pay for rooting a value that can never be collected (#6997). +//! +//! LOWERING (#7493): every test here measures `js_shadow_slot_bind` / +//! `js_shadow_frame_enter` call sites, which only the SHADOW-STACK lowering +//! emits — so every test pins `NativeRootsPin::shadow()`. Native roots express +//! the same root set as `ptr addrspace(1)` allocas that LLVM's RS4GC pass +//! relocates; there is no bind call to count. +//! +//! That distinction was not cosmetic for the two `numeric_only_*_emits_no_rooting` +//! tests. They assert `bind_calls(&ir) == 0`, and under the post-#7370 default +//! that count is zero for EVERY program, rooted or not — so they were passing +//! vacuously, which is CLAUDE.md hazard 4 ("the gate runs but its subject never +//! did"). Pinning them makes them assert their subject again; they now fail for +//! a real reason, tracked in #7504 (since #7487 a pooled temp root also emits +//! `js_shadow_slot_bind`, so a whole-module bind count no longer isolates the +//! scalar-replaced slots it means to measure). A red test that is measuring +//! something beats a green one that is not. +//! +//! The native-roots side of this contract — that a scalar-replaced field +//! holding a heap value becomes a relocatable `addrspace(1)` slot, and a +//! numeric one does not — has NO equivalent assertion anywhere today. #7502. +use perry_codegen::testing::NativeRootsPin; use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::types::Type; use perry_hir::{Expr, Module, ModuleInitKind, Stmt}; @@ -219,6 +240,7 @@ fn frame_slot_count(ir: &str) -> u32 { /// and the read therefore swept the value (#6968). #[test] fn scalar_replaced_object_field_holding_a_heap_value_is_bound() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_object_field_root.ts", vec![ @@ -283,6 +305,7 @@ fn scalar_replaced_object_field_holding_a_heap_value_is_bound() { /// exactly what this module builds (`Type::Any`). #[test] fn numeric_only_scalar_replaced_object_emits_no_rooting() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_object_numeric.ts", vec![ @@ -310,6 +333,7 @@ fn numeric_only_scalar_replaced_object_emits_no_rooting() { /// one alloca per element, and element 0 is the only reference to its value. #[test] fn scalar_replaced_array_element_holding_a_heap_value_is_bound() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_array_element_root.ts", vec![ @@ -337,6 +361,7 @@ fn scalar_replaced_array_element_holding_a_heap_value_is_bound() { /// …and its numeric twin stays free. #[test] fn numeric_only_scalar_replaced_array_emits_no_rooting() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_array_numeric.ts", vec![ @@ -369,6 +394,7 @@ fn numeric_only_scalar_replaced_array_emits_no_rooting() { /// both compilers: only the extra binds can come from the part slots. #[test] fn scalar_replaced_split_parts_are_bound() { + let _pin = NativeRootsPin::shadow(); let source = Stmt::Let { id: 1, name: "s".to_string(), @@ -421,6 +447,7 @@ fn scalar_replaced_split_parts_are_bound() { /// object-literal initializer is not the only store site. #[test] fn later_store_into_a_scalar_replaced_field_is_bound() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_object_field_reassign.ts", vec![ @@ -463,6 +490,7 @@ fn later_store_into_a_scalar_replaced_field_is_bound() { /// the bind altogether (0), which would un-root the alloca and reopen #6968. #[test] fn repeated_stores_into_one_scalar_slot_bind_once() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_field_two_stores.ts", vec![ @@ -505,6 +533,7 @@ fn repeated_stores_into_one_scalar_slot_bind_once() { /// `js_shadow_slot_bind`), so the old compiler produces 0 and fails. #[test] fn every_store_into_a_hoisted_scalar_slot_shades_its_value() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_field_two_stores_barrier.ts", vec![ @@ -560,6 +589,7 @@ fn every_store_into_a_hoisted_scalar_slot_shades_its_value() { /// `bind < first_branch` claim fails on the old compiler. #[test] fn bind_is_hoisted_into_the_entry_block_ahead_of_the_storing_loop() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_field_loop_bind.ts", vec![ @@ -630,6 +660,7 @@ fn bind_is_hoisted_into_the_entry_block_ahead_of_the_storing_loop() { /// store, so the `undef_stores > 0` claim fails on the old compiler. #[test] fn scalar_replaced_array_element_slots_are_initialized_before_the_bind() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_array_element_init.ts", vec![ @@ -675,6 +706,7 @@ fn scalar_replaced_array_element_slots_are_initialized_before_the_bind() { /// for the entry region too. #[test] fn numeric_only_scalar_replaced_literal_emits_no_entry_rooting() { + let _pin = NativeRootsPin::shadow(); let ir = ir_for( "scalar_numeric_no_entry_rooting.ts", vec![ diff --git a/crates/perry-codegen/tests/shadow_slot_hygiene.rs b/crates/perry-codegen/tests/shadow_slot_hygiene.rs index 11a1b364ef..5b99c38bae 100644 --- a/crates/perry-codegen/tests/shadow_slot_hygiene.rs +++ b/crates/perry-codegen/tests/shadow_slot_hygiene.rs @@ -9,7 +9,24 @@ //! shifted by a numeric local) are unchanged. What they no longer prove is //! that a call is what executes; `expr::shadow_inline`'s unit tests cover the //! emitted shape. +//! +//! LOWERING (#7493): **every** test in this file asserts on the SHADOW-STACK +//! lowering and pins it with `NativeRootsPin::shadow()`. That is not a style +//! choice — the file's whole subject is the shadow frame's own mechanics (slot +//! reservation, bind/clear ordering, slot indices, the post-init frame region), +//! which the native-roots lowering does not have: it puts roots in +//! `ptr addrspace(1)` allocas and lets LLVM's RS4GC pass relocate them, with no +//! frame, no slot index and no bind. The two are different lowerings of the +//! same root-set analysis (#7340), so there is nothing here to translate. +//! +//! Since #7370 native roots are the DEFAULT on this target, so these pins are +//! load-bearing: without them the file was 0/12 red on `main`. But note what +//! that means for coverage — this suite now tests a lowering that no longer +//! ships on aarch64/x86_64. The equivalent native-roots assertions are tracked +//! in #7502; where a mechanic has no native-side counterpart today, that issue +//! names it. +use perry_codegen::testing::NativeRootsPin; use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::types::Type; use perry_hir::{Expr, Function, Module, ModuleInitKind, Stmt}; @@ -629,6 +646,7 @@ fn init_body_function_name(ir: &str) -> String { #[test] fn function_shadow_slots_clear_dead_values_and_skip_numeric_roots() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8(compile_module(&shadow_hygiene_module(), empty_opts()).unwrap()) .expect("LLVM IR should be UTF-8"); @@ -669,6 +687,7 @@ fn function_shadow_slots_clear_dead_values_and_skip_numeric_roots() { /// covers the inline sites too. #[test] fn duplicate_var_declarations_keep_every_slot_inside_the_frame() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module(&duplicate_var_decl_shadow_module(), empty_opts()).unwrap(), ) @@ -707,6 +726,7 @@ fn duplicate_var_declarations_keep_every_slot_inside_the_frame() { #[test] fn entry_module_top_level_shadow_frame_starts_after_init_prelude() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module(&top_level_shadow_module("entry_shadow.ts"), entry_opts()).unwrap(), ) @@ -737,6 +757,7 @@ fn entry_module_top_level_shadow_frame_starts_after_init_prelude() { #[test] fn entry_module_top_level_shadow_slots_update_and_clear() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module( &top_level_shadow_module("entry_shadow_slots.ts"), @@ -774,6 +795,7 @@ fn entry_module_top_level_shadow_slots_update_and_clear() { #[test] fn non_entry_module_init_body_gets_post_init_shadow_frame() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module( &top_level_shadow_module("non_entry_shadow.ts"), @@ -809,6 +831,7 @@ fn non_entry_module_init_body_gets_post_init_shadow_frame() { #[test] fn top_level_loop_body_shadow_slots_clear_each_iteration() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8(compile_module(&top_level_loop_shadow_module(), entry_opts()).unwrap()) .expect("LLVM IR should be UTF-8"); @@ -836,6 +859,7 @@ fn top_level_loop_body_shadow_slots_clear_each_iteration() { #[test] fn immutable_index_alias_binds_once_but_keeps_incremental_root_barrier() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module(&persistent_index_alias_shadow_module(), entry_opts()).unwrap(), ) @@ -867,6 +891,7 @@ fn immutable_index_alias_binds_once_but_keeps_incremental_root_barrier() { #[test] fn flat_const_row_aliases_do_not_reserve_shadow_slots() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module(&flat_const_row_alias_shadow_module(), entry_opts()).unwrap(), ) @@ -891,6 +916,7 @@ fn flat_const_row_aliases_do_not_reserve_shadow_slots() { #[test] fn reassigned_any_from_number_to_pointer_reserves_and_updates_shadow_slot() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8(compile_module(&reassigned_any_shadow_module(), empty_opts()).unwrap()) .expect("LLVM IR should be UTF-8"); @@ -912,6 +938,7 @@ fn reassigned_any_from_number_to_pointer_reserves_and_updates_shadow_slot() { #[test] fn mixed_any_writes_keep_alias_shadow_slots_precise() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8(compile_module(&mixed_any_alias_shadow_module(), empty_opts()).unwrap()) .expect("LLVM IR should be UTF-8"); @@ -936,6 +963,7 @@ fn mixed_any_writes_keep_alias_shadow_slots_precise() { #[test] fn closure_body_write_to_captured_outer_local_is_visible_to_shadow_analysis() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module(&closure_captured_write_shadow_module(), empty_opts()).unwrap(), ) @@ -1038,6 +1066,7 @@ fn canonical_str_shadow_module() -> Module { /// drops the generic GC-type-byte tower for the 3-arm tag dispatch. #[test] fn canonical_str_local_keeps_shadow_binding_and_tag_dispatched_ops() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8(compile_module(&canonical_str_shadow_module(), empty_opts()).unwrap()) .expect("LLVM IR should be UTF-8"); diff --git a/crates/perry-codegen/tests/temp_root_argument_temporaries.rs b/crates/perry-codegen/tests/temp_root_argument_temporaries.rs index 5b016cf20e..cf52348a4a 100644 --- a/crates/perry-codegen/tests/temp_root_argument_temporaries.rs +++ b/crates/perry-codegen/tests/temp_root_argument_temporaries.rs @@ -10,6 +10,19 @@ //! `cargo-test`, so a lowering path that quietly goes back to threading an //! accumulator through an SSA register fails here rather than three weeks //! later under a narrowed forced scan. +//! +//! LOWERING (#7493): nothing in this file is pinned, because nothing in it is +//! lowering-dependent — `PERRY_RS4GC=0` moves it not at all (3/7 either way). +//! Its failures are #7487's, not #7370's, and are tracked in #7503: temp roots +//! were re-lowered onto pooled frame allocas, so `js_gc_temp_root_push` / +//! `_get` / `_truncate` now appear only on the FFI fallback arm that neither +//! lowering takes. The four positive assertions therefore fail, and all three +//! `!ir.contains("call i32 @js_gc_temp_root_push")` negatives — the entire +//! passing half of this suite — are VACUOUS: they hold for every program, +//! rooted or not. The #6951 contract is still emitted (store into an entry +//! alloca, root-bind, re-load after the allocating call); only the spelling +//! these assertions search for is gone, so the suite currently proves nothing +//! in either direction. use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::{Expr, Module, ModuleInitKind, Stmt}; diff --git a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs index fa45571f7f..be304ba418 100644 --- a/crates/perry-codegen/tests/temp_root_operand_temporaries.rs +++ b/crates/perry-codegen/tests/temp_root_operand_temporaries.rs @@ -18,7 +18,25 @@ //! forces a conservative native-stack scan that pins the temporary by accident. //! Equally important is the negative half: the shapes that were always safe //! must still emit no rooting calls at all. - +//! +//! LOWERING (#7493). Two tests here assert on the SHADOW-STACK spelling of the +//! `this`-slot root (`js_shadow_slot_bind`) and pin `NativeRootsPin::shadow()`; +//! `a_collection_free_construction_emits_no_this_slot_root` needed the pin even +//! though it was *passing*, because under the post-#7370 native-roots default +//! its `!contains("@js_shadow_slot_bind")` is true of every program (hazard 4: +//! the gate ran, its subject did not). +//! +//! The rest of this file is lowering-INDEPENDENT and deliberately unpinned — +//! but READ #7503 BEFORE TRUSTING IT. #7487 re-lowered temp roots onto pooled +//! frame allocas, so `js_gc_temp_root_push` / `_get` / `_set` / `_truncate` are +//! now emitted only on the FFI fallback arm, which neither lowering takes here. +//! Every positive assertion spelled that way fails, and — worse — every +//! `!ir.contains("call i32 @js_gc_temp_root_push")` passes vacuously. The +//! contract itself is intact (the value is stored into an entry alloca, root- +//! bound, and re-loaded after the allocating call); only the spelling these +//! assertions look for is gone. + +use perry_codegen::testing::NativeRootsPin; use perry_codegen::{compile_module, AppMetadata, CompileOptions}; use perry_hir::{Class, Expr, Module, ModuleInitKind, Stmt}; @@ -959,6 +977,7 @@ fn the_inline_ctor_result_slot_never_carries_an_instance_address() { /// and no bind names the `this` slot. #[test] fn the_inline_ctor_this_slot_is_bound_as_a_shadow_slot() { + let _pin = NativeRootsPin::shadow(); let ir = String::from_utf8( compile_module( &module_with_new_running_ctor("new_inst_this_slot.ts"), @@ -1246,6 +1265,7 @@ fn a_put_value_set_key_is_re_derived_below_the_value() { /// `lower_new_impl_inner` and this fails. #[test] fn a_collection_free_construction_emits_no_this_slot_root() { + let _pin = NativeRootsPin::shadow(); // `module_with_new` is the bare `Pair` class: no fields, no ctor, no // heritage — the exact shape `construction_runs_user_code` answers `false` // for.