From dec6dc4baf9225691c15dade1fef5b8da2bb0299 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simon=20Z=C3=BCnd?= <szuend@chromium.org>
Date: Tue, 19 Nov 2019 08:00:02 +0100
Subject: [PATCH] Prevent stack frame cache usage during isolate serialization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Individual frames of a stack frame in the frame cache might point
to the JSFunction of that corresponding stack frame. It is illegal to
serialize JSFunction objects in the isolate snapshot, so the attempt
to serialize the stack frame cache results in a crash. This can happen
when a warmup script is run, before a snapshot is created.

This CL fixes the crash by not utilizing the stack frame cache in case
the serializer is enabled.

Change-Id: I8b79a06b8cff36e1f54b54d3d8e5397b07ba52e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1923068
Auto-Submit: Simon Zünd <szuend@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65026}
---
 src/execution/isolate.cc      |  3 ++-
 test/cctest/test-serialize.cc | 48 +++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
index 1d2e1faa8b..ef11e5ad53 100644
--- a/src/execution/isolate.cc
+++ b/src/execution/isolate.cc
@@ -749,7 +749,8 @@ class FrameArrayBuilder {
     for (int i = 0; i < frame_count; ++i) {
       // Caching stack frames only happens for user JS frames.
       const bool cache_frame =
-          enable_frame_caching && !elements_->IsAnyWasmFrame(i) &&
+          enable_frame_caching && !isolate_->serializer_enabled() &&
+          !elements_->IsAnyWasmFrame(i) &&
           elements_->Function(i).shared().IsUserJavaScript();
       if (cache_frame) {
         MaybeHandle<StackTraceFrame> maybe_frame =
diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc
index 8fadbca918..f7f28d960c 100644
--- a/test/cctest/test-serialize.cc
+++ b/test/cctest/test-serialize.cc
@@ -3992,5 +3992,53 @@ UNINITIALIZED_TEST(SnapshotCreatorAnonClassWithKeep) {
   delete[] blob.data;
 }
 
+class DisableLazySourcePositionScope {
+ public:
+  DisableLazySourcePositionScope()
+      : backup_value_(FLAG_enable_lazy_source_positions) {
+    FLAG_enable_lazy_source_positions = false;
+  }
+  ~DisableLazySourcePositionScope() {
+    FLAG_enable_lazy_source_positions = backup_value_;
+  }
+
+ private:
+  bool backup_value_;
+};
+
+UNINITIALIZED_TEST(NoStackFrameCacheSerialization) {
+  // Checks that exceptions caught are not cached in the
+  // stack frame cache during serialization. The individual frames
+  // can point to JSFunction objects, which need to be stored in a
+  // context snapshot, *not* isolate snapshot.
+  DisableAlwaysOpt();
+  DisableLazySourcePositionScope lazy_scope;
+
+  v8::SnapshotCreator creator;
+  v8::Isolate* isolate = creator.GetIsolate();
+  isolate->SetCaptureStackTraceForUncaughtExceptions(true);
+  {
+    v8::HandleScope handle_scope(isolate);
+    {
+      v8::Local<v8::Context> context = v8::Context::New(isolate);
+      v8::Context::Scope context_scope(context);
+      v8::TryCatch try_catch(isolate);
+      CompileRun(R"(
+        function foo() { throw new Error('bar'); }
+        function bar() {
+          foo();
+        }
+        bar();
+      )");
+
+      creator.SetDefaultContext(context);
+    }
+  }
+  v8::StartupData blob =
+      creator.CreateBlob(v8::SnapshotCreator::FunctionCodeHandling::kKeep);
+
+  delete[] blob.data;
+}
+
 }  // namespace internal
 }  // namespace v8