[snapshot] Re-enable shared read-only heap with --stress-snapshot

Disable the checksum comparison for the read-only snapshot when
--stress-snapshot is used, since it's possible that it would be
corrupted. This corruption is not important as the purpose of
stress-snapshot is not to produce a useable snapshot, but to test that
the serialization/deserialization does not fail for any given objects.

Since the --stress-snapshot flag's value is now used outside of d8,
this also moves it to flag-definitions.h.

Cq-Include-Trybots: luci.v8.try:v8_linux64_gc_stress_custom_snapshot_dbg_ng
Bug: v8:11750
Change-Id: Iedcf1cfb5afa5f16ac19a76820b62b5b93948f2c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2882810
Commit-Queue: Dan Elphick <delphick@chromium.org>
Auto-Submit: Dan Elphick <delphick@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74480}

src/d8/d8.cc

@@ -3971,25 +3971,6 @@ bool Shell::SetOptions(int argc, char* argv[]) {
                strcmp(argv[i], "--no-stress-opt") == 0) {
       options.stress_opt = false;
       argv[i] = nullptr;
-    } else if (strcmp(argv[i], "--stress-snapshot") == 0) {
-      options.stress_snapshot = true;
-      // Incremental marking is incompatible with the stress_snapshot mode;
-      // specifically, serialization may clear bytecode arrays from shared
-      // function infos which the MarkCompactCollector (running concurrently)
-      // may still need. See also https://crbug.com/v8/10882.
-      //
-      // We thus force the implication
-      //
-      //   --stress-snapshot ~~> --no-incremental-marking
-      //
-      // Note: This is not an issue in production because we don't clear SFI's
-      // there (that only happens in mksnapshot and in --stress-snapshot mode).
-      i::FLAG_incremental_marking = false;
-      argv[i] = nullptr;
-    } else if (strcmp(argv[i], "--nostress-snapshot") == 0 ||
-               strcmp(argv[i], "--no-stress-snapshot") == 0) {
-      options.stress_snapshot = false;
-      argv[i] = nullptr;
     } else if (strcmp(argv[i], "--noalways-opt") == 0 ||
                strcmp(argv[i], "--no-always-opt") == 0) {
       no_always_opt = true;
@@ -4244,7 +4225,7 @@ int Shell::RunMain(Isolate* isolate, bool last_run) {
       DisposeModuleEmbedderData(context);
     }
     WriteLcovData(isolate, options.lcov_file);
-    if (last_run && options.stress_snapshot) {
+    if (last_run && i::FLAG_stress_snapshot) {
       static constexpr bool kClearRecompilableData = true;
       i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
       i::Handle<i::Context> i_context = Utils::OpenHandle(*context);

src/d8/d8.h

@@ -362,7 +362,6 @@ class ShellOptions {
   DisallowReassignment<bool> simulate_errors = {"simulate-errors", false};
   DisallowReassignment<bool> stress_opt = {"stress-opt", false};
   DisallowReassignment<int> stress_runs = {"stress-runs", 1};
-  DisallowReassignment<bool> stress_snapshot = {"stress-snapshot", false};
   DisallowReassignment<bool> interactive_shell = {"shell", false};
   bool test_shell = false;
   DisallowReassignment<bool> expected_to_throw = {"throws", false};

src/flags/flag-definitions.h

@@ -377,6 +377,17 @@ DEFINE_BOOL(icu_timezone_data, true, "get information about timezones from ICU")
 #define V8_SHARED_RO_HEAP_BOOL false
 #endif
 
+DEFINE_BOOL(stress_snapshot, false,
+            "disables sharing of the read-only heap for testing")
+// Incremental marking is incompatible with the stress_snapshot mode;
+// specifically, serialization may clear bytecode arrays from shared function
+// infos which the MarkCompactCollector (running concurrently) may still need.
+// See also https://crbug.com/v8/10882.
+//
+// Note: This is not an issue in production because we don't clear SFI's
+// there (that only happens in mksnapshot and in --stress-snapshot mode).
+DEFINE_NEG_IMPLICATION(stress_snapshot, incremental_marking)
+
 DEFINE_BOOL(lite_mode, V8_LITE_BOOL,
             "enables trade-off of performance for memory savings")
 

src/heap/read-only-spaces.cc

@@ -56,7 +56,18 @@ void ReadOnlyArtifacts::VerifyChecksum(SnapshotData* read_only_snapshot_data,
     CHECK_WITH_MSG(snapshot_checksum,
                    "Attempt to create the read-only heap after already "
                    "creating from a snapshot.");
-    CHECK_EQ(read_only_blob_checksum_, snapshot_checksum);
+    if (!FLAG_stress_snapshot) {
+      // --stress-snapshot is only intended to check how well the
+      // serializer/deserializer copes with unexpected objects, and is not
+      // intended to test whether the newly deserialized Isolate would actually
+      // work since it serializes a currently running Isolate, which is not
+      // supported. As a result, it's possible that it will create a new
+      // read-only snapshot that is not compatible with the original one (for
+      // instance due to the string table being re-ordered). Since we won't
+      // acutally use that new Isoalte, we're ok with any potential corruption.
+      // See crbug.com/1043058.
+      CHECK_EQ(read_only_blob_checksum_, snapshot_checksum);
+    }
   } else {
     // If there's no checksum, then that means the read-only heap objects are
     // being created.