[serializer] ensure that immortal immovable roots are correctly deserialized.

Immortal immovable roots must be allocated on the first page of the space.
If serializing the root list exceeds the first page, immortal immovable root
objects might end up outside of the first page. That could cause missing
write barriers.

We now iterate the root list twice. The first time we only serialize immortal
immovable root objects. The second time we serialize the rest.

R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/1811913002

Cr-Commit-Position: refs/heads/master@{#34859}

src/compiler.cc

@@ -1727,8 +1727,8 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
     // On the first compile, there are no existing shared function info for
     // inner functions yet, so do not try to find them. All bets are off for
     // live edit though.
-    DCHECK(script->FindSharedFunctionInfo(literal).is_null() ||
-           isolate->debug()->live_edit_enabled());
+    SLOW_DCHECK(script->FindSharedFunctionInfo(literal).is_null() ||
+                isolate->debug()->live_edit_enabled());
   } else {
     maybe_existing = script->FindSharedFunctionInfo(literal);
   }

src/globals.h

@@ -476,7 +476,8 @@ enum VisitMode {
   VISIT_ALL_IN_SCAVENGE,
   VISIT_ALL_IN_SWEEP_NEWSPACE,
   VISIT_ONLY_STRONG,
-  VISIT_ONLY_STRONG_FOR_SERIALIZATION
+  VISIT_ONLY_STRONG_FOR_SERIALIZATION,
+  VISIT_ONLY_STRONG_ROOT_LIST,
 };
 
 // Flag indicating whether code is built into the VM (one of the natives files).

src/heap/heap.cc

@@ -4737,6 +4737,10 @@ void Heap::IterateSmiRoots(ObjectVisitor* v) {
 void Heap::IterateStrongRoots(ObjectVisitor* v, VisitMode mode) {
   v->VisitPointers(&roots_[0], &roots_[kStrongRootListLength]);
   v->Synchronize(VisitorSynchronization::kStrongRootList);
+  // The serializer/deserializer iterates the root list twice, first to pick
+  // off immortal immovable roots to make sure they end up on the first page,
+  // and then again for the rest.
+  if (mode == VISIT_ONLY_STRONG_ROOT_LIST) return;
 
   isolate_->bootstrapper()->Iterate(v);
   v->Synchronize(VisitorSynchronization::kBootstrapper);
@@ -4765,6 +4769,9 @@ void Heap::IterateStrongRoots(ObjectVisitor* v, VisitMode mode) {
 
   // Iterate over global handles.
   switch (mode) {
+    case VISIT_ONLY_STRONG_ROOT_LIST:
+      UNREACHABLE();
+      break;
     case VISIT_ONLY_STRONG:
     case VISIT_ONLY_STRONG_FOR_SERIALIZATION:
       isolate_->global_handles()->IterateStrongRoots(v);

src/heap/heap.h

@@ -272,6 +272,10 @@ namespace internal {
   V(JSMessageObjectMap)                 \
   V(ForeignMap)                         \
   V(NeanderMap)                         \
+  V(NanValue)                           \
+  V(InfinityValue)                      \
+  V(MinusZeroValue)                     \
+  V(MinusInfinityValue)                 \
   V(EmptyWeakCell)                      \
   V(empty_string)                       \
   PRIVATE_SYMBOL_LIST(V)

src/objects.cc

@@ -13491,7 +13491,7 @@ void SharedFunctionInfo::SetScript(Handle<SharedFunctionInfo> shared,
   }
 
 #ifdef DEBUG
-  {
+  if (FLAG_enable_slow_asserts) {
     WeakFixedArray::Iterator iterator(*list);
     SharedFunctionInfo* next;
     while ((next = iterator.Next<SharedFunctionInfo>())) {

src/snapshot/deserializer.cc

@@ -83,6 +83,7 @@ void Deserializer::Deserialize(Isolate* isolate) {
 
   {
     DisallowHeapAllocation no_gc;
+    isolate_->heap()->IterateStrongRoots(this, VISIT_ONLY_STRONG_ROOT_LIST);
     isolate_->heap()->IterateSmiRoots(this);
     isolate_->heap()->IterateStrongRoots(this, VISIT_ONLY_STRONG);
     isolate_->heap()->RepairFreeListsAfterDeserialization();

src/snapshot/serializer.cc

@@ -96,20 +96,10 @@ void Serializer::SerializeDeferredObjects() {
   sink_->Put(kSynchronize, "Finished with deferred objects");
 }
 
-bool Serializer::ShouldBeSkipped(Object** current) {
-  Object** roots = isolate()->heap()->roots_array_start();
-  return current == &roots[Heap::kStoreBufferTopRootIndex] ||
-         current == &roots[Heap::kStackLimitRootIndex] ||
-         current == &roots[Heap::kRealStackLimitRootIndex];
-}
-
 void Serializer::VisitPointers(Object** start, Object** end) {
   for (Object** current = start; current < end; current++) {
     if ((*current)->IsSmi()) {
-      sink_->Put(kOnePointerRawData, "Smi");
-      for (int i = 0; i < kPointerSize; i++) {
-        sink_->Put(reinterpret_cast<byte*>(current)[i], "Byte");
-      }
+      PutSmi(Smi::cast(*current));
     } else {
       SerializeObject(HeapObject::cast(*current), kPlain, kStartOfObject, 0);
     }
@@ -240,6 +230,12 @@ void Serializer::PutRoot(int root_index, HeapObject* object,
   }
 }
 
+void Serializer::PutSmi(Smi* smi) {
+  sink_->Put(kOnePointerRawData, "Smi");
+  byte* bytes = reinterpret_cast<byte*>(&smi);
+  for (int i = 0; i < kPointerSize; i++) sink_->Put(bytes[i], "Byte");
+}
+
 void Serializer::PutBackReference(HeapObject* object, BackReference reference) {
   DCHECK(BackReferenceIsAlreadyAllocated(reference));
   sink_->PutInt(reference.reference(), "BackRefValue");
@@ -308,6 +304,13 @@ Code* Serializer::CopyCode(Code* code) {
   return Code::cast(HeapObject::FromAddress(&code_buffer_.first()));
 }
 
+bool Serializer::HasNotExceededFirstPageOfEachSpace() {
+  for (int i = 0; i < kNumberOfPreallocatedSpaces; i++) {
+    if (!completed_chunks_[i].is_empty()) return false;
+  }
+  return true;
+}
+
 void Serializer::ObjectSerializer::SerializePrologue(AllocationSpace space,
                                                      int size, Map* map) {
   if (serializer_->code_address_map_) {

src/snapshot/serializer.h

@@ -155,9 +155,13 @@ class Serializer : public SerializerDeserializer {
   virtual void SerializeObject(HeapObject* o, HowToCode how_to_code,
                                WhereToPoint where_to_point, int skip) = 0;
 
+  void VisitPointers(Object** start, Object** end) override;
+
   void PutRoot(int index, HeapObject* object, HowToCode how, WhereToPoint where,
                int skip);
 
+  void PutSmi(Smi* smi);
+
   void PutBackReference(HeapObject* object, BackReference reference);
 
   // Emit alignment prefix if necessary, return required padding space in bytes.
@@ -183,13 +187,11 @@ class Serializer : public SerializerDeserializer {
     return external_reference_encoder_.Encode(addr);
   }
 
+  bool HasNotExceededFirstPageOfEachSpace();
+
   // GetInt reads 4 bytes at once, requiring padding at the end.
   void Pad();
 
-  // Some roots should not be serialized, because their actual value depends on
-  // absolute addresses and they are reset after deserialization, anyway.
-  bool ShouldBeSkipped(Object** current);
-
   // We may not need the code address map for logging for every instance
   // of the serializer.  Initialize it on demand.
   void InitializeCodeAddressMap();
@@ -227,8 +229,6 @@ class Serializer : public SerializerDeserializer {
   friend class SnapshotData;
 
  private:
-  void VisitPointers(Object** start, Object** end) override;
-
   CodeAddressMap* code_address_map_;
   // Objects from the same space are put into chunks for bulk-allocation
   // when deserializing. We have to make sure that each chunk fits into a

src/snapshot/startup-serializer.cc

@@ -14,9 +14,8 @@ StartupSerializer::StartupSerializer(
     Isolate* isolate, SnapshotByteSink* sink,
     FunctionCodeHandling function_code_handling)
     : Serializer(isolate, sink),
-      root_index_wave_front_(0),
-      serializing_builtins_(false),
-      function_code_handling_(function_code_handling) {
+      function_code_handling_(function_code_handling),
+      serializing_builtins_(false) {
   InitializeCodeAddressMap();
 }
 
@@ -43,15 +42,12 @@ void StartupSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
   }
 
   int root_index = root_index_map_.Lookup(obj);
-  bool is_immortal_immovable_root = false;
   // We can only encode roots as such if it has already been serialized.
   // That applies to root indices below the wave front.
   if (root_index != RootIndexMap::kInvalidRootIndex) {
-    if (root_index < root_index_wave_front_) {
+    if (root_has_been_serialized_.test(root_index)) {
       PutRoot(root_index, obj, how_to_code, where_to_point, skip);
       return;
-    } else {
-      is_immortal_immovable_root = Heap::RootIsImmortalImmovable(root_index);
     }
   }
 
@@ -64,7 +60,8 @@ void StartupSerializer::SerializeObject(HeapObject* obj, HowToCode how_to_code,
                                      where_to_point);
   object_serializer.Serialize();
 
-  if (is_immortal_immovable_root) {
+  if (serializing_immortal_immovables_roots_ &&
+      root_index != RootIndexMap::kInvalidRootIndex) {
     // Make sure that the immortal immovable root has been included in the first
     // chunk of its reserved space , so that it is deserialized onto the first
     // page of its space and stays immortal immovable.
@@ -101,29 +98,57 @@ void StartupSerializer::SerializeStrongReferences() {
   CHECK_EQ(0, isolate->eternal_handles()->NumberOfHandles());
   // We don't support serializing installed extensions.
   CHECK(!isolate->has_installed_extensions());
+  // First visit immortal immovables to make sure they end up in the first page.
+  serializing_immortal_immovables_roots_ = true;
+  isolate->heap()->IterateStrongRoots(this, VISIT_ONLY_STRONG_ROOT_LIST);
+  // Check that immortal immovable roots are allocated on the first page.
+  CHECK(HasNotExceededFirstPageOfEachSpace());
+  serializing_immortal_immovables_roots_ = false;
+  // Visit the rest of the strong roots.
   isolate->heap()->IterateSmiRoots(this);
   isolate->heap()->IterateStrongRoots(this,
                                       VISIT_ONLY_STRONG_FOR_SERIALIZATION);
 }
 
 void StartupSerializer::VisitPointers(Object** start, Object** end) {
-  for (Object** current = start; current < end; current++) {
-    if (start == isolate()->heap()->roots_array_start()) {
-      root_index_wave_front_ =
-          Max(root_index_wave_front_, static_cast<intptr_t>(current - start));
-    }
-    if (ShouldBeSkipped(current)) {
-      sink_->Put(kSkip, "Skip");
-      sink_->PutInt(kPointerSize, "SkipOneWord");
-    } else if ((*current)->IsSmi()) {
-      sink_->Put(kOnePointerRawData, "Smi");
-      for (int i = 0; i < kPointerSize; i++) {
-        sink_->Put(reinterpret_cast<byte*>(current)[i], "Byte");
+  if (start == isolate()->heap()->roots_array_start()) {
+    // Serializing the root list needs special handling:
+    // - The first pass over the root list only serializes immortal immovables.
+    // - The second pass over the root list serializes the rest.
+    // - Only root list elements that have been fully serialized can be
+    //   referenced via as root by using kRootArray bytecodes.
+    int skip = 0;
+    for (Object** current = start; current < end; current++) {
+      int root_index = static_cast<int>(current - start);
+      if (RootShouldBeSkipped(root_index)) {
+        skip += kPointerSize;
+        continue;
+      } else {
+        if ((*current)->IsSmi()) {
+          FlushSkip(skip);
+          PutSmi(Smi::cast(*current));
+        } else {
+          SerializeObject(HeapObject::cast(*current), kPlain, kStartOfObject,
+                          skip);
+        }
+        root_has_been_serialized_.set(root_index);
+        skip = 0;
       }
-    } else {
-      SerializeObject(HeapObject::cast(*current), kPlain, kStartOfObject, 0);
     }
+    FlushSkip(skip);
+  } else {
+    Serializer::VisitPointers(start, end);
+  }
+}
+
+bool StartupSerializer::RootShouldBeSkipped(int root_index) {
+  if (root_index == Heap::kStoreBufferTopRootIndex ||
+      root_index == Heap::kStackLimitRootIndex ||
+      root_index == Heap::kRealStackLimitRootIndex) {
+    return true;
   }
+  return Heap::RootIsImmortalImmovable(root_index) !=
+         serializing_immortal_immovables_roots_;
 }
 
 }  // namespace internal

src/snapshot/startup-serializer.h

@@ -5,6 +5,7 @@
 #ifndef V8_SNAPSHOT_STARTUP_SERIALIZER_H_
 #define V8_SNAPSHOT_STARTUP_SERIALIZER_H_
 
+#include <bitset>
 #include "src/snapshot/serializer.h"
 
 namespace v8 {
@@ -20,9 +21,10 @@ class StartupSerializer : public Serializer {
   ~StartupSerializer() override;
 
   // Serialize the current state of the heap.  The order is:
-  // 1) Strong references.
-  // 2) Partial snapshot cache.
-  // 3) Weak references (e.g. the string table).
+  // 1) Immortal immovable roots
+  // 2) Remaining strong references.
+  // 3) Partial snapshot cache.
+  // 4) Weak references (e.g. the string table).
   void SerializeStrongReferences();
   void SerializeWeakReferencesAndDeferred();
 
@@ -34,9 +36,16 @@ class StartupSerializer : public Serializer {
                        WhereToPoint where_to_point, int skip) override;
   void Synchronize(VisitorSynchronization::SyncTag tag) override;
 
-  intptr_t root_index_wave_front_;
-  bool serializing_builtins_;
+  // Some roots should not be serialized, because their actual value depends on
+  // absolute addresses and they are reset after deserialization, anyway.
+  // In the first pass over the root list, we only serialize immortal immovable
+  // roots. In the second pass, we serialize the rest.
+  bool RootShouldBeSkipped(int root_index);
+
   FunctionCodeHandling function_code_handling_;
+  bool serializing_builtins_;
+  bool serializing_immortal_immovables_roots_;
+  std::bitset<Heap::kStrongRootListLength> root_has_been_serialized_;
   DISALLOW_COPY_AND_ASSIGN(StartupSerializer);
 };
 

test/cctest/cctest.status

@@ -130,6 +130,7 @@
   'test-api/Threading4': [PASS, ['mode == debug', SLOW]],
   'test-debug/CallFunctionInDebugger': [PASS, ['mode == debug', SLOW]],
   'test-strings/StringOOM*': [PASS, ['mode == debug', SKIP]],
+  'test-serialize/CustomSnapshotDataBlobImmortalImmovableRoots': [PASS, ['mode == debug', SKIP]],
 }],  # ALWAYS
 
 ##############################################################################

test/cctest/test-serialize.cc

@@ -854,6 +854,36 @@ TEST(CustomSnapshotDataBlobWithWarmup) {
   isolate->Dispose();
 }
 
+TEST(CustomSnapshotDataBlobImmortalImmovableRoots) {
+  DisableTurbofan();
+  // Flood the startup snapshot with shared function infos. If they are
+  // serialized before the immortal immovable root, the root will no longer end
+  // up on the first page.
+  Vector<const uint8_t> source =
+      ConstructSource(STATIC_CHAR_VECTOR("var a = [];"),
+                      STATIC_CHAR_VECTOR("a.push(function() {return 7});"),
+                      STATIC_CHAR_VECTOR("\0"), 10000);
+
+  v8::StartupData data = v8::V8::CreateSnapshotDataBlob(
+      reinterpret_cast<const char*>(source.start()));
+
+  v8::Isolate::CreateParams params;
+  params.snapshot_blob = &data;
+  params.array_buffer_allocator = CcTest::array_buffer_allocator();
+
+  v8::Isolate* isolate = v8::Isolate::New(params);
+  {
+    v8::Isolate::Scope i_scope(isolate);
+    v8::HandleScope h_scope(isolate);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    delete[] data.data;  // We can dispose of the snapshot blob now.
+    v8::Context::Scope c_scope(context);
+    CHECK_EQ(7, CompileRun("a[0]()")->Int32Value(context).FromJust());
+  }
+  isolate->Dispose();
+  source.Dispose();
+}
+
 TEST(TestThatAlwaysSucceeds) {
 }