[snapshot] clear internal fields that are serialized by callback

This is to ensure the snapshot is deterministic.

Internal fields can contain:
- reference to heap object
- embedder-defined aligned pointer
- a smi

The latter two are not distinguishable by V8, so if the serializer
callback returns non-zero value, we consider it to be an aligned pointer
and clear it to ensure that the snapshot does not contain memory
addresses that may not be deterministic. If the callback returns
{ nullptr, 0 } as result, we consider it to be a smi or some in-place
data that we then serialize verbatim.


R=jgruber@chromium.org

Bug: chromium:870584
Change-Id: I3cf9abf135ffd28d8138fa32636b12596b076e13
Reviewed-on: https://chromium-review.googlesource.com/c/1304441
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57272}

include/v8.h

@@ -7058,6 +7058,10 @@ class V8_EXPORT EmbedderHeapTracer {
 /**
  * Callback and supporting data used in SnapshotCreator to implement embedder
  * logic to serialize internal fields.
+ * Internal fields that directly reference V8 objects are serialized without
+ * calling this callback. Internal fields that contain aligned pointers are
+ * serialized by this callback if it returns non-zero result. Otherwise it is
+ * serialized verbatim.
  */
 struct SerializeInternalFieldsCallback {
   typedef StartupData (*CallbackFunction)(Local<Object> holder, int index,

src/objects/js-objects-inl.h

@@ -268,6 +268,10 @@ Object* JSObject::GetEmbedderField(int index) {
   return READ_FIELD(this, GetHeaderSize() + (kPointerSize * index));
 }
 
+Address JSObject::GetEmbedderFieldRaw(int index) {
+  return GetEmbedderField(index)->ptr();
+}
+
 void JSObject::SetEmbedderField(int index, Object* value) {
   DCHECK(index < GetEmbedderFieldCount() && index >= 0);
   // Internal objects do follow immediately after the header, whereas in-object
@@ -279,12 +283,18 @@ void JSObject::SetEmbedderField(int index, Object* value) {
 }
 
 void JSObject::SetEmbedderField(int index, Smi value) {
+  SetEmbedderFieldRaw(index, value->ptr());
+}
+
+void JSObject::SetEmbedderFieldRaw(int index, Address value) {
   DCHECK(index < GetEmbedderFieldCount() && index >= 0);
   // Internal objects do follow immediately after the header, whereas in-object
   // properties are at the end of the object. Therefore there is no need
   // to adjust the index here.
   int offset = GetHeaderSize() + (kPointerSize * index);
-  WRITE_FIELD(this, offset, value);
+  Address field_addr = FIELD_ADDR(this, offset);
+  base::Relaxed_Store(reinterpret_cast<base::AtomicWord*>(field_addr),
+                      static_cast<base::AtomicWord>(value));
 }
 
 bool JSObject::IsUnboxedDoubleField(FieldIndex index) {

src/objects/js-objects.h

@@ -551,8 +551,10 @@ class JSObject : public JSReceiver {
   inline int GetEmbedderFieldCount() const;
   inline int GetEmbedderFieldOffset(int index);
   inline Object* GetEmbedderField(int index);
+  inline Address GetEmbedderFieldRaw(int index);
   inline void SetEmbedderField(int index, Object* value);
   inline void SetEmbedderField(int index, Smi value);
+  inline void SetEmbedderFieldRaw(int index, Address value);
 
   // Returns true when the object is potentially a wrapper that gets special
   // garbage collection treatment.

src/snapshot/partial-serializer.cc

@@ -47,7 +47,14 @@ void PartialSerializer::Serialize(Context** o, bool include_global_proxy) {
   VisitRootPointer(Root::kPartialSnapshotCache, nullptr,
                    ObjectSlot(reinterpret_cast<Address>(o)));
   SerializeDeferredObjects();
-  SerializeEmbedderFields();
+
+  // Add section for embedder-serialized embedder fields.
+  if (!embedder_fields_sink_.data()->empty()) {
+    sink_.Put(kEmbedderFieldsData, "embedder fields data");
+    sink_.Append(embedder_fields_sink_);
+    sink_.Put(kSynchronize, "Finished with embedder fields data");
+  }
+
   Pad();
 }
 
@@ -89,12 +96,8 @@ void PartialSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
   // Clear literal boilerplates and feedback.
   if (obj->IsFeedbackVector()) FeedbackVector::cast(obj)->ClearSlots(isolate());
 
-  if (obj->IsJSObject()) {
-    JSObject* jsobj = JSObject::cast(obj);
-    if (jsobj->GetEmbedderFieldCount() > 0) {
-      DCHECK_NOT_NULL(serialize_embedder_fields_.callback);
-      embedder_field_holders_.push_back(jsobj);
-    }
+  if (SerializeJSObjectWithEmbedderFields(obj, how_to_code, where_to_point)) {
+    return;
   }
 
   if (obj->IsJSFunction()) {
@@ -124,35 +127,94 @@ bool PartialSerializer::ShouldBeInThePartialSnapshotCache(HeapObject* o) {
                          .fixed_cow_array_map();
 }
 
-void PartialSerializer::SerializeEmbedderFields() {
-  if (embedder_field_holders_.empty()) return;
+namespace {
+bool DataIsEmpty(const StartupData& data) { return data.raw_size == 0; }
+}  // anonymous namespace
+
+bool PartialSerializer::SerializeJSObjectWithEmbedderFields(
+    Object* obj, HowToCode how_to_code, WhereToPoint where_to_point) {
+  if (!obj->IsJSObject()) return false;
+  JSObject* js_obj = JSObject::cast(obj);
+  int embedder_fields_count = js_obj->GetEmbedderFieldCount();
+  if (embedder_fields_count == 0) return false;
+  CHECK_GT(embedder_fields_count, 0);
+  DCHECK_NOT_NULL(serialize_embedder_fields_.callback);
+  DCHECK(!js_obj->NeedsRehashing());
+
   DisallowHeapAllocation no_gc;
   DisallowJavascriptExecution no_js(isolate());
   DisallowCompilation no_compile(isolate());
-  DCHECK_NOT_NULL(serialize_embedder_fields_.callback);
-  sink_.Put(kEmbedderFieldsData, "embedder fields data");
-  while (!embedder_field_holders_.empty()) {
+
   HandleScope scope(isolate());
-    Handle<JSObject> obj(embedder_field_holders_.back(), isolate());
-    embedder_field_holders_.pop_back();
-    SerializerReference reference = reference_map()->LookupReference(*obj);
-    DCHECK(reference.is_back_reference());
-    int embedder_fields_count = obj->GetEmbedderFieldCount();
-    for (int i = 0; i < embedder_fields_count; i++) {
-      if (obj->GetEmbedderField(i)->IsHeapObject()) continue;
+  Handle<JSObject> obj_handle(js_obj, isolate());
+  v8::Local<v8::Object> api_obj = v8::Utils::ToLocal(obj_handle);
+
+  std::vector<Address> original_values;
+  std::vector<StartupData> serialized_data;
 
+  // 1) Iterate embedder fields. Hold onto the original value of the fields.
+  //    Ignore references to heap objects since these are to be handled by the
+  //    serializer. For aligned pointers, call the serialize callback. Hold
+  //    onto the result.
+  for (int i = 0; i < embedder_fields_count; i++) {
+    Address address = js_obj->GetEmbedderFieldRaw(i);
+    original_values.push_back(address);
+    ObjectPtr object_ptr(address);
+    Object* object(object_ptr);
+    if (object->IsHeapObject()) {
+      DCHECK(isolate()->heap()->Contains(HeapObject::cast(object)));
+      serialized_data.push_back({nullptr, 0});
+    } else {
       StartupData data = serialize_embedder_fields_.callback(
-          v8::Utils::ToLocal(obj), i, serialize_embedder_fields_.data);
-      sink_.Put(kNewObject + reference.space(), "embedder field holder");
-      PutBackReference(*obj, reference);
-      sink_.PutInt(i, "embedder field index");
-      sink_.PutInt(data.raw_size, "embedder fields data size");
-      sink_.PutRaw(reinterpret_cast<const byte*>(data.data), data.raw_size,
-                   "embedder fields data");
-      delete[] data.data;
+          api_obj, i, serialize_embedder_fields_.data);
+      serialized_data.push_back(data);
     }
   }
-  sink_.Put(kSynchronize, "Finished with embedder fields data");
+
+  // 2) Embedder fields for which the embedder callback produced non-zero
+  //    serialized data should be considered aligned pointers to objects owned
+  //    by the embedder. Clear these memory addresses to avoid non-determism
+  //    in the snapshot. This is done separately to step 1 to no not interleave
+  //    with embedder callbacks.
+  for (int i = 0; i < embedder_fields_count; i++) {
+    if (!DataIsEmpty(serialized_data[i])) {
+      js_obj->SetEmbedderFieldRaw(i, kNullAddress);
+    }
+  }
+
+  // 3) Serialize the object. References from embedder fields to heap objects or
+  //    smis are serialized regularly.
+  ObjectSerializer(this, js_obj, &sink_, how_to_code, where_to_point)
+      .Serialize();
+
+  // 4) Obtain back reference for the serialized object.
+  SerializerReference reference = reference_map()->LookupReference(js_obj);
+  DCHECK(reference.is_back_reference());
+
+  // 5) Write data returned by the embedder callbacks into a separate sink,
+  //    headed by the back reference. Restore the original embedder fields.
+  for (int i = 0; i < embedder_fields_count; i++) {
+    // Restore original values from potentially cleared fields.
+    js_obj->SetEmbedderFieldRaw(i, original_values[i]);
+    StartupData data = serialized_data[i];
+    if (DataIsEmpty(data)) continue;
+    embedder_fields_sink_.Put(kNewObject + reference.space(),
+                              "embedder field holder");
+    embedder_fields_sink_.PutInt(reference.chunk_index(), "BackRefChunkIndex");
+    embedder_fields_sink_.PutInt(reference.chunk_offset(),
+                                 "BackRefChunkOffset");
+    embedder_fields_sink_.PutInt(i, "embedder field index");
+    embedder_fields_sink_.PutInt(data.raw_size, "embedder fields data size");
+    embedder_fields_sink_.PutRaw(reinterpret_cast<const byte*>(data.data),
+                                 data.raw_size, "embedder fields data");
+    delete[] data.data;
+  }
+
+  // 6) The content of the separate sink is appended eventually to the default
+  //    sink. The ensures that during deserialization, we call the deserializer
+  //    callback at the end, and can guarantee that the deserialized objects are
+  //    in a consistent state. See PartialSerializer::Serialize.
+  return true;
 }
 
 void PartialSerializer::CheckRehashability(HeapObject* obj) {

src/snapshot/partial-serializer.h

@@ -31,17 +31,20 @@ class PartialSerializer : public Serializer {
 
   bool ShouldBeInThePartialSnapshotCache(HeapObject* o);
 
-  void SerializeEmbedderFields();
+  bool SerializeJSObjectWithEmbedderFields(Object* obj, HowToCode how_to_code,
+                                           WhereToPoint where_to_point);
 
   void CheckRehashability(HeapObject* obj);
 
   StartupSerializer* startup_serializer_;
-  std::vector<JSObject*> embedder_field_holders_;
   v8::SerializeEmbedderFieldsCallback serialize_embedder_fields_;
   // Indicates whether we only serialized hash tables that we can rehash.
   // TODO(yangguo): generalize rehashing, and remove this flag.
   bool can_be_rehashed_;
   Context* context_;
+
+  // Used to store serialized data for embedder fields.
+  SnapshotByteSink embedder_fields_sink_;
   DISALLOW_COPY_AND_ASSIGN(PartialSerializer);
 };
 

src/snapshot/snapshot-source-sink.cc

@@ -32,6 +32,9 @@ void SnapshotByteSink::PutRaw(const byte* data, int number_of_bytes,
   data_.insert(data_.end(), data, data + number_of_bytes);
 }
 
+void SnapshotByteSink::Append(const SnapshotByteSink& other) {
+  data_.insert(data_.end(), other.data_.begin(), other.data_.end());
+}
 
 int SnapshotByteSource::GetBlob(const byte** data) {
   int size = GetInt();

src/snapshot/snapshot-source-sink.h

@@ -96,6 +96,8 @@ class SnapshotByteSink {
 
   void PutInt(uintptr_t integer, const char* description);
   void PutRaw(const byte* data, int number_of_bytes, const char* description);
+
+  void Append(const SnapshotByteSink& other);
   int Position() const { return static_cast<int>(data_.size()); }
 
   const std::vector<byte>* data() const { return &data_; }

test/cctest/test-serialize.cc

@@ -798,7 +798,14 @@ struct InternalFieldData {
 
 v8::StartupData SerializeInternalFields(v8::Local<v8::Object> holder, int index,
                                         void* data) {
+  if (data == reinterpret_cast<void*>(2000)) {
+    // Used for SnapshotCreatorTemplates test. We check that none of the fields
+    // have been cleared yet.
+    CHECK_NOT_NULL(holder->GetAlignedPointerFromInternalField(1));
+  } else {
     CHECK_EQ(reinterpret_cast<void*>(2016), data);
+  }
+  if (index != 1) return {nullptr, 0};
   InternalFieldData* embedder_field = static_cast<InternalFieldData*>(
       holder->GetAlignedPointerFromInternalField(index));
   if (embedder_field == nullptr) return {nullptr, 0};
@@ -2879,8 +2886,8 @@ TEST(SnapshotCreatorTemplates) {
 
   {
     InternalFieldData* a1 = new InternalFieldData{11};
-    InternalFieldData* b0 = new InternalFieldData{20};
-    InternalFieldData* c0 = new InternalFieldData{30};
+    InternalFieldData* b1 = new InternalFieldData{20};
+    InternalFieldData* c1 = new InternalFieldData{30};
 
     v8::SnapshotCreator creator(original_external_references);
     v8::Isolate* isolate = creator.GetIsolate();
@@ -2916,19 +2923,23 @@ TEST(SnapshotCreatorTemplates) {
           v8::External::New(isolate, nullptr);
       v8::Local<v8::External> field_external =
           v8::External::New(isolate, &serialized_static_field);
+
       a->SetInternalField(0, b);
+      b->SetInternalField(0, c);
+
       a->SetAlignedPointerInInternalField(1, a1);
-      b->SetAlignedPointerInInternalField(0, b0);
-      b->SetInternalField(1, c);
-      c->SetAlignedPointerInInternalField(0, c0);
-      c->SetInternalField(1, null_external);
-      c->SetInternalField(2, field_external);
+      b->SetAlignedPointerInInternalField(1, b1);
+      c->SetAlignedPointerInInternalField(1, c1);
+
+      a->SetInternalField(2, null_external);
+      b->SetInternalField(2, field_external);
+      c->SetInternalField(2, v8_num(35));
       CHECK(context->Global()->Set(context, v8_str("a"), a).FromJust());
 
       CHECK_EQ(0u,
                creator.AddContext(context, v8::SerializeInternalFieldsCallback(
                                                SerializeInternalFields,
-                                               reinterpret_cast<void*>(2016))));
+                                               reinterpret_cast<void*>(2000))));
       CHECK_EQ(0u, creator.AddTemplate(callback));
       CHECK_EQ(1u, creator.AddTemplate(global_template));
     }
@@ -2936,8 +2947,8 @@ TEST(SnapshotCreatorTemplates) {
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
 
     delete a1;
-    delete b0;
-    delete c0;
+    delete b1;
+    delete c1;
   }
 
   {
@@ -2993,30 +3004,34 @@ TEST(SnapshotCreatorTemplates) {
                                       .ToLocalChecked();
         v8::Local<v8::Object> b =
             a->GetInternalField(0)->ToObject(context).ToLocalChecked();
+        v8::Local<v8::Object> c =
+            b->GetInternalField(0)->ToObject(context).ToLocalChecked();
+
         InternalFieldData* a1 = reinterpret_cast<InternalFieldData*>(
             a->GetAlignedPointerFromInternalField(1));
         v8::Local<v8::Value> a2 = a->GetInternalField(2);
 
-        InternalFieldData* b0 = reinterpret_cast<InternalFieldData*>(
-            b->GetAlignedPointerFromInternalField(0));
-        v8::Local<v8::Object> c =
-            b->GetInternalField(1)->ToObject(context).ToLocalChecked();
+        InternalFieldData* b1 = reinterpret_cast<InternalFieldData*>(
+            b->GetAlignedPointerFromInternalField(1));
         v8::Local<v8::Value> b2 = b->GetInternalField(2);
 
-        InternalFieldData* c0 = reinterpret_cast<InternalFieldData*>(
-            c->GetAlignedPointerFromInternalField(0));
-        v8::Local<v8::Value> c1 = c->GetInternalField(1);
+        v8::Local<v8::Value> c0 = c->GetInternalField(0);
+        InternalFieldData* c1 = reinterpret_cast<InternalFieldData*>(
+            c->GetAlignedPointerFromInternalField(1));
         v8::Local<v8::Value> c2 = c->GetInternalField(2);
 
+        CHECK(c0->IsUndefined());
+
         CHECK_EQ(11u, a1->data);
-        CHECK(a2->IsUndefined());
-        CHECK_EQ(20u, b0->data);
-        CHECK(b2->IsUndefined());
-        CHECK_EQ(30u, c0->data);
-        CHECK(c1->IsExternal());
-        CHECK_NULL(v8::Local<v8::External>::Cast(c1)->Value());
+        CHECK_EQ(20u, b1->data);
+        CHECK_EQ(30u, c1->data);
+
+        CHECK(a2->IsExternal());
+        CHECK_NULL(v8::Local<v8::External>::Cast(a2)->Value());
+        CHECK(b2->IsExternal());
         CHECK_EQ(static_cast<void*>(&serialized_static_field),
-                 v8::Local<v8::External>::Cast(c2)->Value());
+                 v8::Local<v8::External>::Cast(b2)->Value());
+        CHECK(c2->IsInt32() && c2->Int32Value(context).FromJust() == 35);
 
         // Accessing out of bound returns empty MaybeHandle.
         CHECK(v8::ObjectTemplate::FromSnapshot(isolate, 2).IsEmpty());