Fix WebAssembly.Memory deserialization in more complex objects

The wasm memory deserialization didn't properly increment the object id, so
wouldn't work properly if the memory object (or its contained
SharedArrayBuffer) where included multiple times in the object.

Bug: v8:6895
Change-Id: I5c4c25bad2ec6152883c5a7321038aba1950480a
Reviewed-on: https://chromium-review.googlesource.com/721630Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48767}

src/value-serializer.cc

@@ -1735,28 +1735,34 @@ MaybeHandle<JSObject> ValueDeserializer::ReadWasmModule() {
   return result;
 }
 
-MaybeHandle<JSObject> ValueDeserializer::ReadWasmMemory() {
+MaybeHandle<WasmMemoryObject> ValueDeserializer::ReadWasmMemory() {
+  uint32_t id = next_id_++;
+
   if (!FLAG_experimental_wasm_threads) {
-    return MaybeHandle<JSObject>();
+    return MaybeHandle<WasmMemoryObject>();
   }
 
   int32_t maximum_pages;
   if (!ReadZigZag<int32_t>().To(&maximum_pages)) {
-    return MaybeHandle<JSObject>();
+    return MaybeHandle<WasmMemoryObject>();
   }
 
   SerializationTag tag;
   if (!ReadTag().To(&tag) || tag != SerializationTag::kSharedArrayBuffer) {
-    return MaybeHandle<JSObject>();
+    return MaybeHandle<WasmMemoryObject>();
   }
 
   const bool is_shared = true;
   Handle<JSArrayBuffer> buffer;
   if (!ReadTransferredJSArrayBuffer(is_shared).ToHandle(&buffer)) {
-    return MaybeHandle<JSObject>();
+    return MaybeHandle<WasmMemoryObject>();
   }
 
-  return WasmMemoryObject::New(isolate_, buffer, maximum_pages);
+  Handle<WasmMemoryObject> result =
+      WasmMemoryObject::New(isolate_, buffer, maximum_pages);
+
+  AddObjectWithID(id, result);
+  return result;
 }
 
 MaybeHandle<JSObject> ValueDeserializer::ReadHostObject() {

src/value-serializer.h

@@ -273,7 +273,7 @@ class ValueDeserializer {
       Handle<JSArrayBuffer> buffer) WARN_UNUSED_RESULT;
   MaybeHandle<JSObject> ReadWasmModule() WARN_UNUSED_RESULT;
   MaybeHandle<JSObject> ReadWasmModuleTransfer() WARN_UNUSED_RESULT;
-  MaybeHandle<JSObject> ReadWasmMemory() WARN_UNUSED_RESULT;
+  MaybeHandle<WasmMemoryObject> ReadWasmMemory() WARN_UNUSED_RESULT;
   MaybeHandle<JSObject> ReadHostObject() WARN_UNUSED_RESULT;
 
   /*

src/wasm/wasm-objects.h

@@ -158,9 +158,8 @@ class WasmMemoryObject : public JSObject {
   uint32_t current_pages();
   inline bool has_maximum_pages();
 
-  static Handle<WasmMemoryObject> New(Isolate* isolate,
-                                      Handle<JSArrayBuffer> buffer,
-                                      int32_t maximum);
+  V8_EXPORT_PRIVATE static Handle<WasmMemoryObject> New(
+      Isolate* isolate, Handle<JSArrayBuffer> buffer, int32_t maximum);
 
   static int32_t Grow(Isolate*, Handle<WasmMemoryObject>, uint32_t pages);
   static void SetupNewBufferWithSameBackingStore(

test/mjsunit/wasm/worker-memory.js

@@ -11,30 +11,59 @@
   assertThrows(() => worker.postMessage(memory), Error);
 })();
 
-(function TestPostMessageSharedMemory() {
-  let workerScript =
-    `onmessage = function(memory) {
-       // Can't use assert in a worker.
-       if (!(memory instanceof WebAssembly.Memory)) {
-         postMessage("Error: memory is not a WebAssembly.Memory");
-         return;
+// Can't use assert in a worker.
+let workerHelpers =
+  `function assertTrue(value, msg) {
+    if (!value) {
+      postMessage("Error: " + msg);
+      throw new Error("Exit");  // To stop testing.
     }
-
-       if (!(memory.buffer instanceof SharedArrayBuffer)) {
-         postMessage("Error: memory.buffer is not a SharedArrayBuffer");
-         return;
    }
 
-       if (memory.buffer.byteLength != 65536) {
-         postMessage("Error: memory.buffer.byteLength is not 1 page");
-         return;
+   function assertIsWasmMemory(memory, expectedSize) {
+      assertTrue(memory instanceof WebAssembly.Memory,
+                 "object is not a WebAssembly.Memory");
+
+      assertTrue(memory.buffer instanceof SharedArrayBuffer,
+                 "object.buffer is not a SharedArrayBuffer");
+
+      assertTrue(memory.buffer.byteLength == expectedSize,
+                 "object.buffer.byteLength is not " + expectedSize + " bytes");
    }
+`;
 
+(function TestPostMessageSharedMemory() {
+  let workerScript = workerHelpers +
+    `onmessage = function(memory) {
+       assertIsWasmMemory(memory, 65536);
        postMessage("OK");
      };`;
+
   let worker = new Worker(workerScript);
   let memory = new WebAssembly.Memory({initial: 1, maximum: 2, shared: true});
   worker.postMessage(memory);
   assertEquals("OK", worker.getMessage());
   worker.terminate();
 })();
+
+(function TestPostMessageComplexObjectWithSharedMemory() {
+  let workerScript = workerHelpers +
+    `onmessage = function(obj) {
+       assertIsWasmMemory(obj.memories[0], 65536);
+       assertIsWasmMemory(obj.memories[1], 65536);
+       assertTrue(obj.buffer instanceof SharedArrayBuffer,
+                  "buffer is not a SharedArrayBuffer");
+       assertTrue(obj.memories[0] === obj.memories[1], "memories aren't equal");
+       assertTrue(obj.memories[0].buffer === obj.buffer,
+                  "buffers aren't equal");
+       assertTrue(obj.foo === 1, "foo is not 1");
+       postMessage("OK");
+     };`;
+
+  let worker = new Worker(workerScript);
+  let memory = new WebAssembly.Memory({initial: 1, maximum: 2, shared: true});
+  let obj = {memories: [memory, memory], buffer: memory.buffer, foo: 1};
+  worker.postMessage(obj);
+  assertEquals("OK", worker.getMessage());
+  worker.terminate();
+})();

test/unittests/value-serializer-unittest.cc

@@ -11,6 +11,7 @@
 #include "src/api.h"
 #include "src/base/build_config.h"
 #include "src/objects-inl.h"
+#include "src/wasm/wasm-objects.h"
 #include "test/unittests/test-utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -2233,21 +2234,20 @@ TEST_F(ValueSerializerTest, DecodeInvalidDataView) {
 class ValueSerializerTestWithSharedArrayBufferTransfer
     : public ValueSerializerTest {
  protected:
-  static const size_t kTestByteLength = 4;
-
   ValueSerializerTestWithSharedArrayBufferTransfer()
-      : serializer_delegate_(this) {
-    const uint8_t data[kTestByteLength] = {0x00, 0x01, 0x80, 0xff};
-    memcpy(data_, data, kTestByteLength);
+      : serializer_delegate_(this) {}
+
+  void InitializeData(const std::vector<uint8_t>& data) {
+    data_ = data;
     {
       Context::Scope scope(serialization_context());
       input_buffer_ =
-          SharedArrayBuffer::New(isolate(), &data_, kTestByteLength);
+          SharedArrayBuffer::New(isolate(), data_.data(), data_.size());
     }
     {
       Context::Scope scope(deserialization_context());
       output_buffer_ =
-          SharedArrayBuffer::New(isolate(), &data_, kTestByteLength);
+          SharedArrayBuffer::New(isolate(), data_.data(), data_.size());
     }
   }
 
@@ -2305,7 +2305,7 @@ class ValueSerializerTestWithSharedArrayBufferTransfer
 
  private:
   static bool flag_was_enabled_;
-  uint8_t data_[kTestByteLength];
+  std::vector<uint8_t> data_;
   Local<SharedArrayBuffer> input_buffer_;
   Local<SharedArrayBuffer> output_buffer_;
 };
@@ -2315,6 +2315,8 @@ bool ValueSerializerTestWithSharedArrayBufferTransfer::flag_was_enabled_ =
 
 TEST_F(ValueSerializerTestWithSharedArrayBufferTransfer,
        RoundTripSharedArrayBufferTransfer) {
+  InitializeData({0x00, 0x01, 0x80, 0xff});
+
   EXPECT_CALL(serializer_delegate_,
               GetSharedArrayBufferId(isolate(), input_buffer()))
       .WillRepeatedly(Return(Just(0U)));
@@ -2350,6 +2352,40 @@ TEST_F(ValueSerializerTestWithSharedArrayBufferTransfer,
       });
 }
 
+TEST_F(ValueSerializerTestWithSharedArrayBufferTransfer,
+       RoundTripWebAssemblyMemory) {
+  bool flag_was_enabled = i::FLAG_experimental_wasm_threads;
+  i::FLAG_experimental_wasm_threads = true;
+
+  std::vector<uint8_t> data = {0x00, 0x01, 0x80, 0xff};
+  data.resize(65536);
+  InitializeData(data);
+
+  EXPECT_CALL(serializer_delegate_,
+              GetSharedArrayBufferId(isolate(), input_buffer()))
+      .WillRepeatedly(Return(Just(0U)));
+
+  RoundTripTest(
+      [this]() -> Local<Value> {
+        const int32_t kMaxPages = 1;
+        auto i_isolate = reinterpret_cast<i::Isolate*>(isolate());
+        i::Handle<i::JSArrayBuffer> obj = Utils::OpenHandle(*input_buffer());
+        return Utils::Convert<i::WasmMemoryObject, Value>(
+            i::WasmMemoryObject::New(i_isolate, obj, kMaxPages));
+      },
+      [this](Local<Value> value) {
+        EXPECT_TRUE(EvaluateScriptForResultBool(
+            "result instanceof WebAssembly.Memory"));
+        EXPECT_TRUE(
+            EvaluateScriptForResultBool("result.buffer.byteLength === 65536"));
+        EXPECT_TRUE(
+            EvaluateScriptForResultBool("new Uint8Array(result.buffer, 0, "
+                                        "4).toString() === '0,1,128,255'"));
+      });
+
+  i::FLAG_experimental_wasm_threads = flag_was_enabled;
+}
+
 TEST_F(ValueSerializerTest, UnsupportedHostObject) {
   InvalidEncodeTest("new ExampleHostObject()");
   InvalidEncodeTest("({ a: new ExampleHostObject() })");