Reland "[inspector] Re-enable Debugger#restartFrame"

This is a reland of commit 8278cb50

The reland adds the RestartFrameTrampoline to the list of
builtins that the deoptimizer is allowed to return from for
control flow integrity.

Original change's description:
> [inspector] Re-enable Debugger#restartFrame
>
> Doc: https://bit.ly/revive-restart-frame
>
> This CL "undeprecates" Debugger#restartFrame and adds a new optional
> "mode" parameter for back-wards compatibility. Moreover, the return
> values are all deprecated. They were never actually used in the
> DevTools frontend and the same information is available from the
> Debugger#paused event that fires once execution stops at the
> beginning of the restarted function.
>
> The CL also re-baselines all the restart-frame inspector tests that
> now run successfully.
>
> R=bmeurer@chromium.org, kimanh@chromium.org
>
> Bug: chromium:1303521
> Change-Id: I34bddeb1f2f4ff3dee58dd82e779c111495566f3
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3616505
> Reviewed-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@{#80491}

Bug: chromium:1303521
Change-Id: I13e2f8b5011795a38e541310622b8333a3d08049
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3644624Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80534}

include/js_protocol.pdl

@@ -273,18 +273,35 @@ domain Debugger
     parameters
       BreakpointId breakpointId
 
-  # Restarts particular call frame from the beginning.
-  deprecated command restartFrame
+  # Restarts particular call frame from the beginning. The old, deprecated
+  # behavior of `restartFrame` is to stay paused and allow further CDP commands
+  # after a restart was scheduled. This can cause problems with restarting, so
+  # we now continue execution immediatly after it has been scheduled until we
+  # reach the beginning of the restarted frame.
+  #
+  # To stay back-wards compatible, `restartFrame` now expects a `mode`
+  # parameter to be present. If the `mode` parameter is missing, `restartFrame`
+  # errors out.
+  #
+  # The various return values are deprecated and `callFrames` is always empty.
+  # Use the call frames from the `Debugger#paused` events instead, that fires
+  # once V8 pauses at the beginning of the restarted function.
+  command restartFrame
     parameters
       # Call frame identifier to evaluate on.
       CallFrameId callFrameId
+      # The `mode` parameter must be present and set to 'StepInto', otherwise
+      # `restartFrame` will error out.
+      experimental optional enum mode
+        # Pause at the beginning of the restarted function
+        StepInto
     returns
       # New stack trace.
-      array of CallFrame callFrames
+      deprecated array of CallFrame callFrames
       # Async stack trace, if any.
-      optional Runtime.StackTrace asyncStackTrace
+      deprecated optional Runtime.StackTrace asyncStackTrace
       # Async stack trace, if any.
-      experimental optional Runtime.StackTraceId asyncStackTraceId
+      deprecated optional Runtime.StackTraceId asyncStackTraceId
 
   # Resumes JavaScript execution.
   command resume

src/deoptimizer/deoptimizer-cfi-builtins.cc

@@ -16,6 +16,7 @@ void construct_stub_create_deopt_addr();
 void construct_stub_invoke_deopt_addr();
 void Builtins_BaselineOrInterpreterEnterAtBytecode();
 void Builtins_BaselineOrInterpreterEnterAtNextBytecode();
+void Builtins_RestartFrameTrampoline();
 typedef void (*function_ptr)();
 }
 
@@ -34,6 +35,7 @@ constexpr function_ptr builtins[] = {
     &construct_stub_invoke_deopt_addr,
     &Builtins_BaselineOrInterpreterEnterAtBytecode,
     &Builtins_BaselineOrInterpreterEnterAtNextBytecode,
+    &Builtins_RestartFrameTrampoline,
 };
 
 bool Deoptimizer::IsValidReturnAddress(Address address) {

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

@@ -1050,11 +1050,30 @@ Response V8DebuggerAgentImpl::setScriptSource(
 }
 
 Response V8DebuggerAgentImpl::restartFrame(
-    const String16& callFrameId,
+    const String16& callFrameId, Maybe<String16> mode,
     std::unique_ptr<Array<CallFrame>>* newCallFrames,
     Maybe<protocol::Runtime::StackTrace>* asyncStackTrace,
     Maybe<protocol::Runtime::StackTraceId>* asyncStackTraceId) {
-  return Response::ServerError("Frame restarting not supported");
+  if (!isPaused()) return Response::ServerError(kDebuggerNotPaused);
+  if (!mode.isJust()) {
+    return Response::ServerError(
+        "Restarting frame without 'mode' not supported");
+  }
+  CHECK_EQ(mode.fromJust(),
+           protocol::Debugger::RestartFrame::ModeEnum::StepInto);
+
+  InjectedScript::CallFrameScope scope(m_session, callFrameId);
+  Response response = scope.initialize();
+  if (!response.IsSuccess()) return response;
+  int callFrameOrdinal = static_cast<int>(scope.frameOrdinal());
+
+  if (!m_debugger->restartFrame(m_session->contextGroupId(),
+                                callFrameOrdinal)) {
+    return Response::ServerError("Restarting frame failed");
+  }
+  m_session->releaseObjectGroup(kBacktraceObjectGroup);
+  *newCallFrames = std::make_unique<Array<CallFrame>>();
+  return Response::Success();
 }
 
 Response V8DebuggerAgentImpl::getScriptSource(

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

@@ -90,7 +90,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
       Maybe<protocol::Runtime::StackTraceId>* optOutAsyncStackTraceId,
       Maybe<protocol::Runtime::ExceptionDetails>* optOutCompileError) override;
   Response restartFrame(
-      const String16& callFrameId,
+      const String16& callFrameId, Maybe<String16> mode,
       std::unique_ptr<protocol::Array<protocol::Debugger::CallFrame>>*
           newCallFrames,
       Maybe<protocol::Runtime::StackTrace>* asyncStackTrace,

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 : Frame restarting not supported
+        message : Restarting frame failed
     }
     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 : Frame restarting not supported
+    message : Restarting frame failed
 }
 
 Check that a generator function cannot be restarted.
@@ -29,7 +29,7 @@ Restarting function "generatorFn" ...
 Failed to restart function "generatorFn":
 {
     code : -32000
-    message : Frame restarting not supported
+    message : Restarting frame failed
 }
 
 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 : Frame restarting not supported
+    message : Restarting frame failed
 }

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

@@ -12,5 +12,5 @@ Restarting function "entrypoint" ...
 Failed to restart function "entrypoint":
 {
     code : -32000
-    message : Frame restarting not supported
+    message : Restarting frame failed
 }

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

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

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

@@ -12,8 +12,9 @@ Pause stack:
 
 Optimization status for function "h" after we paused? optimized
 Restarting function "g" ...
-Failed to restart function "g":
-{
-    code : -32000
-    message : Frame restarting not supported
-}
+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

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

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

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

@@ -38,8 +38,35 @@ Evaluating z:
     }
 }
 Restarting function "foo" ...
-Failed to restart 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:
 {
-    code : -32000
-    message : Frame restarting not supported
+    id : <messageId>
+    result : {
+        result : {
+            type : undefined
+        }
+    }
+}
+Evaluating z:
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : undefined
+        }
+    }
 }

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

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

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

@@ -15,8 +15,125 @@ Pause stack:
   F:26 (canBeRestarted = true)
 
 Restarting function "A" ...
-Failed to restart function "A":
-{
-    code : -32000
-    message : Frame restarting not supported
-}
+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