1375300 - When paused, declaring a variable doesn’t work.

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Jason Laster [:jlast]]

reposting from github: https://github.com/devtools-html/debugger.html/issues/3074

If you’re paused at a breakpoint and try to declare something in the “Console” panel, it doesn’t work.

> var x = 3;
> let y = 2;
> const f = 4;


https://cloud.githubusercontent.com/assets/128755/26747119/befc393c-47c1-11e7-99da-3104247500aa.png

Comment 1

[Jason Laster [:jlast]]

Sharing Jim's comment: 

Curious: if you change the example to use `var` instead of `let`, does it work?

SpiderMonkey itself does have the ability to inject these declarations into the frame. The Debugger.Frame.prototype.eval function is specified to inject them in non-strict code:

> If <i>code</i> is not strict mode code, then variable declarations in <i>code</i> affect the environment of this frame. (In the terms used by the ECMAScript specification, the `VariableEnvironment` of the execution context for the eval code is the `VariableEnvironment` of the execution context that this frame represents.) If implementation restrictions prevent SpiderMonkey from extending this frame's environment as requested, this call throws an Error exception.

The reason strict mode code is treated differently is that `eval` was meant to behave like an actual `eval` call placed in the code. In strict mode, eval cannot affect its caller's environment. And in fact, a `let` in an eval (shown in the screen shot) never affects its caller's environment, strict mode or not.

However, it may be the case that, for developer tools, the old, non-strict, invasive behavior is what's always wanted, even in strict mode code, and even for `let`. I *think* this can be implemented.

Probably changes to DebuggerGenericEval. I can't really imagine the details; I'd need to ask Shu-yu Guo for help.

Comment 2

[Brian Grinstead [:bgrins]]

Yes I believe this has to do with the let binding as `var` works. I remember a few `let` related Debugger API bugs but I'm not sure if there is one on file for this specifically.  Asking Shu to see if knows of a duplicate or has any ideas on how to fix this.

Comment 3

[Shu-yu Guo [:shu]]

Jim's comment is correct. eval doesn't let lexical declarations (let or const) escape out of the eval.

The desired behavior, I imagine, is not eval, but copy-pasting the typed code in place at the breakpoint. This, unfortunately, is pretty much impossible to implement faithfully, because bindings are optimized in the frontend. Moreover, with the exception of the global lexical scope, lexical scopes (scopes that hold lets and consts) are expressively non-extensible. The engine relies on that invariant everywhere.

1. The frontend tries to statically resolve names wherever possible:

function f(x) {
  // breakpoint here
  print(x);
}

Suppose I called f("arg") and hit the breakpoint. If I typed |let x = "let"| in the Console then resumed, is the desired printed value "let"? If so, this would require an on-stack recompilation of f's bytecode, something we don't support right now.

2. The frontend omits scope information if a scope has no bindings:

function f() {
  let x = "outer";
  {
    // breakpoint here
    print(x);
  }
  print(x);
}

Suppose I called f() and hit the breakpoint. If I typed |let x = "inner"| in the Console then resumed, is the desired output "inner" then "outer"? Since the inner block scope originally had no values, there's no place for the new inner |let x| to go. To support such behavior, like the above, would require an on-stack recompilation of f's bytecode.


I don't think it's possible to support being able to inject bindings into any lexical scope for already-compiled code.

However, it is doable, but still a bit of work, for compartments in debug mode at the start of page load (e.g., if someone opened the Debugger panel and reloaded), to compile all code in such a deoptimized way that injecting lexical bindings is possible. The tradeoff here is that the performance of all code on the page will be very slow. We've certainly had cases before where someone pauses, pastes some loop into the Console, and then complain about the speed.

The possibilities here:

A. Continue not supporting injection of let and const bindings when paused. Print a nice warning when someone tries to do this to tell them to use var. Educate harder that the Console is semantically equivalent to direct eval, not pasting code in place.

B. Deoptimize everything if debug mode is on during page load.

C. Have some option that says "deoptimize everything" instead of doing it by default.

Comment 4

[Shu-yu Guo [:shu]]

Out of curiosity, what is a debugging use case for introducing a new binding?

Comment 5

[Jason Laster [:jlast]]

Thanks for the writeup, I agree that injecting bindings creates more problems than it is worth. I think we can go with A and show a nicer warning that makes it clear that they should use var.

It looks like `var x = 3` does not work either and we should fix that.

http://g.recordit.co/isPGhWb7S3.gif

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Jason Laster [:jlast] from comment #5)
> Thanks for the writeup, I agree that injecting bindings creates more
> problems than it is worth. I think we can go with A and show a nicer warning
> that makes it clear that they should use var.

Makes sense to me

> It looks like `var x = 3` does not work either and we should fix that.
> 
> http://g.recordit.co/isPGhWb7S3.gif

Interesting, in 54 I see this when paused:

  > var x = 1;
  > x;

Error: variable 'x' has been optimized out

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> (In reply to Jason Laster [:jlast] from comment #5)
> > Thanks for the writeup, I agree that injecting bindings creates more
> > problems than it is worth. I think we can go with A and show a nicer warning
> > that makes it clear that they should use var.
> 
> Makes sense to me
> 
> > It looks like `var x = 3` does not work either and we should fix that.
> > 
> > http://g.recordit.co/isPGhWb7S3.gif
> 
> Interesting, in 54 I see this when paused:
> 
>   > var x = 1;
>   > x;
> 
> Error: variable 'x' has been optimized out

Can I get STR for either the undefined or the optimized out behavior, preferably with a file I can run locally? When testing with the following trivial example, I'm able to declare:

<html>
<head>
<script>
  function f() {
    console.log("hello world");
  }
  f();
</script>
</head>
<body>
</body>
</html>

If I break on the console.log, I get:

  > var x = 42; 
  undefined 
  > x 
  42

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Shu-yu Guo [:shu] from comment #7)
> Can I get STR for either the undefined or the optimized out behavior,
> preferably with a file I can run locally? When testing with the following
> trivial example, I'm able to declare:

Ah, this seems dependent on the page. Is this test case reproducible for you?

* ./mach run --profile /tmp/test-console
* This opens "Nightly Start Page" in a clean profile
* Open the debugger
* Click the 'pause' icon (to pause on next script execution)
* Click in the search box (to trigger the pause)
* Now in the console: 

 > var x = 42; 
 undefined 
 > x 
 ReferenceError: x is not defined

Comment 9

[Brian Grinstead [:bgrins]]

Also, I'm not sure how to hit the "Error: variable 'x' has been optimized out" error from Comment 6 anymore

Comment 10

[Jason Laster [:jlast]]

Following up, it seems like there are some cases where you can pause in the debugger and type `var x = 42` and then type `x` and see 42. Some cases return a reference error: `ReferenceError: x is not defined`.


Happy Case: comment 7
Sad Cases: comment 8, http://firefox-dev.tools/debugger-examples/examples/todomvc/ -> breakpoint in todo-view.js#34

It'd be nice to see if we can narrow the test case down a bit.

Comment 11

[Ted Campbell [:tcampbell]]

Bleh.. Looking at it in the debugger, it seems the distinction is that when the script is in strict mode, the debug eval becomes strict and loses the ability to pollute outside. In the "happy" case, we pollute the var into the scripts environment.