diff --git a/changelog.d/7358-string-borrow-across-alloc.md b/changelog.d/7358-string-borrow-across-alloc.md new file mode 100644 index 0000000000..0f93c47dd8 --- /dev/null +++ b/changelog.d/7358-string-borrow-across-alloc.md @@ -0,0 +1,20 @@ +### Fixed + +- **A string's bytes could be read after the string moved.** Two runtime + helpers derived a raw pointer into a `StringHeader`'s payload, then allocated, + then copied *from that pointer*. An evacuating minor inside the allocation + relocates the string, so the copy read retired from-space — + `js_buffer_from_string` (`Buffer.from(str)`) and `js_text_encoder_encode_llvm` + (`TextEncoder.encode`). + + Not observable from output: evacuation copies rather than zeroes, so the stale + address still held the correct bytes and both produced right answers. + `PERRY_GC_PROTECT_FROMSPACE=1` unmaps retired from-space and turns the latent + read into a fault — which is how they were found (#7341). + + Fixed with a no-move window rather than a root: the borrow is a raw slice + handed to a callee that reads it at an arbitrary point, so rooting the header + would leave the slice stale anyway. Each window covers one bounded + allocate-and-copy. + + Closes **13 of the 54** quarantine catches across the gap suite. diff --git a/crates/perry-runtime/src/buffer/from.rs b/crates/perry-runtime/src/buffer/from.rs index 1ff26a2654..21fff8c32d 100644 --- a/crates/perry-runtime/src/buffer/from.rs +++ b/crates/perry-runtime/src/buffer/from.rs @@ -20,6 +20,19 @@ pub extern "C" fn js_buffer_from_string( return buffer_alloc(0); } + // #7341: `str_bytes` borrows the StringHeader's payload, and every arm of + // `buffer_from_str_bytes` allocates before reading it — the UTF-8 arm most + // plainly: `buffer_alloc(len)` then `copy_nonoverlapping(str_bytes...)`. + // An evacuating minor inside that allocation relocates the string, leaving + // the slice pointing at retired from-space, and the copy reads it. That is + // the stale-`memmove` fault the from-space quarantine reports. + // + // A no-move window rather than a root: the borrow is not a value we can + // reload, it is a raw slice handed to a callee that reads it at an + // arbitrary point, so rooting the header would still leave the slice + // stale. The window covers one bounded allocate-and-copy — the same + // argument #7249 made for the globalThis bootstrap. + let _no_move = crate::gc::GcSuppressScope::new(); unsafe { let len = (*str_ptr).byte_len as usize; let data_ptr = (str_ptr as *const u8).add(std::mem::size_of::()); diff --git a/crates/perry-runtime/src/text.rs b/crates/perry-runtime/src/text.rs index d382dc7cbf..ae13be192b 100644 --- a/crates/perry-runtime/src/text.rs +++ b/crates/perry-runtime/src/text.rs @@ -249,6 +249,11 @@ pub extern "C" fn js_text_decoder_ignore_bom(handle: f64) -> f64 { #[no_mangle] pub extern "C" fn js_text_encoder_encode_llvm(value: f64) -> i64 { let str_ptr = text_encoder_string_ptr(value); + // #7341: `data_ptr` points into the StringHeader's payload and is read by + // the copy BELOW `buffer_alloc`. An evacuating minor inside that + // allocation relocates the string and the copy reads retired from-space — + // the same stale-`memmove` fault as `js_buffer_from_string`. + let _no_move = crate::gc::GcSuppressScope::new(); let (data_ptr, len) = unsafe { let l = (*str_ptr).byte_len as usize; let d = (str_ptr as *const u8).add(std::mem::size_of::());