From eef303028f9f06a6c64c20e5b537fd6db6ac69a7 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 23 Aug 2024 13:53:18 +0200
Subject: [PATCH] src: remove cached data tag from snapshot metadata

This only served as a preemptive check, but serializing this in
the snapshot would make it unreproducible on different hardware.
In the current cached data version tag, the V8 version can already
be checked as part of the existing Node.js version check. The V8
flags aren't necessarily important for snapshot/code cache mismatches
(only a small subset are), and the CPU features currently don't
matter, so doing an exact match is stricter than necessary.
Removing the check to help making the snapshot more reproducible on
different hardware.

PR-URL: /~https://github.com/nodejs/node/pull/54122
Refs: /~https://github.com/nodejs/build/issues/3043
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
---
 src/env.cc                    |  1 -
 src/env.h                     |  2 --
 src/node_snapshotable.cc      | 23 -----------------------
 test/parallel/parallel.status |  7 +++++++
 4 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/src/env.cc b/src/env.cc
index f2c0105fa28291..852a8651c8a769 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -320,7 +320,6 @@ std::ostream& operator<<(std::ostream& output, const SnapshotMetadata& i) {
          << "  \"" << i.node_version << "\", // node_version\n"
          << "  \"" << i.node_arch << "\", // node_arch\n"
          << "  \"" << i.node_platform << "\", // node_platform\n"
-         << "  " << i.v8_cache_version_tag << ", // v8_cache_version_tag\n"
          << "  " << i.flags << ", // flags\n"
          << "}";
   return output;
diff --git a/src/env.h b/src/env.h
index ee119eb3556b6d..6177e52d25272d 100644
--- a/src/env.h
+++ b/src/env.h
@@ -549,8 +549,6 @@ struct SnapshotMetadata {
   std::string node_version;
   std::string node_arch;
   std::string node_platform;
-  // Result of v8::ScriptCompiler::CachedDataVersionTag().
-  uint32_t v8_cache_version_tag;
   SnapshotFlags flags;
 };
 
diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
index 426066fb9d63d4..fe04a8ee8d708b 100644
--- a/src/node_snapshotable.cc
+++ b/src/node_snapshotable.cc
@@ -43,7 +43,6 @@ using v8::Isolate;
 using v8::Local;
 using v8::Object;
 using v8::ObjectTemplate;
-using v8::ScriptCompiler;
 using v8::SnapshotCreator;
 using v8::StartupData;
 using v8::String;
@@ -542,7 +541,6 @@ SnapshotMetadata SnapshotDeserializer::Read() {
   result.node_version = ReadString();
   result.node_arch = ReadString();
   result.node_platform = ReadString();
-  result.v8_cache_version_tag = ReadArithmetic<uint32_t>();
   result.flags = static_cast<SnapshotFlags>(ReadArithmetic<uint32_t>());
 
   if (is_debug) {
@@ -570,9 +568,6 @@ size_t SnapshotSerializer::Write(const SnapshotMetadata& data) {
   written_total += WriteString(data.node_arch);
   Debug("Write Node.js platform %s\n", data.node_platform);
   written_total += WriteString(data.node_platform);
-  Debug("Write V8 cached data version tag %" PRIx32 "\n",
-        data.v8_cache_version_tag);
-  written_total += WriteArithmetic<uint32_t>(data.v8_cache_version_tag);
   Debug("Write snapshot flags %" PRIx32 "\n",
         static_cast<uint32_t>(data.flags));
   written_total += WriteArithmetic<uint32_t>(static_cast<uint32_t>(data.flags));
@@ -697,23 +692,6 @@ bool SnapshotData::Check() const {
     return false;
   }
 
-  if (metadata.type == SnapshotMetadata::Type::kFullyCustomized &&
-      !WithoutCodeCache(metadata.flags)) {
-    uint32_t current_cache_version = v8::ScriptCompiler::CachedDataVersionTag();
-    if (metadata.v8_cache_version_tag != current_cache_version) {
-      // For now we only do this check for the customized snapshots - we know
-      // that the flags we use in the default snapshot are limited and safe
-      // enough so we can relax the constraints for it.
-      fprintf(stderr,
-              "Failed to load the startup snapshot because it was built with "
-              "a different version of V8 or with different V8 configurations.\n"
-              "Expected tag %" PRIx32 ", read %" PRIx32 "\n",
-              current_cache_version,
-              metadata.v8_cache_version_tag);
-      return false;
-    }
-  }
-
   // TODO(joyeecheung): check incompatible Node.js flags.
   return true;
 }
@@ -1180,7 +1158,6 @@ ExitCode SnapshotBuilder::CreateSnapshot(SnapshotData* out,
                                    per_process::metadata.versions.node,
                                    per_process::metadata.arch,
                                    per_process::metadata.platform,
-                                   v8::ScriptCompiler::CachedDataVersionTag(),
                                    config->flags};
 
   // We cannot resurrect the handles from the snapshot, so make sure that
diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
index da93f6ca89ac83..8328ae196f63bd 100644
--- a/test/parallel/parallel.status
+++ b/test/parallel/parallel.status
@@ -19,6 +19,13 @@ test-fs-read-stream-concurrent-reads: PASS, FLAKY
 # /~https://github.com/nodejs/node/issues/52630
 test-error-serdes: PASS, FLAKY
 
+# Until V8 provides a better way to check for flag mismatch without
+# making the code cache/snapshot unreproducible, disable the test
+# for a preemptive check now. It should idealy fail more gracefully
+# with a better checking mechanism.
+# /~https://github.com/nodejs/build/issues/3043
+test-snapshot-incompatible: SKIP
+
 [$system==win32]
 
 # Windows on x86