Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

Should length-tracking TAs be considered OOB when O.[[ByteOffset]] == bufferByteLength? #68

Closed
marjakh opened this issue Jul 29, 2021 · 2 comments · Fixed by #70
Closed

Comments

@marjakh
Copy link

marjakh commented Jul 29, 2021

When O.[[ArrayLength]] is auto, and O.[[ByteOffset]] is exactly bufferByteLength, IsIntegerIndexedObjectOutOfBounds returns true. This matters because many functions don't throw for zero-length TAs but do throw for out-of-bounds TAs. Also, byteOffset returns 0 for out of bounds TAs.

Example:

let ab = new ArrayBuffer(10, {maxByteLength: 20});
let a = new Uint8Array(ab, 8);
ab.resize(8); 

-> a is now out of bounds!

I thought, maybe the rationale is that "the buffer is so small that no element will fit", but that doesn't generalize. If the TA has element size > 1, then bufferByteLength == O.[[ByteOffset]] + 1 is not out of bounds but still no element will fit.

This also creates a surprising situation where resizing to 0 makes all length-tracking TAs with offset 0 go out of bounds. I would expect them to be zero-length but not OOB at that point.

My mental model is: if it's a length-tracking TA, the only way to make it go out of bounds is to make the buffer length less than the offset. Length-tracking zero-offset TA should always track the full buffer and never go out of bounds.

Suggestion: change the condition in IsIntegerIndexedOutOfBounds step 7

from
If byteOffsetStart ≥ bufferByteLength or byteOffsetEnd > bufferByteLength, then return true.
to
If byteOffsetStart > bufferByteLength or byteOffsetEnd > bufferByteLength, then return true.

test262 would also need to be updated because they assert this.

@marjakh
Copy link
Author

marjakh commented Jul 29, 2021

cc @jugglinmike

pull bot pushed a commit to Mu-L/v8 that referenced this issue Jul 29, 2021
This CL assumes tc39/proposal-resizablearraybuffer#68
is indeed a spec bug.

Bug: v8:11111
Change-Id: I8d24f0d07f7ab40ba01b8c422868ad189d6f7e5a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3060478
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76001}
@syg
Copy link
Collaborator

syg commented Aug 2, 2021

Ah, interesting edge-case discrepancy.

This also creates a surprising situation where resizing to 0 makes all length-tracking TAs with offset 0 go out of bounds. I would expect them to be zero-length but not OOB at that point.

I find this pretty line of reasoning pretty compelling. Your fix seems right to me, I'll open a PR and update plenary on this change at the next meeting.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants