Reland "[roheap] Check that ro-heap is always passed the same read-only snapshot"

This is a reland of a5fa211f

des_checksum and call_once_run were undefined and unused respectively when
shared read-only heap was enabled. Fixed with a copious amounts of USE.

Original change's description:
> [roheap] Check that ro-heap is always passed the same read-only snapshot
>
> Previously the ReadOnlyHeap simply discarded all but the first
> ReadOnlyDeseralizer. ClearSharedHeapForTest should be called if using a
> new ReadOnlyDeserializer (this might change in the future).
>
> Remove an obsolete 'StartupSerializerRootMapDependencies' test. It used
> to test Map::WeakCellForMap which doesn't exist anymore and was
> difficult to adapt to a shared read-only heap.
>
> Bug: v8:7464
> Change-Id: I64b8e953b0e3466e003541ec8a9321e439a01d33
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1660612
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Dan Elphick <delphick@chromium.org>
> Commit-Queue: Maciej Goszczycki <goszczycki@google.com>
> Cr-Commit-Position: refs/heads/master@{#62250}

TBR: yangguo@chromium.org
Bug: v8:7464
Change-Id: Id66e781be890c5ed03d066f8c62de703d5cb435e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1667415Reviewed-by: Dan Elphick <delphick@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62302}

src/heap/read-only-heap.cc

@@ -28,16 +28,39 @@ ReadOnlyHeap* ReadOnlyHeap::shared_ro_heap_ = nullptr;
 void ReadOnlyHeap::SetUp(Isolate* isolate, ReadOnlyDeserializer* des) {
   DCHECK_NOT_NULL(isolate);
 #ifdef V8_SHARED_RO_HEAP
-  // Make sure we are only sharing read-only space when deserializing. Otherwise
-  // we would be trying to create heap objects inside an already initialized
-  // read-only space. Use ClearSharedHeapForTest if you need a new read-only
-  // space.
-  DCHECK_IMPLIES(shared_ro_heap_ != nullptr, des != nullptr);
-
-  base::CallOnce(&setup_ro_heap_once, [isolate, des]() {
-    shared_ro_heap_ = CreateAndAttachToIsolate(isolate);
-    if (des != nullptr) shared_ro_heap_->DeseralizeIntoIsolate(isolate, des);
-  });
+  bool call_once_ran = false;
+  base::Optional<Checksum> des_checksum;
+#ifdef DEBUG
+  if (des != nullptr) des_checksum = des->GetChecksum();
+#endif  // DEBUG
+
+  base::CallOnce(&setup_ro_heap_once,
+                 [isolate, des, des_checksum, &call_once_ran]() {
+                   shared_ro_heap_ = CreateAndAttachToIsolate(isolate);
+                   if (des != nullptr) {
+#ifdef DEBUG
+                     shared_ro_heap_->read_only_blob_checksum_ = des_checksum;
+#endif  // DEBUG
+                     shared_ro_heap_->DeseralizeIntoIsolate(isolate, des);
+                   }
+                   call_once_ran = true;
+                 });
+
+  USE(call_once_ran);
+  USE(des_checksum);
+#ifdef DEBUG
+  const base::Optional<Checksum> last_checksum =
+      shared_ro_heap_->read_only_blob_checksum_;
+  if (last_checksum || des_checksum) {
+    // The read-only heap was set up from a snapshot. Make sure it's the always
+    // the same snapshot.
+    CHECK_EQ(last_checksum, des_checksum);
+  } else {
+    // The read-only heap objects were created. Make sure this happens only
+    // once, during this call.
+    CHECK(call_once_ran);
+  }
+#endif  // DEBUG
 
   isolate->heap()->SetUpFromReadOnlyHeap(shared_ro_heap_);
   if (des != nullptr) {

src/heap/read-only-heap.h

@@ -5,7 +5,10 @@
 #ifndef V8_HEAP_READ_ONLY_HEAP_H_
 #define V8_HEAP_READ_ONLY_HEAP_H_
 
+#include <utility>
+
 #include "src/base/macros.h"
+#include "src/base/optional.h"
 #include "src/objects/heap-object.h"
 #include "src/objects/objects.h"
 #include "src/roots/roots.h"
@@ -61,6 +64,8 @@ class ReadOnlyHeap final {
   ReadOnlySpace* read_only_space() const { return read_only_space_; }
 
  private:
+  using Checksum = std::pair<uint32_t, uint32_t>;
+
   // Creates a new read-only heap and attaches it to the provided isolate.
   static ReadOnlyHeap* CreateAndAttachToIsolate(Isolate* isolate);
   // Runs the read-only deserailizer and calls InitFromIsolate to complete
@@ -77,10 +82,15 @@ class ReadOnlyHeap final {
   std::vector<Object> read_only_object_cache_;
 
 #ifdef V8_SHARED_RO_HEAP
+#ifdef DEBUG
+  // The checksum of the blob the read-only heap was deserialized from, if any.
+  base::Optional<Checksum> read_only_blob_checksum_;
+#endif  // DEBUG
+
   Address read_only_roots_[kEntriesCount];
 
   V8_EXPORT_PRIVATE static ReadOnlyHeap* shared_ro_heap_;
-#endif
+#endif  // V8_SHARED_RO_HEAP
 
   explicit ReadOnlyHeap(ReadOnlySpace* ro_space) : read_only_space_(ro_space) {}
   DISALLOW_COPY_AND_ASSIGN(ReadOnlyHeap);

src/snapshot/deserializer.h

@@ -5,6 +5,7 @@
 #ifndef V8_SNAPSHOT_DESERIALIZER_H_
 #define V8_SNAPSHOT_DESERIALIZER_H_
 
+#include <utility>
 #include <vector>
 
 #include "src/objects/allocation-site.h"
@@ -39,6 +40,9 @@ class V8_EXPORT_PRIVATE Deserializer : public SerializerDeserializer {
   ~Deserializer() override;
 
   void SetRehashability(bool v) { can_rehash_ = v; }
+  std::pair<uint32_t, uint32_t> GetChecksum() const {
+    return source_.GetChecksum();
+  }
 
  protected:
   // Create a deserializer from a snapshot byte source.

src/snapshot/snapshot-source-sink.h

@@ -5,7 +5,10 @@
 #ifndef V8_SNAPSHOT_SNAPSHOT_SOURCE_SINK_H_
 #define V8_SNAPSHOT_SNAPSHOT_SOURCE_SINK_H_
 
+#include <utility>
+
 #include "src/base/logging.h"
+#include "src/snapshot/serializer-common.h"
 #include "src/utils/utils.h"
 
 namespace v8 {
@@ -66,6 +69,11 @@ class SnapshotByteSource final {
   int position() { return position_; }
   void set_position(int position) { position_ = position; }
 
+  std::pair<uint32_t, uint32_t> GetChecksum() const {
+    Checksum checksum(Vector<const byte>(data_, length_));
+    return {checksum.a(), checksum.b()};
+  }
+
  private:
   const byte* data_;
   int length_;

test/cctest/test-serialize.cc

@@ -148,8 +148,10 @@ namespace {
 
 // Convenience wrapper around the convenience wrapper.
 v8::StartupData CreateSnapshotDataBlob(const char* embedded_source) {
-  return CreateSnapshotDataBlobInternal(
+  v8::StartupData data = CreateSnapshotDataBlobInternal(
       v8::SnapshotCreator::FunctionCodeHandling::kClear, embedded_source);
+  ReadOnlyHeap::ClearSharedHeapForTest();
+  return data;
 }
 
 }  // namespace
@@ -275,53 +277,6 @@ UNINITIALIZED_TEST(StartupSerializerOnce32K) {
   TestStartupSerializerOnceImpl();
 }
 
-UNINITIALIZED_TEST(StartupSerializerRootMapDependencies) {
-  DisableAlwaysOpt();
-  v8::SnapshotCreator snapshot_creator;
-  v8::Isolate* isolate = snapshot_creator.GetIsolate();
-  {
-    v8::Isolate::Scope isolate_scope(isolate);
-    v8::HandleScope handle_scope(isolate);
-    Isolate* internal_isolate = reinterpret_cast<Isolate*>(isolate);
-    // Here is interesting retaining path:
-    // - FreeSpaceMap
-    // - Map for Map types itself
-    // - NullValue
-    // - Internalized one byte string
-    // - Map for Internalized one byte string
-    // - TheHoleValue
-    // - HeapNumber
-    // HeapNumber objects require kDoubleUnaligned on 32-bit
-    // platforms. So, without special measures we're risking to serialize
-    // object, requiring alignment before FreeSpaceMap is fully serialized.
-    v8::internal::Handle<Map> map(
-        ReadOnlyRoots(internal_isolate).one_byte_internalized_string_map(),
-        internal_isolate);
-    // Need to avoid DCHECKs inside SnapshotCreator.
-    snapshot_creator.SetDefaultContext(v8::Context::New(isolate));
-  }
-
-  v8::StartupData startup_data = snapshot_creator.CreateBlob(
-      v8::SnapshotCreator::FunctionCodeHandling::kKeep);
-
-  v8::Isolate::CreateParams params;
-  params.snapshot_blob = &startup_data;
-  params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  isolate = v8::Isolate::New(params);
-
-  {
-    v8::HandleScope handle_scope(isolate);
-    v8::Isolate::Scope isolate_scope(isolate);
-
-    v8::Local<v8::Context> env = v8::Context::New(isolate);
-    env->Enter();
-
-    SanityCheck(isolate);
-  }
-  isolate->Dispose();
-  delete[] startup_data.data;
-}
-
 UNINITIALIZED_TEST(StartupSerializerTwice) {
   DisableAlwaysOpt();
   v8::Isolate* isolate = TestSerializer::NewIsolateInitialized();
@@ -811,6 +766,7 @@ void TestCustomSnapshotDataBlobWithIrregexpCode(
   DisableEmbeddedBlobRefcounting();
   v8::StartupData data1 =
       CreateSnapshotDataBlobInternal(function_code_handling, source);
+  ReadOnlyHeap::ClearSharedHeapForTest();
 
   v8::Isolate::CreateParams params1;
   params1.snapshot_blob = &data1;
@@ -958,6 +914,7 @@ void TypedArrayTestHelper(
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   v8::Isolate::CreateParams create_params;
   create_params.snapshot_blob = &blob;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -1085,6 +1042,7 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobDetachedArrayBuffer) {
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   v8::Isolate::CreateParams create_params;
   create_params.snapshot_blob = &blob;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -1155,6 +1113,7 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   v8::Isolate::CreateParams create_params;
   create_params.snapshot_blob = &blob;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -1208,6 +1167,7 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobTypedArrayNoEmbedderFieldCallback) {
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   v8::Isolate::CreateParams create_params;
   create_params.snapshot_blob = &blob;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2591,6 +2551,7 @@ UNINITIALIZED_TEST(SnapshotCreatorMultipleContexts) {
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   v8::Isolate::CreateParams params;
   params.snapshot_blob = &blob;
   params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2729,6 +2690,7 @@ UNINITIALIZED_TEST(SnapshotCreatorExternalReferences) {
 
   // Deserialize with the original external reference.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2754,6 +2716,7 @@ UNINITIALIZED_TEST(SnapshotCreatorExternalReferences) {
 
   // Deserialize with some other external reference.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2802,6 +2765,7 @@ UNINITIALIZED_TEST(SnapshotCreatorShortExternalReferences) {
 
   // Deserialize with an incomplete list of external references.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2862,6 +2826,7 @@ UNINITIALIZED_TEST(SnapshotCreatorNoExternalReferencesDefault) {
 
   // Deserialize with an incomplete list of external references.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2911,6 +2876,7 @@ UNINITIALIZED_TEST(SnapshotCreatorPreparseDataAndNoOuterScope) {
 
   // Deserialize with an incomplete list of external references.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -2950,6 +2916,7 @@ UNINITIALIZED_TEST(SnapshotCreatorArrayJoinWithKeep) {
   DisableAlwaysOpt();
   DisableEmbeddedBlobRefcounting();
   v8::StartupData blob = CreateCustomSnapshotArrayJoinWithKeep();
+  ReadOnlyHeap::ClearSharedHeapForTest();
 
   // Deserialize with an incomplete list of external references.
   {
@@ -2977,6 +2944,7 @@ TEST(SnapshotCreatorNoExternalReferencesCustomFail1) {
 
   // Deserialize with an incomplete list of external references.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3002,6 +2970,7 @@ TEST(SnapshotCreatorNoExternalReferencesCustomFail2) {
 
   // Deserialize with an incomplete list of external references.
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3120,6 +3089,7 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
   }
 
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3533,6 +3503,7 @@ UNINITIALIZED_TEST(SnapshotCreatorIncludeGlobalProxy) {
   }
 
   {
+    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3667,6 +3638,7 @@ UNINITIALIZED_TEST(ReinitializeHashSeedNotRehashable) {
     CHECK(!blob.CanBeRehashed());
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   i::FLAG_hash_seed = 1337;
   v8::Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3802,6 +3774,7 @@ UNINITIALIZED_TEST(WeakArraySerializationInSnapshot) {
         creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
   }
 
+  ReadOnlyHeap::ClearSharedHeapForTest();
   v8::Isolate::CreateParams create_params;
   create_params.snapshot_blob = &blob;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();