Fix: Don't use Isolate during scope resolution.

Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org, rossberg@chromium.org
BUG=

Committed: https://crrev.com/1e08974e86d197c631b9961c6a1d190aeefeccc7
Cr-Commit-Position: refs/heads/master@{#37893}

[marja, 2016-07-19 11:36:43]

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

[commit-bot: I haz the power, 2016-07-19 11:36:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[marja, 2016-07-19 11:44:20]

Description was changed from

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org
BUG=
==========

to

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org
BUG=
==========

[marja, 2016-07-19 11:44:20]

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

[marja, 2016-07-19 11:44:46]

Description was changed from

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org
BUG=
==========

to

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org, rossberg@chromium.org
BUG=
==========

[marja, 2016-07-19 11:45:15]

rossberg@, ptal

[Toon Verwaest, 2016-07-19 11:49:43]

lgtm with comment

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

https://codereview.chromium.org/2158343002/diff/1/src/ast/scopes.cc#newcode1411
src/ast/scopes.cc:1411: //        !var->IsStackLocal());
We shouldn't leave old code around in comments. Either find a working way to
express it, or remove it altogether.

[marja, 2016-07-19 11:53:08]

oops, didn't mean to leave these comment around, rossberg@, don't look at this
yet :)

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

https://codereview.chromium.org/2158343002/diff/1/src/ast/scopes.cc#newcode1411
src/ast/scopes.cc:1411: //        !var->IsStackLocal());
On 2016/07/19 11:49:42, Toon Verwaest wrote:
> We shouldn't leave old code around in comments. Either find a working way to
> express it, or remove it altogether.

Oops, this was unintentional, will fix

[marja, 2016-07-19 12:02:37]

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

[Toon Verwaest, 2016-07-19 12:02:37]

lgtm

[commit-bot: I haz the power, 2016-07-19 12:02:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[marja, 2016-07-19 12:03:37]

Fixed leftover code.

Probably that check didn't make sense in the previous state, because Variables
don't have Handle<String> names that early. But now it should make sense.

rossberg@, ptal

Sending this before try jobs have completed because timing is hard. :/

[rossberg, 2016-07-19 12:08:36]

Nice! LGTM

[commit-bot: I haz the power, 2016-07-19 12:37:24]

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

[commit-bot: I haz the power, 2016-07-19 12:37:25]

Dry run: This issue passed the CQ dry run.

[marja, 2016-07-20 07:48:17]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2016-07-20 07:48:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 07:52:37]

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

[commit-bot: I haz the power, 2016-07-20 07:52:39]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/19808)

[marja, 2016-07-20 08:02:37]

The CQ bit was checked by marja@chromium.org

[marja, 2016-07-20 08:02:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org, rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/2158343002/#ps40001
(title: "rebased")

[commit-bot: I haz the power, 2016-07-20 08:02:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[marja, 2016-07-20 08:06:59]

The CQ bit was unchecked by marja@chromium.org

[marja, 2016-07-20 08:30:29]

The CQ bit was checked by marja@chromium.org

[marja, 2016-07-20 08:30:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org, rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/2158343002/#ps80001
(title: "(streamlined the rebase)")

[commit-bot: I haz the power, 2016-07-20 08:30:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-20 08:57:57]

Description was changed from

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org, rossberg@chromium.org
BUG=
==========

to

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org, rossberg@chromium.org
BUG=
==========

[commit-bot: I haz the power, 2016-07-20 08:57:58]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-20 08:58:00]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-20 08:58:59]

Description was changed from

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org, rossberg@chromium.org
BUG=
==========

to

==========
Fix: Don't use Isolate during scope resolution.

Using Isolate is unsafe, because we might parse (and do scope analysis)
on a background thread.

The illegal access happens when encountering f(arguments) { ... }.

Kudos to verwaest@ for finding this bug.

R=verwaest@chromium.org, rossberg@chromium.org
BUG=

Committed: https://crrev.com/1e08974e86d197c631b9961c6a1d190aeefeccc7
Cr-Commit-Position: refs/heads/master@{#37893}
==========

[commit-bot: I haz the power, 2016-07-20 08:59:00]

Patchset 5 (id:??) landed as
https://crrev.com/1e08974e86d197c631b9961c6a1d190aeefeccc7
Cr-Commit-Position: refs/heads/master@{#37893}