Able to repro on ToT.

I bisected it to: https://codereview.chromium.org/2525243002
I think that we incorrectly restore some optimized by ignition variables somewhere in FrameInspector::UpdateStackLocalsFromMaterializedObject.
Yang, please take a look.

I am experiencing similar symptoms with Google Chrome 69.0.3497.81. The failure is silent. But it is still quite clear, because after completing the change (when the field is no longer in "edit mode", the value shown returns to the unmodified value.

Is your failure also silent?

This can be worked around by using "Copy property path" on the variable to modify in Scope. The path can then be copied into the console, and then a value assigned. Modifications through the console persist.

Test case for this: 

Debug = debug.Debug

function listener(event, exec_state, event_data, data) {
  if (event == Debug.DebugEvent.Break) {
    exec_state.frame(0).evaluate('a = false');
  }
};

Debug.setListener(listener);

function f(){
  let a = true;
  if (a) {
    assertTrue(a);
  } else {
    assertFalse(a);
  }
}

Debug.setBreakPoint(f, 2, 0);

f();



Run with

out.gn/x64.debug/d8 --allow-natives-syntax --enable-inspector test/mjsunit/mjsunit.js test/debugger/test-api.js test.js

The reason is that the bytecode we produce is this:


  214 E> 0x31f79faafed2 @    0 : a2                StackCheck 
  228 S> 0x31f79faafed3 @    1 : 10                LdaTrue 
         0x31f79faafed4 @    2 : 26 fb             Star r0                            // Write true into stack slot for "a"
  236 S> 0x31f79faafed6 @    4 : 05 0d             DebugBreak1 
  249 S> 0x31f79faafed8 @    6 : 13 00 00          LdaGlobal [0], [0]                 // Load "assertTrue" function
         0x31f79faafedb @    9 : 26 fa             Star r1
  249 E> 0x31f79faafedd @   11 : 5b fa fb 02       CallUndefinedReceiver1 r1, r0, [2] // Call assertTrue on a
         0x31f79faafee1 @   15 : 88 0b             Jump [11] (0x31f79faafeec @ 26)
  279 S> 0x31f79faafee3 @   17 : 13 01 04          LdaGlobal [1], [4]
         0x31f79faafee6 @   20 : 26 fa             Star r1
  279 E> 0x31f79faafee8 @   22 : 5b fa fb 06       CallUndefinedReceiver1 r1, r0, [6]
         0x31f79faafeec @   26 : 0d                LdaUndefined 
  299 S> 0x31f79faafeed @   27 : a6                Return

Constant pool (size = 2)
0x31f79faafda1: [FixedArray] in OldSpace
 - map: 0x39d00bb828b9 <Map>
 - length: 2
           0: 0x31f79faa2a39 <String[10]: assertTrue>
           1: 0x31f79faa2a61 <String[11]: assertFalse>


We simply don't have a after assigning to a. This is not the case if we insert a function call in between, e.g a "console.log(a)".

Ross and Leszek: is this a performance optimization? Can we consider removing it in favor of debugging? I do admit though that this is somewhat an edge case.

This is caused by the bytecode register optimizer optimizing away the Ldar r0 after the Star r0 (since we know the accumulator already has the right value). We rely on the bytecode register optimizer to avoid a lot of unnecessary register move bytecodes (both a space and speed saving), so I don't think we can remove it for debugging.

One option might be to have the debugger also track register / accumulator aliasing and overwrite the value in the accumulator as well as the local register if the values alias.

I don't think a simple aliasing check would work for

let a = true
b = a
// break, change a
if (b) ...

Also simply replacing the accumulator value doesn't make the code take another branch where there is no branch bytecode in the first place.

The branch bytecode here is hidden by the debug break, no?

Huh. Good point. I totally missed that. Thanks.

re #8 - So you mean we shouldn't change 'b' in the example above? This seems even more of an edge case since the code makes it only possible for b to be equal to a (and if there were any kind of branches in this code to make it conditional the register optimizer would drop all aliasing and it wouldn't be a problem).

Definitely an edge case, just saying that aliasing checks could also create incorrect behaviour -- arguably more incorrect, since having values be randomly immutable is less weird than values being randomly mutable.

If we really want correctness, we could run a simple-ish pass over the bytecode to find the most recent Star in this BB (or null of there was another accumulator-writing bytecode in-between).

Er, I mean Ldar

Just looking at Ldar wouldn't work (e.g., see the code above, the accumulator is loaded with LdaTrue, and the Star is used to store it into the register we are interested in, there is no Ldar involved). If we involved Star and Mov as well (for bytecodes that involve the local that the debugger is updating) we might be able to get most of the interesting edge-cases (e.g., in the case of #8 we would walk back until the Star <b>, which would teach us that the accumulator aliases 'b' and we shouldn't touch it when modifying 'a', even though they alias, whereas in the example on #6 we see Star <a> and treat the accumulator as aliasing 'a').

Right, true, something like that anyway.

 Issue 789241  has been merged into this issue.