Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions benchmarks/json_polyglot/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,83 @@ echo
echo "Wrote $(pwd)/RESULTS.md"
cat RESULTS.md

# ---------------------------------------------------------------------------
# Cross-runtime correctness gate (#7264).
#
# Every workload prints `checksum:` — a fold over the parsed data plus the
# length of the re-serialized blob. Perry, node and bun must agree exactly; a
# disagreement means one of them produced different DATA, which is a
# correctness bug, not a measurement artifact.
#
# This used to be checked only inside the optional JSON publisher below, so a
# mismatch (a) did not fail a plain `run.sh` at all, and (b) under
# run_public_baseline.sh surfaced ~40 minutes later as a confusing "could not
# load json-polyglot.json" at final assembly. Check it here, immediately after
# the runs, unconditionally, and name the offending runtimes.
# ---------------------------------------------------------------------------
checksum_failed=0
for workload in roundtrip field_access; do
# Mirror the publisher's cell selection: perry's optimized row, node/bun's
# idiomatic row. Emits one "<runtime> <checksum>" line per runtime found.
observed=$(awk -F'\t' -v w="$workload" '
$1 != w { next }
{
split($3, parts, " ")
runtime = parts[1]
want = (runtime == "perry") ? "optimized" : "idiomatic"
if (runtime != "perry" && runtime != "node" && runtime != "bun") next
if ($2 != want || runtime in seen) next
split($5, cs, ",")
if (cs[1] == "") next
seen[runtime] = cs[1]
order[++n] = runtime
}
END { for (i = 1; i <= n; i++) print order[i], seen[order[i]] }
' "$RAW_RESULTS_FILE")
# A publishing run REQUIRES all three: the publisher indexes
# selected[(workload, runtime)] for perry, node and bun unconditionally, so
# a missing row is a hard error there — and "they all agree" is vacuous when
# only one runtime reported. A local run without node/bun installed is a
# legitimate use of this script, so only enforce presence when publishing.
if [[ -n "$PUBLIC_JSON_OUT" ]]; then
for required in perry node bun; do
if ! grep -q "^$required " <<< "$observed"; then
checksum_failed=1
{
echo
echo "ERROR: json_polyglot/$workload — no checksum row for '$required'."
echo " A published artifact compares perry against BOTH node and bun;"
echo " with a runtime missing there is nothing to compare against."
echo " Install it (or drop \$PUBLIC_BENCH_JSON_OUT for a local run)."
} >&2
fi
done
fi
[[ -z "$observed" ]] && continue
distinct=$(awk '{ print $2 }' <<< "$observed" | sort -u | wc -l | tr -d ' ')
if [[ "$distinct" -ne 1 ]]; then
checksum_failed=1
{
echo
echo "ERROR: json_polyglot/$workload — runtimes disagree on the DATA."
echo " Each line is <runtime> <checksum>; they must all be equal."
sed 's/^/ /' <<< "$observed"
echo " This is a correctness bug in whichever runtime is the odd one out,"
echo " not benchmark noise. The results above are not publishable."
} >&2
fi
done
Comment on lines +597 to +610

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether node/bun runtimes are ever treated as optional in this harness.
rg -n "NODE_BIN|BUN_BIN|PERRY_BIN|command -v (node|bun)" benchmarks/json_polyglot/run.sh

Repository: PerryTS/perry

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file size =="
wc -l benchmarks/json_polyglot/run.sh

echo "== relevant section =="
sed -n '520,605p' benchmarks/json_polyglot/run.sh

echo "== run_bench references/samples =="
sed -n '230,280p' benchmarks/json_polyglot/run.sh

echo "== runtime/workload references =="
rg -n "node|bun|perry|PERRY|run_bench|RAW_RESULTS_FILE|distinct|observed|checksum_failed|workload|roundtrip|field_access" benchmarks/json_polyglot/run.sh

Repository: PerryTS/perry

Length of output: 18886


Require perry, node, and bun checksum rows before accepting agreement.

The checksum gate currently only checks checksum consistency across any present rows. If a workload has only one perry/node/bun checksum row, distinct is 1 and the gate passes, even though the gate states Perry, node, and bun must agree exactly. Require runtime_count == 3 before accepting a single checksum value.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@benchmarks/json_polyglot/run.sh` around lines 578 - 591, The checksum gate in
the workload loop must require all three Perry, node, and bun rows before
accepting agreement. Update the condition using the existing observed-row count
and distinct checksum logic so it fails unless runtime_count equals 3 and
distinct equals 1, while preserving the current failure reporting.

if [[ "$checksum_failed" -ne 0 ]]; then
echo >&2
echo "json_polyglot: FAILED the cross-runtime checksum gate — not publishing." >&2
exit 1
fi

if [[ -n "$PUBLIC_JSON_OUT" ]]; then
mkdir -p "$(dirname "$PUBLIC_JSON_OUT")"
# Never leave a stale artifact behind: a previous run's file must not be
# mistaken for this run's output if the publisher aborts.
rm -f "$PUBLIC_JSON_OUT"
PYTHONPATH="$(cd "$PERRY_ROOT" && pwd)" python3 - "$RAW_RESULTS_FILE" "$PUBLIC_JSON_OUT" "$(cd "$PERRY_ROOT" && pwd)" "$RUNS" "$PIN_NOTE" <<'PY'
import json, shutil, subprocess, sys
from datetime import datetime, timezone
Expand Down Expand Up @@ -614,5 +689,19 @@ component = {
}
Path(output_path).write_text(json.dumps(component, indent=2) + "\n")
PY
# The script runs under `set -uo pipefail`, NOT `-e`: without this check a
# publisher that aborted via `raise SystemExit(...)` still fell through to
# the success line below and run.sh exited 0 having written nothing (#7264).
publish_status=$?
if [[ "$publish_status" -ne 0 || ! -s "$PUBLIC_JSON_OUT" ]]; then
{
echo
echo "ERROR: json_polyglot did not publish machine-readable results."
echo " The publisher exited $publish_status and $PUBLIC_JSON_OUT is missing or empty."
echo " The message above this line is the reason; nothing downstream can use this leg."
} >&2
[[ "$publish_status" -eq 0 ]] && publish_status=1
exit "$publish_status"
fi
echo "Machine-readable results: $PUBLIC_JSON_OUT"
fi
17 changes: 17 additions & 0 deletions changelog.d/7265-json-array-element-overflow-fields.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
fix(json): `JSON.stringify` silently dropped every array element's properties past the inline-slot floor (#7264).

`JSON.stringify(arr)` truncated **every** element of a homogeneous array of objects to 4 properties — no error, no warning, just short output — as soon as the array came from `JSON.parse` and any property of any element had been read. `JSON.stringify(JSON.parse(x))` is a routine idiom, so anything that parses JSON, reads a field, and re-serialises (a proxy, a cache layer, a request handler) could emit truncated records.

**Root cause.** The array fast path builds one shape template from element 0 and reuses its pre-formatted key prefixes for every element. It sized that template from `min(keys_len, field_count)`. `keys_len` is the *logical* property count; `field_count` is *physical* — it never exceeds the object's inline slot allocation. An object grown by name past `INLINE_SLOT_FLOOR` keeps `field_count` pinned at the floor and parks the remaining values in overflow storage. `JSON.parse`'s lazy-tape materializer builds exactly that shape (`js_object_alloc(0, 0)` plus one `js_object_set_field_by_name` per key), so a 6-key record reports `field_count == 4` and the template emitted — and read — only 4 slots.

Only the *array* path was wrong, which is what made this so hard to see: the single-object path already knew the invariant (its `has_overflow_fields` guard bails out of the template when `keys_len > field_count`), so `JSON.stringify(parsed[0])`, `parsed.map(o => JSON.stringify(o))` and `Object.keys(parsed[0])` all reported the truth while the whole-array result did not.

`build_shape_prefix_template` now sizes the template from `keys_len` and routes slot reads through `template_field_bits` / `field_bits_at`, which read inline below `max(field_count, INLINE_SLOT_FLOOR)` and fall through to `js_object_get_field`'s overflow lookup above it — the same rule `stringify_object_inner` has used since #307. The `min` was never needed for the opposite skew either: a pre-sized object (`js_object_alloc(0, 8)` holding 2 keys) has `field_count > keys_len`, and `keys_len` already stops at the last real key.

Latent since the shape template landed (v0.5.65), where it only bit objects with 9+ properties; exposed for ordinary 5–8-field records by 6958a5a8d (#6712), which lowered `INLINE_SLOT_FLOOR` from 8 to 4. That is why the `2026-07-13` public baseline passed the same checksum gate this failed.

**Second defect, same emitter.** The template's primitive-only fast path is chosen by *sampling element 0*, then trusted every later element to be primitive too. A function- or symbol-valued property in a later element must be omitted per `SerializeJSONObject`, but the fast path had already written the key prefix and rendered the closure as `null` — emitting a member that must not exist (`{"f2":null}` where node emits nothing). Only the general path pre-scanned for it. It now rolls the buffer back and defers to the slow path, exactly as it already did for a stray `undefined`. Applies to inline slots as much as overflow ones.

**Harness.** `benchmarks/json_polyglot/run.sh` printed `Machine-readable results: <path>` and exited 0 while its embedded publisher had already aborted via `raise SystemExit(...)` and written nothing — the script runs under `set -uo pipefail`, not `-e`. `run_public_baseline.sh` therefore sailed past it and failed ~40 minutes later with a misleading "could not load json-polyglot.json". Two changes: a cross-runtime checksum gate now runs unconditionally right after the benchmark cells, names the disagreeing runtimes, and fails the leg immediately; and the publisher's exit status plus the existence of a non-empty output file are both checked before the success line prints. A stale artifact from a previous run is also removed up front so it can't be mistaken for this run's output.

Verified: `benchmarks/json_polyglot/bench_field_access.ts` now checksums `2552985550` and `bench.ts` `53735550` under perry, node 26.5.1 and bun 1.3.14 alike (perry previously gave `2538318800` on the first), unblocking the public-baseline regeneration in #7257. Field counts 1–12 round-trip byte-identically; the untouched-array, one-read and loop-read forms all agree; the nested-subtree shape from the report is byte-identical. New `--lib` unit tests in `crates/perry-runtime/src/json/mod.rs` (which run per-PR, unlike `crates/*/tests/*.rs`) plus `test-files/test_gap_json_array_element_overflow_fields.ts`. Full 471-test gap suite and `cargo test -p perry-runtime --lib` show no regressions.
158 changes: 157 additions & 1 deletion crates/perry-runtime/src/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//! - `parser` — `DirectParser` recursive-descent implementation
//! - `parse_api` — `js_json_parse` / `js_json_parse_typed_array` FFI
//! - `stringify` — core stringify traversal (object/array/scalar emitters)
//! - `stringify_shape_template` — homogeneous-array shape templates
//! - `stringify_api` — `js_json_stringify*` / `js_json_get_*` FFI entries
//! - `replacer` — replacer + pretty/indent + array-replacer variants
//! - `reviver` — `JSON.parse(text, reviver)` support
Expand All @@ -33,6 +34,7 @@ mod stringify;
mod stringify_api;
mod stringify_buffer;
mod stringify_scalars;
mod stringify_shape_template;
mod stringify_tojson_probe;

// Public FFI re-exports — preserve the `crate::json::js_json_*` path used by
Expand Down Expand Up @@ -63,12 +65,12 @@ pub(crate) use simd::find_string_terminator;
pub(crate) use stringify::{
arm_to_json_result_guard, estimate_json_size, is_closure_value, is_object_pointer,
is_symbol_value, object_get_to_json, stringify_value, write_escaped_string, write_number,
ShapeTemplate,
};
pub(crate) use stringify_api::{redirect_lazy_to_materialized, try_stringify_lazy_array};
pub(crate) use stringify_buffer::{
stringify_buffer, stringify_buffer_pretty, stringify_typed_array, stringify_typed_array_pretty,
};
pub(crate) use stringify_shape_template::ShapeTemplate;
pub(crate) use stringify_tojson_probe::{
current_to_json_key_arg, invalidate_object_proto_tojson_state, reset_to_json_key,
set_to_json_key_index, set_to_json_key_str, set_to_json_key_value, to_json_definitely_absent,
Expand Down Expand Up @@ -935,4 +937,158 @@ mod tests {
"\"a\\bb\\fc\\u000bd\""
);
}

/// Materialize `src` exactly the way `JSON.parse` does once a property
/// read forces the lazy tape to become real objects, and return both the
/// value and the resulting `(field_count, keys_len)` of element 0.
unsafe fn materialized_array(src: &[u8]) -> (JSValue, u32, u32) {
let tape = crate::json_tape::build_tape(src).expect("tape builds for valid JSON");
let value = crate::json_tape::materialize(&tape, src);
let arr = (value.bits() & POINTER_MASK) as *mut crate::ArrayHeader;
let elem0 = crate::array::js_array_get(arr, 0);
let obj = (elem0.bits() & POINTER_MASK) as *const crate::ObjectHeader;
(value, (*obj).field_count, (*(*obj).keys_array).length)
}

#[test]
fn stringify_array_keeps_every_property_of_name_grown_elements() {
// #7264: `JSON.stringify` silently dropped every property past the
// 4th from EVERY element of a homogeneous array.
//
// The array fast path built its shape template with
// `min(keys_len, field_count)`. `field_count` is PHYSICAL — it never
// exceeds the object's inline slot allocation, so an object grown by
// name past `INLINE_SLOT_FLOOR` reports the floor (4) while the
// remaining values live in overflow storage. `JSON.parse`'s tape
// materializer produces exactly that shape (`js_object_alloc(0, 0)` +
// `js_object_set_field_by_name` per key), so `JSON.stringify` of a
// parsed-and-touched array truncated every record to 4 fields.
//
// Latent since the shape template landed (v0.5.65, 9-field threshold);
// exposed for ordinary 5–8-field records by #6712 (floor 8 → 4).
let src = br#"[{"f0":0,"f1":1,"f2":2,"f3":3,"f4":4,"f5":5},{"f0":10,"f1":11,"f2":12,"f3":13,"f4":14,"f5":15}]"#;
unsafe {
let (value, field_count, keys_len) = materialized_array(src);
// Assert the gate's SUBJECT is live: this test is only meaningful
// while the materializer really does leave `field_count` below
// `keys_len` (the overflow shape). If a future change pre-sizes
// the allocation, this assertion fires and the test must be
// re-pointed rather than silently passing on the easy path.
assert!(
field_count < keys_len,
"precondition lost: tape-materialized element is no longer an \
overflow shape (field_count={field_count}, keys_len={keys_len}) — \
re-point this regression test at a shape that still is"
);
assert_eq!(keys_len, 6);

let output = js_json_stringify(f64::from_bits(value.bits()), TYPE_ARRAY);
assert_eq!(
str_from_header(output).unwrap(),
std::str::from_utf8(src).unwrap(),
"array-element serialization must emit all {keys_len} properties"
);
}
}

#[test]
fn stringify_array_round_trips_every_field_count_across_the_inline_floor() {
// The truncation was invisible at ≤ INLINE_SLOT_FLOOR fields and
// produced IDENTICAL output for 5, 6 and 8 — the tell that emission
// stopped at the floor. Sweep across the boundary.
for n in 1..=10usize {
let record = |base: usize| {
let fields: Vec<String> =
(0..n).map(|f| format!("\"f{f}\":{}", base + f)).collect();
format!("{{{}}}", fields.join(","))
};
let src = format!("[{},{}]", record(0), record(100));
unsafe {
let tape = crate::json_tape::build_tape(src.as_bytes()).expect("tape");
let value = crate::json_tape::materialize(&tape, src.as_bytes());
let output = js_json_stringify(f64::from_bits(value.bits()), TYPE_ARRAY);
assert_eq!(
str_from_header(output).unwrap(),
src.as_str(),
"{n}-field records must round-trip"
);
}
}
}

#[test]
fn stringify_array_keeps_nested_objects_and_arrays_past_the_inline_floor() {
// The shape reported in #7264: the 5th property is a nested object and
// the 4th an array, so the dropped field was a whole subtree.
let src = br#"[{"id":0,"name":"item_0","value":0,"tags":["tag_0","tag_0"],"nested":{"x":0,"y":0}},{"id":1,"name":"item_1","value":3,"tags":["tag_1","tag_1"],"nested":{"x":1,"y":2}}]"#;
unsafe {
let (value, field_count, keys_len) = materialized_array(src);
assert!(field_count < keys_len, "precondition: overflow shape");
let output = js_json_stringify(f64::from_bits(value.bits()), TYPE_ARRAY);
assert_eq!(
str_from_header(output).unwrap(),
std::str::from_utf8(src).unwrap()
);
}
}

#[test]
fn stringify_array_omits_a_function_valued_property_of_a_later_element() {
// Second defect found in the same emitter while fixing #7264. The
// template's primitive-only fast path is chosen by SAMPLING element 0;
// it then trusted every later element to be primitive too. A function
// (or symbol) value in a later element must be OMITTED per
// SerializeJSONObject, but the fast path had already written the key
// prefix and rendered the closure as `null` — emitting a member that
// must not exist. Only the general (non-primitive-only) path pre-scanned
// for it. Applies to inline slots as much as overflow ones.
let src = br#"[{"f0":0,"f1":1,"f2":2,"f3":3,"f4":4,"f5":5},{"f0":10,"f1":11,"f2":12,"f3":13,"f4":14,"f5":15}]"#;
unsafe {
let (value, _, _) = materialized_array(src);
let arr = (value.bits() & POINTER_MASK) as *mut crate::ArrayHeader;
let elem1 = crate::array::js_array_get(arr, 1);
let obj1 = (elem1.bits() & POINTER_MASK) as *mut crate::ObjectHeader;
// `f4` is at index 4 — an OVERFLOW slot on this shape, so this also
// covers the fallback plumbing added for overflow reads.
let closure = crate::closure::js_closure_alloc(std::ptr::null(), 0);
let key = js_string_from_bytes(b"f4".as_ptr(), 2);
crate::object::js_object_set_field_by_name(
obj1,
key,
f64::from_bits(POINTER_TAG | (closure as u64 & POINTER_MASK)),
);

let output = js_json_stringify(f64::from_bits(value.bits()), TYPE_ARRAY);
assert_eq!(
str_from_header(output).unwrap(),
r#"[{"f0":0,"f1":1,"f2":2,"f3":3,"f4":4,"f5":5},{"f0":10,"f1":11,"f2":12,"f3":13,"f5":15}]"#
);
}
}

#[test]
fn stringify_array_stops_at_the_last_real_key_when_slots_were_pre_sized() {
// The opposite skew, and the reason the truncating `min` existed: an
// object allocated with MORE inline slots than it has keys
// (`js_object_alloc(0, 8)` holding 2 properties). `keys_len` alone must
// still stop at the last real key rather than dumping padding slots.
unsafe {
let mut arr = crate::array::js_array_alloc(2);
for base in [1.0f64, 10.0f64] {
let obj = crate::object::js_object_alloc(0, 8);
for (i, name) in ["a", "b"].iter().enumerate() {
let key = js_string_from_bytes(name.as_ptr(), name.len() as u32);
crate::object::js_object_set_field_by_name(obj, key, base + i as f64);
}
assert!((*obj).field_count >= (*(*obj).keys_array).length);
arr = crate::array::js_array_push(arr, JSValue::object_ptr(obj as *mut u8));
}
let boxed = crate::value::js_nanbox_pointer(arr as i64);
let output = js_json_stringify(boxed, TYPE_ARRAY);
assert_eq!(
str_from_header(output).unwrap(),
r#"[{"a":1,"b":2},{"a":10,"b":11}]"#
);
}
}
}
Loading
Loading