An invalid array length or a failed allocation stops the program (#483) - #489
Merged
Merged
Conversation
Array sizes were never checked. `a.length = -1` made the length a huge unsigned size, `a.length = 2^42` asked the allocator for a block it could not give, and growth zeroed the new slots through the null it got back: an access violation in every model since #482, and before it, a length ignored or stored with no block behind it. TypeScript throws a RangeError for a length that is not an integer in [0, 2^32 - 1]. tslang now stops the program instead, as a failing assert does (the message, after the output printed before it, with the file and line ahead of time): - `length =` with a number checks it is >= 0, <= 2^32 - 1 and survives the round trip through an index (no fraction, not NaN), in MLIRGen, before the conversion drops what would show it; - growth (ArrayLayout::ensureCapacity: push, unshift, splice, a longer `length =`) checks the length it needs is at most 2^32 - 1 - which a negative integer, sign-extended, is not - that the byte count does not overflow, and that the allocator gave a block ("Out of memory"); - a new array of a length (`new A(n)` for `type A = number[]`) checks the same. "Invalid array length" is the message for a length, as TypeScript's RangeError says; "Out of memory" for a failed allocation. Neither is an exception: try/catch does not see them. AssertLogic's failure path is shared by `assert` and the new mid-lowering `check`, and it depends on LLVMCodeHelperBase only, so ArrayLayout can use it. Closes #483 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Fixes #483 with the design chosen there: an invalid array length or a failed allocation stops the program, as a failing assert does - not an exception.
The bug
Array sizes were never checked.
a.length = -1made the length a huge unsigned size;a.length = 2^42asked the allocator for a block it could not give, and growth zeroed the new slots through the null it got back - an access violation under every model since #482 (before it: the length ignored under gc, or stored with no block behind it).The fix
TypeScript throws a
RangeErrorfor a length that is not an integer in [0, 2^32 - 1]. tslang now stops withInvalid array length(andOut of memoryfor a failed allocation), after flushing what the program printed, with the file and line ahead of time - the same mechanism as a failingassert. try/catch does not see it.length =with a number (MLIRGen): checked to be >= 0, <= 2^32 - 1 and to survive the round trip through an index (no fraction, not NaN), before the conversion to an index drops what would show it.ArrayLayout::ensureCapacity:push,unshift,splice, a longerlength =): the needed length is at most 2^32 - 1 (a negative integer, sign-extended, is not), the byte count does not overflow, and the allocator gave a block.new A(n)fortype A = number[]): the same checks.AssertLogic's failure path is now shared byassertand a new mid-loweringcheck, and depends onLLVMCodeHelperBaseonly, soArrayLayoutcan use it.assertlowers as before.Tests
array-length/{negative, huge, fraction, negative_integer, new_negative}.ts, each under gc, rc, none and own (20 tests): the output must showprinted before, thenassertion failed: Invalid array length, and never thelength:line after it.00array_length_valid.ts: lengths from a number, shrinking, growth, a new array of a length still work.Out of memorypath has no test: a valid length just under the limit asks for about 32 GB, which under rc was granted (Windows commits lazily) and under gc ran past 30 seconds.Gates
ctest -C Release -j 12 --timeout 300, staged default library)ctest -j 8 --timeout 300)weakref_basic, gc-only by design (#420)Closes #483
🤖 Generated with Claude Code