[inspector] added Debugger.setReturnValue

DebugBreak bytecode fetches current return value from debugger prior
dispatching original handler. So we can change its value on break.

R=leszeks@chromium.org,rmcilroy@chromium.org

Bug: chromium:656150
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I82d0bc82ff49923a748c0084d252d0fd214a2db8
Reviewed-on: https://chromium-review.googlesource.com/731679Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49122}

src/api.cc

@@ -10186,6 +10186,13 @@ void debug::GlobalLexicalScopeNames(
   }
 }
 
+void debug::SetReturnValue(v8::Isolate* v8_isolate,
+                           v8::Local<v8::Value> value) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  if (!isolate->debug()->break_id()) return;
+  isolate->debug()->set_return_value(*Utils::OpenHandle(*value));
+}
+
 Local<String> CpuProfileNode::GetFunctionName() const {
   const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this);
   i::Isolate* isolate = node->isolate();

src/debug/debug-interface.h

@@ -485,6 +485,8 @@ void QueryObjects(v8::Local<v8::Context> context,
 void GlobalLexicalScopeNames(v8::Local<v8::Context> context,
                              v8::PersistentValueVector<v8::String>* names);
 
+void SetReturnValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
+
 }  // namespace debug
 }  // namespace v8
 

src/inspector/js_protocol.json

@@ -698,6 +698,14 @@
                 ],
                 "description": "Changes value of variable in a callframe. Object-based scopes are not supported and must be mutated manually."
             },
+            {
+                "name": "setReturnValue",
+                "parameters": [
+                    { "name": "newValue", "$ref": "Runtime.CallArgument", "description": "New return value." }
+                ],
+                "experimental": true,
+                "description": "Changes return value in top frame. Available only at return break position."
+            },
             {
                 "name": "setAsyncCallStackDepth",
                 "parameters": [

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

@@ -1058,6 +1058,7 @@ Response V8DebuggerAgentImpl::setVariableValue(
   if (scopeNumber != 0) {
     return Response::Error("Could not find scope with given number");
   }
+
   if (!scopeIterator->SetVariableValue(toV8String(m_isolate, variableName),
                                        newValue) ||
       scope.tryCatch().HasCaught()) {
@@ -1066,6 +1067,29 @@ Response V8DebuggerAgentImpl::setVariableValue(
   return Response::OK();
 }
 
+Response V8DebuggerAgentImpl::setReturnValue(
+    std::unique_ptr<protocol::Runtime::CallArgument> protocolNewValue) {
+  if (!enabled()) return Response::Error(kDebuggerNotEnabled);
+  if (!isPaused()) return Response::Error(kDebuggerNotPaused);
+  auto iterator = v8::debug::StackTraceIterator::Create(m_isolate);
+  if (iterator->Done()) {
+    return Response::Error("Could not find top call frame");
+  }
+  if (iterator->GetReturnValue().IsEmpty()) {
+    return Response::Error(
+        "Could not update return value at non-return position");
+  }
+  InjectedScript::ContextScope scope(m_session, iterator->GetContextId());
+  Response response = scope.initialize();
+  if (!response.isSuccess()) return response;
+  v8::Local<v8::Value> newValue;
+  response = scope.injectedScript()->resolveCallArgument(protocolNewValue.get(),
+                                                         &newValue);
+  if (!response.isSuccess()) return response;
+  v8::debug::SetReturnValue(m_isolate, newValue);
+  return Response::OK();
+}
+
 Response V8DebuggerAgentImpl::setAsyncCallStackDepth(int depth) {
   if (!enabled()) return Response::Error(kDebuggerNotEnabled);
   m_state->setInteger(DebuggerAgentState::asyncCallStackDepth, depth);

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

@@ -100,6 +100,8 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
       int scopeNumber, const String16& variableName,
       std::unique_ptr<protocol::Runtime::CallArgument> newValue,
       const String16& callFrame) override;
+  Response setReturnValue(
+      std::unique_ptr<protocol::Runtime::CallArgument> newValue) override;
   Response setAsyncCallStackDepth(int depth) override;
   Response setBlackboxPatterns(
       std::unique_ptr<protocol::Array<String16>> patterns) override;

src/interpreter/bytecodes.h

@@ -325,19 +325,20 @@ namespace interpreter {
                                                                                \
   /* Debug Breakpoints - one for each possible size of unscaled bytecodes */   \
   /* and one for each operand widening prefix bytecode                    */   \
-  V(DebugBreak0, AccumulatorUse::kRead)                                        \
-  V(DebugBreak1, AccumulatorUse::kRead, OperandType::kReg)                     \
-  V(DebugBreak2, AccumulatorUse::kRead, OperandType::kReg, OperandType::kReg)  \
-  V(DebugBreak3, AccumulatorUse::kRead, OperandType::kReg, OperandType::kReg,  \
+  V(DebugBreak0, AccumulatorUse::kReadWrite)                                   \
+  V(DebugBreak1, AccumulatorUse::kReadWrite, OperandType::kReg)                \
+  V(DebugBreak2, AccumulatorUse::kReadWrite, OperandType::kReg,                \
     OperandType::kReg)                                                         \
-  V(DebugBreak4, AccumulatorUse::kRead, OperandType::kReg, OperandType::kReg,  \
+  V(DebugBreak3, AccumulatorUse::kReadWrite, OperandType::kReg,                \
     OperandType::kReg, OperandType::kReg)                                      \
-  V(DebugBreak5, AccumulatorUse::kRead, OperandType::kRuntimeId,               \
+  V(DebugBreak4, AccumulatorUse::kReadWrite, OperandType::kReg,                \
+    OperandType::kReg, OperandType::kReg, OperandType::kReg)                   \
+  V(DebugBreak5, AccumulatorUse::kReadWrite, OperandType::kRuntimeId,          \
     OperandType::kReg, OperandType::kReg)                                      \
-  V(DebugBreak6, AccumulatorUse::kRead, OperandType::kRuntimeId,               \
+  V(DebugBreak6, AccumulatorUse::kReadWrite, OperandType::kRuntimeId,          \
     OperandType::kReg, OperandType::kReg, OperandType::kReg)                   \
-  V(DebugBreakWide, AccumulatorUse::kRead)                                     \
-  V(DebugBreakExtraWide, AccumulatorUse::kRead)                                \
+  V(DebugBreakWide, AccumulatorUse::kReadWrite)                                \
+  V(DebugBreakExtraWide, AccumulatorUse::kReadWrite)                           \
                                                                                \
   /* Block Coverage */                                                         \
   V(IncBlockCounter, AccumulatorUse::kNone, OperandType::kIdx)                 \

src/interpreter/interpreter-generator.cc

@@ -2863,9 +2863,12 @@ IGNITION_HANDLER(Debugger, InterpreterAssembler) {
   IGNITION_HANDLER(Name, InterpreterAssembler) {                           \
     Node* context = GetContext();                                          \
     Node* accumulator = GetAccumulator();                                  \
-    Node* original_handler =                                               \
+    Node* result_pair =                                                    \
         CallRuntime(Runtime::kDebugBreakOnBytecode, context, accumulator); \
+    Node* return_value = Projection(0, result_pair);                       \
+    Node* original_handler = Projection(1, result_pair);                   \
     MaybeDropFrames(context);                                              \
+    SetAccumulator(return_value);                                          \
     DispatchToBytecodeHandler(original_handler);                           \
   }
 DEBUG_BREAK_BYTECODE_LIST(DEBUG_BREAK);

src/runtime/runtime-debug.cc

@@ -26,11 +26,13 @@
 namespace v8 {
 namespace internal {
 
-RUNTIME_FUNCTION(Runtime_DebugBreakOnBytecode) {
+RUNTIME_FUNCTION_RETURN_PAIR(Runtime_DebugBreakOnBytecode) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(Object, value, 0);
   HandleScope scope(isolate);
+  // Return value can be changed by debugger. Last set value will be used as
+  // return value.
   ReturnValueScope result_scope(isolate->debug());
   isolate->debug()->set_return_value(*value);
 
@@ -53,8 +55,9 @@ RUNTIME_FUNCTION(Runtime_DebugBreakOnBytecode) {
     // sees the return bytecode rather than the DebugBreak.
     interpreted_frame->PatchBytecodeArray(bytecode_array);
   }
-  return isolate->interpreter()->GetBytecodeHandler(
-      bytecode, interpreter::OperandScale::kSingle);
+  return MakePair(isolate->debug()->return_value(),
+                  isolate->interpreter()->GetBytecodeHandler(
+                      bytecode, interpreter::OperandScale::kSingle));
 }
 
 

src/runtime/runtime.h

@@ -136,7 +136,6 @@ namespace internal {
 
 #define FOR_EACH_INTRINSIC_DEBUG(F)             \
   F(HandleDebuggerStatement, 0, 1)              \
-  F(DebugBreakOnBytecode, 1, 1)                 \
   F(SetDebugEventListener, 2, 1)                \
   F(ScheduleBreak, 0, 1)                        \
   F(DebugGetInternalProperties, 1, 1)           \
@@ -666,7 +665,8 @@ namespace internal {
   F(WasmCompileLazy, 0, 1)
 
 #define FOR_EACH_INTRINSIC_RETURN_PAIR(F) \
-  F(LoadLookupSlotForCall, 1, 2)
+  F(LoadLookupSlotForCall, 1, 2)          \
+  F(DebugBreakOnBytecode, 1, 2)
 
 // Most intrinsics are implemented in the runtime/ directory, but ICs are
 // implemented in ic.cc for now.

test/inspector/debugger/change-return-value-expected.txt

+Checks that we can update return value on pause
+
+Running test: testError
+Set return value not at return position
+{
+    error : {
+        code : -32000
+        message : Could not update return value at non-return position
+    }
+    id : <messageId>
+}
+
+Running test: testUndefined
+Break at return position..
+Update return value to 42..
+Dump actual return value
+{
+    result : {
+        description : 42
+        type : number
+        value : 42
+    }
+}
+
+Running test: testArrow
+Break at return position..
+Update return value to 239..
+Dump actual return value
+{
+    result : {
+        description : 239
+        type : number
+        value : 239
+    }
+}

test/inspector/debugger/change-return-value.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Checks that we can update return value on pause');
+
+InspectorTest.runAsyncTestSuite([
+  async function testError() {
+    Protocol.Debugger.enable();
+    let evaluation = Protocol.Runtime.evaluate({
+      expression: 'function foo() { debugger; } foo()',
+      returnByValue: true
+    });
+    let {params:{callFrames}} = await Protocol.Debugger.oncePaused();
+    InspectorTest.log('Set return value not at return position');
+    let result = await Protocol.Debugger.setReturnValue({
+      newValue: { value: 42 },
+    });
+    InspectorTest.logMessage(result);
+    await Protocol.Debugger.disable();
+  },
+
+  async function testUndefined() {
+    Protocol.Debugger.enable();
+    let evaluation = Protocol.Runtime.evaluate({
+      expression: 'function foo() { debugger; } foo()',
+      returnByValue: true
+    });
+    InspectorTest.log('Break at return position..');
+    await Protocol.Debugger.oncePaused();
+    Protocol.Debugger.stepInto();
+    let {params:{callFrames}} = await Protocol.Debugger.oncePaused();
+    InspectorTest.log('Update return value to 42..');
+    Protocol.Debugger.setReturnValue({
+      newValue: { value: 42 },
+    });
+    Protocol.Debugger.resume();
+    let {result} = await evaluation;
+    InspectorTest.log('Dump actual return value');
+    InspectorTest.logMessage(result);
+    await Protocol.Debugger.disable();
+  },
+
+  async function testArrow() {
+    Protocol.Debugger.enable();
+    Protocol.Debugger.pause();
+    let evaluation = Protocol.Runtime.evaluate({
+      expression: '(() => 42)()',
+      returnByValue: true
+    });
+    InspectorTest.log('Break at return position..');
+    await Protocol.Debugger.oncePaused();
+    Protocol.Debugger.stepInto();
+    await Protocol.Debugger.oncePaused();
+    Protocol.Debugger.stepInto();
+    let {params:{callFrames}} = await Protocol.Debugger.oncePaused();
+    InspectorTest.log('Update return value to 239..');
+    Protocol.Debugger.setReturnValue({
+      newValue: { value: 239 },
+    });
+    Protocol.Debugger.resume();
+    let {result} = await evaluation;
+    InspectorTest.log('Dump actual return value');
+    InspectorTest.logMessage(result);
+    await Protocol.Debugger.disable();
+  }
+]);