[snapshot] Clear reconstructable data prior to d8 stress_snapshot run

The serializer currently cannot handle a heap state containing
arbitrary compiled Code objects. As a quick fix for the
--stress-snapshot d8 flag, we clear compiled data from the isolate
prior to the serialize-deserialize-verify pass.

With this change, mjsunit tests pass on x64.

The %SerializeDeserializeNow() runtime function would require more
work, since it is not possible to mutate the heap to this extent while
still preserving a runnable host context and isolate. We will need
another solution there.

Drive-by: Skip the stress_snapshot variant except for the mjsunit
suite.

Tbr: machenbach@chromium.org
Bug: v8:10493,v8:10416
Change-Id: Ie110da8b51613fcd69c7f391d3cf8589d6b04dd8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2182429
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67585}

src/api/api.cc

@@ -774,79 +774,11 @@ StartupData SnapshotCreator::CreateBlob(
     isolate->heap()->CompactWeakArrayLists(internal::AllocationType::kOld);
   }
 
-  if (function_code_handling == FunctionCodeHandling::kClear) {
-    // Clear out re-compilable data from all shared function infos. Any
-    // JSFunctions using these SFIs will have their code pointers reset by the
-    // context serializer.
-    //
-    // We have to iterate the heap and collect handles to each clearable SFI,
-    // before we disable allocation, since we have to allocate UncompiledDatas
-    // to be able to recompile them.
-    //
-    // Compiled irregexp code is also flushed by collecting and clearing any
-    // seen JSRegExp objects.
-    i::HandleScope scope(isolate);
-    std::vector<i::Handle<i::SharedFunctionInfo>> sfis_to_clear;
-
-    {  // Heap allocation is disallowed within this scope.
-      i::HeapObjectIterator heap_iterator(isolate->heap());
-      for (i::HeapObject current_obj = heap_iterator.Next();
-           !current_obj.is_null(); current_obj = heap_iterator.Next()) {
-        if (current_obj.IsSharedFunctionInfo()) {
-          i::SharedFunctionInfo shared =
-              i::SharedFunctionInfo::cast(current_obj);
-          if (shared.CanDiscardCompiled()) {
-            sfis_to_clear.emplace_back(shared, isolate);
-          }
-        } else if (current_obj.IsJSRegExp()) {
-          i::JSRegExp regexp = i::JSRegExp::cast(current_obj);
-          if (regexp.HasCompiledCode()) {
-            regexp.DiscardCompiledCodeForSerialization();
-          }
-        }
-      }
-    }
-
-    // Must happen after heap iteration since SFI::DiscardCompiled may allocate.
-    for (i::Handle<i::SharedFunctionInfo> shared : sfis_to_clear) {
-      i::SharedFunctionInfo::DiscardCompiled(isolate, shared);
-    }
-  }
+  i::Snapshot::ClearReconstructableDataForSerialization(
+      isolate, function_code_handling == FunctionCodeHandling::kClear);
 
   i::DisallowHeapAllocation no_gc_from_here_on;
 
-  i::HeapObjectIterator heap_iterator(isolate->heap());
-  for (i::HeapObject current_obj = heap_iterator.Next(); !current_obj.is_null();
-       current_obj = heap_iterator.Next()) {
-    if (current_obj.IsJSFunction()) {
-      i::JSFunction fun = i::JSFunction::cast(current_obj);
-
-      // Complete in-object slack tracking for all functions.
-      fun.CompleteInobjectSlackTrackingIfActive();
-
-      // Also, clear out feedback vectors, or any optimized code.
-      // Note that checking for fun.IsOptimized() || fun.IsInterpreted() is not
-      // sufficient because the function can have a feedback vector even if it
-      // is not compiled (e.g. when the bytecode was flushed). On the other
-      // hand, only checking for the feedback vector is not sufficient because
-      // there can be multiple functions sharing the same feedback vector. So we
-      // need all these checks.
-      if (fun.IsOptimized() || fun.IsInterpreted() ||
-          !fun.raw_feedback_cell().value().IsUndefined()) {
-        fun.raw_feedback_cell().set_value(
-            i::ReadOnlyRoots(isolate).undefined_value());
-        fun.set_code(isolate->builtins()->builtin(i::Builtins::kCompileLazy));
-      }
-#ifdef DEBUG
-      if (function_code_handling == FunctionCodeHandling::kClear) {
-        DCHECK(fun.shared().HasWasmExportedFunctionData() ||
-               fun.shared().HasBuiltinId() || fun.shared().IsApiFunction() ||
-               fun.shared().HasUncompiledDataWithoutPreparseData());
-      }
-#endif  // DEBUG
-    }
-  }
-
   // Create a vector with all contexts and clear associated Persistent fields.
   // Note these contexts may be dead after calling Clear(), but will not be
   // collected until serialization completes and the DisallowHeapAllocation

src/d8/d8.cc

@@ -3046,9 +3046,12 @@ int Shell::RunMain(Isolate* isolate, bool last_run) {
       DisposeModuleEmbedderData(context);
     }
     WriteLcovData(isolate, options.lcov_file);
-    if (options.stress_snapshot) {
+    if (last_run && options.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);
+      i::Snapshot::ClearReconstructableDataForSerialization(
+          i_isolate, kClearRecompilableData);
       i::Snapshot::SerializeDeserializeAndVerifyForTesting(i_isolate,
                                                            i_context);
     }

src/snapshot/snapshot.cc

@@ -10,6 +10,7 @@
 #include "src/execution/isolate-inl.h"
 #include "src/init/bootstrapper.h"
 #include "src/logging/counters.h"
+#include "src/objects/js-regexp-inl.h"
 #include "src/snapshot/context-deserializer.h"
 #include "src/snapshot/context-serializer.h"
 #include "src/snapshot/read-only-deserializer.h"
@@ -188,6 +189,79 @@ MaybeHandle<Context> Snapshot::NewContextFromSnapshot(
   return result;
 }
 
+// static
+void Snapshot::ClearReconstructableDataForSerialization(
+    Isolate* isolate, bool clear_recompilable_data) {
+  // Clear SFIs and JSRegExps.
+
+  if (clear_recompilable_data) {
+    HandleScope scope(isolate);
+    std::vector<i::Handle<i::SharedFunctionInfo>> sfis_to_clear;
+    {  // Heap allocation is disallowed within this scope.
+      i::HeapObjectIterator it(isolate->heap());
+      for (i::HeapObject o = it.Next(); !o.is_null(); o = it.Next()) {
+        if (o.IsSharedFunctionInfo()) {
+          i::SharedFunctionInfo shared = i::SharedFunctionInfo::cast(o);
+          if (shared.script().IsScript() &&
+              Script::cast(shared.script()).type() == Script::TYPE_EXTENSION) {
+            continue;  // Don't clear extensions, they cannot be recompiled.
+          }
+          if (shared.CanDiscardCompiled()) {
+            sfis_to_clear.emplace_back(shared, isolate);
+          }
+        } else if (o.IsJSRegExp()) {
+          i::JSRegExp regexp = i::JSRegExp::cast(o);
+          if (regexp.HasCompiledCode()) {
+            regexp.DiscardCompiledCodeForSerialization();
+          }
+        }
+      }
+    }
+
+    // Must happen after heap iteration since SFI::DiscardCompiled may allocate.
+    for (i::Handle<i::SharedFunctionInfo> shared : sfis_to_clear) {
+      i::SharedFunctionInfo::DiscardCompiled(isolate, shared);
+    }
+  }
+
+  // Clear JSFunctions.
+
+  i::HeapObjectIterator it(isolate->heap());
+  for (i::HeapObject o = it.Next(); !o.is_null(); o = it.Next()) {
+    if (!o.IsJSFunction()) continue;
+
+    i::JSFunction fun = i::JSFunction::cast(o);
+    fun.CompleteInobjectSlackTrackingIfActive();
+
+    i::SharedFunctionInfo shared = fun.shared();
+    if (shared.script().IsScript() &&
+        Script::cast(shared.script()).type() == Script::TYPE_EXTENSION) {
+      continue;  // Don't clear extensions, they cannot be recompiled.
+    }
+
+    // Also, clear out feedback vectors, or any optimized code.
+    // Note that checking for fun.IsOptimized() || fun.IsInterpreted() is
+    // not sufficient because the function can have a feedback vector even
+    // if it is not compiled (e.g. when the bytecode was flushed). On the
+    // other hand, only checking for the feedback vector is not sufficient
+    // because there can be multiple functions sharing the same feedback
+    // vector. So we need all these checks.
+    if (fun.IsOptimized() || fun.IsInterpreted() ||
+        !fun.raw_feedback_cell().value().IsUndefined()) {
+      fun.raw_feedback_cell().set_value(
+          i::ReadOnlyRoots(isolate).undefined_value());
+      fun.set_code(isolate->builtins()->builtin(i::Builtins::kCompileLazy));
+    }
+#ifdef DEBUG
+    if (clear_recompilable_data) {
+      DCHECK(fun.shared().HasWasmExportedFunctionData() ||
+             fun.shared().HasBuiltinId() || fun.shared().IsApiFunction() ||
+             fun.shared().HasUncompiledDataWithoutPreparseData());
+    }
+#endif  // DEBUG
+  }
+}
+
 // static
 void Snapshot::SerializeDeserializeAndVerifyForTesting(
     Isolate* isolate, Handle<Context> default_context) {

src/snapshot/snapshot.h

@@ -41,6 +41,13 @@ class Snapshot : public AllStatic {
   V8_EXPORT_PRIVATE static constexpr SerializerFlags kDefaultSerializerFlags =
       {};
 
+  // In preparation for serialization, clear data from the given isolate's heap
+  // that 1. can be reconstructed and 2. is not suitable for serialization. The
+  // `clear_recompilable_data` flag controls whether compiled objects are
+  // cleared from shared function infos and regexp objects.
+  V8_EXPORT_PRIVATE static void ClearReconstructableDataForSerialization(
+      Isolate* isolate, bool clear_recompilable_data);
+
   // Serializes the given isolate and contexts. Each context may have an
   // associated callback to serialize internal fields. The default context must
   // be passed at index 0.

test/benchmarks/benchmarks.status

@@ -82,4 +82,9 @@
   'octane/typescript': [SKIP],
 }],  # 'predictable'
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/cctest/cctest.status

@@ -631,4 +631,9 @@
   'test-regexp/UnicodePropertyEscapeCodeSize': [SKIP],
 }],  # no_i18n == True
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/debugger/debugger.status

@@ -162,4 +162,9 @@
   'regress/regress-crbug-1032042': [SKIP],
 }],  # 'isolates'
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/debugging/debugging.status

@@ -2,4 +2,11 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-[]
+[
+
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
+]

test/fuzzer/fuzzer.status

@@ -13,4 +13,9 @@
   'wasm_compile/*': [SKIP],
 }], # lite_mode or variant == jitless
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/inspector/inspector.status

@@ -86,4 +86,9 @@
   'debugger/wasm-*': [SKIP],
 }],  # 'arch == s390 or arch == s390x'
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/intl/intl.status

@@ -66,4 +66,9 @@
   'regress-7770': [SKIP],
 }],  # 'system == android'
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/message/message.status

@@ -64,4 +64,9 @@
   'asm-*': [SKIP],
 }], # lite_mode or variant == jitless
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/mjsunit/mjsunit.status

@@ -1192,12 +1192,16 @@
 }],  # variant == assert_types
 
 ##############################################################################
-['variant == stress_snapshot', {
-  # TODO(jgruber): Remove this wildcard line ASAP.
+['variant == stress_snapshot and arch != x64', {
+  # Deserialization fails due to read-only snapshot checksum verification.
+  # https://crbug.com/v8/10491
   '*': [SKIP],
+}],
 
+['variant == stress_snapshot and arch == x64', {
   # Crashes the serializer due to recursion.
   'deep-recursion': [SKIP],
+  'string-replace-gc': [SKIP],
   # Check failed: !field_type.NowStable() || field_type.NowContains(value).
   'eval': [SKIP],
   'regress/regress-737588': [SKIP],

test/mozilla/mozilla.status

@@ -1063,4 +1063,9 @@
   '*': [SKIP],
 }],  # variant == no_wasm_traps
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/test262/test262.status

@@ -775,4 +775,9 @@
   'intl402/DateTimeFormat/prototype/resolvedOptions/basic': [SKIP],
 }],  # system == windows
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/unittests/unittests.status

@@ -48,4 +48,9 @@
   'DecompressionOptimizerTest.*': [SKIP],
 }], # not pointer_compression
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/wasm-api-tests/wasm-api-tests.status

@@ -3,9 +3,15 @@
 # found in the LICENSE file.
 
 [
+
 ['lite_mode or variant == jitless', {
   # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
   '*': [SKIP],
 }], # lite_mode or variant == jitless
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/wasm-js/wasm-js.status

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 [
+
 [ALWAYS, {
   # These are slow, and not useful to run for the proposals:
   'proposals/reference-types/limits': [SKIP],
@@ -34,4 +35,9 @@
   '*': [SKIP],
 }], # lite_mode or variant == jitless
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]

test/wasm-spec-tests/wasm-spec-tests.status

@@ -99,5 +99,9 @@
   '*': [SKIP],
 }], # lite_mode or variant == jitless
 
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
 
 ]

test/webkit/webkit.status

@@ -136,5 +136,9 @@
   'fast/js/string-capitalization': [FAIL],
 }],  # variant == no_wasm_traps
 
-##############################################################################
+################################################################################
+['variant == stress_snapshot', {
+  '*': [SKIP],  # only relevant for mjsunit tests.
+}],
+
 ]