diff --git a/changelog.d/7373-json-parse-stale-input.md b/changelog.d/7373-json-parse-stale-input.md new file mode 100644 index 0000000000..1ba936f061 --- /dev/null +++ b/changelog.d/7373-json-parse-stale-input.md @@ -0,0 +1,22 @@ +### Fixed + +- **`JSON.parse` could read its input after the source string moved.** Both + `js_json_parse` and `js_json_parse_result` derived a byte slice from the + source `StringHeader`, then called `gc_check_trigger()` — a deliberate + collection point that sheds parse-churn garbage between iterations — and only + *then* pushed the string's GC root and suppressed collection. An evacuating + minor at that trigger moved the string, leaving the parser reading retired + from-space for the entire parse. + + The root now precedes the trigger, and the slice is re-derived from the rooted + value afterwards. Ordering is the whole fix: pushing the root *after* the + collection roots an address the collector has already moved away from, so + re-deriving from that slot returns the same stale pointer — verified by trying + exactly that first and measuring no change. + + Invisible from output, because evacuation copies rather than zeroes and the + stale address still held the right bytes. Found with + `PERRY_GC_PROTECT_FROMSPACE=1`, which faults on the first `peek()`. + + Closes **8 of the 31** remaining quarantine catches (#7341) — the largest + single cluster. diff --git a/crates/perry-runtime/src/json/parse_api.rs b/crates/perry-runtime/src/json/parse_api.rs index 5600afc6cd..f4378c696a 100644 --- a/crates/perry-runtime/src/json/parse_api.rs +++ b/crates/perry-runtime/src/json/parse_api.rs @@ -113,11 +113,48 @@ pub unsafe fn js_json_parse_result(text_ptr: *const StringHeader) -> Result()); + std::slice::from_raw_parts(data_ptr, len) + }; let mut parser = DirectParser::new(bytes); let result = parser.parse_value(); parse_root_push(result); @@ -249,6 +286,24 @@ pub unsafe extern "C" fn js_json_parse(text_ptr: *const StringHeader) -> JSValue // in `gc_check_trigger` protects adversarial cases (previous stringify // result strings sharing blocks with interned keys) from retrigger // thrash when block-persistence keeps everything alive. + // #7341: root the source string BEFORE `gc_check_trigger`, and re-derive + // the input slice from the rooted value afterwards. + // + // The `gc_check_trigger()` immediately below is deliberate — see the + // comment above, it is what keeps parse-churn garbage shedding between + // iterations — but it is a COLLECTION POINT, and both `bytes` and + // `text_ptr` were derived above it. An evacuating minor there moves the + // source string, and the parser then reads the pre-collection address for + // the entire parse; the from-space quarantine reports it as a fault at + // `parse_value + 36`, on the very first `peek()`. + // + // Pushing the root first is what makes the re-read work. The old order + // pushed it AFTER the trigger, which roots an address the collector has + // already moved away from — so re-deriving from that slot returns the same + // stale pointer and fixes nothing. Rooting first means the collector + // rewrites the slot, and the re-read yields the post-move payload. + let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); + crate::gc::gc_check_trigger(); // Suppress GC for the duration of the parse. Parse is synchronous and @@ -257,7 +312,12 @@ pub unsafe extern "C" fn js_json_parse(text_ptr: *const StringHeader) -> JSValue // cycles walking an ever-growing live set (issue #59). crate::gc::gc_suppress(); - let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); + let bytes = { + let moved = crate::json::parse_root_get(text_root); + let hdr = moved.as_string_ptr(); + let data_ptr = (hdr as *const u8).add(std::mem::size_of::()); + std::slice::from_raw_parts(data_ptr, len) + }; let mut parser = DirectParser::new(bytes); let result = parser.parse_value();