Reland "[snapshot] Clear irregexp code prior to serialization"

This is a reland of 1efe6259

Original change's description:
> [snapshot] Clear irregexp code prior to serialization
> 
> Compiled irregexp code should be cleared unless
> FunctionCodeHandling::kKeep is passed.
> 
> Bug: v8:8572
> Change-Id: Icb74cc6e0f39a69f8383b05f1638cf0e3be1807c
> Reviewed-on: https://chromium-review.googlesource.com/c/1373773
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58272}

Bug: v8:8572
Change-Id: I2dfe0c1b4419c47fe2a1916c3bc16800a30309c5
Reviewed-on: https://chromium-review.googlesource.com/c/1379937
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58308}

src/api.cc

@@ -751,19 +751,31 @@ StartupData SnapshotCreator::CreateBlob(
     // 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;
 
-    i::HeapIterator heap_iterator(isolate->heap());
-    while (i::HeapObject* 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);
+    {  // Heap allocation is disallowed within this scope.
+      i::HeapIterator heap_iterator(isolate->heap());
+      while (i::HeapObject* 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();
+          }
         }
       }
     }
-    i::AllowHeapAllocation allocate_for_discard;
+
+    // Must happen after heap iteration since SFI::DiscardCompiled may allocate.
     for (i::Handle<i::SharedFunctionInfo> shared : sfis_to_clear) {
       i::SharedFunctionInfo::DiscardCompiled(isolate, shared);
     }

src/objects/js-regexp-inl.h

@@ -26,7 +26,7 @@ ACCESSORS(JSRegExp, flags, Object, kFlagsOffset)
 ACCESSORS(JSRegExp, source, Object, kSourceOffset)
 ACCESSORS(JSRegExp, last_index, Object, kLastIndexOffset)
 
-JSRegExp::Type JSRegExp::TypeTag() {
+JSRegExp::Type JSRegExp::TypeTag() const {
   Object* data = this->data();
   if (data->IsUndefined()) return JSRegExp::NOT_COMPILED;
   Smi smi = Smi::cast(FixedArray::cast(data)->get(kTagIndex));
@@ -66,7 +66,7 @@ Object* JSRegExp::CaptureNameMap() {
   return value;
 }
 
-Object* JSRegExp::DataAt(int index) {
+Object* JSRegExp::DataAt(int index) const {
   DCHECK(TypeTag() != NOT_COMPILED);
   return FixedArray::cast(data())->get(index);
 }
@@ -78,6 +78,17 @@ void JSRegExp::SetDataAt(int index, Object* value) {
   FixedArray::cast(data())->set(index, value);
 }
 
+bool JSRegExp::HasCompiledCode() const {
+  return DataAt(kIrregexpLatin1CodeIndex)->IsCode() ||
+         DataAt(kIrregexpUC16CodeIndex)->IsCode();
+}
+
+void JSRegExp::DiscardCompiledCodeForSerialization() {
+  DCHECK(HasCompiledCode());
+  SetDataAt(kIrregexpLatin1CodeIndex, Smi::FromInt(kUninitializedValue));
+  SetDataAt(kIrregexpUC16CodeIndex, Smi::FromInt(kUninitializedValue));
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/objects/js-regexp.h

@@ -95,13 +95,13 @@ class JSRegExp : public JSObject {
                                           Handle<String> source,
                                           Handle<String> flags_string);
 
-  inline Type TypeTag();
+  inline Type TypeTag() const;
   // Number of captures (without the match itself).
   inline int CaptureCount();
   inline Flags GetFlags();
   inline String Pattern();
   inline Object* CaptureNameMap();
-  inline Object* DataAt(int index);
+  inline Object* DataAt(int index) const;
   // Set implementation data after the object has been prepared.
   inline void SetDataAt(int index, Object* value);
 
@@ -113,6 +113,9 @@ class JSRegExp : public JSObject {
     }
   }
 
+  inline bool HasCompiledCode() const;
+  inline void DiscardCompiledCodeForSerialization();
+
   DECL_CAST2(JSRegExp)
 
   // Dispatched behavior.

test/cctest/test-serialize.cc

@@ -43,6 +43,7 @@
 #include "src/objects-inl.h"
 #include "src/objects/js-array-buffer-inl.h"
 #include "src/objects/js-array-inl.h"
+#include "src/objects/js-regexp-inl.h"
 #include "src/runtime/runtime.h"
 #include "src/snapshot/code-serializer.h"
 #include "src/snapshot/natives.h"
@@ -163,7 +164,9 @@ bool RunExtraCode(v8::Isolate* isolate, v8::Local<v8::Context> context,
   return true;
 }
 
-v8::StartupData CreateSnapshotDataBlob(const char* embedded_source = nullptr) {
+v8::StartupData CreateSnapshotDataBlob(
+    v8::SnapshotCreator::FunctionCodeHandling function_code_handling,
+    const char* embedded_source) {
   // Create a new isolate and a new context from scratch, optionally run
   // a script to embed, and serialize to create a snapshot blob.
   DisableEmbeddedBlobRefcounting();
@@ -180,12 +183,16 @@ v8::StartupData CreateSnapshotDataBlob(const char* embedded_source = nullptr) {
       }
       snapshot_creator.SetDefaultContext(context);
     }
-    result = snapshot_creator.CreateBlob(
-        v8::SnapshotCreator::FunctionCodeHandling::kClear);
+    result = snapshot_creator.CreateBlob(function_code_handling);
   }
   return result;
 }
 
+v8::StartupData CreateSnapshotDataBlob(const char* embedded_source = nullptr) {
+  return CreateSnapshotDataBlob(
+      v8::SnapshotCreator::FunctionCodeHandling::kClear, embedded_source);
+}
+
 v8::StartupData WarmUpSnapshotDataBlob(v8::StartupData cold_snapshot_blob,
                                        const char* warmup_source) {
   CHECK(cold_snapshot_blob.raw_size > 0 && cold_snapshot_blob.data != nullptr);
@@ -818,7 +825,11 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobStringNotInternalized) {
   FreeCurrentEmbeddedBlob();
 }
 
-UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCode) {
+#ifndef V8_INTERPRETED_REGEXP
+namespace {
+
+void TestCustomSnapshotDataBlobWithIrregexpCode(
+    v8::SnapshotCreator::FunctionCodeHandling function_code_handling) {
   DisableAlwaysOpt();
   const char* source =
       "var re = /\\/\\*[^*]*\\*+([^/*][^*]*\\*+)*\\//;\n"
@@ -827,7 +838,8 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCode) {
       "function h() { return '// this is a comment'.search(re); }\n"
       "f(); f(); g(); g();";
 
-  v8::StartupData data1 = CreateSnapshotDataBlob(source);
+  v8::StartupData data1 =
+      CreateSnapshotDataBlob(function_code_handling, source);
 
   v8::Isolate::CreateParams params1;
   params1.snapshot_blob = &data1;
@@ -840,6 +852,15 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCode) {
     v8::HandleScope h_scope(isolate1);
     v8::Local<v8::Context> context = v8::Context::New(isolate1);
     v8::Context::Scope c_scope(context);
+    {
+      // Check that compiled irregexp code has not been flushed prior to
+      // serialization.
+      i::Handle<i::JSRegExp> re =
+          Utils::OpenHandle(*CompileRun("re").As<v8::RegExp>());
+      CHECK_EQ(re->HasCompiledCode(),
+               function_code_handling ==
+                   v8::SnapshotCreator::FunctionCodeHandling::kKeep);
+    }
     {
       v8::Maybe<int32_t> result =
           CompileRun("f()")->Int32Value(isolate1->GetCurrentContext());
@@ -861,6 +882,19 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCode) {
   FreeCurrentEmbeddedBlob();
 }
 
+}  // namespace
+
+UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCodeKeepCode) {
+  TestCustomSnapshotDataBlobWithIrregexpCode(
+      v8::SnapshotCreator::FunctionCodeHandling::kKeep);
+}
+
+UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCodeClearCode) {
+  TestCustomSnapshotDataBlobWithIrregexpCode(
+      v8::SnapshotCreator::FunctionCodeHandling::kClear);
+}
+#endif  // V8_INTERPRETED_REGEXP
+
 UNINITIALIZED_TEST(SnapshotChecksum) {
   DisableAlwaysOpt();
   const char* source1 = "function f() { return 42; }";