[debug] Convert usages of FixedArray::length to ulength [v8/v8 : main]
0 views
Skip to first unread message
Arash Kazemi (Gerrit)
Feb 6, 2026, 9:40:18 AMFeb 6
to Omer Katz, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Omer Katz
Arash Kazemi voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Omer Katz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 6, 2026, 3:01:21 PMFeb 6
to Arash Kazemi, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi
Omer Katz voted and added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Arash Kazemi
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 9, 2026, 3:50:34 PMFeb 9
to Arash Kazemi, Jakob Linke, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi and Jakob Linke
Omer Katz added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Arash Kazemi
- Jakob Linke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Linke (Gerrit)
Feb 10, 2026, 2:50:07 AMFeb 10
to Arash Kazemi, Omer Katz, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi
Jakob Linke voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jakob Linke
| Code-Review | +1 |
3 comments
File src/debug/debug-interface.cc
Line 276, Patchset 7 (Latest):
uint32_t keys_len = keys->ulength().value();Jakob Linke . unresolved
q: `SafeHeapObjectSize` is just for porting and ulength will return a plain uint32_t once done?
Line 695, Patchset 7 (Latest):
int lines_end_len = static_cast<int>(line_ends->ulength().value());Jakob Linke . unresolved
Why not uint32_t here too? And addtl a `DCHECK_GT(lines_end_len, 0)` for the subtractions below.
File src/debug/debug-wasm-objects.cc
Line 436, Patchset 7 (Latest):
return values->ulength().value() - 2;Jakob Linke . unresolved
DCHECK_GE(.., 2)
Related details
Attention is currently required from:
- Arash Kazemi
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 10, 2026, 4:06:55 AMFeb 10
to Jakob Linke, Omer Katz, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Jakob Linke
Arash Kazemi added 3 comments![]( "Open in Gerrit")
File src/debug/debug-interface.cc
Line 276, Patchset 7:
uint32_t keys_len = keys->ulength().value();Jakob Linke . unresolved
q: `SafeHeapObjectSize` is just for porting and ulength will return a plain uint32_t once done?
Arash Kazemi
The idea would be to keep `SafeHeapObjectSize` (renamed to something more generic) at least until implicit casts warnings can be enabled on the codebase. This way all callsites have to be more explicit when handling lengths by calling `.value()`. Of course, you could also end up writing `int something = length().value()` but it should hopefully be less likely.
Line 695, Patchset 7:
int lines_end_len = static_cast<int>(line_ends->ulength().value());Jakob Linke . resolved
Why not uint32_t here too? And addtl a `DCHECK_GT(lines_end_len, 0)` for the subtractions below.
Arash Kazemi
Done
File src/debug/debug-wasm-objects.cc
Line 436, Patchset 7:
return values->ulength().value() - 2;Jakob Linke . resolved
Arash KazemiDCHECK_GE(.., 2)
Done
Related details
Attention is currently required from:
- Jakob Linke
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Linke (Gerrit)
Feb 10, 2026, 5:59:04 AMFeb 10
to Arash Kazemi, Omer Katz, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi
Jakob Linke voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jakob Linke
| Code-Review | +1 |
3 comments
Patchset-level comments
File src/debug/debug-interface.cc
Line 276, Patchset 7:
uint32_t keys_len = keys->ulength().value();Jakob Linke . resolved
Arash Kazemiq: `SafeHeapObjectSize` is just for porting and ulength will return a plain uint32_t once done?
The idea would be to keep `SafeHeapObjectSize` (renamed to something more generic) at least until implicit casts warnings can be enabled on the codebase. This way all callsites have to be more explicit when handling lengths by calling `.value()`. Of course, you could also end up writing `int something = length().value()` but it should hopefully be less likely.
Jakob Linke
I see.
Of course, you could also end up writing int something = length().value() but it should hopefully be less likely.
Yeah exactly; I hope we're going in the right direction here wrt preventing mistakes vs. readable/simple code. Personally I'd much rather work with plain uint32_t.
Line 698, Patchset 9 (Latest):
return Just(GetSmiValue(line_ends, lines_end_len - 1));Jakob Linke . unresolved
nit: There's one more here.
Related details
Attention is currently required from:
- Arash Kazemi
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 10, 2026, 6:51:05 AMFeb 10
to Jakob Linke, Omer Katz, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Jakob Linke
Arash Kazemi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Arash Kazemi
| Commit-Queue | +1 |
1 comment
File src/debug/debug-interface.cc
nit: There's one more here.
Attention is currently required from:
- Jakob Linke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 10, 2026, 8:05:31 AMFeb 10
to Arash Kazemi, Jakob Linke, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi and Jakob Linke
Omer Katz voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Arash Kazemi
- Jakob Linke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 11, 2026, 7:33:49 AMFeb 11
to Omer Katz, Jakob Linke, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Jakob Linke
Arash Kazemi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Arash Kazemi
Attention is currently required from:
- Jakob Linke
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Linke (Gerrit)
Feb 12, 2026, 1:24:52 AMFeb 12
to Arash Kazemi, Omer Katz, V8 LUCI CQ, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi
Jakob Linke voted![]( "Open in Gerrit")
Attention is currently required from:
- Arash Kazemi
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Feb 12, 2026, 1:27:04 AMFeb 12
to Arash Kazemi, Jakob Linke, Omer Katz, devtools-...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[debug] Convert usages of FixedArray::length to ulength
Improve robustness in the sandbox attacker model by using unsigned
integer length for FixedArray (see: crrev.com/c/7544875). Additionally,
BreakPointInfo::GetBreakPointCount and DebugInfo::GetBreakPointCount now
return uint32_t.
Bug: 375937549
Change-Id: I8d70ac38a3c9d99e29361d907643f61d3843bdab
Reviewed-by: Omer Katz <omer...@chromium.org>
Auto-Submit: Arash Kazemi <ara...@chromium.org>
Reviewed-by: Jakob Linke <jgr...@chromium.org>
Commit-Queue: Jakob Linke <jgr...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105216}
Files:
- M src/debug/debug-evaluate.cc
- M src/debug/debug-interface.cc
- M src/debug/debug-property-iterator.cc
- M src/debug/debug-scopes.cc
- M src/debug/debug-wasm-objects.cc
- M src/debug/debug.cc
- M src/debug/liveedit.cc
- M src/objects/debug-objects.cc
- M src/objects/debug-objects.h
Change size: M
Delta: 9 files changed, 75 insertions(+), 47 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Jakob Linke, +1 by Omer Katz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 16, 2026, 12:26:05 PMFeb 16
to Omer Katz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Omer Katz
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Omer Katz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 16, 2026, 2:55:10 PMFeb 16
to Arash Kazemi, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi
Omer Katz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Omer Katz
| Code-Review | +1 |
2 comments
Patchset-level comments
File src/objects/source-text-module.cc
Line 145, Patchset 2 (Latest):
for (uint32_t i = 0; i < names_len; ++i) {Omer Katz . unresolved
`names_len` can be `const`, as well the other places where we read the length into a variable.
I didn't think about this in the previous CLs, so let's leave them as is. No need to go back and update all locations. Let's just add `const` from now on.
Related details
Attention is currently required from:
- Arash Kazemi
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 17, 2026, 3:59:46 AMFeb 17
to Michael Lippautz, Omer Katz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz and Omer Katz
Arash Kazemi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Arash Kazemi
| Commit-Queue | +1 |
1 comment
File src/objects/source-text-module.cc
`names_len` can be `const`, as well the other places where we read the length into a variable.
I didn't think about this in the previous CLs, so let's leave them as is. No need to go back and update all locations. Let's just add `const` from now on.
Arash Kazemi
Good point, I have added const in this CL and will try to address the rest in follow-up CLs.
Related details
Attention is currently required from:
- Michael Lippautz
- Omer Katz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 17, 2026, 4:35:07 AMFeb 17
to Arash Kazemi, Michael Lippautz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi and Michael Lippautz
Omer Katz voted and added 16 comments![]( "Open in Gerrit")
Votes added by Omer Katz
| Code-Review | +1 |
16 comments
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Omer Katz . resolved
lgtm % highlighted some more places that can be const
File src/builtins/accessors.cc
File src/diagnostics/objects-debug.cc
Line 1130, Patchset 3 (Latest):
uint32_t index = TransitionArray::kProtoTransitionHeaderSize + i;Omer Katz . unresolved
const
File src/objects/js-function.cc
Line 452, Patchset 3 (Latest):
uint32_t nof_bound_arguments = function->bound_arguments()->ulength().value();Omer Katz . unresolved
const
Line 459, Patchset 3 (Latest):
uint32_t length = function->bound_arguments()->ulength().value();Omer Katz . unresolved
const
Line 482, Patchset 3 (Latest):
uint32_t target_length = target->length();Omer Katz . unresolved
const
File src/objects/js-objects.cc
Line 5478, Patchset 3 (Latest):
uint32_t capacity = elements()->ulength().value();Omer Katz . unresolved
const
Line 5660, Patchset 3 (Latest):
uint32_t c = object->elements()->ulength().value();Omer Katz . unresolved
const
File src/objects/source-text-module.cc
Line 145, Patchset 2:
for (uint32_t i = 0; i < names_len; ++i) {Omer Katz . resolved
Arash Kazemi`names_len` can be `const`, as well the other places where we read the length into a variable.
I didn't think about this in the previous CLs, so let's leave them as is. No need to go back and update all locations. Let's just add `const` from now on.
Good point, I have added const in this CL and will try to address the rest in follow-up CLs.
Omer Katz
I don't think we need to go back and update all the other locations from the previous CLs. I assume we'll add const to them whenever we touch them again.
File src/objects/transitions-inl.h
Line 126, Patchset 3 (Latest):
int32_t transitions = raw.ToSmi().value();Omer Katz . unresolved
const
File src/objects/transitions.cc
Line 398, Patchset 3 (Latest):
uint32_t grow_by = new_capacity - capacity;Omer Katz . unresolved
const
File src/runtime/runtime-wasm.cc
Line 1734, Patchset 3 (Latest):
uint32_t length = wasm_function->length();Omer Katz . unresolved
const
Related details
Attention is currently required from:
- Arash Kazemi
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 17, 2026, 4:43:23 AMFeb 17
to Arash Kazemi, Michael Lippautz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi and Michael Lippautz
Omer Katz added 2 comments![]( "Open in Gerrit")
File src/objects/js-objects.cc
Line 4747, Patchset 3 (Latest):
uint32_t length = IsJSArray(object)Omer Katz . resolved
Omer Katzconst
on second thought, this one is pointless since you're returning immediately after.
File src/objects/transitions-inl.h
Line 126, Patchset 3 (Latest):
int32_t transitions = raw.ToSmi().value();Omer Katz . resolved
Omer Katzconst
on second thought, this one is pointless since you're returning immediately after.
Omer Katz (Gerrit)
Feb 17, 2026, 5:00:47 AMFeb 17
to Arash Kazemi, Michael Lippautz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi and Michael Lippautz
Omer Katz voted and added 2 comments![]( "Open in Gerrit")
Votes added by Omer Katz
| Code-Review | +1 |
2 comments
Patchset-level comments
Related details
Attention is currently required from:
- Arash Kazemi
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 17, 2026, 5:08:12 AMFeb 17
to Omer Katz, Michael Lippautz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Michael Lippautz and Omer Katz
Arash Kazemi added 12 comments![]( "Open in Gerrit")
File src/builtins/accessors.cc
Line 350, Patchset 3:
uint32_t length = function->length();Omer Katz . resolved
const
Arash Kazemi
Done
File src/diagnostics/objects-debug.cc
Line 1130, Patchset 3:
uint32_t index = TransitionArray::kProtoTransitionHeaderSize + i;Omer Katz . resolved
const
Arash Kazemi
Done
File src/objects/js-function.cc
Line 452, Patchset 3:
uint32_t nof_bound_arguments = function->bound_arguments()->ulength().value();Omer Katz . unresolved
const
Arash Kazemi
This is modified inside the while loop.
Line 459, Patchset 3:
uint32_t length = function->bound_arguments()->ulength().value();Omer Katz . resolved
const
Arash Kazemi
Done
Line 482, Patchset 3:
uint32_t target_length = target->length();Omer Katz . resolved
const
Arash Kazemi
Done
File src/objects/js-objects.cc
Line 4469, Patchset 3:
uint32_t length = IsJSArray(*object)Omer Katz . resolved
const
Arash Kazemi
Done
Line 4758, Patchset 3:
uint32_t length = IsJSArray(object)Omer Katz . resolved
const
Arash Kazemi
Done
Line 4768, Patchset 3:
uint32_t length = IsJSArray(object)Omer Katz . resolved
const
Arash Kazemi
Done
Line 5478, Patchset 3:
uint32_t capacity = elements()->ulength().value();Omer Katz . resolved
const
Arash Kazemi
Done
Line 5660, Patchset 3:
uint32_t c = object->elements()->ulength().value();Omer Katz . resolved
const
Arash Kazemi
Done
File src/objects/transitions.cc
Line 398, Patchset 3:
uint32_t grow_by = new_capacity - capacity;Omer Katz . resolved
const
Arash Kazemi
Done
File src/runtime/runtime-wasm.cc
Line 1734, Patchset 3:
uint32_t length = wasm_function->length();Omer Katz . resolved
const
Attention is currently required from:
- Michael Lippautz
- Omer Katz
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omer Katz (Gerrit)
Feb 17, 2026, 5:30:44 AMFeb 17
to Arash Kazemi, Michael Lippautz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi and Michael Lippautz
Omer Katz voted and added 1 comment![]( "Open in Gerrit")
Votes added by Omer Katz
| Code-Review | +1 |
1 comment
File src/objects/js-function.cc
Line 452, Patchset 3:
uint32_t nof_bound_arguments = function->bound_arguments()->ulength().value();Omer Katz . resolved
Arash Kazemiconst
This is modified inside the while loop.
Attention is currently required from:
- Arash Kazemi
- Michael Lippautz
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Lippautz (Gerrit)
Feb 17, 2026, 1:43:57 PMFeb 17
to Arash Kazemi, Omer Katz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Attention needed from Arash Kazemi
Michael Lippautz voted and added 1 comment![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Arash Kazemi
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arash Kazemi (Gerrit)
Feb 17, 2026, 2:00:52 PMFeb 17
to Michael Lippautz, Omer Katz, V8 LUCI CQ, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
Arash Kazemi voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Feb 18, 2026, 2:40:00 AMFeb 18
to Arash Kazemi, Michael Lippautz, Omer Katz, AyeAye, devtools-...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[objects] Convert usages of FixedArray::length to ulength
Improve robustness in the sandbox attacker model by using unsigned
integer length for FixedArray (see: crrev.com/c/7544875).
Bug: 375937549
Change-Id: Id65adb8b8a6b8756e89397d2f3711ceec2e1dbcb
Reviewed-by: Michael Lippautz <mlip...@chromium.org>
Commit-Queue: Arash Kazemi <ara...@chromium.org>
Reviewed-by: Omer Katz <omer...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105305}
Files:
- M src/builtins/accessors.cc
- M src/debug/liveedit.cc
- M src/diagnostics/objects-debug.cc
- M src/diagnostics/objects-printer.cc
- M src/init/bootstrapper.cc
- M src/objects/js-function-inl.h
- M src/objects/js-function.cc
- M src/objects/js-function.h
- M src/objects/js-objects.cc
- M src/objects/script.cc
- M src/objects/shared-function-info.cc
- M src/objects/shared-function-info.h
- M src/objects/source-text-module.cc
- M src/objects/transitions-inl.h
- M src/objects/transitions.cc
- M src/objects/transitions.h
- M src/runtime/runtime-wasm.cc
Change size: L
Delta: 17 files changed, 181 insertions(+), 135 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Michael Lippautz, +1 by Omer Katz
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages