Reland "Reland "[builtins] Verify Isolate compatibility with the embedded blob""

This is a reland of 1e3582b5

Original change's description:
> Reland "[builtins] Verify Isolate compatibility with the embedded blob"
>
> This is a reland of b022e825
>
> Original change's description:
> > [builtins] Verify Isolate compatibility with the embedded blob
> >
> > Embedded builtins (= the embedded blob) have a few dependencies on the
> > snapshot state. For instance, they require that metadata stored on
> > builtin Code objects as well as the builtins constant table remain
> > unchanged from mksnapshot-time. Embedders may violate these
> > assumptions by accident, e.g. by loading a snapshot generated with
> > different build flags, leading to seemingly unrelated failures later
> > on.
> >
> > This CL introduces an Isolate hash stored in the embedded blob which
> > hashes relevant parts of builtin Code objects and the builtins
> > constant table. It's verified in Isolate::Init in debug builds.
> >
> > Bug: v8:8723
> > Change-Id: Ifc9bdbe6f56ea67d8984f162afa73a3572cfbba8
> > Reviewed-on: https://chromium-review.googlesource.com/c/1442641
> > Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> > Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#59177}
>
> Tbr: yangguo@chromium.org,sigurds@chromium.org
> Bug: v8:8723
> Change-Id: I1dd001783f0f1fae21a9809c8639e40f55b8f663
> Reviewed-on: https://chromium-review.googlesource.com/c/1445985
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59236}

Tbr: yangguo@chromium.org,sigurds@chromium.org
Bug: v8:8723
Change-Id: I8e4ae2f09d16b693c1fb814477d8487385046ee4
Reviewed-on: https://chromium-review.googlesource.com/c/1448312Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59250}

src/isolate.cc

@@ -213,7 +213,7 @@ void Isolate::SetEmbeddedBlob(const uint8_t* blob, uint32_t blob_size) {
   // Verify that the contents of the embedded blob are unchanged from
   // serialization-time, just to ensure the compiler isn't messing with us.
   EmbeddedData d = EmbeddedData::FromBlob();
-  CHECK_EQ(d.Hash(), d.CreateHash());
+  CHECK_EQ(d.EmbeddedBlobHash(), d.CreateEmbeddedBlobHash());
 #endif  // DEBUG
 }
 
@@ -244,6 +244,45 @@ uint32_t Isolate::CurrentEmbeddedBlobSize() {
       std::memory_order::memory_order_relaxed);
 }
 
+size_t Isolate::HashIsolateForEmbeddedBlob() {
+  DCHECK(builtins_.is_initialized());
+  DCHECK(FLAG_embedded_builtins);
+  DCHECK(Builtins::AllBuiltinsAreIsolateIndependent());
+
+  DisallowHeapAllocation no_gc;
+
+  static constexpr size_t kSeed = 0;
+  size_t hash = kSeed;
+
+  // Hash data sections of builtin code objects.
+  for (int i = 0; i < Builtins::builtin_count; i++) {
+    Code code = heap_.builtin(i);
+
+    DCHECK(Internals::HasHeapObjectTag(code.ptr()));
+    uint8_t* const code_ptr =
+        reinterpret_cast<uint8_t*>(code.ptr() - kHeapObjectTag);
+
+    // These static asserts ensure we don't miss relevant fields. We don't hash
+    // instruction size and flags since they change when creating the off-heap
+    // trampolines. Other data fields must remain the same.
+    STATIC_ASSERT(Code::kInstructionSizeOffset == Code::kDataStart);
+    STATIC_ASSERT(Code::kFlagsOffset == Code::kInstructionSizeOffsetEnd + 1);
+    STATIC_ASSERT(Code::kSafepointTableOffsetOffset ==
+                  Code::kFlagsOffsetEnd + 1);
+    static constexpr int kStartOffset = Code::kSafepointTableOffsetOffset;
+
+    for (int j = kStartOffset; j < Code::kHeaderPaddingStart; j++) {
+      hash = base::hash_combine(hash, size_t{code_ptr[j]});
+    }
+  }
+
+  // The builtins constants table is also tightly tied to embedded builtins.
+  hash = base::hash_combine(
+      hash, static_cast<size_t>(heap_.builtins_constants_table()->length()));
+
+  return hash;
+}
+
 void ThreadLocalTop::Initialize(Isolate* isolate) {
   *this = ThreadLocalTop();
   isolate_ = isolate;
@@ -3111,6 +3150,13 @@ void CreateOffHeapTrampolines(Isolate* isolate) {
   }
 }
 
+#ifdef DEBUG
+bool IsolateIsCompatibleWithEmbeddedBlob(Isolate* isolate) {
+  EmbeddedData d = EmbeddedData::FromBlob(isolate);
+  return (d.IsolateHash() == isolate->HashIsolateForEmbeddedBlob());
+}
+#endif  // DEBUG
+
 }  // namespace
 
 void Isolate::InitializeDefaultEmbeddedBlob() {
@@ -3338,6 +3384,12 @@ bool Isolate::Init(StartupDeserializer* des) {
   // Initialize the builtin entry table.
   Builtins::UpdateBuiltinEntryTable(this);
 
+  // Verify that the current heap state (usually deserialized from the snapshot)
+  // is compatible with the embedded blob. If this DCHECK fails, we've likely
+  // loaded a snapshot generated by a different V8 version or build-time
+  // configuration.
+  DCHECK(IsolateIsCompatibleWithEmbeddedBlob(this));
+
 #ifndef V8_TARGET_ARCH_ARM
   // The IET for profiling should always be a full on-heap Code object.
   DCHECK(!Code::cast(heap_.interpreter_entry_trampoline_for_profiling())

src/isolate.h

@@ -1470,6 +1470,11 @@ class Isolate final : private HiddenFactory {
     return builtins_constants_table_builder_;
   }
 
+  // Hashes bits of the Isolate that are relevant for embedded builtins. In
+  // particular, the embedded blob requires builtin Code object layout and the
+  // builtins constants table to remain unchanged from build-time.
+  size_t HashIsolateForEmbeddedBlob();
+
   static const uint8_t* CurrentEmbeddedBlob();
   static uint32_t CurrentEmbeddedBlobSize();
   static bool CurrentEmbeddedBlobIsBinaryEmbedded();

src/snapshot/embedded-data.cc

@@ -232,6 +232,13 @@ EmbeddedData EmbeddedData::FromIsolate(Isolate* isolate) {
   // between two builtins with int3's (on x64/ia32).
   ZapCode(reinterpret_cast<Address>(blob), blob_size);
 
+  // Hash relevant parts of the Isolate's heap and store the result.
+  {
+    STATIC_ASSERT(IsolateHashSize() == kSizetSize);
+    const size_t hash = isolate->HashIsolateForEmbeddedBlob();
+    std::memcpy(blob + IsolateHashOffset(), &hash, IsolateHashSize());
+  }
+
   // Write the metadata tables.
   DCHECK_EQ(MetadataSize(), sizeof(metadata[0]) * metadata.size());
   std::memcpy(blob + MetadataOffset(), metadata.data(), MetadataSize());
@@ -254,12 +261,14 @@ EmbeddedData EmbeddedData::FromIsolate(Isolate* isolate) {
   FinalizeEmbeddedCodeTargets(isolate, &d);
 
   // Hash the blob and store the result.
-  STATIC_ASSERT(HashSize() == kSizetSize);
-  const size_t hash = d.CreateHash();
-  std::memcpy(blob + HashOffset(), &hash, HashSize());
+  {
+    STATIC_ASSERT(EmbeddedBlobHashSize() == kSizetSize);
+    const size_t hash = d.CreateEmbeddedBlobHash();
+    std::memcpy(blob + EmbeddedBlobHashOffset(), &hash, EmbeddedBlobHashSize());
 
-  DCHECK_EQ(hash, d.CreateHash());
-  DCHECK_EQ(hash, d.Hash());
+    DCHECK_EQ(hash, d.CreateEmbeddedBlobHash());
+    DCHECK_EQ(hash, d.EmbeddedBlobHash());
+  }
 
   if (FLAG_serialization_statistics) d.PrintStatistics();
 
@@ -281,10 +290,10 @@ uint32_t EmbeddedData::InstructionSizeOfBuiltin(int i) const {
   return metadata[i].instructions_length;
 }
 
-size_t EmbeddedData::CreateHash() const {
-  STATIC_ASSERT(HashOffset() == 0);
-  STATIC_ASSERT(HashSize() == kSizetSize);
-  return base::hash_range(data_ + HashSize(), data_ + size_);
+size_t EmbeddedData::CreateEmbeddedBlobHash() const {
+  STATIC_ASSERT(EmbeddedBlobHashOffset() == 0);
+  STATIC_ASSERT(EmbeddedBlobHashSize() == kSizetSize);
+  return base::hash_range(data_ + EmbeddedBlobHashSize(), data_ + size_);
 }
 
 void EmbeddedData::PrintStatistics() const {
@@ -311,7 +320,8 @@ void EmbeddedData::PrintStatistics() const {
   const int k90th = embedded_count * 0.90;
   const int k99th = embedded_count * 0.99;
 
-  const int metadata_size = static_cast<int>(HashSize() + MetadataSize());
+  const int metadata_size = static_cast<int>(
+      EmbeddedBlobHashSize() + IsolateHashSize() + MetadataSize());
 
   PrintF("EmbeddedData:\n");
   PrintF("  Total size:                         %d\n",

src/snapshot/embedded-data.h

@@ -71,9 +71,13 @@ class EmbeddedData final {
     return (size == 0) ? 0 : PadAndAlign(size);
   }
 
-  size_t CreateHash() const;
-  size_t Hash() const {
-    return *reinterpret_cast<const size_t*>(data_ + HashOffset());
+  size_t CreateEmbeddedBlobHash() const;
+  size_t EmbeddedBlobHash() const {
+    return *reinterpret_cast<const size_t*>(data_ + EmbeddedBlobHashOffset());
+  }
+
+  size_t IsolateHash() const {
+    return *reinterpret_cast<const size_t*>(data_ + IsolateHashOffset());
   }
 
   struct Metadata {
@@ -88,15 +92,20 @@ class EmbeddedData final {
   // The layout of the blob is as follows:
   //
   // [0] hash of the remaining blob
-  // [1] metadata of instruction stream 0
+  // [1] hash of embedded-blob-relevant heap objects
+  // [2] metadata of instruction stream 0
   // ... metadata
   // ... instruction streams
 
   static constexpr uint32_t kTableSize = Builtins::builtin_count;
-  static constexpr uint32_t HashOffset() { return 0; }
-  static constexpr uint32_t HashSize() { return kSizetSize; }
+  static constexpr uint32_t EmbeddedBlobHashOffset() { return 0; }
+  static constexpr uint32_t EmbeddedBlobHashSize() { return kSizetSize; }
+  static constexpr uint32_t IsolateHashOffset() {
+    return EmbeddedBlobHashOffset() + EmbeddedBlobHashSize();
+  }
+  static constexpr uint32_t IsolateHashSize() { return kSizetSize; }
   static constexpr uint32_t MetadataOffset() {
-    return HashOffset() + HashSize();
+    return IsolateHashOffset() + IsolateHashSize();
   }
   static constexpr uint32_t MetadataSize() {
     return sizeof(struct Metadata) * kTableSize;

test/cctest/cctest.status

@@ -222,6 +222,8 @@
   # support.
   'test-serialize/CustomSnapshotDataBlobWithWarmup': [SKIP],
   'test-serialize/SnapshotDataBlobWithWarmup': [SKIP],
+  # Fails the embedded blob <-> Isolate verification step.
+  'test-serialize/CustomSnapshotDataBlobWithLocker': [SKIP],
 
   # https://crbug.com/v8/7763
   'test-lockers/ExtensionsRegistration': [SKIP],

test/cctest/test-serialize.cc

@@ -1337,9 +1337,9 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobOutdatedContextWithOverflow) {
 
 UNINITIALIZED_TEST(CustomSnapshotDataBlobWithLocker) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  DisableEmbeddedBlobRefcounting();
   v8::Isolate* isolate0 = v8::Isolate::New(create_params);
   {
     v8::Locker locker(isolate0);
@@ -2575,8 +2575,9 @@ TEST(Regress503552) {
   delete cache_data;
 }
 
-TEST(SnapshotCreatorMultipleContexts) {
+UNINITIALIZED_TEST(SnapshotCreatorMultipleContexts) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -2635,6 +2636,7 @@ TEST(SnapshotCreatorMultipleContexts) {
 
   isolate->Dispose();
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 static int serialized_static_field = 314;
@@ -2697,8 +2699,9 @@ intptr_t replaced_external_references[] = {
 intptr_t short_external_references[] = {
     reinterpret_cast<intptr_t>(SerializedCallbackReplacement), 0};
 
-TEST(SnapshotCreatorExternalReferences) {
+UNINITIALIZED_TEST(SnapshotCreatorExternalReferences) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator(original_external_references);
@@ -2785,10 +2788,12 @@ TEST(SnapshotCreatorExternalReferences) {
   CHECK_EQ(3, serializable_two_byte_resource.dispose_count());
 
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
-TEST(SnapshotCreatorShortExternalReferences) {
+UNINITIALIZED_TEST(SnapshotCreatorShortExternalReferences) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator(original_external_references);
@@ -2827,6 +2832,7 @@ TEST(SnapshotCreatorShortExternalReferences) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 v8::StartupData CreateSnapshotWithDefaultAndCustom() {
@@ -2863,8 +2869,9 @@ v8::StartupData CreateSnapshotWithDefaultAndCustom() {
   return creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
 }
 
-TEST(SnapshotCreatorNoExternalReferencesDefault) {
+UNINITIALIZED_TEST(SnapshotCreatorNoExternalReferencesDefault) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob = CreateSnapshotWithDefaultAndCustom();
 
   // Deserialize with an incomplete list of external references.
@@ -2885,6 +2892,7 @@ TEST(SnapshotCreatorNoExternalReferencesDefault) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 v8::StartupData CreateCustomSnapshotWithPreparseDataAndNoOuterScope() {
@@ -2910,8 +2918,9 @@ v8::StartupData CreateCustomSnapshotWithPreparseDataAndNoOuterScope() {
   return creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
 }
 
-TEST(SnapshotCreatorPreparseDataAndNoOuterScope) {
+UNINITIALIZED_TEST(SnapshotCreatorPreparseDataAndNoOuterScope) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob = CreateCustomSnapshotWithPreparseDataAndNoOuterScope();
 
   // Deserialize with an incomplete list of external references.
@@ -2930,6 +2939,7 @@ TEST(SnapshotCreatorPreparseDataAndNoOuterScope) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 v8::StartupData CreateCustomSnapshotArrayJoinWithKeep() {
@@ -2950,8 +2960,9 @@ v8::StartupData CreateCustomSnapshotArrayJoinWithKeep() {
   return creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kKeep);
 }
 
-TEST(SnapshotCreatorArrayJoinWithKeep) {
+UNINITIALIZED_TEST(SnapshotCreatorArrayJoinWithKeep) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob = CreateCustomSnapshotArrayJoinWithKeep();
 
   // Deserialize with an incomplete list of external references.
@@ -2971,6 +2982,7 @@ TEST(SnapshotCreatorArrayJoinWithKeep) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(SnapshotCreatorNoExternalReferencesCustomFail1) {
@@ -3023,8 +3035,9 @@ TEST(SnapshotCreatorNoExternalReferencesCustomFail2) {
   delete[] blob.data;
 }
 
-TEST(SnapshotCreatorUnknownExternalReferences) {
+UNINITIALIZED_TEST(SnapshotCreatorUnknownExternalReferences) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::SnapshotCreator creator;
   v8::Isolate* isolate = creator.GetIsolate();
   {
@@ -3045,10 +3058,12 @@ TEST(SnapshotCreatorUnknownExternalReferences) {
       creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
 
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
-TEST(SnapshotCreatorTemplates) {
+UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
 
   {
@@ -3212,10 +3227,12 @@ TEST(SnapshotCreatorTemplates) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
-TEST(SnapshotCreatorAddData) {
+UNINITIALIZED_TEST(SnapshotCreatorAddData) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
 
   {
@@ -3412,6 +3429,7 @@ TEST(SnapshotCreatorAddData) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(SnapshotCreatorUnknownHandles) {
@@ -3439,8 +3457,9 @@ TEST(SnapshotCreatorUnknownHandles) {
   delete[] blob.data;
 }
 
-TEST(SnapshotCreatorIncludeGlobalProxy) {
+UNINITIALIZED_TEST(SnapshotCreatorIncludeGlobalProxy) {
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
 
   {
@@ -3628,6 +3647,7 @@ TEST(SnapshotCreatorIncludeGlobalProxy) {
     isolate->Dispose();
   }
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(ReinitializeHashSeedNotRehashable) {
@@ -3749,7 +3769,7 @@ UNINITIALIZED_TEST(ReinitializeHashSeedRehashable) {
   FreeCurrentEmbeddedBlob();
 }
 
-TEST(SerializationStats) {
+UNINITIALIZED_TEST(SerializationStats) {
   FLAG_profile_deserialization = true;
   FLAG_always_opt = false;
   v8::StartupData blob = CreateSnapshotDataBlob();
@@ -3764,6 +3784,8 @@ TEST(SerializationStats) {
     }
     PrintF("Embedded blob is %d bytes\n", embedded_blob_size);
   }
+
+  FreeCurrentEmbeddedBlob();
 }
 
 void CheckSFIsAreWeak(WeakFixedArray sfis, Isolate* isolate) {
@@ -3782,10 +3804,11 @@ void CheckSFIsAreWeak(WeakFixedArray sfis, Isolate* isolate) {
   CHECK_GT(no_of_weak, 0);
 }
 
-TEST(WeakArraySerializizationInSnapshot) {
+UNINITIALIZED_TEST(WeakArraySerializationInSnapshot) {
   const char* code = "var my_func = function() { }";
 
   DisableAlwaysOpt();
+  DisableEmbeddedBlobRefcounting();
   i::FLAG_allow_natives_syntax = true;
   v8::StartupData blob;
   {
@@ -3827,10 +3850,11 @@ TEST(WeakArraySerializizationInSnapshot) {
     // Verify that the pointers in shared_function_infos are weak.
     WeakFixedArray sfis =
         Script::cast(function->shared()->script())->shared_function_infos();
-    CheckSFIsAreWeak(sfis, CcTest::i_isolate());
+    CheckSFIsAreWeak(sfis, reinterpret_cast<i::Isolate*>(isolate));
   }
   isolate->Dispose();
   delete[] blob.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(WeakArraySerializationInCodeCache) {