[debugger] fix debug-evaluate wrt shadowed context var.

[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow
context variables incorrectly, since the stack variable is
not visible in the context chain.

To fix this, we limit local variables accessible by debug-
evaluate to the ones directly referenced inside the function.
What is not referenced by the function itself, is considered
optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to
the native context, and write back changes after debug-
evaluate. Changes to the original context chain will however
be overwritten. This already happens for catch and block
scopes though.

Also fix a crash caused by declaring variables inside debug-
evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N

Committed: https://crrev.com/089edbfa97eab324bc463829ea03c167fdc6d45c
Cr-Commit-Position: refs/heads/master@{#32828}

[Yang, 2015-12-07 08:04:42]

The CQ bit was checked by yangguo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-07 08:04:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1500933002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1500933002/1

[commit-bot: I haz the power, 2015-12-07 08:26:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-07 08:26:30]

Dry run: This issue passed the CQ dry run.

[Yang, 2015-12-07 10:32:22]

On 2015/12/07 08:26:30, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

ping

[Michael Starzinger, 2015-12-07 10:42:57]

https://codereview.chromium.org/1500933002/diff/1/src/debug/debug-evaluate.cc
File src/debug/debug-evaluate.cc (right):

https://codereview.chromium.org/1500933002/diff/1/src/debug/debug-evaluate.cc...
src/debug/debug-evaluate.cc:278: while (!context->IsNativeContext() &&
!context->IsScriptContext()) {
This iteration seems to walk the context chain until the first context allocated
receiver is hit. But I am not sure this guarantees that the receiver is actually
the correct one that is lexically visible at the evaluation point. Consider the
following example:

function f() {
  (() => this);                     // context allocate 'this' within f
  function g() {
    (() => %DebugEvaluate("this"))  // evaluates to wrong 'this'?
  }
  g.call("yep!");
}
f.call("nope!");

[Michael Starzinger, 2015-12-07 10:53:56]

LGTM.

[Yang, 2015-12-08 13:19:45]

On 2015/12/07 10:53:56, Michael Starzinger wrote:
> LGTM.

The old approach did not work because of incomplete context chains (we skip
allocating contexts if possible).
I reimplemented this. Now resolving "this" in debug-evaluate is only possible if
the arrow function itself references "this".

[Yang, 2015-12-08 13:20:04]

yangguo@chromium.org changed reviewers:
+ rossberg@chromium.org

[Yang, 2015-12-08 13:20:05]

Andreas, do you want to take a look at this?

[rossberg, 2015-12-08 17:09:17]

On 2015/12/08 13:20:05, Yang wrote:
> Andreas, do you want to take a look at this?

LGTM, but Michael might want to revisit the new implementation.

[Yang, 2015-12-08 17:43:50]

On 2015/12/08 17:09:17, rossberg wrote:
> On 2015/12/08 13:20:05, Yang wrote:
> > Andreas, do you want to take a look at this?
> 
> LGTM, but Michael might want to revisit the new implementation.

So... now that I think about this, the whole debug evaluate implementation is
broken. Consider this case:

(function() {
  var x = 1;
  (() => x); // context allocate x.
  (function() {
    var x = 2; // stack allocate shadowing x.
    (function() {
      debugger;
    })();
  })();
})();

If we debug evaluate "x" at the debugger statement, we would see x == 1, because
the stack allocated one is not on the context chain and there is no indication
that there is anything missing.

So evaluating "this" in arrow functions is just a special case of stack-var
shadowing context-var, and one that occurs often since every non-arrow function
scope and the global scope has a "this".

I'd like to find a generic solution to this. What I have in mind is:
- the scope iterator already reparses the function to get complete scope
information on the function
- from this, we can find out which variable names from outer contexts the
function references
- we can build a with-context with these variable names and values that we
looked up
- this way, debug-evaluate can only access variables that are also available to
the function we are evaluating in. Other vars, even though context-allocated,
are hidden.
- we would need to write back changes to the with-context. We already do similar
trick when we materialize and dematerialize stack locals.

WDYT?

[rossberg, 2015-12-08 17:46:47]

On 2015/12/08 17:43:50, Yang wrote:
> On 2015/12/08 17:09:17, rossberg wrote:
> > On 2015/12/08 13:20:05, Yang wrote:
> > > Andreas, do you want to take a look at this?
> > 
> > LGTM, but Michael might want to revisit the new implementation.
> 
> So... now that I think about this, the whole debug evaluate implementation is
> broken. Consider this case:
> 
> (function() {
>   var x = 1;
>   (() => x); // context allocate x.
>   (function() {
>     var x = 2; // stack allocate shadowing x.
>     (function() {
>       debugger;
>     })();
>   })();
> })();
> 
> If we debug evaluate "x" at the debugger statement, we would see x == 1,
because
> the stack allocated one is not on the context chain and there is no indication
> that there is anything missing.
> 
> So evaluating "this" in arrow functions is just a special case of stack-var
> shadowing context-var, and one that occurs often since every non-arrow
function
> scope and the global scope has a "this".
> 
> I'd like to find a generic solution to this. What I have in mind is:
> - the scope iterator already reparses the function to get complete scope
> information on the function
> - from this, we can find out which variable names from outer contexts the
> function references
> - we can build a with-context with these variable names and values that we
> looked up
> - this way, debug-evaluate can only access variables that are also available
to
> the function we are evaluating in. Other vars, even though context-allocated,
> are hidden.
> - we would need to write back changes to the with-context. We already do
similar
> trick when we materialize and dematerialize stack locals.
> 
> WDYT?

Makes sense -- if you really feel like implementing this. :)

FWIW, I was aware that we already do the wrong thing for other variables, but I
thought this was in the spirit of the debugger being best effort.

[Yang, 2015-12-08 18:10:26]

On 2015/12/08 17:46:47, rossberg wrote:
> On 2015/12/08 17:43:50, Yang wrote:
> > On 2015/12/08 17:09:17, rossberg wrote:
> > > On 2015/12/08 13:20:05, Yang wrote:
> > > > Andreas, do you want to take a look at this?
> > > 
> > > LGTM, but Michael might want to revisit the new implementation.
> > 
> > So... now that I think about this, the whole debug evaluate implementation
is
> > broken. Consider this case:
> > 
> > (function() {
> >   var x = 1;
> >   (() => x); // context allocate x.
> >   (function() {
> >     var x = 2; // stack allocate shadowing x.
> >     (function() {
> >       debugger;
> >     })();
> >   })();
> > })();
> > 
> > If we debug evaluate "x" at the debugger statement, we would see x == 1,
> because
> > the stack allocated one is not on the context chain and there is no
indication
> > that there is anything missing.
> > 
> > So evaluating "this" in arrow functions is just a special case of stack-var
> > shadowing context-var, and one that occurs often since every non-arrow
> function
> > scope and the global scope has a "this".
> > 
> > I'd like to find a generic solution to this. What I have in mind is:
> > - the scope iterator already reparses the function to get complete scope
> > information on the function
> > - from this, we can find out which variable names from outer contexts the
> > function references
> > - we can build a with-context with these variable names and values that we
> > looked up
> > - this way, debug-evaluate can only access variables that are also available
> to
> > the function we are evaluating in. Other vars, even though
context-allocated,
> > are hidden.
> > - we would need to write back changes to the with-context. We already do
> similar
> > trick when we materialize and dematerialize stack locals.
> > 
> > WDYT?
> 
> Makes sense -- if you really feel like implementing this. :)
> 
> FWIW, I was aware that we already do the wrong thing for other variables, but
I
> thought this was in the spirit of the debugger being best effort.

Best effort, yes, but it should not be incorrect :)

[rossberg, 2015-12-09 08:48:40]

On 2015/12/08 18:10:26, Yang wrote:
> On 2015/12/08 17:46:47, rossberg wrote:
> > On 2015/12/08 17:43:50, Yang wrote:
> > > On 2015/12/08 17:09:17, rossberg wrote:
> > > > On 2015/12/08 13:20:05, Yang wrote:
> > > > > Andreas, do you want to take a look at this?
> > > > 
> > > > LGTM, but Michael might want to revisit the new implementation.
> > > 
> > > So... now that I think about this, the whole debug evaluate implementation
> is
> > > broken. Consider this case:
> > > 
> > > (function() {
> > >   var x = 1;
> > >   (() => x); // context allocate x.
> > >   (function() {
> > >     var x = 2; // stack allocate shadowing x.
> > >     (function() {
> > >       debugger;
> > >     })();
> > >   })();
> > > })();
> > > 
> > > If we debug evaluate "x" at the debugger statement, we would see x == 1,
> > because
> > > the stack allocated one is not on the context chain and there is no
> indication
> > > that there is anything missing.
> > > 
> > > So evaluating "this" in arrow functions is just a special case of
stack-var
> > > shadowing context-var, and one that occurs often since every non-arrow
> > function
> > > scope and the global scope has a "this".
> > > 
> > > I'd like to find a generic solution to this. What I have in mind is:
> > > - the scope iterator already reparses the function to get complete scope
> > > information on the function
> > > - from this, we can find out which variable names from outer contexts the
> > > function references
> > > - we can build a with-context with these variable names and values that we
> > > looked up
> > > - this way, debug-evaluate can only access variables that are also
available
> > to
> > > the function we are evaluating in. Other vars, even though
> context-allocated,
> > > are hidden.
> > > - we would need to write back changes to the with-context. We already do
> > similar
> > > trick when we materialize and dematerialize stack locals.
> > > 
> > > WDYT?
> > 
> > Makes sense -- if you really feel like implementing this. :)
> > 
> > FWIW, I was aware that we already do the wrong thing for other variables,
but
> I
> > thought this was in the spirit of the debugger being best effort.
> 
> Best effort, yes, but it should not be incorrect :)

Well, hasn't it always been? And won't it always be to some extent?

[Yang, 2015-12-09 14:47:02]

On 2015/12/09 08:48:40, rossberg wrote:
> On 2015/12/08 18:10:26, Yang wrote:
> > On 2015/12/08 17:46:47, rossberg wrote:
> > > On 2015/12/08 17:43:50, Yang wrote:
> > > > On 2015/12/08 17:09:17, rossberg wrote:
> > > > > On 2015/12/08 13:20:05, Yang wrote:
> > > > > > Andreas, do you want to take a look at this?
> > > > > 
> > > > > LGTM, but Michael might want to revisit the new implementation.
> > > > 
> > > > So... now that I think about this, the whole debug evaluate
implementation
> > is
> > > > broken. Consider this case:
> > > > 
> > > > (function() {
> > > >   var x = 1;
> > > >   (() => x); // context allocate x.
> > > >   (function() {
> > > >     var x = 2; // stack allocate shadowing x.
> > > >     (function() {
> > > >       debugger;
> > > >     })();
> > > >   })();
> > > > })();
> > > > 
> > > > If we debug evaluate "x" at the debugger statement, we would see x == 1,
> > > because
> > > > the stack allocated one is not on the context chain and there is no
> > indication
> > > > that there is anything missing.
> > > > 
> > > > So evaluating "this" in arrow functions is just a special case of
> stack-var
> > > > shadowing context-var, and one that occurs often since every non-arrow
> > > function
> > > > scope and the global scope has a "this".
> > > > 
> > > > I'd like to find a generic solution to this. What I have in mind is:
> > > > - the scope iterator already reparses the function to get complete scope
> > > > information on the function
> > > > - from this, we can find out which variable names from outer contexts
the
> > > > function references
> > > > - we can build a with-context with these variable names and values that
we
> > > > looked up
> > > > - this way, debug-evaluate can only access variables that are also
> available
> > > to
> > > > the function we are evaluating in. Other vars, even though
> > context-allocated,
> > > > are hidden.
> > > > - we would need to write back changes to the with-context. We already do
> > > similar
> > > > trick when we materialize and dematerialize stack locals.
> > > > 
> > > > WDYT?
> > > 
> > > Makes sense -- if you really feel like implementing this. :)
> > > 
> > > FWIW, I was aware that we already do the wrong thing for other variables,
> but
> > I
> > > thought this was in the spirit of the debugger being best effort.
> > 
> > Best effort, yes, but it should not be incorrect :)
> 
> Well, hasn't it always been? And won't it always be to some extent?

I uploaded the newest attempt at this.

This is not perfect:we now copy accessibly context locals into a with-context,
and afterwards copy it back. If debug-evaluate calls a function that mutates the
original context, then those changes are overwritten. I think we can live with
this though - it already works this way for block and catch contexts, so doing
it for function contexts just makes the behavior more consistent.

We should think about whether we do want to do that write back at all though.

I think we should generally think about whether we want to update

[rossberg, 2015-12-09 14:53:47]

https://codereview.chromium.org/1500933002/diff/60001/src/ast/scopes.cc
File src/ast/scopes.cc (right):

https://codereview.chromium.org/1500933002/diff/60001/src/ast/scopes.cc#newco...
src/ast/scopes.cc:847: for (int i = 0; i < unresolved_.length(); i++) {
This scares me! It assumes that all non-locals are still unresolved, which is
very (too) brittle. It may not even be the case for all variables (we may
resolve some more eagerly, I'm not sure, or may want to do so in the future).

[Yang, 2015-12-09 15:15:09]

On 2015/12/09 14:53:47, rossberg wrote:
> https://codereview.chromium.org/1500933002/diff/60001/src/ast/scopes.cc
> File src/ast/scopes.cc (right):
> 
>
https://codereview.chromium.org/1500933002/diff/60001/src/ast/scopes.cc#newco...
> src/ast/scopes.cc:847: for (int i = 0; i < unresolved_.length(); i++) {
> This scares me! It assumes that all non-locals are still unresolved, which is
> very (too) brittle. It may not even be the case for all variables (we may
> resolve some more eagerly, I'm not sure, or may want to do so in the future).

actually they have already been resolved at this point. it's just that the list
is still populated with those variable proxies, since we never remove from this
list.

[Yang, 2015-12-10 08:34:37]

Description was changed from

==========
[debugger] correctly resolve "this" for arrow functions.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

to

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow context variables
incorrectly, since the stack variable is not visible in the context chain.

To fix this, we limit local variables accessible by debug-evaluate to the ones
directly referenced inside the function. What is not referenced by the function
itself, is considered optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to the native context,
and write back changes after debug-evaluate. Changes to the original context
chain will however be overwritten. This already happens for catch and block
scopes though

Also fix a crash caused by declaring variables inside debug-evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

[Yang, 2015-12-10 08:39:20]

Description was changed from

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow context variables
incorrectly, since the stack variable is not visible in the context chain.

To fix this, we limit local variables accessible by debug-evaluate to the ones
directly referenced inside the function. What is not referenced by the function
itself, is considered optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to the native context,
and write back changes after debug-evaluate. Changes to the original context
chain will however be overwritten. This already happens for catch and block
scopes though

Also fix a crash caused by declaring variables inside debug-evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

to

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow
context variables incorrectly, since the stack variable is
not visible in the context chain.

To fix this, we limit local variables accessible by debug-
evaluate to the ones directly referenced inside the function.
What is not referenced by the function itself, is considered
optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to
the native context, and write back changes after debug-
evaluate. Changes to the original context chain will however
be overwritten. This already happens for catch and block
scopes though.

Also fix a crash caused by declaring variables inside debug-
evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

[Yang, 2015-12-10 09:08:47]

On 2015/12/09 15:15:09, Yang wrote:
> On 2015/12/09 14:53:47, rossberg wrote:
> > https://codereview.chromium.org/1500933002/diff/60001/src/ast/scopes.cc
> > File src/ast/scopes.cc (right):
> > 
> >
>
https://codereview.chromium.org/1500933002/diff/60001/src/ast/scopes.cc#newco...
> > src/ast/scopes.cc:847: for (int i = 0; i < unresolved_.length(); i++) {
> > This scares me! It assumes that all non-locals are still unresolved, which
is
> > very (too) brittle. It may not even be the case for all variables (we may
> > resolve some more eagerly, I'm not sure, or may want to do so in the
future).
> 
> actually they have already been resolved at this point. it's just that the
list
> is still populated with those variable proxies, since we never remove from
this
> list.

Polished this, added explaination to CL, more test cases and fixed a crash bug
along the way.

[Yang, 2015-12-10 11:18:06]

The CQ bit was checked by yangguo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-10 11:18:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1500933002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1500933002/140001

[commit-bot: I haz the power, 2015-12-10 11:53:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-10 11:53:25]

Dry run: This issue passed the CQ dry run.

[Yang, 2015-12-10 13:06:39]

On 2015/12/10 11:53:25, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

Ping.

- If the concern is that debugger is trying to do scope resolution... well it is
not really. Both debug-evaluate and the compiler parse and do a scope analysis
upfront, so their source of truth is the same. What I require from the scope
analysis is, aside from the nested scope chain that we already collect, a list
of variable names that are not stack-allocated.
- If the concern is that Scope::unresolved_ is a bad name when iterating through
it after everything has been resolved, then maybe I can rename it?
- If the concern is that Scope::unresolved_ may not contain all context and
global variable names, then should I collect the variable names separately in
Scope::LookupLocal?

[Yang, 2015-12-11 11:57:39]

On 2015/12/10 13:06:39, Yang wrote:
> On 2015/12/10 11:53:25, commit-bot: I haz the power wrote:
> > Dry run: This issue passed the CQ dry run.
> 
> Ping.
> 
> - If the concern is that debugger is trying to do scope resolution... well it
is
> not really. Both debug-evaluate and the compiler parse and do a scope analysis
> upfront, so their source of truth is the same. What I require from the scope
> analysis is, aside from the nested scope chain that we already collect, a list
> of variable names that are not stack-allocated.
> - If the concern is that Scope::unresolved_ is a bad name when iterating
through
> it after everything has been resolved, then maybe I can rename it?
> - If the concern is that Scope::unresolved_ may not contain all context and
> global variable names, then should I collect the variable names separately in
> Scope::LookupLocal?

Added TODO regarding unresolved_.

[rossberg, 2015-12-11 12:01:21]

Thanks for the TODO. LGTM until we have something better :)

[Michael Starzinger, 2015-12-11 14:35:12]

LGTM as an interim solution.

https://codereview.chromium.org/1500933002/diff/160001/src/ast/scopes.h
File src/ast/scopes.h (right):

https://codereview.chromium.org/1500933002/diff/160001/src/ast/scopes.h#newcode9
src/ast/scopes.h:9: #include "src/hashmap.h"
nit: The HashMap type could be forward declared instead.

[Yang, 2015-12-14 09:46:57]

On 2015/12/11 14:35:12, Michael Starzinger wrote:
> LGTM as an interim solution.
> 
> https://codereview.chromium.org/1500933002/diff/160001/src/ast/scopes.h
> File src/ast/scopes.h (right):
> 
>
https://codereview.chromium.org/1500933002/diff/160001/src/ast/scopes.h#newcode9
> src/ast/scopes.h:9: #include "src/hashmap.h"
> nit: The HashMap type could be forward declared instead.

HashMap is not a class, but a typedef. The only way to "forward declare" it
would be to include the whole line

typedef TemplateHashMapImpl<FreeStoreAllocationPolicy> HashMap;

Which I think is worse than the #include. WDYT?

[Yang, 2015-12-14 09:48:00]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-12-14 09:48:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1500933002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1500933002/160001

[Michael Starzinger, 2015-12-14 09:49:57]

On 2015/12/14 09:46:57, Yang wrote:
> On 2015/12/11 14:35:12, Michael Starzinger wrote:
> > LGTM as an interim solution.
> > 
> > https://codereview.chromium.org/1500933002/diff/160001/src/ast/scopes.h
> > File src/ast/scopes.h (right):
> > 
> >
>
https://codereview.chromium.org/1500933002/diff/160001/src/ast/scopes.h#newcode9
> > src/ast/scopes.h:9: #include "src/hashmap.h"
> > nit: The HashMap type could be forward declared instead.
> 
> HashMap is not a class, but a typedef. The only way to "forward declare" it
> would be to include the whole line
> 
> typedef TemplateHashMapImpl<FreeStoreAllocationPolicy> HashMap;
> 
> Which I think is worse than the #include. WDYT?

Ack. Didn't realize it is a typedef. Yes, you are right, duplicating the typedef
would be worse. Please disregard my comment in this case. Sorry for the noise.

[commit-bot: I haz the power, 2015-12-14 10:24:39]

Description was changed from

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow
context variables incorrectly, since the stack variable is
not visible in the context chain.

To fix this, we limit local variables accessible by debug-
evaluate to the ones directly referenced inside the function.
What is not referenced by the function itself, is considered
optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to
the native context, and write back changes after debug-
evaluate. Changes to the original context chain will however
be overwritten. This already happens for catch and block
scopes though.

Also fix a crash caused by declaring variables inside debug-
evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

to

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow
context variables incorrectly, since the stack variable is
not visible in the context chain.

To fix this, we limit local variables accessible by debug-
evaluate to the ones directly referenced inside the function.
What is not referenced by the function itself, is considered
optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to
the native context, and write back changes after debug-
evaluate. Changes to the original context chain will however
be overwritten. This already happens for catch and block
scopes though.

Also fix a crash caused by declaring variables inside debug-
evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

[commit-bot: I haz the power, 2015-12-14 10:24:41]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-12-14 10:25:20]

Description was changed from

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow
context variables incorrectly, since the stack variable is
not visible in the context chain.

To fix this, we limit local variables accessible by debug-
evaluate to the ones directly referenced inside the function.
What is not referenced by the function itself, is considered
optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to
the native context, and write back changes after debug-
evaluate. Changes to the original context chain will however
be overwritten. This already happens for catch and block
scopes though.

Also fix a crash caused by declaring variables inside debug-
evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N
==========

to

==========
[debugger] fix debug-evaluate wrt shadowed context var.

Debug-evaluate used to resolve stack variables that shadow
context variables incorrectly, since the stack variable is
not visible in the context chain.

To fix this, we limit local variables accessible by debug-
evaluate to the ones directly referenced inside the function.
What is not referenced by the function itself, is considered
optimized out and not accessible by debug-evaluate.

To achieve this, we duplicate the entire context chain up to
the native context, and write back changes after debug-
evaluate. Changes to the original context chain will however
be overwritten. This already happens for catch and block
scopes though.

Also fix a crash caused by declaring variables inside debug-
evaluate.

R=mstarzinger@chromium.org
BUG=v8:4593
LOG=N

Committed: https://crrev.com/089edbfa97eab324bc463829ea03c167fdc6d45c
Cr-Commit-Position: refs/heads/master@{#32828}
==========

[commit-bot: I haz the power, 2015-12-14 10:25:21]

Patchset 9 (id:??) landed as
https://crrev.com/089edbfa97eab324bc463829ea03c167fdc6d45c
Cr-Commit-Position: refs/heads/master@{#32828}