Fix memmove sizes in array shift, unshift and splice - #325
Merged
Merged
Conversation
… corresponding tests
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.
Problem
Two bugs in the element moves of the array mutation lowerings (
LowerToLLVM.cpp):MemoryMoveOpLoweringscaled the count by a pointer's size. It multiplied the count by the size of the source operand's type, but the operands are plain pointers. For elements wider than 8 bytes,shift,unshiftandsplicemoved only part of each element:unshifton such an array segfaulted, even under-mm=gc.ArrayUnshiftOpLoweringmoved the new element count instead of the old one, writingitemselements past the end of the reallocated block. Under-mm=gcand-mm=nonenothing noticed. Under-mm=rcthe debug CRT heap caught it on free, which is whytest-jit-rc-owned-array-opsandtest-compile-rc-corpus-00owned-array-opsfail on debug builds (CI builds Release, where the heap doesn't check).Fix
MemoryMoveOp's count is now in bytes, likeMemoryCopyOp's.unshiftandshift, plus both branches ofsplice.unshiftmoves the old count.Tests
00array_move_wide_elements.ts(compile, jit, rc/none corpus):shift,unshift, andsplicein both directions (growing and shrinking) on an array of tuples. It fails on the previous compiler withassertion failed: shift second.00owned_array_opspasses undergc,rcandnone.gc-shared-auto,gc-defaultlib-collector×2) are an unrelated CRT report-hook breakpoint and a default-library CRT flag, fixed separately.🤖 Generated with Claude Code