[websnapshot] Only serialize actual elements

We set the JSArray's length to the elements's length before,
which is wrong when the elements have the slack part. We could
serialize the correct length and only the actual elements excluding
the slack part for JSArray's elements. And we do the same thing
for the objects to avoid serializing unnecessary elements.

Bug: v8:13304
Change-Id: Ib68e06f409bfcab5c57fb5532e188aa0099d1140
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3905061Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83380}

src/web-snapshot/web-snapshot.cc

@@ -1592,7 +1592,28 @@ void WebSnapshotSerializer::SerializeObject(Handle<JSObject> object) {
   }
 
   // Elements.
-  SerializeElements(object, object_serializer_);
+  ElementsKind kind = object->GetElementsKind();
+  // We only serialize the actual elements excluding the slack part.
+  DCHECK(!IsDoubleElementsKind(kind));
+  if (!IsDictionaryElementsKind(kind)) {
+    uint32_t elements_length = object->elements().length();
+    if (IsHoleyElementsKindForRead(kind)) {
+      uint32_t max_element_index = 0;
+      FixedArray elements = FixedArray::cast(object->elements());
+      for (int i = elements_length - 1; i >= 0; i--) {
+        if (!elements.is_the_hole(isolate_, i)) {
+          max_element_index = i + 1;
+          break;
+        }
+      }
+      return SerializeElements(object, object_serializer_,
+                               Just(max_element_index));
+    } else {
+      return SerializeElements(object, object_serializer_,
+                               Just(elements_length));
+    }
+  }
+  SerializeElements(object, object_serializer_, Nothing<uint32_t>());
 }
 
 // Format (serialized array):
@@ -1606,11 +1627,17 @@ void WebSnapshotSerializer::SerializeObject(Handle<JSObject> object) {
 //     - Element index
 //     - Serialized value
 void WebSnapshotSerializer::SerializeArray(Handle<JSArray> array) {
-  SerializeElements(array, array_serializer_);
+  uint32_t length;
+  if (!array->length().ToUint32(&length)) {
+    Throw("Invalid array length");
+    return;
+  }
+  SerializeElements(array, array_serializer_, Just(length));
 }
 
 void WebSnapshotSerializer::SerializeElements(Handle<JSObject> object,
-                                              ValueSerializer& serializer) {
+                                              ValueSerializer& serializer,
+                                              Maybe<uint32_t> length) {
   // TODO(v8:11525): Handle sealed & frozen elements correctly. (Also: handle
   // sealed & frozen objects.)
 
@@ -1634,9 +1661,8 @@ void WebSnapshotSerializer::SerializeElements(Handle<JSObject> object,
       serializer.WriteUint32(ElementsType::kDense);
       Handle<FixedArray> elements =
           handle(FixedArray::cast(object->elements()), isolate_);
-      uint32_t length = static_cast<uint32_t>(elements->length());
-      serializer.WriteUint32(length);
-      for (uint32_t i = 0; i < length; ++i) {
+      serializer.WriteUint32(length.ToChecked());
+      for (uint32_t i = 0; i < length.ToChecked(); ++i) {
         WriteValue(handle(elements->get(i), isolate_), serializer);
       }
       break;
@@ -1646,9 +1672,8 @@ void WebSnapshotSerializer::SerializeElements(Handle<JSObject> object,
       serializer.WriteUint32(ElementsType::kDense);
       Handle<FixedDoubleArray> elements =
           handle(FixedDoubleArray::cast(object->elements()), isolate_);
-      uint32_t length = static_cast<uint32_t>(elements->length());
-      serializer.WriteUint32(length);
-      for (uint32_t i = 0; i < length; ++i) {
+      serializer.WriteUint32(length.ToChecked());
+      for (uint32_t i = 0; i < length.ToChecked(); ++i) {
         if (!elements->is_the_hole(i)) {
           double double_value = elements->get_scalar(i);
           Handle<Object> element_value =

src/web-snapshot/web-snapshot.h

@@ -314,7 +314,8 @@ class V8_EXPORT WebSnapshotSerializer
   void SerializeClass(Handle<JSFunction> function);
   void SerializeContext(Handle<Context> context, uint32_t id);
   void SerializeArray(Handle<JSArray> array);
-  void SerializeElements(Handle<JSObject> object, ValueSerializer& serializer);
+  void SerializeElements(Handle<JSObject> object, ValueSerializer& serializer,
+                         Maybe<uint32_t> length);
   void SerializeObject(Handle<JSObject> object);
   void SerializeArrayBufferView(Handle<JSArrayBufferView> array_buffer_view,
                                 ValueSerializer& serializer);

test/mjsunit/web-snapshot/web-snapshot-array.js

@@ -107,3 +107,27 @@ d8.file.execute('test/mjsunit/web-snapshot/web-snapshot-helpers.js');
   // cctests.
   assertEquals('foobarfoo', foo.array.join(''));
 })();
+
+(function TestArrayWithSlackElements() {
+  function createObjects() {
+    globalThis.foo = {
+      array: [],
+      doubleArray: [],
+      objectArray: []
+    };
+    for (let i = 0; i < 100; ++i) {
+      globalThis.foo.array.push(i);
+      globalThis.foo.doubleArray.push(i + 0.1);
+      globalThis.foo.objectArray.push({});
+    }
+  }
+  const { foo } = takeAndUseWebSnapshot(createObjects, ['foo']);
+  assertEquals(100, foo.array.length);
+  assertEquals(100, foo.doubleArray.length);
+  assertEquals(100, foo.objectArray.length);
+  for (let i = 0; i < 100; ++i){
+    assertEquals(i, foo.array[i]);
+    assertEquals(i + 0.1, foo.doubleArray[i]);
+    assertEquals({}, foo.objectArray[i]);
+  }
+})();

test/mjsunit/web-snapshot/web-snapshot-object.js

@@ -186,3 +186,19 @@ d8.file.execute('test/mjsunit/web-snapshot/web-snapshot-helpers.js');
   assertEquals(['4394967296'], Object.getOwnPropertyNames(obj));
   assertEquals['lol', obj[4394967296]];
 })();
+
+(function TestObjectWithSlackElements() {
+  function createObjects() {
+    globalThis.foo = {};
+    globalThis.bar = {};
+    for (let i = 0; i < 100; ++i) {
+      globalThis.foo[i] = i;
+      globalThis.bar[i] = {};
+    }
+  }
+  const { foo, bar } = takeAndUseWebSnapshot(createObjects, ['foo', 'bar']);
+  for (let i = 0; i < 100; ++i) {
+    assertEquals(i, foo[i]);
+    assertEquals({}, bar[i]);
+  }
+})();

test/unittests/web-snapshot/web-snapshot-unittest.cc

@@ -1079,5 +1079,57 @@ TEST_F(WebSnapshotTest, ConstructorFunctionKinds) {
   }
 }
 
+TEST_F(WebSnapshotTest, SlackElementsInObjects) {
+  v8::Isolate* isolate = v8_isolate();
+  v8::HandleScope scope(isolate);
+
+  WebSnapshotData snapshot_data;
+  {
+    v8::Local<v8::Context> new_context = v8::Context::New(isolate);
+    v8::Context::Scope context_scope(new_context);
+    const char* snapshot_source =
+        "var foo = {};"
+        "for (let i = 0; i < 100; ++i) {"
+        "  foo[i] = i;"
+        "}"
+        "var bar = {};"
+        "for (let i = 0; i < 100; ++i) {"
+        "  bar[i] = {};"
+        "}";
+
+    RunJS(snapshot_source);
+    v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 2);
+    exports->Set(isolate, 0,
+                 v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked());
+    exports->Set(isolate, 1,
+                 v8::String::NewFromUtf8(isolate, "bar").ToLocalChecked());
+    WebSnapshotSerializer serializer(isolate);
+    CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
+    CHECK(!serializer.has_error());
+    CHECK_NOT_NULL(snapshot_data.buffer);
+  }
+
+  {
+    v8::Local<v8::Context> new_context = v8::Context::New(isolate);
+    v8::Context::Scope context_scope(new_context);
+    WebSnapshotDeserializer deserializer(isolate, snapshot_data.buffer,
+                                         snapshot_data.buffer_size);
+    CHECK(deserializer.Deserialize());
+    CHECK(!deserializer.has_error());
+
+    Handle<JSObject> foo =
+        Handle<JSObject>::cast(Utils::OpenHandle<v8::Object, JSReceiver>(
+            RunJS("foo").As<v8::Object>()));
+    CHECK_EQ(100, foo->elements().length());
+    CHECK_EQ(HOLEY_ELEMENTS, foo->GetElementsKind());
+
+    Handle<JSObject> bar =
+        Handle<JSObject>::cast(Utils::OpenHandle<v8::Object, JSReceiver>(
+            RunJS("bar").As<v8::Object>()));
+    CHECK_EQ(100, bar->elements().length());
+    CHECK_EQ(HOLEY_ELEMENTS, bar->GetElementsKind());
+  }
+}
+
 }  // namespace internal
 }  // namespace v8