[immutable-arraybuffer] Implement immutable arraybuffer [v8/v8 : main]

0 views
Skip to first unread message

Olivier Flückiger (Gerrit)

unread,
Dec 8, 2025, 9:08:17 AM12/8/25
to Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
Attention needed from Marja Hölttä

Olivier Flückiger voted and added 1 comment

Votes added by Olivier Flückiger

Auto-Submit+1
Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 7 (Latest):
Olivier Flückiger . resolved

ptal

Open in Gerrit

Related details

Attention is currently required from:
  • Marja Hölttä
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
Gerrit-Change-Number: 7223550
Gerrit-PatchSet: 7
Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
Gerrit-Comment-Date: Mon, 08 Dec 2025 14:08:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
unsatisfied_requirement
open
diffy

Marja Hölttä (Gerrit)

unread,
Dec 9, 2025, 6:44:24 AM12/9/25
to Olivier Flückiger, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
Attention needed from Olivier Flückiger

Marja Hölttä added 15 comments

Patchset-level comments
File-level comment, Patchset 7:
Marja Hölttä . resolved

first round of comments; I didn't yet read all of this

File src/builtins/builtins-array-gen.cc
Line 47, Patchset 7: len(), TypedArrayAccessMode::kWrite);
Marja Hölttä . unresolved

I find it a bit odd that we're passing an accessmode to TypedArraySpeciesCreateByLength. Why... what? We're just creating a TypedArray so we can never fail to write into it? Also what it would mean to do TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kRead) vs TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kWrite)? (Is there a comment about this somewhere?)

Line 96, Patchset 7: // The only way how this can bailout is because of a detached or out of bounds
// buffer.
Marja Hölttä . unresolved

This is out of date now?

File src/builtins/builtins-arraybuffer.cc
Line 261, Patchset 7: size_t current_len = array_buffer->GetByteLength();
Marja Hölttä . unresolved

Are you reloading the length on purpose? (If yes, please add a comment.) We already have it as `len` above. If you reload, we need to think about what happens if the sandbox attacker changes it inbetween...

File src/builtins/builtins-sharedarraybuffer-gen.cc
Line 35, Patchset 7: TypedArrayAccessMode access_mode);
Marja Hölttä . unresolved

this could also have kWrite as a default param like the code before, wdyt? (I don't have a super strong opinion here)

Line 64, Patchset 7: TypedArrayAccessMode access_mode) {
Marja Hölttä . unresolved

ditto

File src/builtins/builtins-typed-array-gen.h
Line 55, Patchset 7: TypedArrayAccessMode access_mode = TypedArrayAccessMode::kRead);
Marja Hölttä . unresolved

ditto

Line 51, Patchset 7: TypedArrayAccessMode access_mode = TypedArrayAccessMode::kRead);
Marja Hölttä . unresolved

Having kRead as a default access mode sounds a bit scary! Then we might just forget to update a call site which actually does writing.. Is this important to have this way?

File src/builtins/builtins-typed-array.cc
Line 240, Patchset 7:BUILTIN(TypedArrayPrototypeIncludes) {
Marja Hölttä . unresolved

Related to the comment about testing (see below), these don't seem to be tested in the test you're adding. Does test262 have tests which ensure that these work for immutable ABs? (Maybe relevant to add them here anyway...)

File src/builtins/typed-array-createtypedarray.tq
Line 485, Patchset 7:transitioning macro TypedArraySpeciesCreateMaybeOOB(
Marja Hölttä . unresolved

Can you add a comment explaining why we can have OOB here? I'm confused!

File test/mjsunit/immutable-arraybuffer.js
Line 7, Patchset 7:const ab = new ArrayBuffer(10);
Marja Hölttä . unresolved

Resizable arraybuffers can also be made immutable, right? Can they be resized after? Could you make all the tests below also test resizable immutable arraybuffers too?

Line 8, Patchset 7:const immutable = ab.transferToImmutable();
Marja Hölttä . unresolved

could you add a test that a SAB cannot be transferToImmutable()d?

Line 47, Patchset 7:const slicedImmutable = immutable.sliceToImmutable(0, 5);
Marja Hölttä . unresolved

What about calling sliceToImmutable on a non-immutable buffer?

Line 79, Patchset 7: ta.set([1]);
Marja Hölttä . unresolved

Could also use
```assertThrows(() => ta.set([1]), TypeError);```

Line 105, Patchset 7:})();
Marja Hölttä . unresolved

Can you also add tests for all the code paths you're modifying, like, the Atomics stuff doesn't seem to be covered here. (Looks like you can still use atomics with a non-shared ArrayBuffer, so immutability is relevant there.)

Open in Gerrit

Related details

Attention is currently required from:
  • Olivier Flückiger
Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
    Gerrit-Change-Number: 7223550
    Gerrit-PatchSet: 7
    Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
    Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Comment-Date: Tue, 09 Dec 2025 11:44:18 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    open
    diffy

    Olivier Flückiger (Gerrit)

    unread,
    Dec 9, 2025, 7:32:21 AM12/9/25
    to Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Marja Hölttä

    Olivier Flückiger voted and added 5 comments

    Votes added by Olivier Flückiger

    Auto-Submit+1

    5 comments

    File src/builtins/builtins-array-gen.cc
    Line 47, Patchset 7: len(), TypedArrayAccessMode::kWrite);
    Marja Hölttä . unresolved

    I find it a bit odd that we're passing an accessmode to TypedArraySpeciesCreateByLength. Why... what? We're just creating a TypedArray so we can never fail to write into it? Also what it would mean to do TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kRead) vs TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kWrite)? (Is there a comment about this somewhere?)

    Olivier Flückiger

    This implementation is following the spec text.

    In particular subarray passes kRead.

    File src/builtins/builtins-sharedarraybuffer-gen.cc
    Line 35, Patchset 7: TypedArrayAccessMode access_mode);
    Marja Hölttä . unresolved

    this could also have kWrite as a default param like the code before, wdyt? (I don't have a super strong opinion here)

    Olivier Flückiger

    Since this is a function from the spec and the spec has kRead as default I would find that confusing. Thus I opted not having a default.

    File src/builtins/builtins-typed-array-gen.h
    Line 51, Patchset 7: TypedArrayAccessMode access_mode = TypedArrayAccessMode::kRead);
    Marja Hölttä . unresolved

    Having kRead as a default access mode sounds a bit scary! Then we might just forget to update a call site which actually does writing.. Is this important to have this way?

    Olivier Flückiger

    It's the default from the spec. But I agree that no default is better.

    File src/builtins/builtins-typed-array.cc
    Line 240, Patchset 7:BUILTIN(TypedArrayPrototypeIncludes) {
    Marja Hölttä . unresolved

    Related to the comment about testing (see below), these don't seem to be tested in the test you're adding. Does test262 have tests which ensure that these work for immutable ABs? (Maybe relevant to add them here anyway...)

    Olivier Flückiger

    I mean I am not really changing anything here, except passing kRead, which ignores immutability.

    Most of this should be covered in test262. But not all the tests are merged upstream.

    File src/builtins/typed-array-createtypedarray.tq
    Line 485, Patchset 7:transitioning macro TypedArraySpeciesCreateMaybeOOB(
    Marja Hölttä . unresolved

    Can you add a comment explaining why we can have OOB here? I'm confused!

    Olivier Flückiger

    This was always the case. I am just changing the name to reflect what it is actually doing. We are calling `ValidateTypedArray` which had a "TODO verify not OOB" in it. I checked all the callsites and they deal with OOB, but still I decided to give it an appropriate name.

    Will add a comment.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Marja Hölttä
    Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
    Gerrit-Change-Number: 7223550
    Gerrit-PatchSet: 9
    Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
    Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
    Gerrit-Comment-Date: Tue, 09 Dec 2025 12:32:16 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Marja Hölttä <ma...@chromium.org>
    unsatisfied_requirement
    open
    diffy

    Olivier Flückiger (Gerrit)

    unread,
    Dec 9, 2025, 7:41:16 AM12/9/25
    to Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Marja Hölttä

    Olivier Flückiger added 1 comment

    File src/builtins/builtins-array-gen.cc
    Line 47, Patchset 7: len(), TypedArrayAccessMode::kWrite);
    Marja Hölttä . unresolved

    I find it a bit odd that we're passing an accessmode to TypedArraySpeciesCreateByLength. Why... what? We're just creating a TypedArray so we can never fail to write into it? Also what it would mean to do TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kRead) vs TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kWrite)? (Is there a comment about this somewhere?)

    Olivier Flückiger

    This implementation is following the spec text.

    In particular subarray passes kRead.

    Olivier Flückiger

    Ah and to your actual question this guards against the constructor being overwritten and returning an immutable array.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Marja Hölttä
    Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
    Gerrit-Change-Number: 7223550
    Gerrit-PatchSet: 9
    Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
    Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
    Gerrit-Comment-Date: Tue, 09 Dec 2025 12:41:10 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Olivier Flückiger <ol...@chromium.org>
    Comment-In-Reply-To: Marja Hölttä <ma...@chromium.org>
    unsatisfied_requirement
    open
    diffy

    Marja Hölttä (Gerrit)

    unread,
    Dec 9, 2025, 8:42:50 AM12/9/25
    to Olivier Flückiger, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Olivier Flückiger

    Marja Hölttä added 2 comments

    File src/builtins/builtins-array-gen.cc
    Line 47, Patchset 7: len(), TypedArrayAccessMode::kWrite);
    Marja Hölttä . unresolved

    I find it a bit odd that we're passing an accessmode to TypedArraySpeciesCreateByLength. Why... what? We're just creating a TypedArray so we can never fail to write into it? Also what it would mean to do TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kRead) vs TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kWrite)? (Is there a comment about this somewhere?)

    Olivier Flückiger

    This implementation is following the spec text.

    In particular subarray passes kRead.

    Olivier Flückiger

    Ah and to your actual question this guards against the constructor being overwritten and returning an immutable array.

    Marja Hölttä

    Ah, could you add a test for that? And a comment

    File test/mjsunit/immutable-arraybuffer.js
    Marja Hölttä . unresolved

    Can you also add tests for all the code paths you're modifying, like, the Atomics stuff doesn't seem to be covered here. (Looks like you can still use atomics with a non-shared ArrayBuffer, so immutability is relevant there.)

    Marja Hölttä

    Ah, I just now noticed there are two test files added, I guess many of the things are in the other file... pls ignore the comments if so :)

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Olivier Flückiger
    Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
    Gerrit-Change-Number: 7223550
    Gerrit-PatchSet: 9
    Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
    Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Comment-Date: Tue, 09 Dec 2025 13:42:44 +0000
    unsatisfied_requirement
    open
    diffy

    Olivier Flückiger (Gerrit)

    unread,
    Dec 9, 2025, 8:54:03 AM12/9/25
    to Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Marja Hölttä

    Olivier Flückiger added 4 comments

    File src/builtins/builtins-array-gen.cc
    Line 96, Patchset 7: // The only way how this can bailout is because of a detached or out of bounds
    // buffer.
    Marja Hölttä . resolved

    This is out of date now?

    Olivier Flückiger

    Done

    File src/builtins/builtins-arraybuffer.cc
    Line 261, Patchset 7: size_t current_len = array_buffer->GetByteLength();
    Marja Hölttä . unresolved

    Are you reloading the length on purpose? (If yes, please add a comment.) We already have it as `len` above. If you reload, we need to think about what happens if the sandbox attacker changes it inbetween...

    Olivier Flückiger

    It can change as a side-effect of getting IntegerValue of start and end. I actually also forgot to re-check if we are detached. Added a scope so we don't accidentally re-use the old len.

    File src/builtins/builtins-sharedarraybuffer-gen.cc
    Line 64, Patchset 7: TypedArrayAccessMode access_mode) {
    Marja Hölttä . resolved

    ditto

    Olivier Flückiger

    Done

    File test/mjsunit/immutable-arraybuffer.js
    Marja Hölttä . unresolved

    Can you also add tests for all the code paths you're modifying, like, the Atomics stuff doesn't seem to be covered here. (Looks like you can still use atomics with a non-shared ArrayBuffer, so immutability is relevant there.)

    Marja Hölttä

    Ah, I just now noticed there are two test files added, I guess many of the things are in the other file... pls ignore the comments if so :)

    Olivier Flückiger

    yeah, let me merge them and make sure your comments are addresses.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Marja Hölttä
    Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
    Gerrit-Change-Number: 7223550
    Gerrit-PatchSet: 9
    Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
    Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
    Gerrit-Comment-Date: Tue, 09 Dec 2025 13:53:58 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Marja Hölttä <ma...@chromium.org>
    unsatisfied_requirement
    open
    diffy

    Olivier Flückiger (Gerrit)

    unread,
    Dec 9, 2025, 10:30:26 AM12/9/25
    to Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Marja Hölttä

    Olivier Flückiger voted and added 11 comments

    Votes added by Olivier Flückiger

    Auto-Submit+1
    Commit-Queue+1

    11 comments

    Patchset-level comments
    File-level comment, Patchset 10 (Latest):
    Olivier Flückiger . resolved

    addressed first round of comments. ptal again

    File src/builtins/builtins-array-gen.cc
    Line 47, Patchset 7: len(), TypedArrayAccessMode::kWrite);
    Marja Hölttä . resolved

    I find it a bit odd that we're passing an accessmode to TypedArraySpeciesCreateByLength. Why... what? We're just creating a TypedArray so we can never fail to write into it? Also what it would mean to do TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kRead) vs TypedArraySpeciesCreateByLength(..., TypedArrayAccessMode::kWrite)? (Is there a comment about this somewhere?)

    Olivier Flückiger

    This implementation is following the spec text.

    In particular subarray passes kRead.

    Olivier Flückiger

    Ah and to your actual question this guards against the constructor being overwritten and returning an immutable array.

    Marja Hölttä

    Ah, could you add a test for that? And a comment

    Olivier Flückiger

    Done

    File src/builtins/builtins-arraybuffer.cc
    Line 261, Patchset 7: size_t current_len = array_buffer->GetByteLength();
    Marja Hölttä . unresolved

    Are you reloading the length on purpose? (If yes, please add a comment.) We already have it as `len` above. If you reload, we need to think about what happens if the sandbox attacker changes it inbetween...

    Olivier Flückiger

    It can change as a side-effect of getting IntegerValue of start and end. I actually also forgot to re-check if we are detached. Added a scope so we don't accidentally re-use the old len.

    Olivier Flückiger

    And I also had to move around some more code since I realized that the spec mandates a different order for the exceptions.

    For mutable arrays we (1) throw if the result buffer is smaller than requested, (2) throw if the source got detached, (3) then copy up to end or the current source size.

    For immutable ones we (1) know the result buffer is big enough, (2) throw if the source got detached, (3) throw if the source shrunk below end.

    Line 261, Patchset 7: size_t current_len = array_buffer->GetByteLength();
    Marja Hölttä . resolved

    Are you reloading the length on purpose? (If yes, please add a comment.) We already have it as `len` above. If you reload, we need to think about what happens if the sandbox attacker changes it inbetween...

    Olivier Flückiger

    It can change as a side-effect of getting IntegerValue of start and end. I actually also forgot to re-check if we are detached. Added a scope so we don't accidentally re-use the old len.

    Olivier Flückiger

    Done

    File src/builtins/builtins-typed-array-gen.h
    Line 55, Patchset 7: TypedArrayAccessMode access_mode = TypedArrayAccessMode::kRead);
    Marja Hölttä . resolved

    ditto

    Olivier Flückiger

    Done

    Line 51, Patchset 7: TypedArrayAccessMode access_mode = TypedArrayAccessMode::kRead);
    Marja Hölttä . resolved

    Having kRead as a default access mode sounds a bit scary! Then we might just forget to update a call site which actually does writing.. Is this important to have this way?

    Olivier Flückiger

    It's the default from the spec. But I agree that no default is better.

    Olivier Flückiger

    Done

    File test/mjsunit/immutable-arraybuffer.js
    Line 7, Patchset 7:const ab = new ArrayBuffer(10);
    Marja Hölttä . resolved

    Resizable arraybuffers can also be made immutable, right? Can they be resized after? Could you make all the tests below also test resizable immutable arraybuffers too?

    Olivier Flückiger

    Done

    Line 8, Patchset 7:const immutable = ab.transferToImmutable();
    Marja Hölttä . resolved

    could you add a test that a SAB cannot be transferToImmutable()d?

    Olivier Flückiger

    Done

    Line 47, Patchset 7:const slicedImmutable = immutable.sliceToImmutable(0, 5);
    Marja Hölttä . resolved

    What about calling sliceToImmutable on a non-immutable buffer?

    Olivier Flückiger

    Done

    Marja Hölttä . resolved

    Could also use
    ```assertThrows(() => ta.set([1]), TypeError);```

    Olivier Flückiger

    Done

    Marja Hölttä . resolved

    Can you also add tests for all the code paths you're modifying, like, the Atomics stuff doesn't seem to be covered here. (Looks like you can still use atomics with a non-shared ArrayBuffer, so immutability is relevant there.)

    Marja Hölttä

    Ah, I just now noticed there are two test files added, I guess many of the things are in the other file... pls ignore the comments if so :)

    Olivier Flückiger

    yeah, let me merge them and make sure your comments are addresses.

    Olivier Flückiger

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Marja Hölttä
    Submit Requirements:
    • requirement is not satisfiedCode-Owners
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: v8/v8
    Gerrit-Branch: main
    Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
    Gerrit-Change-Number: 7223550
    Gerrit-PatchSet: 10
    Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
    Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
    Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
    Gerrit-Comment-Date: Tue, 09 Dec 2025 15:30:20 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    unsatisfied_requirement
    open
    diffy

    Marja Hölttä (Gerrit)

    unread,
    Dec 10, 2025, 5:07:30 AM12/10/25
    to Olivier Flückiger, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
    Attention needed from Olivier Flückiger

    Marja Hölttä voted and added 7 comments

    Votes added by Marja Hölttä

    Code-Review+1

    7 comments

    File src/builtins/builtins-arraybuffer.cc
    Line 261, Patchset 7: size_t current_len = array_buffer->GetByteLength();
    Marja Hölttä . unresolved

    Are you reloading the length on purpose? (If yes, please add a comment.) We already have it as `len` above. If you reload, we need to think about what happens if the sandbox attacker changes it inbetween...

    Olivier Flückiger

    It can change as a side-effect of getting IntegerValue of start and end. I actually also forgot to re-check if we are detached. Added a scope so we don't accidentally re-use the old len.

    Olivier Flückiger

    And I also had to move around some more code since I realized that the spec mandates a different order for the exceptions.

    For mutable arrays we (1) throw if the result buffer is smaller than requested, (2) throw if the source got detached, (3) then copy up to end or the current source size.

    For immutable ones we (1) know the result buffer is big enough, (2) throw if the source got detached, (3) throw if the source shrunk below end.

    Marja Hölttä

    SGTM; can you add a test that would fail if you didn't have the correct order of exceptions?

    Line 398, Patchset 12 (Latest): DCHECK_GE(new_array_buffer->GetByteLength(), new_len_size);
    Marja Hölttä . unresolved

    Is this legit in the sandbox sense? GetByteLength will read data from the sandbox so it can be whatever

    Line 402, Patchset 12 (Latest): DCHECK_IMPLIES(to_immutable, new_len_size <= from_byte_length - first_size);
    Marja Hölttä . unresolved

    ditto

    File src/builtins/builtins-sharedarraybuffer-gen.cc
    Line 35, Patchset 7: TypedArrayAccessMode access_mode);
    Marja Hölttä . resolved

    this could also have kWrite as a default param like the code before, wdyt? (I don't have a super strong opinion here)

    Olivier Flückiger

    Since this is a function from the spec and the spec has kRead as default I would find that confusing. Thus I opted not having a default.

    Marja Hölttä

    Acknowledged

    It's actually funny how for the spec the "safe" option is the other one :) We don't want to have a spec bug where default is kWrite but some trivial place forgets to pass kRead, but here we don't want to have bugs the other way around.

    File src/builtins/builtins-typed-array.cc
    Line 240, Patchset 7:BUILTIN(TypedArrayPrototypeIncludes) {
    Marja Hölttä . unresolved

    Related to the comment about testing (see below), these don't seem to be tested in the test you're adding. Does test262 have tests which ensure that these work for immutable ABs? (Maybe relevant to add them here anyway...)

    Olivier Flückiger

    I mean I am not really changing anything here, except passing kRead, which ignores immutability.

    Most of this should be covered in test262. But not all the tests are merged upstream.

    Marja Hölttä

    It'd be quite easy to have a bug where you're using a function which has a kWrite default, and then forget to pass kRead. We'd only notice that if we test all the 30 or so TypedArray funcs for immutable buffers :/

    File src/builtins/typed-array-createtypedarray.tq
    Line 485, Patchset 7:transitioning macro TypedArraySpeciesCreateMaybeOOB(
    Marja Hölttä . resolved

    Can you add a comment explaining why we can have OOB here? I'm confused!

    Olivier Flückiger

    This was always the case. I am just changing the name to reflect what it is actually doing. We are calling `ValidateTypedArray` which had a "TODO verify not OOB" in it. I checked all the callsites and they deal with OOB, but still I decided to give it an appropriate name.

    Will add a comment.

    Marja Hölttä

    Acknowledged

    File test/mjsunit/immutable-arraybuffer.js
    Line 69, Patchset 12 (Latest):TestImmutableBuffer(() => new ArrayBuffer(10), "Fixed-length ArrayBuffer");
    TestImmutableBuffer(() => new ArrayBuffer(10, {maxByteLength: 20}), "Resizable ArrayBuffer");
    Marja Hölttä . unresolved

    The same tests?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Olivier Flückiger
    Submit Requirements:
      • requirement is not satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: v8/v8
      Gerrit-Branch: main
      Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
      Gerrit-Change-Number: 7223550
      Gerrit-PatchSet: 12
      Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
      Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
      Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
      Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
      Gerrit-Comment-Date: Wed, 10 Dec 2025 10:07:24 +0000
      unsatisfied_requirement
      satisfied_requirement
      open
      diffy

      Olivier Flückiger (Gerrit)

      unread,
      Dec 10, 2025, 8:37:28 AM12/10/25
      to Darius Mercadier, Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
      Attention needed from Darius Mercadier

      Olivier Flückiger voted and added 6 comments

      Votes added by Olivier Flückiger

      Auto-Submit+1

      6 comments

      Patchset-level comments
      File-level comment, Patchset 12:
      Olivier Flückiger . resolved

      +darius ptal at compiler/*

      File src/builtins/builtins-arraybuffer.cc
      Line 261, Patchset 7: size_t current_len = array_buffer->GetByteLength();
      Marja Hölttä . resolved

      Are you reloading the length on purpose? (If yes, please add a comment.) We already have it as `len` above. If you reload, we need to think about what happens if the sandbox attacker changes it inbetween...

      Olivier Flückiger

      It can change as a side-effect of getting IntegerValue of start and end. I actually also forgot to re-check if we are detached. Added a scope so we don't accidentally re-use the old len.

      Olivier Flückiger

      And I also had to move around some more code since I realized that the spec mandates a different order for the exceptions.

      For mutable arrays we (1) throw if the result buffer is smaller than requested, (2) throw if the source got detached, (3) then copy up to end or the current source size.

      For immutable ones we (1) know the result buffer is big enough, (2) throw if the source got detached, (3) throw if the source shrunk below end.

      Marja Hölttä

      SGTM; can you add a test that would fail if you didn't have the correct order of exceptions?

      Olivier Flückiger

      Done

      Line 398, Patchset 12: DCHECK_GE(new_array_buffer->GetByteLength(), new_len_size);
      Marja Hölttä . resolved

      Is this legit in the sandbox sense? GetByteLength will read data from the sandbox so it can be whatever

      Olivier Flückiger

      The authorative thing is new_len_size, which at most is as big as a ReadBoundedSizeField. So we won't write more than that into the target buffer.

      The dcheck is just here to ensure we cover all the cases above to catch cases where the buffer would legitimately too small (i.e., without sandbox corruption).

      Line 402, Patchset 12: DCHECK_IMPLIES(to_immutable, new_len_size <= from_byte_length - first_size);
      Marja Hölttä . resolved

      ditto

      Olivier Flückiger

      same

      File src/builtins/builtins-typed-array.cc
      Line 240, Patchset 7:BUILTIN(TypedArrayPrototypeIncludes) {
      Marja Hölttä . resolved

      Related to the comment about testing (see below), these don't seem to be tested in the test you're adding. Does test262 have tests which ensure that these work for immutable ABs? (Maybe relevant to add them here anyway...)

      Olivier Flückiger

      I mean I am not really changing anything here, except passing kRead, which ignores immutability.

      Most of this should be covered in test262. But not all the tests are merged upstream.

      Marja Hölttä

      It'd be quite easy to have a bug where you're using a function which has a kWrite default, and then forget to pass kRead. We'd only notice that if we test all the 30 or so TypedArray funcs for immutable buffers :/

      Olivier Flückiger

      Done

      File test/mjsunit/immutable-arraybuffer.js
      Line 69, Patchset 12:TestImmutableBuffer(() => new ArrayBuffer(10), "Fixed-length ArrayBuffer");

      TestImmutableBuffer(() => new ArrayBuffer(10, {maxByteLength: 20}), "Resizable ArrayBuffer");
      Marja Hölttä . resolved

      The same tests?

      Olivier Flückiger

      Done

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Darius Mercadier
      Submit Requirements:
        • requirement is not satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: v8/v8
        Gerrit-Branch: main
        Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
        Gerrit-Change-Number: 7223550
        Gerrit-PatchSet: 12
        Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
        Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
        Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
        Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
        Gerrit-Attention: Darius Mercadier <dmerc...@chromium.org>
        Gerrit-Comment-Date: Wed, 10 Dec 2025 13:37:23 +0000
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy

        Marja Hölttä (Gerrit)

        unread,
        Dec 10, 2025, 8:39:00 AM12/10/25
        to Olivier Flückiger, Darius Mercadier, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
        Attention needed from Darius Mercadier

        Marja Hölttä voted and added 1 comment

        Votes added by Marja Hölttä

        Code-Review+1

        1 comment

        File src/builtins/builtins-arraybuffer.cc
        Line 351, Patchset 12: THROW_NEW_ERROR_RETURN_FAILURE(
        Marja Hölttä . unresolved

        Do you have a test which goes here?

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Darius Mercadier
        Submit Requirements:
          • requirement is not satisfiedCode-Owners
          • requirement satisfiedCode-Review
          • requirement is not satisfiedNo-Unresolved-Comments
          • requirement satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: v8/v8
          Gerrit-Branch: main
          Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
          Gerrit-Change-Number: 7223550
          Gerrit-PatchSet: 12
          Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
          Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
          Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Attention: Darius Mercadier <dmerc...@chromium.org>
          Gerrit-Comment-Date: Wed, 10 Dec 2025 13:38:54 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: Yes
          unsatisfied_requirement
          satisfied_requirement
          open
          diffy

          Darius Mercadier (Gerrit)

          unread,
          Dec 10, 2025, 8:45:54 AM12/10/25
          to Olivier Flückiger, Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
          Attention needed from Olivier Flückiger

          Darius Mercadier voted and added 2 comments

          Votes added by Darius Mercadier

          Code-Review+1

          2 comments

          Patchset-level comments
          File-level comment, Patchset 13 (Latest):
          Darius Mercadier . resolved

          src/compiler mostly LGTM

          File src/compiler/js-native-context-specialization.cc
          Line 3444, Patchset 13 (Latest): if (!keyed_mode.IsStore() || !v8_flags.js_immutable_arraybuffer ||
          dependencies()->DependOnArrayBufferDetachingProtector()) {
          Darius Mercadier . unresolved

          When this test doesn't path, we'll just go into the regular non-ArrayBuffer path further down. Are you sure that this is correct for ArrayBuffers?

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Olivier Flückiger
          Submit Requirements:
          • requirement satisfiedCode-Owners
          • requirement satisfiedCode-Review
          • requirement is not satisfiedNo-Unresolved-Comments
          • requirement satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: v8/v8
          Gerrit-Branch: main
          Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
          Gerrit-Change-Number: 7223550
          Gerrit-PatchSet: 13
          Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
          Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
          Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Comment-Date: Wed, 10 Dec 2025 13:45:49 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: Yes
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          Olivier Flückiger (Gerrit)

          unread,
          Dec 10, 2025, 9:29:55 AM12/10/25
          to Darius Mercadier, Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
          Attention needed from Darius Mercadier and Marja Hölttä

          Olivier Flückiger voted and added 3 comments

          Votes added by Olivier Flückiger

          Auto-Submit+1
          Commit-Queue+1

          3 comments

          Patchset-level comments
          File-level comment, Patchset 15 (Latest):
          Olivier Flückiger . resolved

          darius, ptal again

          File src/builtins/builtins-arraybuffer.cc
          Line 351, Patchset 12: THROW_NEW_ERROR_RETURN_FAILURE(
          Marja Hölttä . resolved

          Do you have a test which goes here?

          Olivier Flückiger

          yes:
          ```
          99: // Test TypedArray.prototype.map throws TypeError if species creates immutable buffer
          ...
          ```

          File src/compiler/js-native-context-specialization.cc
          Line 3444, Patchset 13: if (!keyed_mode.IsStore() || !v8_flags.js_immutable_arraybuffer ||
          dependencies()->DependOnArrayBufferDetachingProtector()) {
          Darius Mercadier . resolved

          When this test doesn't path, we'll just go into the regular non-ArrayBuffer path further down. Are you sure that this is correct for ArrayBuffers?

          Olivier Flückiger

          aha, good catch. indeed the code below would be wrong for TA.

          turns out the easiest fix is to just implement the check in this case.

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Darius Mercadier
          • Marja Hölttä
          Submit Requirements:
          • requirement is not satisfiedCode-Owners
          • requirement is not satisfiedCode-Review
          • requirement is not satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: v8/v8
          Gerrit-Branch: main
          Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
          Gerrit-Change-Number: 7223550
          Gerrit-PatchSet: 15
          Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
          Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
          Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
          Gerrit-Attention: Darius Mercadier <dmerc...@chromium.org>
          Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
          Gerrit-Comment-Date: Wed, 10 Dec 2025 14:29:50 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: Yes
          Comment-In-Reply-To: Darius Mercadier <dmerc...@chromium.org>
          Comment-In-Reply-To: Marja Hölttä <ma...@chromium.org>
          unsatisfied_requirement
          open
          diffy

          Darius Mercadier (Gerrit)

          unread,
          Dec 10, 2025, 9:34:59 AM12/10/25
          to Olivier Flückiger, Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
          Attention needed from Marja Hölttä and Olivier Flückiger

          Darius Mercadier voted and added 1 comment

          Votes added by Darius Mercadier

          Code-Review+1
          Commit-Queue+2

          1 comment

          Patchset-level comments
          Darius Mercadier . resolved

          LGTM thanks :)

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Marja Hölttä
          • Olivier Flückiger
          Submit Requirements:
            • requirement satisfiedCode-Owners
            • requirement satisfiedCode-Review
            • requirement satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 15
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
            Gerrit-Comment-Date: Wed, 10 Dec 2025 14:34:54 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes
            satisfied_requirement
            open
            diffy

            Olivier Flückiger (Gerrit)

            unread,
            Dec 10, 2025, 9:39:18 AM12/10/25
            to Darius Mercadier, Marja Hölttä, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
            Attention needed from Marja Hölttä

            Olivier Flückiger voted Commit-Queue+0

            Commit-Queue+0
            Open in Gerrit

            Related details

            Attention is currently required from:
            • Marja Hölttä
            Submit Requirements:
            • requirement satisfiedCode-Owners
            • requirement satisfiedCode-Review
            • requirement satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 15
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Attention: Marja Hölttä <ma...@chromium.org>
            Gerrit-Comment-Date: Wed, 10 Dec 2025 14:39:13 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: Yes
            satisfied_requirement
            open
            diffy

            Marja Hölttä (Gerrit)

            unread,
            Dec 10, 2025, 10:07:21 AM12/10/25
            to Olivier Flückiger, Darius Mercadier, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
            Attention needed from Olivier Flückiger

            Marja Hölttä voted Code-Review+1

            Code-Review+1
            Open in Gerrit

            Related details

            Attention is currently required from:
            • Olivier Flückiger
            Submit Requirements:
            • requirement satisfiedCode-Owners
            • requirement satisfiedCode-Review
            • requirement satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 15
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Comment-Date: Wed, 10 Dec 2025 15:07:16 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: Yes
            satisfied_requirement
            open
            diffy

            Marja Hölttä (Gerrit)

            unread,
            Dec 10, 2025, 10:07:41 AM12/10/25
            to Olivier Flückiger, Darius Mercadier, AyeAye, V8 LUCI CQ, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com
            Attention needed from Olivier Flückiger

            Marja Hölttä added 1 comment

            Patchset-level comments
            Marja Hölttä . resolved

            (re-added my LGTM since it got lost)

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Olivier Flückiger
            Submit Requirements:
            • requirement satisfiedCode-Owners
            • requirement satisfiedCode-Review
            • requirement satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 15
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Comment-Date: Wed, 10 Dec 2025 15:07:36 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            satisfied_requirement
            open
            diffy

            V8 LUCI CQ (Gerrit)

            unread,
            Dec 10, 2025, 10:21:39 AM12/10/25
            to Olivier Flückiger, Marja Hölttä, Darius Mercadier, AyeAye, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com

            V8 LUCI CQ submitted the change

            Change information

            Commit message:
            [immutable-arraybuffer] Implement immutable arraybuffer

            Implements the TC39 immutable array buffer proposal
            https://github.com/tc39/proposal-immutable-arraybuffer.

            This CL mostly disables typed array support in the optimizing
            compilers. This part still needs to be done.

            Drive-By:
            * Cleanup error messages for detached vs. out-of-bounds buffers
            * Cleanup naming and implementation around ValidateTypedArray.
            Bug: 450237486
            Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Reviewed-by: Darius Mercadier <dmerc...@chromium.org>
            Reviewed-by: Marja Hölttä <ma...@chromium.org>
            Commit-Queue: Darius Mercadier <dmerc...@chromium.org>
            Auto-Submit: Olivier Flückiger <ol...@chromium.org>
            Cr-Commit-Position: refs/heads/main@{#104246}
            Files:
            • M src/builtins/array-join.tq
            • M src/builtins/arraybuffer.tq
            • M src/builtins/base.tq
            • M src/builtins/builtins-array-gen.cc
            • M src/builtins/builtins-arraybuffer.cc
            • M src/builtins/builtins-dataview.cc
            • M src/builtins/builtins-definitions.h
            • M src/builtins/builtins-sharedarraybuffer-gen.cc
            • M src/builtins/builtins-sharedarraybuffer.cc
            • M src/builtins/builtins-typed-array-gen.cc
            • M src/builtins/builtins-typed-array-gen.h
            • M src/builtins/builtins-typed-array.cc
            • M src/builtins/builtins.cc
            • M src/builtins/data-view.tq
            • M src/builtins/typed-array-at.tq
            • M src/builtins/typed-array-createtypedarray.tq
            • M src/builtins/typed-array-entries.tq
            • M src/builtins/typed-array-every.tq
            • M src/builtins/typed-array-filter.tq
            • M src/builtins/typed-array-find.tq
            • M src/builtins/typed-array-findindex.tq
            • M src/builtins/typed-array-findlast.tq
            • M src/builtins/typed-array-findlastindex.tq
            • M src/builtins/typed-array-foreach.tq
            • M src/builtins/typed-array-from.tq
            • M src/builtins/typed-array-keys.tq
            • M src/builtins/typed-array-of.tq
            • M src/builtins/typed-array-reduce.tq
            • M src/builtins/typed-array-reduceright.tq
            • M src/builtins/typed-array-set.tq
            • M src/builtins/typed-array-slice.tq
            • M src/builtins/typed-array-some.tq
            • M src/builtins/typed-array-sort.tq
            • M src/builtins/typed-array-subarray.tq
            • M src/builtins/typed-array-to-reversed.tq
            • M src/builtins/typed-array-to-sorted.tq
            • M src/builtins/typed-array-values.tq
            • M src/builtins/typed-array-with.tq
            • M src/builtins/typed-array.tq
            • M src/builtins/wasm.tq
            • M src/codegen/code-stub-assembler.cc
            • M src/codegen/code-stub-assembler.h
            • M src/common/globals.h
            • M src/common/message-template.h
            • M src/compiler/js-call-reducer.cc
            • M src/compiler/js-native-context-specialization.cc
            • M src/deoptimizer/deoptimize-reason.h
            • M src/flags/flag-definitions.h
            • M src/ic/accessor-assembler.cc
            • M src/init/bootstrapper.cc
            • M src/init/heap-symbols.h
            • M src/maglev/maglev-graph-builder.cc
            • M src/objects/elements.cc
            • M src/objects/js-array-buffer-inl.h
            • M src/objects/js-array-buffer.cc
            • M src/objects/js-array-buffer.h
            • M src/objects/js-array-buffer.tq
            • M src/roots/static-roots-intl-nowasm.h
            • M src/roots/static-roots-intl-wasm.h
            • M src/roots/static-roots-nointl-nowasm.h
            • M src/roots/static-roots-nointl-wasm.h
            • M test/mjsunit/compiler/typedarray-resizablearraybuffer.js
            • M test/mjsunit/es6/array-iterator-detached.js
            • A test/mjsunit/immutable-arraybuffer.js
            • M test/mjsunit/messages.js
            • M test/test262/test262.status
            • M test/test262/testcfg.py
            • M test/unittests/interpreter/bytecode_expectations/PrivateAccessorAccess.golden
            • M test/unittests/interpreter/bytecode_expectations/PrivateMethodAccess.golden
            • M test/unittests/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden
            Change size: XL
            Delta: 70 files changed, 2448 insertions(+), 1639 deletions(-)
            Branch: refs/heads/main
            Submit Requirements:
            • requirement satisfiedCode-Review: +1 by Darius Mercadier, +1 by Marja Hölttä
            Open in Gerrit
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: merged
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 16
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            open
            diffy
            satisfied_requirement

            chromeperf@appspot.gserviceaccount.com (Gerrit)

            unread,
            Dec 10, 2025, 11:02:57 AM12/10/25
            to Olivier Flückiger, V8 LUCI CQ, Marja Hölttä, Darius Mercadier, AyeAye, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com

            Message from chrom...@appspot.gserviceaccount.com

            📍 Job mac-m1_mini_2020-perf/speedometer-main.crossbench complete.

            See results at: https://pinpoint-dot-chromeperf.appspot.com/job/15fab000b10000

            Open in Gerrit

            Related details

            Attention set is empty
            Submit Requirements:
            • requirement satisfiedCode-Owners
            • requirement satisfiedCode-Review
            • requirement satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 16
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Comment-Date: Wed, 10 Dec 2025 16:02:50 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: No
            satisfied_requirement
            open
            diffy

            chromeperf@appspot.gserviceaccount.com (Gerrit)

            unread,
            Dec 10, 2025, 11:07:04 AM12/10/25
            to Olivier Flückiger, V8 LUCI CQ, Marja Hölttä, Darius Mercadier, AyeAye, verwaes...@chromium.org, dmercadi...@chromium.org, victorgo...@chromium.org, leszek...@chromium.org, v8-flag...@chromium.org, v8-re...@googlegroups.com

            Message from chrom...@appspot.gserviceaccount.com

            📍 Job mac-m1_mini_2020-perf/jetstream-main.crossbench complete.

            See results at: https://pinpoint-dot-chromeperf.appspot.com/job/133a8347310000

            Open in Gerrit

            Related details

            Attention set is empty
            Submit Requirements:
            • requirement satisfiedCode-Owners
            • requirement satisfiedCode-Review
            • requirement satisfiedReview-Enforcement
            Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
            Gerrit-MessageType: comment
            Gerrit-Project: v8/v8
            Gerrit-Branch: main
            Gerrit-Change-Id: I40444163c3b8fbd72e937dc27668a4c6675d7ff1
            Gerrit-Change-Number: 7223550
            Gerrit-PatchSet: 16
            Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Reviewer: Darius Mercadier <dmerc...@chromium.org>
            Gerrit-Reviewer: Marja Hölttä <ma...@chromium.org>
            Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>
            Gerrit-Comment-Date: Wed, 10 Dec 2025 16:07:00 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: No
            satisfied_requirement
            open
            diffy
            Reply all
            Reply to author
            Forward
            0 new messages