[inspector] Add 'canBeRestarted' flag to CallFrames when debugger pauses

Doc: https://bit.ly/revive-restart-frame
Context: https://crrev.com/c/3582395 (whole feature)

This CL adds a new optional flag `canBeRestarted` to every call frame
in Debugger.paused events. As the name suggests, the flag indicates
whether we can restart a particular frame through Debugger.restartFrame
once implemented.

We are not able to safely restart all frames:
  * We don't support WASM frames
  * We don't support frames where resumable functions (async fns,
    generators) and embedder C++ frames are between the top-most
    frame and the to-be-restarted frame.

Note that from a CDP perspective the flag doesn't actually guarantee
a successful restart. CDP clients can issue
CDP commands between the Debugger.paused event and before a user
decides to restart a frame, which can potentially mess
with the stack.

The `canBeRestarted` flag tests are folded into the
Debugger.restartFrame tests. As the feature is not yet fully
implemented we short-circuit most of the tests for now and only
run them up until the first Debugger.restartFrame call fails
(except "fails-for-resumables.js").
This means the tests exercise the `canBeRestarted` flag, but not
the restarting functionality itself.

R=bmeurer@chromium.org, kimanh@chromium.org

Bug: chromium:1303521
Change-Id: I01ab46dc3557ab8383960969fbe03e00604cc5e2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3596160Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80046}

include/js_protocol.pdl

@@ -113,6 +113,11 @@ domain Debugger
       Runtime.RemoteObject this
       # The value being returned, if the function is at return point.
       optional Runtime.RemoteObject returnValue
+      # Valid only while the VM is paused and indicates whether this frame
+      # can be restarted or not. Note that a `true` value here does not
+      # guarantee that Debugger#restartFrame with this CallFrameId will be
+      # successful, but it is very likely.
+      experimental optional boolean canBeRestarted
 
   # Scope description.
   type Scope extends object

src/debug/debug-interface.h

@@ -534,6 +534,7 @@ class V8_EXPORT_PRIVATE StackTraceIterator {
   virtual debug::Location GetSourceLocation() const = 0;
   virtual v8::Local<v8::Function> GetFunction() const = 0;
   virtual std::unique_ptr<ScopeIterator> GetScopeIterator() const = 0;
+  virtual bool CanBeRestarted() const = 0;
 
   virtual v8::MaybeLocal<v8::Value> Evaluate(v8::Local<v8::String> source,
                                              bool throw_on_side_effect) = 0;

src/debug/debug-stack-trace-iterator.cc

@@ -30,11 +30,10 @@ namespace internal {
 DebugStackTraceIterator::DebugStackTraceIterator(Isolate* isolate, int index)
     : isolate_(isolate),
       iterator_(isolate, isolate->debug()->break_frame_id()),
-      is_top_frame_(true) {
+      is_top_frame_(true),
+      resumable_fn_on_stack_(false) {
   if (iterator_.done()) return;
-  std::vector<FrameSummary> frames;
-  iterator_.frame()->Summarize(&frames);
-  inlined_frame_index_ = static_cast<int>(frames.size());
+  UpdateInlineFrameIndexAndResumableFnOnStack();
   Advance();
   for (; !Done() && index > 0; --index) Advance();
 }
@@ -63,9 +62,7 @@ void DebugStackTraceIterator::Advance() {
     frame_inspector_.reset();
     iterator_.Advance();
     if (iterator_.done()) break;
-    std::vector<FrameSummary> frames;
-    iterator_.frame()->Summarize(&frames);
-    inlined_frame_index_ = static_cast<int>(frames.size());
+    UpdateInlineFrameIndexAndResumableFnOnStack();
   }
 }
 
@@ -168,6 +165,51 @@ DebugStackTraceIterator::GetScopeIterator() const {
   return std::make_unique<DebugScopeIterator>(isolate_, frame_inspector_.get());
 }
 
+bool DebugStackTraceIterator::CanBeRestarted() const {
+  DCHECK(!Done());
+
+  if (resumable_fn_on_stack_) return false;
+
+  StackFrame* frame = iterator_.frame();
+  // We do not support restarting WASM frames.
+#if V8_ENABLE_WEBASSEMBLY
+  if (frame->is_wasm()) return false;
+#endif  // V8_ENABLE_WEBASSEMBLY
+
+  // Check that no embedder API calls are between the top-most frame, and the
+  // current frame. While we *could* determine whether embedder
+  // frames are safe to terminate (via the CallDepthScope chain), we don't know
+  // if embedder frames would cancel the termination effectively breaking
+  // restart frame.
+  if (isolate_->thread_local_top()->last_api_entry_ < frame->fp()) {
+    return false;
+  }
+
+  return true;
+}
+
+void DebugStackTraceIterator::UpdateInlineFrameIndexAndResumableFnOnStack() {
+  CHECK(!iterator_.done());
+
+  std::vector<FrameSummary> frames;
+  iterator_.frame()->Summarize(&frames);
+  inlined_frame_index_ = static_cast<int>(frames.size());
+
+  if (resumable_fn_on_stack_) return;
+
+  StackFrame* frame = iterator_.frame();
+  if (!frame->is_java_script()) return;
+
+  std::vector<Handle<SharedFunctionInfo>> shareds;
+  JavaScriptFrame::cast(frame)->GetFunctions(&shareds);
+  for (auto& shared : shareds) {
+    if (IsResumableFunction(shared->kind())) {
+      resumable_fn_on_stack_ = true;
+      return;
+    }
+  }
+}
+
 v8::MaybeLocal<v8::Value> DebugStackTraceIterator::Evaluate(
     v8::Local<v8::String> source, bool throw_on_side_effect) {
   DCHECK(!Done());

src/debug/debug-stack-trace-iterator.h

@@ -30,16 +30,20 @@ class DebugStackTraceIterator final : public debug::StackTraceIterator {
   debug::Location GetSourceLocation() const override;
   v8::Local<v8::Function> GetFunction() const override;
   std::unique_ptr<v8::debug::ScopeIterator> GetScopeIterator() const override;
+  bool CanBeRestarted() const override;
 
   v8::MaybeLocal<v8::Value> Evaluate(v8::Local<v8::String> source,
                                      bool throw_on_side_effect) override;
 
  private:
+  void UpdateInlineFrameIndexAndResumableFnOnStack();
+
   Isolate* isolate_;
   StackTraceFrameIterator iterator_;
   std::unique_ptr<FrameInspector> frame_inspector_;
   int inlined_frame_index_;
   bool is_top_frame_;
+  bool resumable_fn_on_stack_;
 };
 }  // namespace internal
 }  // namespace v8

src/inspector/v8-debugger-agent-impl.cc

@@ -1491,6 +1491,7 @@ Response V8DebuggerAgentImpl::currentCallFrames(
                      .setUrl(String16())
                      .setScopeChain(std::move(scopes))
                      .setThis(std::move(protocolReceiver))
+                     .setCanBeRestarted(iterator->CanBeRestarted())
                      .build();
 
     v8::Local<v8::Function> func = iterator->GetFunction();

test/inspector/debugger/class-fields-scopes-expected.txt

@@ -4,6 +4,7 @@ Running test: testScopesPaused
 [
     [0] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 14
             lineNumber : 2
@@ -76,6 +77,7 @@ Running test: testScopesPaused
     }
     [1] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 12
             lineNumber : 1
@@ -148,6 +150,7 @@ Running test: testScopesPaused
     }
     [2] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 0
             lineNumber : 0
@@ -182,6 +185,7 @@ Running test: testScopesPaused
 [
     [0] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -205,6 +209,7 @@ Running test: testScopesPaused
     }
     [1] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -277,6 +282,7 @@ Running test: testScopesPaused
     }
     [2] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 12
             lineNumber : 1
@@ -329,6 +335,7 @@ Running test: testScopesPaused
     }
     [3] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 0
             lineNumber : 0
@@ -363,6 +370,7 @@ Running test: testScopesPaused
 [
     [0] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -386,6 +394,7 @@ Running test: testScopesPaused
     }
     [1] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -458,6 +467,7 @@ Running test: testScopesPaused
     }
     [2] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 12
             lineNumber : 1
@@ -510,6 +520,7 @@ Running test: testScopesPaused
     }
     [3] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 0
             lineNumber : 0
@@ -544,6 +555,7 @@ Running test: testScopesPaused
 [
     [0] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -567,6 +579,7 @@ Running test: testScopesPaused
     }
     [1] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -639,6 +652,7 @@ Running test: testScopesPaused
     }
     [2] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 12
             lineNumber : 1
@@ -691,6 +705,7 @@ Running test: testScopesPaused
     }
     [3] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 0
             lineNumber : 0
@@ -725,6 +740,7 @@ Running test: testScopesPaused
 [
     [0] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 14
             lineNumber : 7
@@ -797,6 +813,7 @@ Running test: testScopesPaused
     }
     [1] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -820,6 +837,7 @@ Running test: testScopesPaused
     }
     [2] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 2
             lineNumber : 11
@@ -892,6 +910,7 @@ Running test: testScopesPaused
     }
     [3] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 12
             lineNumber : 1
@@ -944,6 +963,7 @@ Running test: testScopesPaused
     }
     [4] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 0
             lineNumber : 0

test/inspector/debugger/class-private-fields-scopes-expected.txt

@@ -4,6 +4,7 @@ Running test: testScopesPaused
 [
     [0] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 16
             lineNumber : 4
@@ -56,6 +57,7 @@ Running test: testScopesPaused
     }
     [1] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 12
             lineNumber : 1
@@ -108,6 +110,7 @@ Running test: testScopesPaused
     }
     [2] : {
         callFrameId : <callFrameId>
+        canBeRestarted : true
         functionLocation : {
             columnNumber : 0
             lineNumber : 0

test/inspector/debugger/restart-frame/fails-for-non-existant-index-expected.txt

@@ -11,7 +11,7 @@ Attempting to restart frame with non-existent index 2
 {
     error : {
         code : -32000
-        message : Restarting frame failed
+        message : Frame restarting not supported
     }
     id : <messageId>
 }

test/inspector/debugger/restart-frame/fails-for-resumables-expected.txt

@@ -13,7 +13,7 @@ Restarting function "asyncFn" ...
 Failed to restart function "asyncFn":
 {
     code : -32000
-    message : Restarting frame failed
+    message : Frame restarting not supported
 }
 
 Check that a generator function cannot be restarted.
@@ -29,7 +29,7 @@ Restarting function "generatorFn" ...
 Failed to restart function "generatorFn":
 {
     code : -32000
-    message : Restarting frame failed
+    message : Frame restarting not supported
 }
 
 Check that a function cannot be restarted when a generator function is on the stack above
@@ -47,5 +47,5 @@ Restarting function "bar" ...
 Failed to restart function "bar":
 {
     code : -32000
-    message : Restarting frame failed
+    message : Frame restarting not supported
 }

test/inspector/debugger/restart-frame/fails-for-resumables.js

@@ -13,7 +13,7 @@ async function testCase(description, snippet, restartFrameIndex) {
 
   const { callFrames, evaluatePromise } = await InspectorTest.evaluateAndWaitForPause(snippet);
   // These are negative tests where the following call is expected to fail.
-  await InspectorTest.restartFrameAndWaitForPause(callFrames, restartFrameIndex);
+  await InspectorTest.restartFrameAndWaitForPause(callFrames, restartFrameIndex, /*quitOnFailure*/ false);
 
   // All snippets are written so a single resume is enough.
   Protocol.Debugger.resume();

test/inspector/debugger/restart-frame/fails-with-embedder-frames-expected.txt

@@ -5,12 +5,12 @@ function breaker() {
 }
 
 Pause stack:
-    breaker:2 (canBeRestarted = true)
-    entrypoint:5 (canBeRestarted = false)
+  breaker:2 (canBeRestarted = true)
+  entrypoint:5 (canBeRestarted = false)
 
 Restarting function "entrypoint" ...
 Failed to restart function "entrypoint":
 {
     code : -32000
-    message : Restarting frame failed
+    message : Frame restarting not supported
 }

test/inspector/debugger/restart-frame/fails-without-mode-param-expected.txt

@@ -6,7 +6,7 @@ restartFrame result:
 {
     error : {
         code : -32000
-        message : Restarting frame without 'mode' not supported
+        message : Frame restarting not supported
     }
     id : <messageId>
 }

test/inspector/debugger/restart-frame/restart-inlined-frame-expected.txt

@@ -6,15 +6,14 @@ Paused at (after evaluation):
 }
 
 Pause stack:
-    f:3 (canBeRestarted = true)
-    g:8 (canBeRestarted = true)
-    h:13 (canBeRestarted = true)
+  f:3 (canBeRestarted = true)
+  g:8 (canBeRestarted = true)
+  h:13 (canBeRestarted = true)
 
 Optimization status for function "h" after we paused? optimized
 Restarting function "g" ...
-Paused at (after restart):
-function g(a, b) {  // We want to restart 'g'.
-  #console.log('g');
-  return 2 + f(a, b);
-
-Called functions: h,g,f,g,f
+Failed to restart function "g":
+{
+    code : -32000
+    message : Frame restarting not supported
+}

test/inspector/debugger/restart-frame/restart-top-frame-debugger-stmt-expected.txt

@@ -5,11 +5,11 @@ Paused at (after evaluation):
   const y = 2;
 
 Pause stack:
-    foo:3 (canBeRestarted = true)
+  foo:3 (canBeRestarted = true)
 
 Restarting function "foo" ...
-Paused at (after restart):
-function foo() {
-  const x = #1;
-  debugger;
-
+Failed to restart function "foo":
+{
+    code : -32000
+    message : Frame restarting not supported
+}

test/inspector/debugger/restart-frame/restart-top-frame-local-variables-expected.txt

@@ -5,7 +5,7 @@ Paused at (after evaluation):
 }
 
 Pause stack:
-    foo:5 (canBeRestarted = true)
+  foo:5 (canBeRestarted = true)
 
 Evaluating x:
 {
@@ -38,35 +38,8 @@ Evaluating z:
     }
 }
 Restarting function "foo" ...
-Paused at (after restart):
-function foo() {
-  var x = #'some var';
-  const y = 'some const';
-
-Evaluating x:
-{
-    id : <messageId>
-    result : {
-        result : {
-            type : undefined
-        }
-    }
-}
-Evaluating y:
+Failed to restart function "foo":
 {
-    id : <messageId>
-    result : {
-        result : {
-            type : undefined
-        }
-    }
-}
-Evaluating z:
-{
-    id : <messageId>
-    result : {
-        result : {
-            type : undefined
-        }
-    }
+    code : -32000
+    message : Frame restarting not supported
 }

test/inspector/debugger/restart-frame/restart-top-frame-with-breakpoint-expected.txt

@@ -5,11 +5,11 @@ Paused at (after evaluation):
 }
 
 Pause stack:
-    foo:3 (canBeRestarted = true)
+  foo:3 (canBeRestarted = true)
 
 Restarting function "foo" ...
-Paused at (after restart):
-function foo() {
-  const x = #1;
-  const y = 2;
-
+Failed to restart function "foo":
+{
+    code : -32000
+    message : Frame restarting not supported
+}

test/inspector/debugger/restart-frame/restart-various-frames-expected.txt

@@ -7,133 +7,16 @@ Paused at (after evaluation):
   return 'Magic value';
 
 Pause stack:
-    A:3 (canBeRestarted = true)
-    B:8 (canBeRestarted = true)
-    C:13 (canBeRestarted = true)
-    D:18 (canBeRestarted = true)
-    E:22 (canBeRestarted = true)
-    F:26 (canBeRestarted = true)
+  A:3 (canBeRestarted = true)
+  B:8 (canBeRestarted = true)
+  C:13 (canBeRestarted = true)
+  D:18 (canBeRestarted = true)
+  E:22 (canBeRestarted = true)
+  F:26 (canBeRestarted = true)
 
 Restarting function "A" ...
-Paused at (after restart):
-function A() {
-  #console.log('A');
-  debugger;
-
-Evaluating to: Magic value
-Called functions: F,E,D,C,B,A,A
-
----- Restarting at frame index 1 ----
-Paused at (after evaluation):
-  console.log('A');
-  #debugger;
-  return 'Magic value';
-
-Pause stack:
-    A:3 (canBeRestarted = true)
-    B:8 (canBeRestarted = true)
-    C:13 (canBeRestarted = true)
-    D:18 (canBeRestarted = true)
-    E:22 (canBeRestarted = true)
-    F:26 (canBeRestarted = true)
-
-Restarting function "B" ...
-Paused at (after restart):
-function B(param1, param2) {
-  #console.log('B');
-  return A();
-
-Evaluating to: Magic value
-Called functions: F,E,D,C,B,A,B,A
-
----- Restarting at frame index 2 ----
-Paused at (after evaluation):
-  console.log('A');
-  #debugger;
-  return 'Magic value';
-
-Pause stack:
-    A:3 (canBeRestarted = true)
-    B:8 (canBeRestarted = true)
-    C:13 (canBeRestarted = true)
-    D:18 (canBeRestarted = true)
-    E:22 (canBeRestarted = true)
-    F:26 (canBeRestarted = true)
-
-Restarting function "C" ...
-Paused at (after restart):
-function C() {
-  #console.log('C');
-  // Function call with argument adapter is intentional.
-
-Evaluating to: Magic value
-Called functions: F,E,D,C,B,A,C,B,A
-
----- Restarting at frame index 3 ----
-Paused at (after evaluation):
-  console.log('A');
-  #debugger;
-  return 'Magic value';
-
-Pause stack:
-    A:3 (canBeRestarted = true)
-    B:8 (canBeRestarted = true)
-    C:13 (canBeRestarted = true)
-    D:18 (canBeRestarted = true)
-    E:22 (canBeRestarted = true)
-    F:26 (canBeRestarted = true)
-
-Restarting function "D" ...
-Paused at (after restart):
-function D() {
-  #console.log('D');
-  // Function call with argument adapter is intentional.
-
-Evaluating to: Magic value
-Called functions: F,E,D,C,B,A,D,C,B,A
-
----- Restarting at frame index 4 ----
-Paused at (after evaluation):
-  console.log('A');
-  #debugger;
-  return 'Magic value';
-
-Pause stack:
-    A:3 (canBeRestarted = true)
-    B:8 (canBeRestarted = true)
-    C:13 (canBeRestarted = true)
-    D:18 (canBeRestarted = true)
-    E:22 (canBeRestarted = true)
-    F:26 (canBeRestarted = true)
-
-Restarting function "E" ...
-Paused at (after restart):
-function E() {
-  #console.log('E');
-  return D();
-
-Evaluating to: Magic value
-Called functions: F,E,D,C,B,A,E,D,C,B,A
-
----- Restarting at frame index 5 ----
-Paused at (after evaluation):
-  console.log('A');
-  #debugger;
-  return 'Magic value';
-
-Pause stack:
-    A:3 (canBeRestarted = true)
-    B:8 (canBeRestarted = true)
-    C:13 (canBeRestarted = true)
-    D:18 (canBeRestarted = true)
-    E:22 (canBeRestarted = true)
-    F:26 (canBeRestarted = true)
-
-Restarting function "F" ...
-Paused at (after restart):
-function F() {
-  #console.log('F');
-  return E();
-
-Evaluating to: Magic value
-Called functions: F,E,D,C,B,A,F,E,D,C,B,A
+Failed to restart function "A":
+{
+    code : -32000
+    message : Frame restarting not supported
+}

test/inspector/inspector.status

@@ -23,9 +23,6 @@
   # Tests that need to run sequentially (e.g. due to memory consumption).
   'runtime/console-messages-limits': [PASS, HEAVY],
   'runtime/regression-732717': [PASS, HEAVY],
-
-  # https://crbug.com/1303521, feature not yet implemented.
-  'debugger/restart-frame/*': [SKIP],
 }],  # ALWAYS
 
 ##############################################################################

test/inspector/protocol-test.js

@@ -519,7 +519,8 @@ InspectorTest.evaluateAndWaitForPause = async (expression) => {
   return { callFrames, evaluatePromise };
 };
 
-InspectorTest.restartFrameAndWaitForPause = async (callFrames, index) => {
+// TODO(crbug.com/1303521): Remove `quitOnFailure` once no longer needed.
+InspectorTest.restartFrameAndWaitForPause = async (callFrames, index, quitOnFailure = true) => {
   const pausedPromise = Protocol.Debugger.oncePaused();
   const frame = callFrames[index];
 
@@ -528,6 +529,9 @@ InspectorTest.restartFrameAndWaitForPause = async (callFrames, index) => {
   if (response.error) {
     InspectorTest.log(`Failed to restart function "${frame.functionName}":`);
     InspectorTest.logMessage(response.error);
+    if (quitOnFailure) {
+      InspectorTest.completeTest();
+    }
     return;
   }
 

test/inspector/runtime/es6-module-expected.txt

@@ -79,6 +79,7 @@ console.log(239)
         callFrames : [
             [0] : {
                 callFrameId : <callFrameId>
+                canBeRestarted : false
                 functionLocation : {
                     columnNumber : 0
                     lineNumber : 0