[sandbox] Access external pointer in JSArrayBuffer via bottlenecks

isheludko · Commit Bot · commit c9d650e64ce3 · 2020-05-04T11:58:23.000Z
Bug: v8:10391 Change-Id: I316a3c5cd986a74d7f46da6d0b85cb3d549be497 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2153209 Commit-Queue: Igor Sheludko <ishell@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#67533}
diff --git a/src/builtins/builtins-sharedarraybuffer-gen.cc b/src/builtins/builtins-sharedarraybuffer-gen.cc
@@ -89,7 +89,7 @@ void SharedArrayBufferBuiltinsAssembler::ValidateSharedTypedArray(
   BIND(&not_float_or_clamped);
   *out_elements_kind = elements_kind;
 
-  TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStore(array_buffer);
+  TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStorePtr(array_buffer);
   TNode<UintPtrT> byte_offset = LoadJSArrayBufferViewByteOffset(array);
   *out_backing_store = RawPtrAdd(backing_store, Signed(byte_offset));
 }
diff --git a/src/builtins/builtins-typed-array-gen.cc b/src/builtins/builtins-typed-array-gen.cc
@@ -65,8 +65,9 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::AllocateEmptyOnHeapBuffer(
 
   StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kByteLengthOffset,
                                  byte_length);
-  StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kBackingStoreOffset,
-                                 IntPtrConstant(0));
+  StoreJSArrayBufferBackingStore(
+      buffer,
+      EncodeExternalPointer(ReinterpretCast<RawPtrT>(IntPtrConstant(0))));
   if (V8_ARRAY_BUFFER_EXTENSION_BOOL) {
     StoreObjectFieldNoWriteBarrier(buffer, JSArrayBuffer::kExtensionOffset,
                                    IntPtrConstant(0));
@@ -239,7 +240,7 @@ TNode<JSArrayBuffer> TypedArrayBuiltinsAssembler::GetBuffer(
 
   TNode<JSArrayBuffer> buffer = LoadJSArrayBufferViewBuffer(array);
   GotoIf(IsDetachedBuffer(buffer), &call_runtime);
-  TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStore(buffer);
+  TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStorePtr(buffer);
   GotoIf(WordEqual(backing_store, IntPtrConstant(0)), &call_runtime);
   var_result = buffer;
   Goto(&done);
diff --git a/src/builtins/data-view.tq b/src/builtins/data-view.tq
@@ -122,16 +122,16 @@ namespace data_view {
   macro LoadDataView8(
       buffer: JSArrayBuffer, offset: uintptr, signed: constexpr bool): Smi {
     if constexpr (signed) {
-      return Convert<Smi>(LoadInt8(buffer.backing_store, offset));
+      return Convert<Smi>(LoadInt8(buffer.backing_store_ptr, offset));
     } else {
-      return Convert<Smi>(LoadUint8(buffer.backing_store, offset));
+      return Convert<Smi>(LoadUint8(buffer.backing_store_ptr, offset));
     }
   }
 
   macro LoadDataView16(
       buffer: JSArrayBuffer, offset: uintptr, requestedLittleEndian: bool,
       signed: constexpr bool): Number {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     let b0: int32;
     let b1: int32;
@@ -158,7 +158,7 @@ namespace data_view {
   macro LoadDataView32(
       buffer: JSArrayBuffer, offset: uintptr, requestedLittleEndian: bool,
       kind: constexpr ElementsKind): Number {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     const b0: uint32 = LoadUint8(dataPointer, offset);
     const b1: uint32 = LoadUint8(dataPointer, offset + 1);
@@ -187,7 +187,7 @@ namespace data_view {
   macro LoadDataViewFloat64(
       buffer: JSArrayBuffer, offset: uintptr,
       requestedLittleEndian: bool): Number {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     const b0: uint32 = LoadUint8(dataPointer, offset);
     const b1: uint32 = LoadUint8(dataPointer, offset + 1);
@@ -329,7 +329,7 @@ namespace data_view {
   macro LoadDataViewBigInt(implicit context: Context)(
       buffer: JSArrayBuffer, offset: uintptr, requestedLittleEndian: bool,
       signed: constexpr bool): BigInt {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     const b0: uint32 = LoadUint8(dataPointer, offset);
     const b1: uint32 = LoadUint8(dataPointer, offset + 1);
@@ -533,13 +533,13 @@ namespace data_view {
       void;
 
   macro StoreDataView8(buffer: JSArrayBuffer, offset: uintptr, value: uint32) {
-    StoreWord8(buffer.backing_store, offset, value & 0xFF);
+    StoreWord8(buffer.backing_store_ptr, offset, value & 0xFF);
   }
 
   macro StoreDataView16(
       buffer: JSArrayBuffer, offset: uintptr, value: uint32,
       requestedLittleEndian: bool) {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     const b0: uint32 = value & 0xFF;
     const b1: uint32 = (value >>> 8) & 0xFF;
@@ -556,7 +556,7 @@ namespace data_view {
   macro StoreDataView32(
       buffer: JSArrayBuffer, offset: uintptr, value: uint32,
       requestedLittleEndian: bool) {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     const b0: uint32 = value & 0xFF;
     const b1: uint32 = (value >>> 8) & 0xFF;
@@ -579,7 +579,7 @@ namespace data_view {
   macro StoreDataView64(
       buffer: JSArrayBuffer, offset: uintptr, lowWord: uint32, highWord: uint32,
       requestedLittleEndian: bool) {
-    const dataPointer: RawPtr = buffer.backing_store;
+    const dataPointer: RawPtr = buffer.backing_store_ptr;
 
     const b0: uint32 = lowWord & 0xFF;
     const b1: uint32 = (lowWord >>> 8) & 0xFF;
diff --git a/src/builtins/typed-array-createtypedarray.tq b/src/builtins/typed-array-createtypedarray.tq
@@ -32,7 +32,7 @@ namespace typed_array {
       // platforms are self-limiting, because we can't allocate an array bigger
       // than our 32-bit arithmetic range anyway. 64 bit platforms could
       // theoretically have an offset up to 2^35 - 1.
-      const backingStore: uintptr = Convert<uintptr>(buffer.backing_store);
+      const backingStore: uintptr = Convert<uintptr>(buffer.backing_store_ptr);
 
       // Assert no overflow has occurred. Only assert if the mock array buffer
       // allocator is NOT used. When the mock array buffer is used, impossibly
@@ -58,10 +58,10 @@ namespace typed_array {
           typedArray, elements, byteOffset);
     } else {
       typed_array::SetJSTypedArrayOffHeapDataPtr(
-          typedArray, buffer.backing_store, byteOffset);
+          typedArray, buffer.backing_store_ptr, byteOffset);
       assert(
           typedArray.data_ptr ==
-          (buffer.backing_store + Convert<intptr>(byteOffset)));
+          (buffer.backing_store_ptr + Convert<intptr>(byteOffset)));
     }
     SetupTypedArrayEmbedderFields(typedArray);
     return typedArray;
diff --git a/src/codegen/code-stub-assembler.cc b/src/codegen/code-stub-assembler.cc
@@ -12635,15 +12635,9 @@ void CodeStubAssembler::ThrowIfArrayBufferViewBufferIsDetached(
   ThrowIfArrayBufferIsDetached(context, buffer, method_name);
 }
 
-TNode<Uint32T> CodeStubAssembler::LoadJSArrayBufferBitField(
+TNode<RawPtrT> CodeStubAssembler::LoadJSArrayBufferBackingStorePtr(
     TNode<JSArrayBuffer> array_buffer) {
-  return LoadObjectField<Uint32T>(array_buffer, JSArrayBuffer::kBitFieldOffset);
-}
-
-TNode<RawPtrT> CodeStubAssembler::LoadJSArrayBufferBackingStore(
-    TNode<JSArrayBuffer> array_buffer) {
-  return LoadObjectField<RawPtrT>(array_buffer,
-                                  JSArrayBuffer::kBackingStoreOffset);
+  return DecodeExternalPointer(LoadJSArrayBufferBackingStore(array_buffer));
 }
 
 TNode<JSArrayBuffer> CodeStubAssembler::LoadJSArrayBufferViewBuffer(
diff --git a/src/codegen/code-stub-assembler.h b/src/codegen/code-stub-assembler.h
@@ -3642,8 +3642,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<BoolT> IsDebugActive();
 
   // JSArrayBuffer helpers
-  TNode<Uint32T> LoadJSArrayBufferBitField(TNode<JSArrayBuffer> array_buffer);
-  TNode<RawPtrT> LoadJSArrayBufferBackingStore(
+  TNode<RawPtrT> LoadJSArrayBufferBackingStorePtr(
       TNode<JSArrayBuffer> array_buffer);
   void ThrowIfArrayBufferIsDetached(SloppyTNode<Context> context,
                                     TNode<JSArrayBuffer> array_buffer,
diff --git a/src/objects/js-array-buffer-inl.h b/src/objects/js-array-buffer-inl.h
@@ -5,6 +5,7 @@
 #ifndef V8_OBJECTS_JS_ARRAY_BUFFER_INL_H_
 #define V8_OBJECTS_JS_ARRAY_BUFFER_INL_H_
 
+#include "src/common/external-pointer.h"
 #include "src/objects/js-array-buffer.h"
 
 #include "src/common/external-pointer-inl.h"
@@ -37,12 +38,27 @@ void JSArrayBuffer::set_byte_length(size_t value) {
   WriteField<size_t>(kByteLengthOffset, value);
 }
 
-void* JSArrayBuffer::backing_store() const {
-  return reinterpret_cast<void*>(ReadField<Address>(kBackingStoreOffset));
+DEF_GETTER(JSArrayBuffer, backing_store, void*) {
+  ExternalPointer_t encoded_value =
+      ReadField<ExternalPointer_t>(kBackingStoreOffset);
+  return reinterpret_cast<void*>(DecodeExternalPointer(isolate, encoded_value));
 }
 
-void JSArrayBuffer::set_backing_store(void* value) {
-  WriteField<Address>(kBackingStoreOffset, reinterpret_cast<Address>(value));
+void JSArrayBuffer::set_backing_store(Isolate* isolate, void* value) {
+  ExternalPointer_t encoded_value =
+      EncodeExternalPointer(isolate, reinterpret_cast<Address>(value));
+  WriteField<ExternalPointer_t>(kBackingStoreOffset, encoded_value);
+}
+
+uint32_t JSArrayBuffer::GetBackingStoreRefForDeserialization() const {
+  ExternalPointer_t encoded_value =
+      ReadField<ExternalPointer_t>(kBackingStoreOffset);
+  return static_cast<uint32_t>(encoded_value);
+}
+
+void JSArrayBuffer::SetBackingStoreRefForSerialization(uint32_t ref) {
+  ExternalPointer_t encoded_value = ref;
+  WriteField<ExternalPointer_t>(kBackingStoreOffset, encoded_value);
 }
 
 ArrayBufferExtension* JSArrayBuffer::extension() const {
diff --git a/src/objects/js-array-buffer.cc b/src/objects/js-array-buffer.cc
@@ -45,7 +45,7 @@ void JSArrayBuffer::Setup(SharedFlag shared,
   }
   set_extension(nullptr);
   if (!backing_store) {
-    set_backing_store(nullptr);
+    set_backing_store(GetIsolate(), nullptr);
     set_byte_length(0);
   } else {
     Attach(std::move(backing_store));
@@ -60,19 +60,20 @@ void JSArrayBuffer::Attach(std::shared_ptr<BackingStore> backing_store) {
   DCHECK_NOT_NULL(backing_store);
   DCHECK_EQ(is_shared(), backing_store->is_shared());
   DCHECK(!was_detached());
-  set_backing_store(backing_store->buffer_start());
+  Isolate* isolate = GetIsolate();
+  set_backing_store(isolate, backing_store->buffer_start());
   set_byte_length(backing_store->byte_length());
   if (backing_store->is_wasm_memory()) set_is_detachable(false);
   if (!backing_store->free_on_destruct()) set_is_external(true);
   if (V8_ARRAY_BUFFER_EXTENSION_BOOL) {
-    Heap* heap = GetIsolate()->heap();
+    Heap* heap = isolate->heap();
     ArrayBufferExtension* extension = EnsureExtension();
     size_t bytes = backing_store->PerIsolateAccountingLength();
     extension->set_accounting_length(bytes);
     extension->set_backing_store(std::move(backing_store));
     heap->AppendArrayBufferExtension(*this, extension);
   } else {
-    GetIsolate()->heap()->RegisterBackingStore(*this, std::move(backing_store));
+    isolate->heap()->RegisterBackingStore(*this, std::move(backing_store));
   }
 }
 
@@ -103,7 +104,7 @@ void JSArrayBuffer::Detach(bool force_for_wasm_memory) {
 
   DCHECK(!is_shared());
   DCHECK(!is_asmjs_memory());
-  set_backing_store(nullptr);
+  set_backing_store(isolate, nullptr);
   set_byte_length(0);
   set_was_detached(true);
 }
diff --git a/src/objects/js-array-buffer.h b/src/objects/js-array-buffer.h
@@ -33,7 +33,8 @@ class JSArrayBuffer : public JSObject {
   DECL_PRIMITIVE_ACCESSORS(byte_length, size_t)
 
   // [backing_store]: backing memory for this array
-  DECL_PRIMITIVE_ACCESSORS(backing_store, void*)
+  DECL_GETTER(backing_store, void*)
+  inline void set_backing_store(Isolate* isolate, void* value);
 
   // [extension]: extension object used for GC
   DECL_PRIMITIVE_ACCESSORS(extension, ArrayBufferExtension*)
@@ -50,7 +51,7 @@ class JSArrayBuffer : public JSObject {
   // is deterministic. Depending on the V8 build mode there could be no padding.
   V8_INLINE void clear_padding();
 
-// Bit positions for [bit_field].
+  // Bit positions for [bit_field].
   DEFINE_TORQUE_GENERATED_JS_ARRAY_BUFFER_FLAGS()
 
   // [is_external]: true indicates that the embedder is in charge of freeing the
@@ -110,6 +111,17 @@ class JSArrayBuffer : public JSObject {
   void YoungMarkExtension();
   void YoungMarkExtensionPromoted();
 
+  //
+  // Serializer/deserializer support.
+  //
+
+  // Backing stores are serialized/deserialized separately. During serialization
+  // the backing store reference is stored in the backing store field and upon
+  // deserialization it is converted back to actual external (off-heap) pointer
+  // value.
+  inline uint32_t GetBackingStoreRefForDeserialization() const;
+  inline void SetBackingStoreRefForSerialization(uint32_t ref);
+
   // Dispatched behavior.
   DECL_PRINTER(JSArrayBuffer)
   DECL_VERIFIER(JSArrayBuffer)
diff --git a/src/objects/js-array-buffer.tq b/src/objects/js-array-buffer.tq
@@ -12,7 +12,7 @@ bitfield struct JSArrayBufferFlags extends uint32 {
 
 extern class JSArrayBuffer extends JSObject {
   byte_length: uintptr;
-  backing_store: RawPtr;
+  backing_store: ExternalPointer;
   @if(V8_ARRAY_BUFFER_EXTENSION_BOOL) extension: RawPtr;
   @ifnot(V8_ARRAY_BUFFER_EXTENSION_BOOL) extension: void;
   bit_field: JSArrayBufferFlags;
@@ -21,6 +21,9 @@ extern class JSArrayBuffer extends JSObject {
   @ifnot(TAGGED_SIZE_8_BYTES) optional_padding: void;
 }
 
+extern operator '.backing_store_ptr' macro LoadJSArrayBufferBackingStorePtr(
+    JSArrayBuffer): RawPtr;
+
 @export
 macro IsDetachedBuffer(buffer: JSArrayBuffer): bool {
   return buffer.bit_field.was_detached;
diff --git a/src/snapshot/context-deserializer.cc b/src/snapshot/context-deserializer.cc
@@ -69,8 +69,7 @@ MaybeHandle<Object> ContextDeserializer::Deserialize(
 
 void ContextDeserializer::SetupOffHeapArrayBufferBackingStores() {
   for (Handle<JSArrayBuffer> buffer : new_off_heap_array_buffers()) {
-    // Serializer writes backing store ref in |backing_store| field.
-    size_t store_index = reinterpret_cast<size_t>(buffer->backing_store());
+    uint32_t store_index = buffer->GetBackingStoreRefForDeserialization();
     auto bs = backing_store(store_index);
     SharedFlag shared =
         bs && bs->is_shared() ? SharedFlag::kShared : SharedFlag::kNotShared;
diff --git a/src/snapshot/deserializer.cc b/src/snapshot/deserializer.cc
@@ -291,7 +291,7 @@ HeapObject Deserializer::PostProcessNewObject(HeapObject obj,
       // The backing store of the JSArrayBuffer has not been correctly restored
       // yet, as that may trigger GC. The backing_store field currently contains
       // a numbered reference to an already deserialized backing store.
-      size_t store_index = reinterpret_cast<size_t>(buffer.backing_store());
+      uint32_t store_index = buffer.GetBackingStoreRefForDeserialization();
       backing_store = backing_stores_[store_index]->buffer_start();
     }
     data_view.set_data_pointer(reinterpret_cast<uint8_t*>(backing_store) +
diff --git a/src/snapshot/object-deserializer.cc b/src/snapshot/object-deserializer.cc
@@ -90,8 +90,7 @@ void ObjectDeserializer::CommitPostProcessedObjects() {
   }
 
   for (Handle<JSArrayBuffer> buffer : new_off_heap_array_buffers()) {
-    // Serializer writes backing store ref in |backing_store| field.
-    size_t store_index = reinterpret_cast<size_t>(buffer->backing_store());
+    uint32_t store_index = buffer->GetBackingStoreRefForDeserialization();
     auto bs = backing_store(store_index);
     SharedFlag shared =
         bs && bs->is_shared() ? SharedFlag::kShared : SharedFlag::kNotShared;
diff --git a/src/snapshot/serializer.cc b/src/snapshot/serializer.cc
@@ -389,10 +389,7 @@ void Serializer::ObjectSerializer::SerializeJSArrayBuffer() {
   // The embedder-allocated backing store only exists for the off-heap case.
   if (backing_store != nullptr) {
     uint32_t ref = SerializeBackingStore(backing_store, byte_length);
-    // To properly share the buffer, we set the backing store ref as an
-    // a backing store address. On deserialization we re-set data pointer
-    // to proper value.
-    buffer.set_backing_store(reinterpret_cast<void*>(static_cast<size_t>(ref)));
+    buffer.SetBackingStoreRefForSerialization(ref);
 
     // Ensure deterministic output by setting extension to null during
     // serialization.
@@ -401,7 +398,7 @@ void Serializer::ObjectSerializer::SerializeJSArrayBuffer() {
 
   SerializeObject();
 
-  buffer.set_backing_store(backing_store);
+  buffer.set_backing_store(serializer_->isolate(), backing_store);
   buffer.set_extension(extension);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ void SharedArrayBufferBuiltinsAssembler::ValidateSharedTypedArray(`
`89`	`89`	`BIND(&not_float_or_clamped);`
`90`	`90`	`*out_elements_kind = elements_kind;`
`91`	`91`
`92`		`- TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStore(array_buffer);`
	`92`	`+ TNode<RawPtrT> backing_store = LoadJSArrayBufferBackingStorePtr(array_buffer);`
`93`	`93`	`TNode<UintPtrT> byte_offset = LoadJSArrayBufferViewByteOffset(array);`
`94`	`94`	`*out_backing_store = RawPtrAdd(backing_store, Signed(byte_offset));`
`95`	`95`	`}`