Skip to content

Commit b66ae39

Browse files
authored
deps: V8: backport 8ca9f77d0f7c
This fixes a crash when loading snapshots that contain empty ArrayBuffer instances: ```js const X = []; X.push(new ArrayBuffer()); v8.startupSnapshot.setDeserializeMainFunction(() => { for (let i = 0; i < 1000000; i++) { // trigger GC X.push(new ArrayBuffer()); } }) ``` Original commit message: [sandbox] Sandboxify JSArrayBuffer::extension external pointer Bug: chromium:1335043 Change-Id: Id8e6883fc652b144f6380ff09b1c18e777e041c2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3706626 Reviewed-by: Michael Lippautz <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Commit-Queue: Samuel Groß <[email protected]> Cr-Commit-Position: refs/heads/main@{#84544} Refs: v8/v8@8ca9f77 PR-URL: #45871 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 5124613 commit b66ae39

File tree

11 files changed

+71
-63
lines changed

11 files changed

+71
-63
lines changed

common.gypi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.6',
39+
'v8_embedder_string': '-node.7',
4040

4141
##### V8 defaults for Node.js #####
4242

deps/v8/include/v8-internal.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,8 @@ constexpr uint64_t kAllExternalPointerTypeTags[] = {
398398
V(kWasmInternalFunctionCallTargetTag, sandboxed, TAG(17)) \
399399
V(kWasmTypeInfoNativeTypeTag, sandboxed, TAG(18)) \
400400
V(kWasmExportedFunctionDataSignatureTag, sandboxed, TAG(19)) \
401-
V(kWasmContinuationJmpbufTag, sandboxed, TAG(20))
401+
V(kWasmContinuationJmpbufTag, sandboxed, TAG(20)) \
402+
V(kArrayBufferExtensionTag, sandboxed, TAG(21))
402403

403404
// All external pointer tags.
404405
#define ALL_EXTERNAL_POINTER_TAGS(V) \

deps/v8/src/builtins/builtins-typed-array-gen.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,15 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
7070
UintPtrConstant(0));
7171
StoreSandboxedPointerToObject(buffer, JSArrayBuffer::kBackingStoreOffset,
7272
EmptyBackingStoreBufferConstant());
73+
#ifdef V8_COMPRESS_POINTERS
74+
// When pointer compression is enabled, the extension slot contains a
75+
// (lazily-initialized) external pointer handle.
76+
StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
77+
ExternalPointerHandleNullConstant());
78+
#else
7379
StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
7480
IntPtrConstant(0));
81+
#endif
7582
for (int offset = JSArrayBuffer::kHeaderSize;
7683
offset < JSArrayBuffer::kSizeWithEmbedderFields; offset += kTaggedSize) {
7784
// TODO(v8:10391, saelo): Handle external pointers in EmbedderDataSlot

deps/v8/src/compiler/code-assembler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,9 @@ class V8_EXPORT_PRIVATE CodeAssembler {
553553
TNode<BoolT> BoolConstant(bool value) {
554554
return value ? Int32TrueConstant() : Int32FalseConstant();
555555
}
556+
TNode<ExternalPointerHandleT> ExternalPointerHandleNullConstant() {
557+
return ReinterpretCast<ExternalPointerHandleT>(Uint32Constant(0));
558+
}
556559

557560
bool IsMapOffsetConstant(Node* node);
558561

deps/v8/src/objects/js-array-buffer-inl.h

Lines changed: 42 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -88,65 +88,60 @@ void JSArrayBuffer::SetBackingStoreRefForSerialization(uint32_t ref) {
8888

8989
ArrayBufferExtension* JSArrayBuffer::extension() const {
9090
#if V8_COMPRESS_POINTERS
91-
// With pointer compression the extension-field might not be
92-
// pointer-aligned. However on ARM64 this field needs to be aligned to
93-
// perform atomic operations on it. Therefore we split the pointer into two
94-
// 32-bit words that we update atomically. We don't have an ABA problem here
95-
// since there can never be an Attach() after Detach() (transitions only
96-
// from NULL --> some ptr --> NULL).
97-
98-
// Synchronize with publishing release store of non-null extension
99-
uint32_t lo = base::AsAtomic32::Acquire_Load(extension_lo());
100-
if (lo & kUninitializedTagMask) return nullptr;
101-
102-
// Synchronize with release store of null extension
103-
uint32_t hi = base::AsAtomic32::Acquire_Load(extension_hi());
104-
uint32_t verify_lo = base::AsAtomic32::Relaxed_Load(extension_lo());
105-
if (lo != verify_lo) return nullptr;
106-
107-
uintptr_t address = static_cast<uintptr_t>(lo);
108-
address |= static_cast<uintptr_t>(hi) << 32;
109-
return reinterpret_cast<ArrayBufferExtension*>(address);
91+
// We need Acquire semantics here when loading the entry, see below.
92+
// Consider adding respective external pointer accessors if non-relaxed
93+
// ordering semantics are ever needed in other places as well.
94+
Isolate* isolate = GetIsolateFromWritableObject(*this);
95+
ExternalPointerHandle handle =
96+
base::AsAtomic32::Acquire_Load(extension_handle_location());
97+
return reinterpret_cast<ArrayBufferExtension*>(
98+
isolate->external_pointer_table().Get(handle, kArrayBufferExtensionTag));
11099
#else
111-
return base::AsAtomicPointer::Acquire_Load(extension_location());
112-
#endif
100+
return base::AsAtomicPointer::Acquire_Load(extension_location());
101+
#endif // V8_COMPRESS_POINTERS
113102
}
114103

115104
void JSArrayBuffer::set_extension(ArrayBufferExtension* extension) {
116105
#if V8_COMPRESS_POINTERS
117-
if (extension != nullptr) {
118-
uintptr_t address = reinterpret_cast<uintptr_t>(extension);
119-
base::AsAtomic32::Relaxed_Store(extension_hi(),
120-
static_cast<uint32_t>(address >> 32));
121-
base::AsAtomic32::Release_Store(extension_lo(),
122-
static_cast<uint32_t>(address));
123-
} else {
124-
base::AsAtomic32::Relaxed_Store(extension_lo(),
125-
0 | kUninitializedTagMask);
126-
base::AsAtomic32::Release_Store(extension_hi(), 0);
127-
}
106+
if (extension != nullptr) {
107+
Isolate* isolate = GetIsolateFromWritableObject(*this);
108+
ExternalPointerTable& table = isolate->external_pointer_table();
109+
110+
// The external pointer handle for the extension is initialized lazily and
111+
// so has to be zero here since, once set, the extension field can only be
112+
// cleared, but not changed.
113+
DCHECK_EQ(0, base::AsAtomic32::Relaxed_Load(extension_handle_location()));
114+
115+
// We need Release semantics here, see above.
116+
ExternalPointerHandle handle = table.AllocateAndInitializeEntry(
117+
isolate, reinterpret_cast<Address>(extension),
118+
kArrayBufferExtensionTag);
119+
base::AsAtomic32::Release_Store(extension_handle_location(), handle);
120+
} else {
121+
// This special handling of nullptr is required as it is used to initialize
122+
// the slot, but is also beneficial when an ArrayBuffer is detached as it
123+
// allows the external pointer table entry to be reclaimed while the
124+
// ArrayBuffer is still alive.
125+
base::AsAtomic32::Release_Store(extension_handle_location(),
126+
kNullExternalPointerHandle);
127+
}
128128
#else
129-
base::AsAtomicPointer::Release_Store(extension_location(), extension);
130-
#endif
131-
WriteBarrier::Marking(*this, extension);
132-
}
133-
134-
ArrayBufferExtension** JSArrayBuffer::extension_location() const {
135-
Address location = field_address(kExtensionOffset);
136-
return reinterpret_cast<ArrayBufferExtension**>(location);
129+
base::AsAtomicPointer::Release_Store(extension_location(), extension);
130+
#endif // V8_COMPRESS_POINTERS
131+
WriteBarrier::Marking(*this, extension);
137132
}
138133

139134
#if V8_COMPRESS_POINTERS
140-
uint32_t* JSArrayBuffer::extension_lo() const {
135+
ExternalPointerHandle* JSArrayBuffer::extension_handle_location() const {
141136
Address location = field_address(kExtensionOffset);
142-
return reinterpret_cast<uint32_t*>(location);
137+
return reinterpret_cast<ExternalPointerHandle*>(location);
143138
}
144-
145-
uint32_t* JSArrayBuffer::extension_hi() const {
146-
Address location = field_address(kExtensionOffset) + sizeof(uint32_t);
147-
return reinterpret_cast<uint32_t*>(location);
139+
#else
140+
ArrayBufferExtension** JSArrayBuffer::extension_location() const {
141+
Address location = field_address(kExtensionOffset);
142+
return reinterpret_cast<ArrayBufferExtension**>(location);
148143
}
149-
#endif
144+
#endif // V8_COMPRESS_POINTERS
150145

151146
void JSArrayBuffer::clear_padding() {
152147
if (FIELD_SIZE(kOptionalPaddingOffset) != 0) {

deps/v8/src/objects/js-array-buffer.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,14 +168,15 @@ class JSArrayBuffer
168168
private:
169169
void DetachInternal(bool force_for_wasm_memory, Isolate* isolate);
170170

171-
inline ArrayBufferExtension** extension_location() const;
172-
173171
#if V8_COMPRESS_POINTERS
174-
static const int kUninitializedTagMask = 1;
175-
176-
inline uint32_t* extension_lo() const;
177-
inline uint32_t* extension_hi() const;
178-
#endif
172+
// When pointer compression is enabled, the pointer to the extension is
173+
// stored in the external pointer table and the object itself only contains a
174+
// 32-bit external pointer handles. This simplifies alignment requirements
175+
// and is also necessary for the sandbox.
176+
inline ExternalPointerHandle* extension_handle_location() const;
177+
#else
178+
inline ArrayBufferExtension** extension_location() const;
179+
#endif // V8_COMPRESS_POINTERS
179180

180181
TQ_OBJECT_CONSTRUCTORS(JSArrayBuffer)
181182
};

deps/v8/src/objects/js-array-buffer.tq

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ extern class JSArrayBuffer extends JSObjectWithEmbedderSlots {
1919
raw_max_byte_length: uintptr;
2020
// A SandboxedPtr if the sandbox is enabled
2121
backing_store: RawPtr;
22-
extension: RawPtr;
22+
extension: ExternalPointer;
2323
bit_field: JSArrayBufferFlags;
2424
// Pads header size to be a multiple of kTaggedSize.
2525
@if(TAGGED_SIZE_8_BYTES) optional_padding: uint32;

deps/v8/src/objects/objects-body-descriptors-inl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,8 @@ class JSArrayBuffer::BodyDescriptor final : public BodyDescriptorBase {
387387
// JSArrayBuffer instances contain raw data that the GC does not know about.
388388
IteratePointers(obj, kPropertiesOrHashOffset, kEndOfTaggedFieldsOffset, v);
389389
IterateJSObjectBodyImpl(map, obj, kHeaderSize, object_size, v);
390+
v->VisitExternalPointer(map, obj.RawExternalPointerField(kExtensionOffset),
391+
kArrayBufferExtensionTag);
390392
}
391393

392394
static inline int SizeOf(Map map, HeapObject object) {

deps/v8/src/sandbox/external-pointer-table.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
9898
// returning the previous value. The same tag is applied both to decode the
9999
// previous value and encode the given value.
100100
//
101-
// This method is atomic and can call be called from background threads.
101+
// This method is atomic and can be called from background threads.
102102
inline Address Exchange(ExternalPointerHandle handle, Address value,
103103
ExternalPointerTag tag);
104104

deps/v8/src/snapshot/deserializer.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@ void Deserializer<Isolate>::PostProcessNewJSReceiver(Map map,
399399
auto buffer = JSArrayBuffer::cast(*obj);
400400
uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
401401
if (store_index == kEmptyBackingStoreRefSentinel) {
402+
buffer.set_extension(nullptr);
402403
buffer.set_backing_store(main_thread_isolate(),
403404
EmptyBackingStoreBuffer());
404405
} else {

deps/v8/test/cctest/heap/test-array-buffer-tracker.cc

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ TEST(ArrayBuffer_NonLivePromotion) {
218218
v8::Isolate* isolate = env->GetIsolate();
219219
Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
220220

221-
JSArrayBuffer raw_ab;
222221
{
223222
v8::HandleScope handle_scope(isolate);
224223
Handle<FixedArray> root =
@@ -235,13 +234,12 @@ TEST(ArrayBuffer_NonLivePromotion) {
235234
CHECK(IsTracked(heap, JSArrayBuffer::cast(root->get(0))));
236235
heap::GcAndSweep(heap, NEW_SPACE);
237236
CHECK(IsTracked(heap, JSArrayBuffer::cast(root->get(0))));
238-
raw_ab = JSArrayBuffer::cast(root->get(0));
237+
ArrayBufferExtension* extension =
238+
JSArrayBuffer::cast(root->get(0)).extension();
239239
root->set(0, ReadOnlyRoots(heap).undefined_value());
240240
heap::SimulateIncrementalMarking(heap, true);
241-
// Prohibit page from being released.
242-
Page::FromHeapObject(raw_ab)->MarkNeverEvacuate();
243241
heap::GcAndSweep(heap, OLD_SPACE);
244-
CHECK(!IsTracked(heap, raw_ab));
242+
CHECK(!IsTracked(heap, extension));
245243
}
246244
}
247245

0 commit comments

Comments
 (0)