Skip to content

Commit 7bdad94

Browse files
addaleaxRafaelGSS
authored andcommitted
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 4454f5f commit 7bdad94

11 files changed

+71
-63
lines changed

common.gypi

+1-1
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.10',
39+
'v8_embedder_string': '-node.11',
4040

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

deps/v8/include/v8-internal.h

+2-1
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

+7
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,15 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
6868
UintPtrConstant(0));
6969
StoreSandboxedPointerToObject(buffer, JSArrayBuffer::kBackingStoreOffset,
7070
EmptyBackingStoreBufferConstant());
71+
#ifdef V8_COMPRESS_POINTERS
72+
// When pointer compression is enabled, the extension slot contains a
73+
// (lazily-initialized) external pointer handle.
74+
StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
75+
ExternalPointerHandleNullConstant());
76+
#else
7177
StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
7278
IntPtrConstant(0));
79+
#endif
7380
for (int offset = JSArrayBuffer::kHeaderSize;
7481
offset < JSArrayBuffer::kSizeWithEmbedderFields; offset += kTaggedSize) {
7582
// TODO(v8:10391, saelo): Handle external pointers in EmbedderDataSlot

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

+3
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,9 @@ class V8_EXPORT_PRIVATE CodeAssembler {
552552
TNode<BoolT> BoolConstant(bool value) {
553553
return value ? Int32TrueConstant() : Int32FalseConstant();
554554
}
555+
TNode<ExternalPointerHandleT> ExternalPointerHandleNullConstant() {
556+
return ReinterpretCast<ExternalPointerHandleT>(Uint32Constant(0));
557+
}
555558

556559
bool IsMapOffsetConstant(Node* node);
557560

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

+42-47
Original file line numberDiff line numberDiff line change
@@ -79,65 +79,60 @@ void JSArrayBuffer::SetBackingStoreRefForSerialization(uint32_t ref) {
7979

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

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

130125
#if V8_COMPRESS_POINTERS
131-
uint32_t* JSArrayBuffer::extension_lo() const {
126+
ExternalPointerHandle* JSArrayBuffer::extension_handle_location() const {
132127
Address location = field_address(kExtensionOffset);
133-
return reinterpret_cast<uint32_t*>(location);
128+
return reinterpret_cast<ExternalPointerHandle*>(location);
134129
}
135-
136-
uint32_t* JSArrayBuffer::extension_hi() const {
137-
Address location = field_address(kExtensionOffset) + sizeof(uint32_t);
138-
return reinterpret_cast<uint32_t*>(location);
130+
#else
131+
ArrayBufferExtension** JSArrayBuffer::extension_location() const {
132+
Address location = field_address(kExtensionOffset);
133+
return reinterpret_cast<ArrayBufferExtension**>(location);
139134
}
140-
#endif
135+
#endif // V8_COMPRESS_POINTERS
141136

142137
void JSArrayBuffer::clear_padding() {
143138
if (FIELD_SIZE(kOptionalPaddingOffset) != 0) {

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

+8-7
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,15 @@ class JSArrayBuffer
160160
class BodyDescriptor;
161161

162162
private:
163-
inline ArrayBufferExtension** extension_location() const;
164-
165163
#if V8_COMPRESS_POINTERS
166-
static const int kUninitializedTagMask = 1;
167-
168-
inline uint32_t* extension_lo() const;
169-
inline uint32_t* extension_hi() const;
170-
#endif
164+
// When pointer compression is enabled, the pointer to the extension is
165+
// stored in the external pointer table and the object itself only contains a
166+
// 32-bit external pointer handles. This simplifies alignment requirements
167+
// and is also necessary for the sandbox.
168+
inline ExternalPointerHandle* extension_handle_location() const;
169+
#else
170+
inline ArrayBufferExtension** extension_location() const;
171+
#endif // V8_COMPRESS_POINTERS
171172

172173
TQ_OBJECT_CONSTRUCTORS(JSArrayBuffer)
173174
};

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

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

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

+2
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

+1-1
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

+1
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ void Deserializer<IsolateT>::PostProcessNewJSReceiver(
397397
auto buffer = JSArrayBuffer::cast(*obj);
398398
uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
399399
if (store_index == kEmptyBackingStoreRefSentinel) {
400+
buffer.set_extension(nullptr);
400401
buffer.set_backing_store(main_thread_isolate(),
401402
EmptyBackingStoreBuffer());
402403
} else {

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

+3-5
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)