Revert "Reland "[builtins] Verify Isolate compatibility with the embedded blob""
This reverts commit 1e3582b5aecfeb0ffdca4bda3f1fd66d433236f6.
Reason for revert: Still fails nosnap: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20nosnap%20-%20debug/22789
Original change's description:
> Reland "[builtins] Verify Isolate compatibility with the embedded blob"
>
> This is a reland of b022e825bd9f45279bb0c493a9081804f17455ae
>
> 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,jgruber@chromium.org
Change-Id: If6082452c739d4de44ed70d3c6355f5282684ac1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8723
Reviewed-on: https://chromium-review.googlesource.com/c/1448311
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59241}diff --git a/src/isolate.cc b/src/isolate.cc
index ebe2463..f87cc40 100644
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -213,7 +213,7 @@
// 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 @@
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;
@@ -3150,13 +3111,6 @@
}
}
-#ifdef DEBUG
-bool IsolateIsCompatibleWithEmbeddedBlob(Isolate* isolate) {
- EmbeddedData d = EmbeddedData::FromBlob(isolate);
- return (d.IsolateHash() == isolate->HashIsolateForEmbeddedBlob());
-}
-#endif // DEBUG
-
} // namespace
void Isolate::InitializeDefaultEmbeddedBlob() {
@@ -3384,12 +3338,6 @@
// 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())
diff --git a/src/isolate.h b/src/isolate.h
index c5f7a8e..3bf6f3a 100644
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -1470,11 +1470,6 @@
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();
diff --git a/src/snapshot/embedded-data.cc b/src/snapshot/embedded-data.cc
index 0488c2f..f5dded1 100644
--- a/src/snapshot/embedded-data.cc
+++ b/src/snapshot/embedded-data.cc
@@ -232,13 +232,6 @@
// 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 @@
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 @@
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 @@
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",
diff --git a/src/snapshot/embedded-data.h b/src/snapshot/embedded-data.h
index 5c5653e..6e28071 100644
--- a/src/snapshot/embedded-data.h
+++ b/src/snapshot/embedded-data.h
@@ -71,13 +71,9 @@
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 @@
// 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;
diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
index d4eb163..af69362 100644
--- a/test/cctest/test-serialize.cc
+++ b/test/cctest/test-serialize.cc
@@ -1337,7 +1337,6 @@
UNINITIALIZED_TEST(CustomSnapshotDataBlobWithLocker) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::Isolate::CreateParams create_params;
create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
DisableEmbeddedBlobRefcounting();
@@ -2576,9 +2575,8 @@
delete cache_data;
}
-UNINITIALIZED_TEST(SnapshotCreatorMultipleContexts) {
+TEST(SnapshotCreatorMultipleContexts) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob;
{
v8::SnapshotCreator creator;
@@ -2637,7 +2635,6 @@
isolate->Dispose();
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
static int serialized_static_field = 314;
@@ -2700,9 +2697,8 @@
intptr_t short_external_references[] = {
reinterpret_cast<intptr_t>(SerializedCallbackReplacement), 0};
-UNINITIALIZED_TEST(SnapshotCreatorExternalReferences) {
+TEST(SnapshotCreatorExternalReferences) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob;
{
v8::SnapshotCreator creator(original_external_references);
@@ -2789,12 +2785,10 @@
CHECK_EQ(3, serializable_two_byte_resource.dispose_count());
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
-UNINITIALIZED_TEST(SnapshotCreatorShortExternalReferences) {
+TEST(SnapshotCreatorShortExternalReferences) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob;
{
v8::SnapshotCreator creator(original_external_references);
@@ -2833,7 +2827,6 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
v8::StartupData CreateSnapshotWithDefaultAndCustom() {
@@ -2870,9 +2863,8 @@
return creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
}
-UNINITIALIZED_TEST(SnapshotCreatorNoExternalReferencesDefault) {
+TEST(SnapshotCreatorNoExternalReferencesDefault) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob = CreateSnapshotWithDefaultAndCustom();
// Deserialize with an incomplete list of external references.
@@ -2893,7 +2885,6 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
v8::StartupData CreateCustomSnapshotWithPreparseDataAndNoOuterScope() {
@@ -2919,9 +2910,8 @@
return creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
}
-UNINITIALIZED_TEST(SnapshotCreatorPreparseDataAndNoOuterScope) {
+TEST(SnapshotCreatorPreparseDataAndNoOuterScope) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob = CreateCustomSnapshotWithPreparseDataAndNoOuterScope();
// Deserialize with an incomplete list of external references.
@@ -2940,7 +2930,6 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
v8::StartupData CreateCustomSnapshotArrayJoinWithKeep() {
@@ -2961,9 +2950,8 @@
return creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kKeep);
}
-UNINITIALIZED_TEST(SnapshotCreatorArrayJoinWithKeep) {
+TEST(SnapshotCreatorArrayJoinWithKeep) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob = CreateCustomSnapshotArrayJoinWithKeep();
// Deserialize with an incomplete list of external references.
@@ -2983,7 +2971,6 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
TEST(SnapshotCreatorNoExternalReferencesCustomFail1) {
@@ -3036,9 +3023,8 @@
delete[] blob.data;
}
-UNINITIALIZED_TEST(SnapshotCreatorUnknownExternalReferences) {
+TEST(SnapshotCreatorUnknownExternalReferences) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::SnapshotCreator creator;
v8::Isolate* isolate = creator.GetIsolate();
{
@@ -3059,12 +3045,10 @@
creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kClear);
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
-UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
+TEST(SnapshotCreatorTemplates) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob;
{
@@ -3228,12 +3212,10 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
-UNINITIALIZED_TEST(SnapshotCreatorAddData) {
+TEST(SnapshotCreatorAddData) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob;
{
@@ -3430,7 +3412,6 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
TEST(SnapshotCreatorUnknownHandles) {
@@ -3458,9 +3439,8 @@
delete[] blob.data;
}
-UNINITIALIZED_TEST(SnapshotCreatorIncludeGlobalProxy) {
+TEST(SnapshotCreatorIncludeGlobalProxy) {
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
v8::StartupData blob;
{
@@ -3648,7 +3628,6 @@
isolate->Dispose();
}
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
UNINITIALIZED_TEST(ReinitializeHashSeedNotRehashable) {
@@ -3770,7 +3749,7 @@
FreeCurrentEmbeddedBlob();
}
-UNINITIALIZED_TEST(SerializationStats) {
+TEST(SerializationStats) {
FLAG_profile_deserialization = true;
FLAG_always_opt = false;
v8::StartupData blob = CreateSnapshotDataBlob();
@@ -3785,8 +3764,6 @@
}
PrintF("Embedded blob is %d bytes\n", embedded_blob_size);
}
-
- FreeCurrentEmbeddedBlob();
}
void CheckSFIsAreWeak(WeakFixedArray sfis, Isolate* isolate) {
@@ -3805,11 +3782,10 @@
CHECK_GT(no_of_weak, 0);
}
-UNINITIALIZED_TEST(WeakArraySerializationInSnapshot) {
+TEST(WeakArraySerializizationInSnapshot) {
const char* code = "var my_func = function() { }";
DisableAlwaysOpt();
- DisableEmbeddedBlobRefcounting();
i::FLAG_allow_natives_syntax = true;
v8::StartupData blob;
{
@@ -3851,11 +3827,10 @@
// Verify that the pointers in shared_function_infos are weak.
WeakFixedArray sfis =
Script::cast(function->shared()->script())->shared_function_infos();
- CheckSFIsAreWeak(sfis, reinterpret_cast<i::Isolate*>(isolate));
+ CheckSFIsAreWeak(sfis, CcTest::i_isolate());
}
isolate->Dispose();
delete[] blob.data;
- FreeCurrentEmbeddedBlob();
}
TEST(WeakArraySerializationInCodeCache) {
