Skip to content

Commit

Permalink
deps: V8: backport 8ca9f77d0f7c
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
addaleax authored and RafaelGSS committed Jan 5, 2023
1 parent 4454f5f commit 7bdad94
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 63 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.10',
'v8_embedder_string': '-node.11',

##### V8 defaults for Node.js #####

Expand Down
3 changes: 2 additions & 1 deletion deps/v8/include/v8-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,8 @@ constexpr uint64_t kAllExternalPointerTypeTags[] = {
V(kWasmInternalFunctionCallTargetTag, sandboxed, TAG(17)) \
V(kWasmTypeInfoNativeTypeTag, sandboxed, TAG(18)) \
V(kWasmExportedFunctionDataSignatureTag, sandboxed, TAG(19)) \
V(kWasmContinuationJmpbufTag, sandboxed, TAG(20))
V(kWasmContinuationJmpbufTag, sandboxed, TAG(20)) \
V(kArrayBufferExtensionTag, sandboxed, TAG(21))

// All external pointer tags.
#define ALL_EXTERNAL_POINTER_TAGS(V) \
Expand Down
7 changes: 7 additions & 0 deletions deps/v8/src/builtins/builtins-typed-array-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
UintPtrConstant(0));
StoreSandboxedPointerToObject(buffer, JSArrayBuffer::kBackingStoreOffset,
EmptyBackingStoreBufferConstant());
#ifdef V8_COMPRESS_POINTERS
// When pointer compression is enabled, the extension slot contains a
// (lazily-initialized) external pointer handle.
StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
ExternalPointerHandleNullConstant());
#else
StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
IntPtrConstant(0));
#endif
for (int offset = JSArrayBuffer::kHeaderSize;
offset < JSArrayBuffer::kSizeWithEmbedderFields; offset += kTaggedSize) {
// TODO(v8:10391, saelo): Handle external pointers in EmbedderDataSlot
Expand Down
3 changes: 3 additions & 0 deletions deps/v8/src/compiler/code-assembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,9 @@ class V8_EXPORT_PRIVATE CodeAssembler {
TNode<BoolT> BoolConstant(bool value) {
return value ? Int32TrueConstant() : Int32FalseConstant();
}
TNode<ExternalPointerHandleT> ExternalPointerHandleNullConstant() {
return ReinterpretCast<ExternalPointerHandleT>(Uint32Constant(0));
}

bool IsMapOffsetConstant(Node* node);

Expand Down
89 changes: 42 additions & 47 deletions deps/v8/src/objects/js-array-buffer-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,65 +79,60 @@ void JSArrayBuffer::SetBackingStoreRefForSerialization(uint32_t ref) {

ArrayBufferExtension* JSArrayBuffer::extension() const {
#if V8_COMPRESS_POINTERS
// With pointer compression the extension-field might not be
// pointer-aligned. However on ARM64 this field needs to be aligned to
// perform atomic operations on it. Therefore we split the pointer into two
// 32-bit words that we update atomically. We don't have an ABA problem here
// since there can never be an Attach() after Detach() (transitions only
// from NULL --> some ptr --> NULL).

// Synchronize with publishing release store of non-null extension
uint32_t lo = base::AsAtomic32::Acquire_Load(extension_lo());
if (lo & kUninitializedTagMask) return nullptr;

// Synchronize with release store of null extension
uint32_t hi = base::AsAtomic32::Acquire_Load(extension_hi());
uint32_t verify_lo = base::AsAtomic32::Relaxed_Load(extension_lo());
if (lo != verify_lo) return nullptr;

uintptr_t address = static_cast<uintptr_t>(lo);
address |= static_cast<uintptr_t>(hi) << 32;
return reinterpret_cast<ArrayBufferExtension*>(address);
// We need Acquire semantics here when loading the entry, see below.
// Consider adding respective external pointer accessors if non-relaxed
// ordering semantics are ever needed in other places as well.
Isolate* isolate = GetIsolateFromWritableObject(*this);
ExternalPointerHandle handle =
base::AsAtomic32::Acquire_Load(extension_handle_location());
return reinterpret_cast<ArrayBufferExtension*>(
isolate->external_pointer_table().Get(handle, kArrayBufferExtensionTag));
#else
return base::AsAtomicPointer::Acquire_Load(extension_location());
#endif
return base::AsAtomicPointer::Acquire_Load(extension_location());
#endif // V8_COMPRESS_POINTERS
}

void JSArrayBuffer::set_extension(ArrayBufferExtension* extension) {
#if V8_COMPRESS_POINTERS
if (extension != nullptr) {
uintptr_t address = reinterpret_cast<uintptr_t>(extension);
base::AsAtomic32::Relaxed_Store(extension_hi(),
static_cast<uint32_t>(address >> 32));
base::AsAtomic32::Release_Store(extension_lo(),
static_cast<uint32_t>(address));
} else {
base::AsAtomic32::Relaxed_Store(extension_lo(),
0 | kUninitializedTagMask);
base::AsAtomic32::Release_Store(extension_hi(), 0);
}
if (extension != nullptr) {
Isolate* isolate = GetIsolateFromWritableObject(*this);
ExternalPointerTable& table = isolate->external_pointer_table();

// The external pointer handle for the extension is initialized lazily and
// so has to be zero here since, once set, the extension field can only be
// cleared, but not changed.
DCHECK_EQ(0, base::AsAtomic32::Relaxed_Load(extension_handle_location()));

// We need Release semantics here, see above.
ExternalPointerHandle handle = table.AllocateAndInitializeEntry(
isolate, reinterpret_cast<Address>(extension),
kArrayBufferExtensionTag);
base::AsAtomic32::Release_Store(extension_handle_location(), handle);
} else {
// This special handling of nullptr is required as it is used to initialize
// the slot, but is also beneficial when an ArrayBuffer is detached as it
// allows the external pointer table entry to be reclaimed while the
// ArrayBuffer is still alive.
base::AsAtomic32::Release_Store(extension_handle_location(),
kNullExternalPointerHandle);
}
#else
base::AsAtomicPointer::Release_Store(extension_location(), extension);
#endif
WriteBarrier::Marking(*this, extension);
}

ArrayBufferExtension** JSArrayBuffer::extension_location() const {
Address location = field_address(kExtensionOffset);
return reinterpret_cast<ArrayBufferExtension**>(location);
base::AsAtomicPointer::Release_Store(extension_location(), extension);
#endif // V8_COMPRESS_POINTERS
WriteBarrier::Marking(*this, extension);
}

#if V8_COMPRESS_POINTERS
uint32_t* JSArrayBuffer::extension_lo() const {
ExternalPointerHandle* JSArrayBuffer::extension_handle_location() const {
Address location = field_address(kExtensionOffset);
return reinterpret_cast<uint32_t*>(location);
return reinterpret_cast<ExternalPointerHandle*>(location);
}

uint32_t* JSArrayBuffer::extension_hi() const {
Address location = field_address(kExtensionOffset) + sizeof(uint32_t);
return reinterpret_cast<uint32_t*>(location);
#else
ArrayBufferExtension** JSArrayBuffer::extension_location() const {
Address location = field_address(kExtensionOffset);
return reinterpret_cast<ArrayBufferExtension**>(location);
}
#endif
#endif // V8_COMPRESS_POINTERS

void JSArrayBuffer::clear_padding() {
if (FIELD_SIZE(kOptionalPaddingOffset) != 0) {
Expand Down
15 changes: 8 additions & 7 deletions deps/v8/src/objects/js-array-buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,15 @@ class JSArrayBuffer
class BodyDescriptor;

private:
inline ArrayBufferExtension** extension_location() const;

#if V8_COMPRESS_POINTERS
static const int kUninitializedTagMask = 1;

inline uint32_t* extension_lo() const;
inline uint32_t* extension_hi() const;
#endif
// When pointer compression is enabled, the pointer to the extension is
// stored in the external pointer table and the object itself only contains a
// 32-bit external pointer handles. This simplifies alignment requirements
// and is also necessary for the sandbox.
inline ExternalPointerHandle* extension_handle_location() const;
#else
inline ArrayBufferExtension** extension_location() const;
#endif // V8_COMPRESS_POINTERS

TQ_OBJECT_CONSTRUCTORS(JSArrayBuffer)
};
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/objects/js-array-buffer.tq
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ extern class JSArrayBuffer extends JSObjectWithEmbedderSlots {
raw_max_byte_length: uintptr;
// A SandboxedPtr if the sandbox is enabled
backing_store: RawPtr;
extension: RawPtr;
extension: ExternalPointer;
bit_field: JSArrayBufferFlags;
// Pads header size to be a multiple of kTaggedSize.
@if(TAGGED_SIZE_8_BYTES) optional_padding: uint32;
Expand Down
2 changes: 2 additions & 0 deletions deps/v8/src/objects/objects-body-descriptors-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ class JSArrayBuffer::BodyDescriptor final : public BodyDescriptorBase {
// JSArrayBuffer instances contain raw data that the GC does not know about.
IteratePointers(obj, kPropertiesOrHashOffset, kEndOfTaggedFieldsOffset, v);
IterateJSObjectBodyImpl(map, obj, kHeaderSize, object_size, v);
v->VisitExternalPointer(map, obj.RawExternalPointerField(kExtensionOffset),
kArrayBufferExtensionTag);
}

static inline int SizeOf(Map map, HeapObject object) {
Expand Down
2 changes: 1 addition & 1 deletion deps/v8/src/sandbox/external-pointer-table.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
// returning the previous value. The same tag is applied both to decode the
// previous value and encode the given value.
//
// This method is atomic and can call be called from background threads.
// This method is atomic and can be called from background threads.
inline Address Exchange(ExternalPointerHandle handle, Address value,
ExternalPointerTag tag);

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/snapshot/deserializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ void Deserializer<IsolateT>::PostProcessNewJSReceiver(
auto buffer = JSArrayBuffer::cast(*obj);
uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
if (store_index == kEmptyBackingStoreRefSentinel) {
buffer.set_extension(nullptr);
buffer.set_backing_store(main_thread_isolate(),
EmptyBackingStoreBuffer());
} else {
Expand Down
8 changes: 3 additions & 5 deletions deps/v8/test/cctest/heap/test-array-buffer-tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ TEST(ArrayBuffer_NonLivePromotion) {
v8::Isolate* isolate = env->GetIsolate();
Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();

JSArrayBuffer raw_ab;
{
v8::HandleScope handle_scope(isolate);
Handle<FixedArray> root =
Expand All @@ -235,13 +234,12 @@ TEST(ArrayBuffer_NonLivePromotion) {
CHECK(IsTracked(heap, JSArrayBuffer::cast(root->get(0))));
heap::GcAndSweep(heap, NEW_SPACE);
CHECK(IsTracked(heap, JSArrayBuffer::cast(root->get(0))));
raw_ab = JSArrayBuffer::cast(root->get(0));
ArrayBufferExtension* extension =
JSArrayBuffer::cast(root->get(0)).extension();
root->set(0, ReadOnlyRoots(heap).undefined_value());
heap::SimulateIncrementalMarking(heap, true);
// Prohibit page from being released.
Page::FromHeapObject(raw_ab)->MarkNeverEvacuate();
heap::GcAndSweep(heap, OLD_SPACE);
CHECK(!IsTracked(heap, raw_ab));
CHECK(!IsTracked(heap, extension));
}
}

Expand Down

0 comments on commit 7bdad94

Please sign in to comment.