Yang, I need your advice, please take a look.
Is it possible to update return value which is stored in accumulator or it can be non-trivial because of some kind of optimizations?
I can only imagine some raw-level builtin method which can just update register value and return without changing accumulator value.

Is the issue that we are not changing the accumulator value when returning from the break? We should be able to change the accumulator value though.

Okay let me see if I understand this correctly: altering the return value never worked, and this is actually a feature request?

During a break from bytecode, we call into Runtime_DebugBreakOnBytecode, with the return value as first argument. This value is stored away via isolate->debug()->set_return_value(), so that it can be inspected.

When we return from the break, we simply continue to use the return value in the accumulator register. However, I think we should be able to simply write the isolate->debug()->return_value() into the accumulator via InterpretedFrame::WriteInterpreterRegister.

For some reason, I found that the accumulator's register index is 5. Ross or Leszek, is this always the case? Is there some constant we could use here?

The accumulator is not an interpreter register as such (i.e. an offset into the register file which is stored on the stack), but rather is a machine register (kInterpreterAccumulatorRegister) during interpreter execution. I'm guessing that the index 5 you're seeing is either the value pushed onto the stack before the runtime call, or the value passed through to the runtime call as a parameter. So, I don't think we can set it from the runtime. Maybe we could return it along with the resume handler, and set it in the DebugBreak bytecode handler?

Using InterpretedFrame::WriteInterpreterRegister somehow worked in my toy example though, but maybe that doesn't work reliably?

But your suggestions works too.

Possibly you were writing out-of-bounds of the register file, and overwriting the rax (i.e. kInterpreterAccumulatorRegister) value saved onto the stack?

One neat thing with returning both accumulator and real handler as a pair is that the accumulator register coincides with the first return register, so we would be able to set it for free as well as kill its liveness over the runtime call (thus avoid saving it for the call). It's not exactly a large or even useful optimization, but at least it shouldn't be a performance penalty.

I agree. But it's not like performance matters when we are debugging. How do you suggest returning the handler? Would we return them as pair, or use some fixed memory location?

I suggest returning as a pair, similar to LoadLookupSlotForCall.

Missed this yesterday, but +1 to Leszek's suggestions. The CallRuntimeForPair bytecode handler does something similar - i.e., use the Projection node to extract both values from the pair returned by the runtime.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f86b4de27260b7fb5703c03400d53530c26f343d

commit f86b4de27260b7fb5703c03400d53530c26f343d
Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org>
Date: Sat Nov 04 00:38:09 2017

[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/731679
Reviewed-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}
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/api.cc
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/debug/debug-interface.h
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/inspector/js_protocol.json
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/inspector/v8-debugger-agent-impl.cc
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/inspector/v8-debugger-agent-impl.h
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/interpreter/bytecodes.h
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/interpreter/interpreter-generator.cc
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/runtime/runtime-debug.cc
[modify] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/src/runtime/runtime.h
[add] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/test/inspector/debugger/change-return-value-expected.txt
[add] https://crrev.com/f86b4de27260b7fb5703c03400d53530c26f343d/test/inspector/debugger/change-return-value.js

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10e57b24797db63ed004d506195bf5c336b48c34

commit 10e57b24797db63ed004d506195bf5c336b48c34
Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org>
Date: Sat Dec 02 03:51:38 2017

[DevTools] make return value editable on pause

With this change user can update return value when paused at
return position.

Bug:  chromium:656150 
Change-Id: I8e31799461be21d839e256cc63f8bf8748875fc1
Reviewed-on: https://chromium-review.googlesource.com/802695
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521195}
[add] https://crrev.com/10e57b24797db63ed004d506195bf5c336b48c34/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger-pause/set-return-value-expected.txt
[add] https://crrev.com/10e57b24797db63ed004d506195bf5c336b48c34/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger-pause/set-return-value.js
[modify] https://crrev.com/10e57b24797db63ed004d506195bf5c336b48c34/third_party/WebKit/Source/devtools/front_end/object_ui/ObjectPropertiesSection.js
[modify] https://crrev.com/10e57b24797db63ed004d506195bf5c336b48c34/third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js
[modify] https://crrev.com/10e57b24797db63ed004d506195bf5c336b48c34/third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js
[modify] https://crrev.com/10e57b24797db63ed004d506195bf5c336b48c34/third_party/WebKit/Source/devtools/front_end/sources/ScopeChainSidebarPane.js