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

This reverts commit a5fa211f30.

Reason for revert: breaks ARM Lite builder:
https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20arm%20-%20sim%20-%20lite/4843

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,delphick@chromium.org,goszczycki@google.com

Change-Id: I099544913bec3bbd67840b1818a6ad6029fdf380
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7464
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1666453
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62264}

src/heap/read-only-heap.cc

@@ -28,39 +28,16 @@ ReadOnlyHeap* ReadOnlyHeap::shared_ro_heap_ = nullptr;
 void ReadOnlyHeap::SetUp(Isolate* isolate, ReadOnlyDeserializer* des) {
   DCHECK_NOT_NULL(isolate);
 #ifdef V8_SHARED_RO_HEAP
-  bool call_once_ran = false;
-#ifdef DEBUG
-  base::Optional<Checksum> des_checksum;
-  if (des != nullptr) des_checksum = des->GetChecksum();
-#endif  // DEBUG
+  // 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, 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);
-                   }
-#ifdef DEBUG
-                   call_once_ran = true;
-#endif  // DEBUG
-                 });
-
-#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
+  base::CallOnce(&setup_ro_heap_once, [isolate, des]() {
+    shared_ro_heap_ = CreateAndAttachToIsolate(isolate);
+    if (des != nullptr) shared_ro_heap_->DeseralizeIntoIsolate(isolate, des);
+  });
 
   isolate->heap()->SetUpFromReadOnlyHeap(shared_ro_heap_);
   if (des != nullptr) {

src/heap/read-only-heap.h

@@ -5,10 +5,7 @@
 #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"
@@ -64,8 +61,6 @@ 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
@@ -82,15 +77,10 @@ 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  // V8_SHARED_RO_HEAP
+#endif
 
   explicit ReadOnlyHeap(ReadOnlySpace* ro_space) : read_only_space_(ro_space) {}
   DISALLOW_COPY_AND_ASSIGN(ReadOnlyHeap);

src/snapshot/deserializer.h

@@ -5,7 +5,6 @@
 #ifndef V8_SNAPSHOT_DESERIALIZER_H_
 #define V8_SNAPSHOT_DESERIALIZER_H_
 
-#include <utility>
 #include <vector>
 
 #include "src/objects/allocation-site.h"
@@ -40,9 +39,6 @@ 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,10 +5,7 @@
 #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 {
@@ -69,11 +66,6 @@ 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,10 +148,8 @@ namespace {
 
 // Convenience wrapper around the convenience wrapper.
 v8::StartupData CreateSnapshotDataBlob(const char* embedded_source) {
-  v8::StartupData data = CreateSnapshotDataBlobInternal(
+  return CreateSnapshotDataBlobInternal(
       v8::SnapshotCreator::FunctionCodeHandling::kClear, embedded_source);
-  ReadOnlyHeap::ClearSharedHeapForTest();
-  return data;
 }
 
 }  // namespace
@@ -277,6 +275,53 @@ 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();
@@ -766,7 +811,6 @@ void TestCustomSnapshotDataBlobWithIrregexpCode(
   DisableEmbeddedBlobRefcounting();
   v8::StartupData data1 =
       CreateSnapshotDataBlobInternal(function_code_handling, source);
-  ReadOnlyHeap::ClearSharedHeapForTest();
 
   v8::Isolate::CreateParams params1;
   params1.snapshot_blob = &data1;
@@ -914,7 +958,6 @@ 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();
@@ -1042,7 +1085,6 @@ 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();
@@ -1113,7 +1155,6 @@ 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();
@@ -1167,7 +1208,6 @@ 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();
@@ -2551,7 +2591,6 @@ 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();
@@ -2690,7 +2729,6 @@ 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();
@@ -2716,7 +2754,6 @@ 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();
@@ -2765,7 +2802,6 @@ 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();
@@ -2826,7 +2862,6 @@ 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();
@@ -2876,7 +2911,6 @@ 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();
@@ -2916,7 +2950,6 @@ UNINITIALIZED_TEST(SnapshotCreatorArrayJoinWithKeep) {
   DisableAlwaysOpt();
   DisableEmbeddedBlobRefcounting();
   v8::StartupData blob = CreateCustomSnapshotArrayJoinWithKeep();
-  ReadOnlyHeap::ClearSharedHeapForTest();
 
   // Deserialize with an incomplete list of external references.
   {
@@ -2944,7 +2977,6 @@ 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();
@@ -2970,7 +3002,6 @@ 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();
@@ -3089,7 +3120,6 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
   }
 
   {
-    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3503,7 +3533,6 @@ UNINITIALIZED_TEST(SnapshotCreatorIncludeGlobalProxy) {
   }
 
   {
-    ReadOnlyHeap::ClearSharedHeapForTest();
     v8::Isolate::CreateParams params;
     params.snapshot_blob = &blob;
     params.array_buffer_allocator = CcTest::array_buffer_allocator();
@@ -3638,7 +3667,6 @@ 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();
@@ -3774,7 +3802,6 @@ 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();