diff --git a/CLAUDE.md b/CLAUDE.md index a1d6728416..a08c936a50 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1397 +**Current Version:** 0.5.1398 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 7facfe1bc0..498459fdcf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1397" +version = "0.5.1398" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1397" +version = "0.5.1398" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1397" +version = "0.5.1398" [[package]] name = "perry-ui-tvos" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1397" +version = "0.5.1398" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 56a036892a..18df552dcd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1397" +version = "0.5.1398" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7693-readdir-options-rooting.md b/changelog.d/7693-readdir-options-rooting.md new file mode 100644 index 0000000000..e4cece5283 --- /dev/null +++ b/changelog.d/7693-readdir-options-rooting.md @@ -0,0 +1,42 @@ +### Fixed + +- **`fs.readdir`'s options object is rooted across the `withFileTypes` key allocation (#7274).** + `fs/dirent.rs::options_with_file_types` decoded a raw `*const ObjectHeader` out of + the NaN-boxed `options` argument, then called + `js_string_from_bytes(b"withFileTypes")` — a collection point — and then + dereferenced the address it had computed *before* the collection. `options_value` + is a plain Rust `f64` local: nothing kept the object alive and nothing rewrote the + pointer. `{ withFileTypes: true }` is a fresh object literal at the call site, so + it is a nursery object — precisely the generation an evacuating minor relocates. + + The allocation is now hoisted above the decode (bound together with + `RuntimeHandle::across_nanbox`, so no pre-collection address is nameable) and the + options value is rooted in a `RuntimeHandleScope`. The decode itself — the + POINTER_TAG / raw-address forms plus the #7259 `is_handle_band` floor — is + factored into a single `options_object_ptr` helper: it appeared three times in + this file, and the drift between two of those copies is exactly what the bug was. + `options_field_value`, 40 lines below, already had the correct shape. + + **Which configuration this bites in**, stated precisely rather than overclaimed: + `gc_check_trigger`'s alloc-point arm engages + `ManualGcScanGuard::force_full_scan(NurseryChurnSlackValve)` (unconditional since + #7682), and when that guard *engages* the conservative native-stack scan both + retains the raw local and makes the copying minor ineligible + (`CopiedMinorFallbackReason::ConservativeStack`) — so the pre-fix code survived. + It does not always engage: `force_full_scan` is a no-op when + `CONSERVATIVE_STACK_SCAN_OVERRIDE` is already set, and an explicit + `PERRY_CONSERVATIVE_STACK_SCAN` env value beats any pin, so + `PERRY_CONSERVATIVE_STACK_SCAN=off` removes the valve and the alloc-point minor + evacuates. The masking mechanism is the bounded valve #7148 documents *as* a + bounded valve, not a guarantee. + + Witness: two knob-free unit tests in + `gc/tests/runtime_roots/fs_options_object.rs` drive a real evacuating minor from + inside the function's own key allocation, in exactly that configuration. The + subject is asserted live — the options object is held by nothing but the function + under test, and a separately rooted sentinel must come back at a different address, + so a cycle that moved nothing cannot certify the file. That assertion paid for + itself immediately: the first draft pinned `force_legacy_gc_pacing()`, which routes + the trigger to the non-moving budgeted stepper, and the test failed rather than + passing vacuously. Sabotage-verified: restoring the decode-then-allocate order + fails the positive test. diff --git a/crates/perry-runtime/src/fs/dirent.rs b/crates/perry-runtime/src/fs/dirent.rs index 95c4d1e929..e1e7a24f67 100644 --- a/crates/perry-runtime/src/fs/dirent.rs +++ b/crates/perry-runtime/src/fs/dirent.rs @@ -116,10 +116,28 @@ pub(crate) unsafe fn build_dirent_object(name: &str, parent_path: &str, kind: Di f64::from_bits(POINTER_TAG | (obj as u64 & 0x0000_FFFF_FFFF_FFFF)) } -/// Decode a NaN-boxed object's `withFileTypes` field as a boolean. -/// Returns false when the options arg is undefined / not an object / -/// the field is absent or falsy. -pub(crate) unsafe fn options_with_file_types(options_value: f64) -> bool { +/// Decode a NaN-boxed options value into a plain-object pointer. +/// +/// Factored out because this file performs the decode in THREE places and +/// every one of them has to be re-done *after* any allocation (#7274): the +/// result is a bare address, so it says nothing once a collection has moved +/// the object. Keeping one copy is what stops the three from drifting again — +/// they already had, which is how `options_with_file_types` ended up +/// dereferencing across `js_string_from_bytes` while its sibling +/// `options_field_value` re-read through a handle. +/// +/// # Not a root +/// +/// This returns the address, it does not keep it alive. The caller owns the +/// `RuntimeHandleScope`; this must be called on a value read back OUT of a +/// live handle, never on a pre-allocation copy. +/// +/// #7259: a POINTER_TAG payload can be a registry handle id rather than a heap +/// address, and `< 0x1000` sits an order of magnitude below `HANDLE_BAND_MAX` — +/// fetch/zlib/proxy ids passed it and were dereferenced as an ObjectHeader (the +/// Linux-only fault class of #1843/#4004/#6271). `is_handle_band` also subsumes +/// the null check that used to follow. +unsafe fn options_object_ptr(options_value: f64) -> Option<*const crate::object::ObjectHeader> { let bits = options_value.to_bits(); let value = crate::value::JSValue::from_bits(bits); let raw_ptr = if value.is_pointer() { @@ -127,18 +145,40 @@ pub(crate) unsafe fn options_with_file_types(options_value: f64) -> bool { } else if bits >> 48 == 0x0000 { (bits & 0x0000_FFFF_FFFF_FFFF) as usize } else { - return false; + return None; }; - // #7259: a POINTER_TAG payload can be a registry handle id rather than a - // heap address, and `< 0x1000` sits an order of magnitude below - // `HANDLE_BAND_MAX` — fetch/zlib/proxy ids passed it and were dereferenced - // as an ObjectHeader (the Linux-only fault class of #1843/#4004/#6271). - // `is_handle_band` also subsumes the null check that used to follow. if crate::value::addr_class::is_handle_band(raw_ptr) { - return false; + return None; } - let obj_ptr = raw_ptr as *const crate::object::ObjectHeader; - let key = crate::string::js_string_from_bytes(b"withFileTypes".as_ptr(), 13); + Some(raw_ptr as *const crate::object::ObjectHeader) +} + +/// Decode a NaN-boxed object's `withFileTypes` field as a boolean. +/// Returns false when the options arg is undefined / not an object / +/// the field is absent or falsy. +/// +/// ★ #7274: `js_string_from_bytes` is a collection point, and under the C4b +/// evacuation policy that collection MOVES. The object pointer therefore cannot +/// be computed before it — this used to decode `obj_ptr` first, allocate the +/// key, and then dereference the pre-collection address, reading swept +/// from-space or an unrelated live object at the recycled address. +/// `{ withFileTypes: true }` is a fresh object literal at the call site, i.e. +/// exactly the young object a minor triggered by the very next allocation is +/// most likely to relocate. +/// +/// The allocation is hoisted above the decode and `options_value` is rooted, so +/// the address is only ever produced from a slot the collector rewrote. This is +/// the shape `options_field_value` (below) already used. +pub(crate) unsafe fn options_with_file_types(options_value: f64) -> bool { + let scope = crate::gc::RuntimeHandleScope::new(); + let options_handle = scope.root_nanbox_f64(options_value); + // Allocate FIRST, decode after. `across_nanbox` binds the two together so + // there is no pre-collection address in scope to reach for by accident. + let (key, refreshed) = options_handle + .across_nanbox(|| crate::string::js_string_from_bytes(b"withFileTypes".as_ptr(), 13)); + let Some(obj_ptr) = options_object_ptr(refreshed) else { + return false; + }; let val = crate::object::js_object_get_field_by_name(obj_ptr, key); crate::value::js_is_truthy(f64::from_bits(val.bits())) != 0 } @@ -174,22 +214,7 @@ pub(crate) unsafe fn options_field_value( ) -> Option { let scope = crate::gc::RuntimeHandleScope::new(); let options_handle = scope.root_nanbox_f64(options_value); - let bits = options_handle.get_nanbox_f64().to_bits(); - let value = crate::value::JSValue::from_bits(bits); - let raw_ptr = if value.is_pointer() { - value.as_pointer::() as usize - } else if bits >> 48 == 0x0000 { - (bits & 0x0000_FFFF_FFFF_FFFF) as usize - } else { - return None; - }; - // #7259: see `options_with_file_types` — a POINTER_TAG payload can be a - // registry handle id, and `is_handle_band` (not `< 0x1000`) is the floor - // that rejects the fetch/zlib/proxy bands. It subsumes the null check too. - if crate::value::addr_class::is_handle_band(raw_ptr) { - return None; - } - let obj_ptr = raw_ptr as *const crate::object::ObjectHeader; + let obj_ptr = options_object_ptr(options_handle.get_nanbox_f64())?; let keys = (*obj_ptr).keys_array; if !keys.is_null() { let key_count = crate::array::js_array_length(keys) as usize; @@ -206,21 +231,12 @@ pub(crate) unsafe fn options_field_value( } } } - let key = crate::string::js_string_from_bytes(field.as_ptr(), field.len() as u32); - let refreshed_bits = options_handle.get_nanbox_f64().to_bits(); - let refreshed_value = crate::value::JSValue::from_bits(refreshed_bits); - let refreshed_ptr = if refreshed_value.is_pointer() { - refreshed_value.as_pointer::() as usize - } else if refreshed_bits >> 48 == 0x0000 { - (refreshed_bits & 0x0000_FFFF_FFFF_FFFF) as usize - } else { - return None; - }; - // #7259: same handle-band floor after the GC-safe re-read of the handle. - if crate::value::addr_class::is_handle_band(refreshed_ptr) { - return None; - } - let refreshed_obj_ptr = refreshed_ptr as *const crate::object::ObjectHeader; + // The key allocation is a collection point; `obj_ptr` above is stale after + // it. `across_nanbox` re-reads the rooted options value and hands back the + // post-collection address (#7274 factored this decode into one helper). + let (key, refreshed) = options_handle + .across_nanbox(|| crate::string::js_string_from_bytes(field.as_ptr(), field.len() as u32)); + let refreshed_obj_ptr = options_object_ptr(refreshed)?; let val = crate::object::js_object_get_field_by_name(refreshed_obj_ptr, key); if val.bits() == crate::value::TAG_UNDEFINED { None diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots.rs b/crates/perry-runtime/src/gc/tests/runtime_roots.rs index 3042135b04..8e0d8ae583 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots.rs @@ -2,6 +2,7 @@ use super::super::*; use super::support::*; use std::cell::Cell; mod callback_scanners; +mod fs_options_object; mod generator_attach_prototype; mod hook_dispatch_handles; mod interned_string_caches; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/fs_options_object.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/fs_options_object.rs new file mode 100644 index 0000000000..2fa11d00de --- /dev/null +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/fs_options_object.rs @@ -0,0 +1,183 @@ +//! #7274 — `fs.readdir`'s options object must survive the allocation that the +//! `withFileTypes` lookup performs. +//! +//! `fs/dirent.rs::options_with_file_types` decoded a raw `*const ObjectHeader` +//! out of the NaN-boxed `options` argument, THEN called +//! `js_string_from_bytes(b"withFileTypes")` — a collection point — and THEN +//! dereferenced the address it computed before the collection. `options_value` +//! is a plain Rust `f64` local, so nothing kept the object alive and nothing +//! rewrote the pointer: a minor landing on that allocation either swept the +//! object outright or evacuated it and left the local naming retired +//! from-space, which the very next allocation recycles. +//! +//! `{ withFileTypes: true }` is a fresh object literal at every call site, so +//! it is a nursery object — precisely the generation the minor triggered by the +//! next allocation relocates. +//! +//! ## Why this lives in a Rust unit test rather than a `.ts` witness +//! +//! `scripts/gc_root_dominance_check.py` reads emitted LLVM IR and this +//! function is hand-written Rust the compiler never sees, so the static gate is +//! structurally blind to it (CLAUDE.md, "a runtime-side cache of a raw heap +//! pointer is a GC root, and the static checker cannot see it"). A `.ts` +//! reproducer would additionally need a real filesystem, a real `readdir`, and +//! GC pressure landing inside one 3-instruction window. Driving the collection +//! from the test is deterministic and needs no knob. +//! +//! ## Which configuration this bites in — stated precisely +//! +//! `gc_check_trigger`'s alloc-point arm engages +//! `ManualGcScanGuard::force_full_scan(NurseryChurnSlackValve)` (unconditional +//! since #7682). When that guard *engages*, the conservative native-stack scan +//! both retains the raw local and makes the copying minor ineligible +//! (`CopiedMinorFallbackReason::ConservativeStack`), so the pre-fix code +//! survives — the address neither dies nor moves. +//! +//! It does not always engage. `force_full_scan` is a no-op whenever +//! `CONSERVATIVE_STACK_SCAN_OVERRIDE` is already set, and +//! `conservative_stack_scan_mode` lets an explicit +//! `PERRY_CONSERVATIVE_STACK_SCAN` env value beat any pin. So +//! `PERRY_CONSERVATIVE_STACK_SCAN=off` — the arm every issue in this family +//! reproduces on — removes the valve and the alloc-point minor evacuates. +//! +//! These tests run in exactly that configuration and say so out loud rather +//! than inheriting it: `CopyingNurseryTestGuard` pins the mode to `Auto`, which +//! `conservative_stack_scan_decision_for` resolves to `SkipDisabled` — the same +//! decision `PERRY_CONSERVATIVE_STACK_SCAN=off` produces. So the honest claim +//! is not "this crashed a shipped default build"; it is "the valve masking it +//! is a bounded valve #7148 documents as such, and with the valve off the +//! read lands on retired from-space, deterministically." +//! +//! ## What makes it able to fail +//! +//! The options object is held by NOTHING except the function under test: the +//! test never installs it in a shadow slot and it is not reachable from any +//! registered root. So the only thing that can keep it alive and correct +//! across the key allocation is the `RuntimeHandleScope` inside +//! `options_with_file_types` itself. +//! +//! And the collection is asserted to have MOVED something — a separately +//! rooted sentinel allocated in the same nursery must come back at a different +//! address. Without that, a cycle that collected nothing would let this file +//! pass while proving nothing (CLAUDE.md, "a gate must assert its subject was +//! live"). Pacing is left at the shipped default deliberately: pinning +//! `force_legacy_gc_pacing` (scavenge off, polls off) routes the trigger to the +//! budgeted stepper, which is non-moving by construction — under that guard the +//! sentinel never moves and the file certifies nothing. That was observed, not +//! assumed. +//! +//! SABOTAGE RECORD: reverting `options_with_file_types` to its pre-fix shape +//! (decode, then allocate, then dereference) fails +//! `readdir_options_object_survives_the_with_file_types_key_allocation` and +//! leaves the negative test green — the negative one is a regression guard, not +//! a discriminator, and is labelled as such below. + +use super::*; + +/// Build `{ withFileTypes: true }` in the nursery and return it NaN-boxed. +/// +/// Runs under suppressed triggers: `js_object_set_field_by_name` allocates the +/// keys array, and a collection there would move `obj` — which this function +/// holds as a bare local — out from under the setup, failing the test for a +/// reason that has nothing to do with the subject. +fn with_file_types_options_object(_trigger_guard: &GcTriggerThresholdTestGuard) -> (f64, usize) { + let obj = crate::object::js_object_alloc(0, 1); + let key = crate::string::js_string_from_bytes(b"withFileTypes".as_ptr(), 13); + crate::object::js_object_set_field_by_name(obj, key, f64::from_bits(crate::value::TAG_TRUE)); + assert!( + crate::arena::pointer_in_nursery(obj as usize), + "the options object must be a movable nursery object or the test \ + exercises nothing" + ); + (f64::from_bits(ptr_bits(obj as usize)), obj as usize) +} + +#[test] +fn readdir_options_object_survives_the_with_file_types_key_allocation() { + let _guard = CopyingNurseryTestGuard::new(1); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + register_runtime_handle_root_scanner_for_tests(); + + let (options_value, options_addr_before) = with_file_types_options_object(&trigger_guard); + + // The liveness witness. Rooted, so the collector must keep it AND rewrite + // it; if its address is unchanged afterwards the cycle did not evacuate and + // this test would be certifying an empty run. + let sentinel_scope = RuntimeHandleScope::new(); + let sentinel = sentinel_scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 0)); + let sentinel_before = sentinel.get_raw_mut_ptr::() as usize; + + force_next_general_arena_alloc_slow(); + trigger_guard.make_arena_trigger_due(); + let before = gc_collection_count(); + + // SUBJECT. The `js_string_from_bytes` inside this call is the collection + // point; everything the function needs afterwards has to come back out of + // its own handle. + let with_file_types = unsafe { crate::fs::options_with_file_types(options_value) }; + + drain_scheduled_minor_gc(before, "withFileTypes key allocation"); + + let sentinel_after = sentinel.get_raw_mut_ptr::() as usize; + assert_ne!( + sentinel_after, sentinel_before, + "the minor did not evacuate — no object moved, so nothing here was \ + exercised and a green result would be meaningless" + ); + assert!( + with_file_types, + "options_with_file_types read `withFileTypes` through the address it \ + computed BEFORE the key allocation (object was at {options_addr_before:#x}); \ + the collection at that allocation moved/reclaimed the object, so the \ + read landed on retired from-space" + ); +} + +/// The negative half: an options object WITHOUT the field must still read +/// `false` across the same collection, rather than picking up a truthy word +/// from whatever was recycled into the from-space bytes. +/// +/// NOT A DISCRIMINATOR — it stays green under the pre-fix code, because a +/// stale read yields not-truthy just as readily as a correct one. It is here as +/// a regression guard on the `None`/false arms of the rewritten decode +/// (`options_object_ptr` returning `None`, the handle-band floor), which the +/// positive test never reaches. Recorded rather than deleted so nobody later +/// reads its passing as evidence about the rooting. +#[test] +fn readdir_options_without_the_field_stays_false_across_the_key_allocation() { + let _guard = CopyingNurseryTestGuard::new(1); + let trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + register_runtime_handle_root_scanner_for_tests(); + + let obj = crate::object::js_object_alloc(0, 1); + let key = crate::string::js_string_from_bytes(b"encoding".as_ptr(), 8); + let encoding = crate::string::js_string_from_bytes(b"utf8".as_ptr(), 4); + crate::object::js_object_set_field_by_name( + obj, + key, + f64::from_bits(string_bits(encoding as usize)), + ); + let options_value = f64::from_bits(ptr_bits(obj as usize)); + + let sentinel_scope = RuntimeHandleScope::new(); + let sentinel = sentinel_scope.root_raw_mut_ptr(crate::object::js_object_alloc(0, 0)); + let sentinel_before = sentinel.get_raw_mut_ptr::() as usize; + + force_next_general_arena_alloc_slow(); + trigger_guard.make_arena_trigger_due(); + let before = gc_collection_count(); + + let with_file_types = unsafe { crate::fs::options_with_file_types(options_value) }; + + drain_scheduled_minor_gc(before, "withFileTypes key allocation"); + + assert_ne!( + sentinel.get_raw_mut_ptr::() as usize, + sentinel_before, + "the minor did not evacuate — nothing here was exercised" + ); + assert!( + !with_file_types, + "an options object with no `withFileTypes` field must read false" + ); +}