fix(runtime): zero-fill new Uint8Array(n) — buffer_alloc returns reclaimed dirty memory#6429
Merged
proggeramlug merged 1 commit intoJul 15, 2026
Conversation
…y memory `js_uint8array_alloc` set the header and relied on the backing block already being zero. It isn't: `buffer_alloc` allocates through `arena_alloc_gc_old`, and the old generation reclaims and re-hands dirty blocks — only pristine mmap pages read as zero. So `new Uint8Array(n)` (and the bool / string / numeric-length sources that funnel through it) could observe stale bytes, which is a spec violation: a length-constructed typed array is zero-initialized. The sibling paths already know this. `js_buffer_alloc` (Buffer.alloc) does `write_bytes(data, fill, size)`, and `zeroed_array_buffer_storage` carries the exact comment — "buffer_alloc does not zero, but ArrayBuffer per ECMAScript spec must observe zero-initialized bytes." `js_uint8array_alloc` was the lone sibling that forgot; this adds the same explicit zero-fill. Latent on macOS (length-constructed arrays land on fresh, zero mmap pages) and on current main even on Linux (they happen to land on clean blocks). It surfaced under an unrelated codegen change (PerryTS#6407 expands BUFFER_PROTOTYPE_METHODS 11->93, shifting -O3/LTO layout) which moved these allocations onto reclaimed dirty blocks: test_gap_uint8array_source_dispatch then printed garbage, non- deterministic bytes for the 'boolean' / 'string' length cases. Reproduced on a Linux box: 20/20 runs correct with this fix, byte-identical to Node; reverting it fails. No new gap test: the defect needs the arena to re-hand a specific dirtied block, which neither macOS nor main-on-Linux reproduces on demand, so any portable test would pass with or without the fix (it would not prove it can fail). The fix is unconditionally correct — zero-filling a spec-zero-initialized buffer can never regress a caller.
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
new Uint8Array(n)can observe uninitialized memory — a spec violation (a length-constructed typed array must be zero-filled), and a genuine memory-disclosure hazard.Root cause
js_uint8array_allocset the header and relied on the backing block already being zero. It isn't.buffer_allocallocates througharena_alloc_gc_old, and the old generation reclaims and re-hands dirty blocks — only pristinemmappages read as zero.The sibling paths already know this:
js_buffer_alloc(Buffer.alloc) doesptr::write_bytes(data, fill, size).zeroed_array_buffer_storagecarries the exact comment: "buffer_allocdoes not zero, but ArrayBuffer per ECMAScript spec must observe zero-initialized bytes."js_uint8array_allocwas the one sibling that forgot. The fix adds the same explicit zero-fill:Why nobody hit it, and how it surfaced
Latent on macOS (length-constructed arrays land on fresh, zero
mmappages), and latent on currentmaineven on Linux (they happen to land on clean blocks). It surfaced under #6407 — an unrelated net-dispatch PR that also expandsBUFFER_PROTOTYPE_METHODSfrom 11 to ~93 entries, perturbing-O3/thin-LTO code+data layout and moving these allocations onto reclaimed dirty blocks.test_gap_uint8array_source_dispatchthen printed garbage, non-deterministic bytes for theboolean(new Uint8Array(true)) andstring(new Uint8Array("2")) length cases:Reproduced on a Linux box: 20/20 runs correct with this fix, byte-identical to Node; reverting it fails. This is why #6407's CI was red on
conformance-smoke (8)— landing this onmainunblocks it on rebase.On the missing test
I deliberately did not add a gap test. The defect requires the arena to re-hand a specific dirtied block, which neither macOS nor
main-on-Linux reproduces on demand — I verified a GC-reuse stress test (fill 0xFF → drop → gc() → realloc → assert zero) passes with and without the fix on macOS. A test that can't fail without the fix isn't a regression guard, it's noise. The fix stands on its own correctness: zero-filling a spec-zero-initialized buffer can never regress a caller, and three sibling allocators already do exactly this.