Revert "[snapshot] add checksum to startup snapshot"

This reverts commit bcb8d49b.

Reason for revert: MSan compile error: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/23025

Original change's description:
> [snapshot] add checksum to startup snapshot
> 
> We already had checksumming for code cache data. We now extend
> checksumming to the startup snapshot to catch data corruption early.
> 
> The performance impact for deserialization is a regression of 1-2%,
> which should be acceptable.
> 
> Sample output for the included test with --profile-deserialization:
> 
> [Verifying snapshot checksum took 0.023 ms]
> [Deserializing isolate (134348 bytes) took 1.891 ms]
> [Verifying snapshot checksum took 0.024 ms]
> [Deserializing isolate (134348 bytes) took 1.654 ms]
> [Deserializing context #0 (47208 bytes) took 0.331 ms]
> Deserialization will reserve:
>     208168 bytes per isolate
>     123368 bytes per context #0
> Snapshot blob consists of:
>     134492 bytes in 6 chunks for startup
>     115272 bytes for builtins
>      47152 bytes in 31 chunks for context #0
> [Verifying snapshot checksum took 0.048 ms]
> [Verifying snapshot checksum took 0.043 ms]
> 
> R=​peria@chromium.org, petermarshall@chromium.org
> 
> Bug: chromium:881417
> Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
> Change-Id: Ibc57520d459c86be8972f731aa35045b5e3751d7
> Reviewed-on: https://chromium-review.googlesource.com/1241874
> Reviewed-by: Peter Marshall <petermarshall@chromium.org>
> Commit-Queue: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56217}

TBR=peria@chromium.org,yangguo@chromium.org,petermarshall@chromium.org

Change-Id: Iccb82092858ab68a5d6ae9552fa716108eda354b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:881417
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1243190Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56221}

src/api.cc

@@ -835,7 +835,6 @@ StartupData SnapshotCreator::CreateBlob(
   }
   data->created_ = true;
 
-  DCHECK(i::Snapshot::VerifyChecksum(&result));
   return result;
 }
 

src/snapshot/builtin-serializer.cc

@@ -34,16 +34,13 @@ void BuiltinSerializer::SerializeBuiltinsAndHandlers() {
     SerializeBuiltin(code);
   }
 
-  // Append the offset table. During deserialization, the offset table is
-  // extracted by BuiltinSnapshotData.
-  const byte* data = reinterpret_cast<const byte*>(&code_offsets_[0]);
-  int data_length = static_cast<int>(sizeof(code_offsets_));
-
   // Pad with kNop since GetInt() might read too far.
-  Pad(data_length);
+  Pad();
 
   // Append the offset table. During deserialization, the offset table is
   // extracted by BuiltinSnapshotData.
+  const byte* data = reinterpret_cast<const byte*>(&code_offsets_[0]);
+  int data_length = static_cast<int>(sizeof(code_offsets_));
   sink_.PutRaw(data, data_length, "BuiltinOffsets");
 }
 

src/snapshot/code-serializer.cc

@@ -336,6 +336,44 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
   return scope.CloseAndEscape(result);
 }
 
+class Checksum {
+ public:
+  explicit Checksum(Vector<const byte> payload) {
+#ifdef MEMORY_SANITIZER
+    // Computing the checksum includes padding bytes for objects like strings.
+    // Mark every object as initialized in the code serializer.
+    MSAN_MEMORY_IS_INITIALIZED(payload.start(), payload.length());
+#endif  // MEMORY_SANITIZER
+    // Fletcher's checksum. Modified to reduce 64-bit sums to 32-bit.
+    uintptr_t a = 1;
+    uintptr_t b = 0;
+    const uintptr_t* cur = reinterpret_cast<const uintptr_t*>(payload.start());
+    DCHECK(IsAligned(payload.length(), kIntptrSize));
+    const uintptr_t* end = cur + payload.length() / kIntptrSize;
+    while (cur < end) {
+      // Unsigned overflow expected and intended.
+      a += *cur++;
+      b += a;
+    }
+#if V8_HOST_ARCH_64_BIT
+    a ^= a >> 32;
+    b ^= b >> 32;
+#endif  // V8_HOST_ARCH_64_BIT
+    a_ = static_cast<uint32_t>(a);
+    b_ = static_cast<uint32_t>(b);
+  }
+
+  bool Check(uint32_t a, uint32_t b) const { return a == a_ && b == b_; }
+
+  uint32_t a() const { return a_; }
+  uint32_t b() const { return b_; }
+
+ private:
+  uint32_t a_;
+  uint32_t b_;
+
+  DISALLOW_COPY_AND_ASSIGN(Checksum);
+};
 
 SerializedCodeData::SerializedCodeData(const std::vector<byte>* payload,
                                        const CodeSerializer* cs) {
@@ -352,14 +390,10 @@ SerializedCodeData::SerializedCodeData(const std::vector<byte>* payload,
   uint32_t padded_payload_offset = POINTER_SIZE_ALIGN(payload_offset);
   uint32_t size =
       padded_payload_offset + static_cast<uint32_t>(payload->size());
-  DCHECK(IsAligned(size, kPointerAlignment));
 
   // Allocate backing store and create result data.
   AllocateData(size);
 
-  // Zero out pre-payload data. Part of that is only used for padding.
-  memset(data_, 0, padded_payload_offset);
-
   // Set header values.
   SetMagicNumber(cs->isolate());
   SetHeaderValue(kVersionHashOffset, Version::Hash());
@@ -384,13 +418,16 @@ SerializedCodeData::SerializedCodeData(const std::vector<byte>* payload,
   CopyBytes(data_ + kHeaderSize + reservation_size,
             reinterpret_cast<const byte*>(stub_keys->data()), stub_keys_size);
 
+  // Zero out any padding before the payload.
+  memset(data_ + payload_offset, 0, padded_payload_offset - payload_offset);
+
   // Copy serialized data.
   CopyBytes(data_ + padded_payload_offset, payload->data(),
             static_cast<size_t>(payload->size()));
 
-  Checksum checksum(ChecksummedContent());
-  SetHeaderValue(kChecksumPartAOffset, checksum.a());
-  SetHeaderValue(kChecksumPartBOffset, checksum.b());
+  Checksum checksum(DataWithoutHeader());
+  SetHeaderValue(kChecksum1Offset, checksum.a());
+  SetHeaderValue(kChecksum2Offset, checksum.b());
 }
 
 SerializedCodeData::SanityCheckResult SerializedCodeData::SanityCheck(
@@ -403,8 +440,8 @@ SerializedCodeData::SanityCheckResult SerializedCodeData::SanityCheck(
   uint32_t cpu_features = GetHeaderValue(kCpuFeaturesOffset);
   uint32_t flags_hash = GetHeaderValue(kFlagHashOffset);
   uint32_t payload_length = GetHeaderValue(kPayloadLengthOffset);
-  uint32_t c1 = GetHeaderValue(kChecksumPartAOffset);
-  uint32_t c2 = GetHeaderValue(kChecksumPartBOffset);
+  uint32_t c1 = GetHeaderValue(kChecksum1Offset);
+  uint32_t c2 = GetHeaderValue(kChecksum2Offset);
   if (version_hash != Version::Hash()) return VERSION_MISMATCH;
   if (source_hash != expected_source_hash) return SOURCE_MISMATCH;
   if (cpu_features != static_cast<uint32_t>(CpuFeatures::SupportedFeatures())) {
@@ -417,7 +454,7 @@ SerializedCodeData::SanityCheckResult SerializedCodeData::SanityCheck(
                          GetHeaderValue(kNumReservationsOffset) * kInt32Size +
                          GetHeaderValue(kNumCodeStubKeysOffset) * kInt32Size);
   if (payload_length > max_payload_length) return LENGTH_MISMATCH;
-  if (!Checksum(ChecksummedContent()).Check(c1, c2)) return CHECKSUM_MISMATCH;
+  if (!Checksum(DataWithoutHeader()).Check(c1, c2)) return CHECKSUM_MISMATCH;
   return CHECK_SUCCESS;
 }
 

src/snapshot/code-serializer.h

@@ -110,8 +110,8 @@ class SerializedCodeData : public SerializedData {
   // [6] number of code stub keys
   // [7] number of reservation size entries
   // [8] payload length
-  // [9] payload checksum part A
-  // [10] payload checksum part B
+  // [9] payload checksum part 1
+  // [10] payload checksum part 2
   // ...  reservations
   // ...  code stub keys
   // ...  serialized payload
@@ -124,12 +124,9 @@ class SerializedCodeData : public SerializedData {
       kNumReservationsOffset + kUInt32Size;
   static const uint32_t kPayloadLengthOffset =
       kNumCodeStubKeysOffset + kUInt32Size;
-  static const uint32_t kChecksumPartAOffset =
-      kPayloadLengthOffset + kUInt32Size;
-  static const uint32_t kChecksumPartBOffset =
-      kChecksumPartAOffset + kUInt32Size;
-  static const uint32_t kUnalignedHeaderSize =
-      kChecksumPartBOffset + kUInt32Size;
+  static const uint32_t kChecksum1Offset = kPayloadLengthOffset + kUInt32Size;
+  static const uint32_t kChecksum2Offset = kChecksum1Offset + kUInt32Size;
+  static const uint32_t kUnalignedHeaderSize = kChecksum2Offset + kUInt32Size;
   static const uint32_t kHeaderSize = POINTER_SIZE_ALIGN(kUnalignedHeaderSize);
 
   // Used when consuming.
@@ -158,7 +155,7 @@ class SerializedCodeData : public SerializedData {
   SerializedCodeData(const byte* data, int size)
       : SerializedData(const_cast<byte*>(data), size) {}
 
-  Vector<const byte> ChecksummedContent() const {
+  Vector<const byte> DataWithoutHeader() const {
     return Vector<const byte>(data_ + kHeaderSize, size_ - kHeaderSize);
   }
 

src/snapshot/serializer-common.h

@@ -350,45 +350,6 @@ class SerializedData {
   DISALLOW_COPY_AND_ASSIGN(SerializedData);
 };
 
-class Checksum {
- public:
-  explicit Checksum(Vector<const byte> payload) {
-#ifdef MEMORY_SANITIZER
-    // Computing the checksum includes padding bytes for objects like strings.
-    // Mark every object as initialized in the code serializer.
-    MSAN_MEMORY_IS_INITIALIZED(payload.start(), payload.length());
-#endif  // MEMORY_SANITIZER
-    // Fletcher's checksum. Modified to reduce 64-bit sums to 32-bit.
-    uintptr_t a = 1;
-    uintptr_t b = 0;
-    const uintptr_t* cur = reinterpret_cast<const uintptr_t*>(payload.start());
-    DCHECK(IsAligned(payload.length(), kIntptrSize));
-    const uintptr_t* end = cur + payload.length() / kIntptrSize;
-    while (cur < end) {
-      // Unsigned overflow expected and intended.
-      a += *cur++;
-      b += a;
-    }
-#if V8_HOST_ARCH_64_BIT
-    a ^= a >> 32;
-    b ^= b >> 32;
-#endif  // V8_HOST_ARCH_64_BIT
-    a_ = static_cast<uint32_t>(a);
-    b_ = static_cast<uint32_t>(b);
-  }
-
-  bool Check(uint32_t a, uint32_t b) const { return a == a_ && b == b_; }
-
-  uint32_t a() const { return a_; }
-  uint32_t b() const { return b_; }
-
- private:
-  uint32_t a_;
-  uint32_t b_;
-
-  DISALLOW_COPY_AND_ASSIGN(Checksum);
-};
-
 }  // namespace internal
 }  // namespace v8
 

src/snapshot/serializer.cc

@@ -327,14 +327,14 @@ void Serializer<AllocatorT>::PutNextChunk(int space) {
 }
 
 template <class AllocatorT>
-void Serializer<AllocatorT>::Pad(int padding_offset) {
+void Serializer<AllocatorT>::Pad() {
   // The non-branching GetInt will read up to 3 bytes too far, so we need
   // to pad the snapshot to make sure we don't read over the end.
   for (unsigned i = 0; i < sizeof(int32_t) - 1; i++) {
     sink_.Put(kNop, "Padding");
   }
   // Pad up to pointer size for checksum.
-  while (!IsAligned(sink_.Position() + padding_offset, kPointerAlignment)) {
+  while (!IsAligned(sink_.Position(), kPointerAlignment)) {
     sink_.Put(kNop, "Padding");
   }
 }

src/snapshot/serializer.h

@@ -210,8 +210,7 @@ class Serializer : public SerializerDeserializer {
   }
 
   // GetInt reads 4 bytes at once, requiring padding at the end.
-  // Use padding_offset to specify the space you want to use after padding.
-  void Pad(int padding_offset = 0);
+  void Pad();
 
   // We may not need the code address map for logging for every instance
   // of the serializer.  Initialize it on demand.

src/snapshot/snapshot-common.cc

@@ -44,7 +44,6 @@ bool Snapshot::Initialize(Isolate* isolate) {
 
   const v8::StartupData* blob = isolate->snapshot_blob();
   CheckVersion(blob);
-  CHECK(VerifyChecksum(blob));
   Vector<const byte> startup_data = ExtractStartupData(blob);
   SnapshotData startup_snapshot_data(startup_data);
   Vector<const byte> builtin_data = ExtractBuiltinData(blob);
@@ -203,22 +202,15 @@ v8::StartupData Snapshot::CreateSnapshotBlob(
   uint32_t num_contexts = static_cast<uint32_t>(context_snapshots.size());
   uint32_t startup_snapshot_offset = StartupSnapshotOffset(num_contexts);
   uint32_t total_length = startup_snapshot_offset;
-  DCHECK(IsAligned(total_length, kPointerAlignment));
   total_length += static_cast<uint32_t>(startup_snapshot->RawData().length());
-  DCHECK(IsAligned(total_length, kPointerAlignment));
   total_length += static_cast<uint32_t>(builtin_snapshot->RawData().length());
-  DCHECK(IsAligned(total_length, kPointerAlignment));
   for (const auto context_snapshot : context_snapshots) {
     total_length += static_cast<uint32_t>(context_snapshot->RawData().length());
-    DCHECK(IsAligned(total_length, kPointerAlignment));
   }
 
   ProfileDeserialization(startup_snapshot, builtin_snapshot, context_snapshots);
 
   char* data = new char[total_length];
-  // Zero out pre-payload data. Part of that is only used for padding.
-  memset(data, 0, StartupSnapshotOffset(num_contexts));
-
   SetHeaderValue(data, kNumberOfContextsOffset, num_contexts);
   SetHeaderValue(data, kRehashabilityOffset, can_be_rehashed ? 1 : 0);
 
@@ -268,13 +260,8 @@ v8::StartupData Snapshot::CreateSnapshotBlob(
     payload_offset += payload_length;
   }
 
-  DCHECK_EQ(total_length, payload_offset);
   v8::StartupData result = {data, static_cast<int>(total_length)};
-
-  Checksum checksum(ChecksummedContent(&result));
-  SetHeaderValue(data, kChecksumPartAOffset, checksum.a());
-  SetHeaderValue(data, kChecksumPartBOffset, checksum.b());
-
+  DCHECK_EQ(total_length, payload_offset);
   return result;
 }
 
@@ -494,19 +481,6 @@ uint32_t Snapshot::ExtractNumContexts(const v8::StartupData* data) {
   return num_contexts;
 }
 
-bool Snapshot::VerifyChecksum(const v8::StartupData* data) {
-  base::ElapsedTimer timer;
-  if (FLAG_profile_deserialization) timer.Start();
-  uint32_t expected_a = GetHeaderValue(data, kChecksumPartAOffset);
-  uint32_t expected_b = GetHeaderValue(data, kChecksumPartBOffset);
-  Checksum checksum(ChecksummedContent(data));
-  if (FLAG_profile_deserialization) {
-    double ms = timer.Elapsed().InMillisecondsF();
-    PrintF("[Verifying snapshot checksum took %0.3f ms]\n", ms);
-  }
-  return checksum.Check(expected_a, expected_b);
-}
-
 void EmbeddedData::PrintStatistics() const {
   DCHECK(FLAG_serialization_statistics);
 
@@ -640,18 +614,12 @@ SnapshotData::SnapshotData(const Serializer<AllocatorT>* serializer) {
   // Calculate sizes.
   uint32_t reservation_size =
       static_cast<uint32_t>(reservations.size()) * kUInt32Size;
-  uint32_t payload_offset = kHeaderSize + reservation_size;
-  uint32_t padded_payload_offset = POINTER_SIZE_ALIGN(payload_offset);
   uint32_t size =
-      padded_payload_offset + static_cast<uint32_t>(payload->size());
-  DCHECK(IsAligned(size, kPointerAlignment));
+      kHeaderSize + reservation_size + static_cast<uint32_t>(payload->size());
 
   // Allocate backing store and create result data.
   AllocateData(size);
 
-  // Zero out pre-payload data. Part of that is only used for padding.
-  memset(data_, 0, padded_payload_offset);
-
   // Set header values.
   SetMagicNumber(serializer->isolate());
   SetHeaderValue(kNumReservationsOffset, static_cast<int>(reservations.size()));
@@ -662,7 +630,7 @@ SnapshotData::SnapshotData(const Serializer<AllocatorT>* serializer) {
             reservation_size);
 
   // Copy serialized data.
-  CopyBytes(data_ + padded_payload_offset, payload->data(),
+  CopyBytes(data_ + kHeaderSize + reservation_size, payload->data(),
             static_cast<size_t>(payload->size()));
 }
 
@@ -681,9 +649,7 @@ std::vector<SerializedData::Reservation> SnapshotData::Reservations() const {
 Vector<const byte> SnapshotData::Payload() const {
   uint32_t reservations_size =
       GetHeaderValue(kNumReservationsOffset) * kUInt32Size;
-  uint32_t padded_payload_offset =
-      POINTER_SIZE_ALIGN(kHeaderSize + reservations_size);
-  const byte* payload = data_ + padded_payload_offset;
+  const byte* payload = data_ + kHeaderSize + reservations_size;
   uint32_t length = GetHeaderValue(kPayloadLengthOffset);
   DCHECK_EQ(data_ + size_, payload + length);
   return Vector<const byte>(payload, length);
@@ -693,21 +659,26 @@ BuiltinSnapshotData::BuiltinSnapshotData(const BuiltinSerializer* serializer)
     : SnapshotData(serializer) {}
 
 Vector<const byte> BuiltinSnapshotData::Payload() const {
-  Vector<const byte> payload = SnapshotData::Payload();
+  uint32_t reservations_size =
+      GetHeaderValue(kNumReservationsOffset) * kUInt32Size;
+  const byte* payload = data_ + kHeaderSize + reservations_size;
   const int builtin_offsets_size = Builtins::builtin_count * kUInt32Size;
-  DCHECK_EQ(data_ + size_, payload.start() + payload.size());
-  DCHECK_GT(payload.size(), builtin_offsets_size);
-  return Vector<const byte>(payload.start(),
-                            payload.size() - builtin_offsets_size);
+  uint32_t payload_length = GetHeaderValue(kPayloadLengthOffset);
+  DCHECK_EQ(data_ + size_, payload + payload_length);
+  DCHECK_GT(payload_length, builtin_offsets_size);
+  return Vector<const byte>(payload, payload_length - builtin_offsets_size);
 }
 
 Vector<const uint32_t> BuiltinSnapshotData::BuiltinOffsets() const {
-  Vector<const byte> payload = SnapshotData::Payload();
+  uint32_t reservations_size =
+      GetHeaderValue(kNumReservationsOffset) * kUInt32Size;
+  const byte* payload = data_ + kHeaderSize + reservations_size;
   const int builtin_offsets_size = Builtins::builtin_count * kUInt32Size;
-  DCHECK_EQ(data_ + size_, payload.start() + payload.size());
-  DCHECK_GT(payload.size(), builtin_offsets_size);
+  uint32_t payload_length = GetHeaderValue(kPayloadLengthOffset);
+  DCHECK_EQ(data_ + size_, payload + payload_length);
+  DCHECK_GT(payload_length, builtin_offsets_size);
   const uint32_t* data = reinterpret_cast<const uint32_t*>(
-      payload.start() + payload.size() - builtin_offsets_size);
+      payload + payload_length - builtin_offsets_size);
   return Vector<const uint32_t>(data, Builtins::builtin_count);
 }
 

src/snapshot/snapshot.h

@@ -183,8 +183,6 @@ class Snapshot : public AllStatic {
   // To be implemented by the snapshot source.
   static const v8::StartupData* DefaultSnapshotBlob();
 
-  static bool VerifyChecksum(const v8::StartupData* data);
-
   // ---------------- Serialization ----------------
 
   static v8::StartupData CreateSnapshotBlob(
@@ -220,12 +218,10 @@ class Snapshot : public AllStatic {
   // Snapshot blob layout:
   // [0] number of contexts N
   // [1] rehashability
-  // [2] checksum part A
-  // [3] checksum part B
-  // [4] (128 bytes) version string
-  // [5] offset to builtins
-  // [6] offset to context 0
-  // [7] offset to context 1
+  // [2] (128 bytes) version string
+  // [3] offset to builtins
+  // [4] offset to context 0
+  // [5] offset to context 1
   // ...
   // ... offset to context N - 1
   // ... startup snapshot data
@@ -237,28 +233,16 @@ class Snapshot : public AllStatic {
   // TODO(yangguo): generalize rehashing, and remove this flag.
   static const uint32_t kRehashabilityOffset =
       kNumberOfContextsOffset + kUInt32Size;
-  static const uint32_t kChecksumPartAOffset =
-      kRehashabilityOffset + kUInt32Size;
-  static const uint32_t kChecksumPartBOffset =
-      kChecksumPartAOffset + kUInt32Size;
   static const uint32_t kVersionStringOffset =
-      kChecksumPartBOffset + kUInt32Size;
+      kRehashabilityOffset + kUInt32Size;
   static const uint32_t kVersionStringLength = 64;
   static const uint32_t kBuiltinOffsetOffset =
       kVersionStringOffset + kVersionStringLength;
   static const uint32_t kFirstContextOffsetOffset =
       kBuiltinOffsetOffset + kUInt32Size;
 
-  static Vector<const byte> ChecksummedContent(const v8::StartupData* data) {
-    const uint32_t kChecksumStart = kVersionStringOffset;
-    return Vector<const byte>(
-        reinterpret_cast<const byte*>(data->data + kChecksumStart),
-        data->raw_size - kChecksumStart);
-  }
-
   static uint32_t StartupSnapshotOffset(int num_contexts) {
-    return POINTER_SIZE_ALIGN(kFirstContextOffsetOffset +
-                              num_contexts * kInt32Size);
+    return kFirstContextOffsetOffset + num_contexts * kInt32Size;
   }
 
   static uint32_t ContextSnapshotOffsetOffset(int index) {

test/cctest/test-serialize.cc

@@ -785,17 +785,6 @@ TEST(CustomSnapshotDataBlob1) {
   delete[] data1.data;  // We can dispose of the snapshot blob now.
 }
 
-TEST(SnapshotChecksum) {
-  DisableAlwaysOpt();
-  const char* source1 = "function f() { return 42; }";
-
-  v8::StartupData data1 = CreateSnapshotDataBlob(source1);
-  CHECK(i::Snapshot::VerifyChecksum(&data1));
-  const_cast<char*>(data1.data)[142] = data1.data[142] ^ 4;  // Flip a bit.
-  CHECK(!i::Snapshot::VerifyChecksum(&data1));
-  delete[] data1.data;  // We can dispose of the snapshot blob now.
-}
-
 struct InternalFieldData {
   uint32_t data;
 };