[array-buffer] Track AB views to avoid protector invalidation [v8/v8 : main]
0 views
Skip to first unread message
Olivier Flückiger (Gerrit)
Dec 22, 2025, 12:26:17 PM12/22/25
to Camillo Bruni, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, AyeAye, Hannes Payer, leszek...@chromium.org, cbruni...@chromium.org, v8-flag...@chromium.org, victorgo...@chromium.org, verwaes...@chromium.org, dmercadi...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni
Olivier Flückiger voted and added 1 comment![]( "Open in Gerrit")
Votes added by Olivier Flückiger
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Camillo Bruni
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camillo Bruni (Gerrit)
Dec 23, 2025, 4:18:28 AM12/23/25
to Olivier Flückiger, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, AyeAye, Hannes Payer, leszek...@chromium.org, cbruni...@chromium.org, v8-flag...@chromium.org, victorgo...@chromium.org, verwaes...@chromium.org, dmercadi...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Olivier Flückiger
Camillo Bruni added 4 comments![]( "Open in Gerrit")
File include/v8-array-buffer.h
Line 355, Patchset 31 (Latest):
void SetDetachKey(v8::Local<v8::String> key);Camillo Bruni . unresolved
technically we should a proper API deprecation for this 😞
File src/diagnostics/objects-debug.cc
Line 2420, Patchset 31 (Latest):
}Camillo Bruni . unresolved
do you plan to allow more values? maybe worth making this stricter here?
File src/objects/js-array-buffer-inl.h
Line 186, Patchset 31 (Latest):
DCHECK(value.IsSmi() || value.IsWeak());Camillo Bruni . unresolved
could this be a stricter check (kNoView, kManyViews)?
File src/runtime/runtime.cc
Line 218, Patchset 31 (Latest):
case Runtime::kArrayBufferDetach:Camillo Bruni . unresolved
nit: I guess we could remove it here?
Related details
Attention is currently required from:
- Olivier Flückiger
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. |
Olivier Flückiger (Gerrit)
Dec 23, 2025, 11:04:15 AM12/23/25
to Camillo Bruni, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, AyeAye, Hannes Payer, leszek...@chromium.org, cbruni...@chromium.org, v8-flag...@chromium.org, victorgo...@chromium.org, verwaes...@chromium.org, dmercadi...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni
Olivier Flückiger added 1 comment![]( "Open in Gerrit")
File include/v8-array-buffer.h
Line 355, Patchset 31 (Latest):
void SetDetachKey(v8::Local<v8::String> key);Camillo Bruni . unresolved
technically we should a proper API deprecation for this 😞
Olivier Flückiger
I know but, I couldn't find any users which would pass anything but Strings. Some wrappers even use String as the type. And the spec says that it is a string...
Related details
Attention is currently required from:
- Camillo Bruni
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. |
Camillo Bruni (Gerrit)
Dec 30, 2025, 11:18:53 AM (10 days ago) 12/30/25
to Olivier Flückiger, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, AyeAye, Hannes Payer, leszek...@chromium.org, cbruni...@chromium.org, v8-flag...@chromium.org, victorgo...@chromium.org, verwaes...@chromium.org, dmercadi...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Olivier Flückiger
Camillo Bruni added 1 comment![]( "Open in Gerrit")
File include/v8-array-buffer.h
Line 355, Patchset 31 (Latest):
void SetDetachKey(v8::Local<v8::String> key);Camillo Bruni . resolved
Olivier Flückigertechnically we should a proper API deprecation for this 😞
I know but, I couldn't find any users which would pass anything but Strings. Some wrappers even use String as the type. And the spec says that it is a string...
Attention is currently required from:
- Olivier Flückiger
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. |
Olivier Flückiger (Gerrit)
Jan 8, 2026, 9:10:55 AM (21 hours ago) Jan 8
to Marja Hölttä, Camillo Bruni, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, AyeAye, Hannes Payer, leszek...@chromium.org, cbruni...@chromium.org, v8-flag...@chromium.org, victorgo...@chromium.org, verwaes...@chromium.org, dmercadi...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni and Marja Hölttä
Olivier Flückiger added 5 comments![]( "Open in Gerrit")
Patchset-level comments
Olivier Flückiger . resolved
ptal
File src/diagnostics/objects-debug.cc
do you plan to allow more values? maybe worth making this stricter here?
Olivier Flückiger
Done
File src/flags/flag-definitions.h
Line 3943, Patchset 35 (Latest):
DEFINE_BOOL(track_array_buffer_views, true,Olivier Flückiger . unresolved
TODO: set to false before landing
File src/objects/js-array-buffer-inl.h
could this be a stricter check (kNoView, kManyViews)?
Olivier Flückiger
Done
File src/runtime/runtime.cc
Line 218, Patchset 31:
case Runtime::kArrayBufferDetach:Camillo Bruni . unresolved
nit: I guess we could remove it here?
Olivier Flückiger
why? I think we want to have it in differential and normal fuzzing...
Related details
Attention is currently required from:
- Camillo Bruni
- Marja Hölttä
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. |
Marja Hölttä (Gerrit)
4:37 AM (2 hours ago) 4:37 AM
to Olivier Flückiger, Camillo Bruni, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, AyeAye, Hannes Payer, leszek...@chromium.org, cbruni...@chromium.org, v8-flag...@chromium.org, victorgo...@chromium.org, verwaes...@chromium.org, dmercadi...@chromium.org, jgrube...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni and Olivier Flückiger
Marja Hölttä added 5 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 36 (Latest):
Marja Hölttä . resolved
i'll have closer look at the logic parts after the new sentinel implementation
File include/v8-array-buffer.h
Line 355, Patchset 36 (Latest):
V8_DEPRECATED("Only Strings are supported as arguments")Marja Hölttä . unresolved
As discussed offline, no need to do this restriction, if we select the sentinels (many views, no views) in a different way.
File src/builtins/builtins-arraybuffer.cc
Line 573, Patchset 36 (Latest):
// only fire the TypedArrayResizing protector.Marja Hölttä . unresolved
sounds like this sentence ends a bit abruptly :) "if we cannot" or such is missing
File test/mjsunit/compiler/typedarray-resizablearraybuffer.js
Line 249, Patchset 36 (Parent):
assertOptimized(Length);Marja Hölttä . unresolved
Why this change? We still kill the protector above...
File test/mjsunit/maglev/typed-array-length-detached-1.js
Line 21, Patchset 36 (Parent):
assertFalse(isMaglevved(foo));Marja Hölttä . unresolved
Why this change?
Related details
Attention is currently required from:
- Camillo Bruni
- Olivier Flückiger
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. |
Reply all
Reply to author
Forward
0 new messages