diff --git a/changelog.d/11858-empty-string-literal-key.md b/changelog.d/11858-empty-string-literal-key.md new file mode 100644 index 0000000000..5e74046a27 --- /dev/null +++ b/changelog.d/11858-empty-string-literal-key.md @@ -0,0 +1 @@ +- **perry-runtime**: an object literal with an empty-string key, `{ "": v }`, no longer aborts with "refusing to publish invalid object shape facts" (or, across two modules, "the static ShapeId ... was refused by the shape mint"). The readers of compiler-packed key names dropped every empty segment, so the key `""` vanished; they now share one decoder that keeps it. This killed the natively compiled OpenCode v1.18.30 TUI at startup, on json5's JSON reviver holder `{ '': root }`. diff --git a/crates/perry-runtime/src/json/parse_api.rs b/crates/perry-runtime/src/json/parse_api.rs index 8ebc92492a..40efbbf38f 100644 --- a/crates/perry-runtime/src/json/parse_api.rs +++ b/crates/perry-runtime/src/json/parse_api.rs @@ -1011,10 +1011,7 @@ pub(crate) unsafe fn build_shape_hint( let packed = std::slice::from_raw_parts(packed_keys, packed_keys_len as usize); // Same parsing as `js_build_class_keys_array`: split on `\0`, // drop empties. - let keys: Vec<&[u8]> = packed - .split(|&b| b == 0) - .filter(|s| !s.is_empty()) - .collect(); + let keys: Vec<&[u8]> = crate::object::packed_key_names(packed); if keys.len() != field_count as usize { return None; } diff --git a/crates/perry-runtime/src/object/alloc.rs b/crates/perry-runtime/src/object/alloc.rs index 2793ada085..241b95a1e3 100644 --- a/crates/perry-runtime/src/object/alloc.rs +++ b/crates/perry-runtime/src/object/alloc.rs @@ -479,10 +479,7 @@ pub extern "C" fn js_build_class_keys_array( return keys.arr(); } let keys_bytes = unsafe { std::slice::from_raw_parts(packed_keys, packed_keys_len as usize) }; - let keys: Vec<&[u8]> = keys_bytes - .split(|&b| b == 0) - .filter(|s| !s.is_empty()) - .collect(); + let keys: Vec<&[u8]> = crate::object::packed_key_names(keys_bytes); // This array is long-lived and never dies. Without the scope, the per-slot // notes in the builder mint a per-object pointer mask for any class with // enough keys, which arms `PERRY_PER_OBJECT_LAYOUTS_ANY` and puts the @@ -575,10 +572,7 @@ pub extern "C" fn js_object_alloc_class_with_keys( } else { let keys_bytes = unsafe { std::slice::from_raw_parts(packed_keys, packed_keys_len as usize) }; - let keys: Vec<&[u8]> = keys_bytes - .split(|&b| b == 0) - .filter(|s| !s.is_empty()) - .collect(); + let keys: Vec<&[u8]> = crate::object::packed_key_names(keys_bytes); // Issue #179: shape-cache keys_array lives in the longlived arena // (see `js_build_class_keys_array` for the rationale). let arr = unsafe { build_longlived_keys_array(ptr::null_mut(), 0, &keys) }; @@ -686,7 +680,7 @@ pub extern "C" fn js_object_alloc_class_dynamic_parent( let bytes = unsafe { std::slice::from_raw_parts(own_packed_keys, own_packed_keys_len as usize) }; - bytes.split(|&b| b == 0).filter(|s| !s.is_empty()).collect() + crate::object::packed_key_names(bytes) }; let merged_len = parent_len as usize + own_keys.len(); // `parent_arr` was read from the memo with no allocation since; the @@ -790,10 +784,7 @@ pub extern "C" fn js_object_alloc_with_shape( } else { let keys_bytes = unsafe { std::slice::from_raw_parts(packed_keys, packed_keys_len as usize) }; - let keys: Vec<&[u8]> = keys_bytes - .split(|&b| b == 0) - .filter(|s| !s.is_empty()) - .collect(); + let keys: Vec<&[u8]> = crate::object::packed_key_names(keys_bytes); // Issue #179: shape-cache keys_array lives in the longlived arena. // The builder roots the unfinished array across its key allocations; // the object is already held in `obj_scope`. diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 24c1e31254..00c9beb5c8 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -1961,3 +1961,48 @@ mod transition_ic_tests; mod wide_field_read_tests; #[cfg(test)] mod wide_object_membership_tests; + +/// The key names in a compiler-packed key list. Codegen writes every name +/// followed by a NUL (`codegen/mod.rs`, `expr/object_literal.rs`, +/// `lower_call/new_alloc.rs`), so the names are the segments between the +/// terminators, and an empty segment is a name: the key `""`. Only the empty +/// segment after the final terminator is dropped. +/// +/// Every reader used to drop ALL empty segments, so `{ "": v }` built a keys +/// array shorter than its count and the shape mint refused the facts (an +/// abort at the literal), and two modules' `{ "": v }` -- equal contents under +/// one static ShapeId -- each built their own empty array, so the second +/// module's static mint was refused (OpenCode's TUI: json5's reviver holder +/// `{ "": root }`). +pub(crate) fn packed_key_names(bytes: &[u8]) -> Vec<&[u8]> { + let mut names: Vec<&[u8]> = bytes.split(|&b| b == 0).collect(); + if names.last().is_some_and(|s| s.is_empty()) { + names.pop(); + } + names +} + +#[cfg(test)] +mod packed_key_names_tests { + use super::packed_key_names; + + #[test] + fn the_empty_key_is_a_name_and_only_the_final_terminator_is_dropped() { + let names = + |b: &[u8]| -> Vec> { packed_key_names(b).iter().map(|s| s.to_vec()).collect() }; + assert_eq!(names(b"a\0b\0"), vec![b"a".to_vec(), b"b".to_vec()]); + assert_eq!( + names(b"\0"), + vec![b"".to_vec()], + "{{ \"\": v }} has one key" + ); + assert_eq!(names(b"\0a\0"), vec![b"".to_vec(), b"a".to_vec()]); + assert_eq!(names(b"a\0\0"), vec![b"a".to_vec(), b"".to_vec()]); + assert_eq!( + names(b"a\0b"), + vec![b"a".to_vec(), b"b".to_vec()], + "an unterminated last name" + ); + assert!(names(b"").is_empty()); + } +} diff --git a/crates/perry-runtime/src/object/static_shapes.rs b/crates/perry-runtime/src/object/static_shapes.rs index 759fb0216a..69400577f1 100644 --- a/crates/perry-runtime/src/object/static_shapes.rs +++ b/crates/perry-runtime/src/object/static_shapes.rs @@ -85,7 +85,7 @@ pub extern "C" fn js_shape_seed_plain( } // SAFETY: compiler-owned static data of `packed_len` bytes. let bytes = unsafe { std::slice::from_raw_parts(packed, packed_len as usize) }; - let names: Vec<&[u8]> = bytes.split(|&b| b == 0).filter(|s| !s.is_empty()).collect(); + let names: Vec<&[u8]> = crate::object::packed_key_names(bytes); if names.len() != count as usize { return 0; } @@ -134,7 +134,7 @@ pub extern "C" fn js_shape_seed_plain_constfn( } // SAFETY: compiler-owned static data of `packed_len` bytes. let bytes = unsafe { std::slice::from_raw_parts(packed, packed_len as usize) }; - let names: Vec<&[u8]> = bytes.split(|&b| b == 0).filter(|s| !s.is_empty()).collect(); + let names: Vec<&[u8]> = crate::object::packed_key_names(bytes); if names.len() != count as usize { invalid_constfn_static_seed(); } diff --git a/crates/perry/tests/empty_string_literal_key.rs b/crates/perry/tests/empty_string_literal_key.rs new file mode 100644 index 0000000000..7f34b91560 --- /dev/null +++ b/crates/perry/tests/empty_string_literal_key.rs @@ -0,0 +1,103 @@ +//! Regression test: an object literal whose key is the empty string, +//! `{ "": v }`, aborted at the literal with "refusing to publish invalid +//! object shape facts". +//! +//! Codegen packs key names as `name\0` per name, so the key `""` is the +//! single byte `\0`. Every runtime reader split the packed names on NUL and +//! dropped empty segments, so the empty key vanished: the keys array came out +//! shorter than its count and the shape mint refused the facts. Across two +//! modules it failed differently: both literals are one content, so they +//! share one static ShapeId, and each module built its own empty array -- +//! different facts under one id, and the second module's mint was refused. +//! That is how the natively compiled OpenCode v1.18.30 TUI died at startup: +//! json5's `parse.js` builds the JSON reviver holder `{ '': root }`. +//! +//! The literal lives in two modules here so the static-id path is exercised, +//! and once inside a function so it is born at a call, not only at init. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn runtime_dir() -> PathBuf { + std::env::var_os("PERRY_RUNTIME_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| { + perry_bin() + .parent() + .expect("compiler directory") + .to_path_buf() + }) +} + +const OTHER_SOURCE: &str = r#" +export const otherHolder = { "": "second module" } +export function wrap(v: unknown) { + return { "": v } +} +"#; + +const MAIN_SOURCE: &str = r#" +import { otherHolder, wrap } from "./other.js" + +const holder = { "": 42 } +console.log("1 holder[\"\"]:", holder[""]) +console.log("2 keys:", JSON.stringify(Object.keys(holder))) +console.log("3 in:", "" in holder) +const mixed = { "": 1, a: 2 } +console.log("4 mixed:", JSON.stringify(mixed), mixed[""], mixed.a) +console.log("5 reviver:", JSON.stringify(JSON.parse("{\"x\":1}", (k, v) => (k === "" ? { root: v } : v)))) +console.log("6 other module:", otherHolder[""]) +console.log("7 built in a function:", JSON.stringify(wrap([1, 2]))) +"#; + +/// Byte-for-byte what node 26.5.1 prints. +const EXPECTED: &str = "\ +1 holder[\"\"]: 42 +2 keys: [\"\"] +3 in: true +4 mixed: {\"\":1,\"a\":2} 1 2 +5 reviver: {\"root\":{\"x\":1}} +6 other module: second module +7 built in a function: {\"\":[1,2]} +"; + +#[test] +fn an_empty_string_key_survives_an_object_literal() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + std::fs::write(root.join("other.ts"), OTHER_SOURCE).unwrap(); + std::fs::write(root.join("main.ts"), MAIN_SOURCE).unwrap(); + + let output = root.join("main_bin"); + let out = Command::new(perry_bin()) + .current_dir(root) + .arg("compile") + .arg(root.join("main.ts")) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_RUNTIME_DIR", runtime_dir()) + .output() + .expect("run perry compile"); + assert!( + out.status.success(), + "empty-key probe must compile; stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary must run, not abort at the literal; stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let stdout = String::from_utf8(run.stdout).expect("UTF-8 stdout"); + assert_eq!(stdout, EXPECTED, "the key `\"\"` is a key like any other"); +}