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

This reverts commit b022e825.

Reason for revert: Fails nosnap: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20nosnap%20-%20debug/22761

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=sigurds@chromium.org,jgruber@chromium.org

Change-Id: I027ae2585bd06290ea3245b1fc23efba544eb470
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8723
Reviewed-on: https://chromium-review.googlesource.com/c/1443059Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59179}

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.EmbeddedBlobHash(), d.CreateEmbeddedBlobHash());
+  CHECK_EQ(d.Hash(), d.CreateHash());
 #endif  // DEBUG
 }
 
@@ -244,45 +244,6 @@ 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;
@@ -3154,13 +3115,6 @@ 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() {
@@ -3390,12 +3344,6 @@ 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));
-
   if (FLAG_print_builtin_code) builtins()->PrintBuiltinCode();
   if (FLAG_print_builtin_size) builtins()->PrintBuiltinSize();
 

src/isolate.h

@@ -1470,11 +1470,6 @@ 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,13 +232,6 @@ 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());
@@ -261,14 +254,12 @@ EmbeddedData EmbeddedData::FromIsolate(Isolate* isolate) {
   FinalizeEmbeddedCodeTargets(isolate, &d);
 
   // Hash the blob and store the result.
-  {
-    STATIC_ASSERT(EmbeddedBlobHashSize() == kSizetSize);
-    const size_t hash = d.CreateEmbeddedBlobHash();
-    std::memcpy(blob + EmbeddedBlobHashOffset(), &hash, EmbeddedBlobHashSize());
+  STATIC_ASSERT(HashSize() == kSizetSize);
+  const size_t hash = d.CreateHash();
+  std::memcpy(blob + HashOffset(), &hash, HashSize());
 
-    DCHECK_EQ(hash, d.CreateEmbeddedBlobHash());
-    DCHECK_EQ(hash, d.EmbeddedBlobHash());
-  }
+  DCHECK_EQ(hash, d.CreateHash());
+  DCHECK_EQ(hash, d.Hash());
 
   if (FLAG_serialization_statistics) d.PrintStatistics();
 
@@ -290,10 +281,10 @@ uint32_t EmbeddedData::InstructionSizeOfBuiltin(int i) const {
   return metadata[i].instructions_length;
 }
 
-size_t EmbeddedData::CreateEmbeddedBlobHash() const {
-  STATIC_ASSERT(EmbeddedBlobHashOffset() == 0);
-  STATIC_ASSERT(EmbeddedBlobHashSize() == kSizetSize);
-  return base::hash_range(data_ + EmbeddedBlobHashSize(), data_ + size_);
+size_t EmbeddedData::CreateHash() const {
+  STATIC_ASSERT(HashOffset() == 0);
+  STATIC_ASSERT(HashSize() == kSizetSize);
+  return base::hash_range(data_ + HashSize(), data_ + size_);
 }
 
 void EmbeddedData::PrintStatistics() const {
@@ -320,8 +311,7 @@ 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>(
-      EmbeddedBlobHashSize() + IsolateHashSize() + MetadataSize());
+  const int metadata_size = static_cast<int>(HashSize() + MetadataSize());
 
   PrintF("EmbeddedData:\n");
   PrintF("  Total size:                         %d\n",

src/snapshot/embedded-data.h

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