[strong] Implement static restrictions on direct eval

[strong] Implement static restrictions on direct eval

Does not entirely disallow the use of 'eval' as an identifier in strong mode,
as originally proposed.

BUG=v8:3956
LOG=N

Committed: https://crrev.com/c983689d39ebd27b0be81542f0721eb5f5d5e3b8
Cr-Commit-Position: refs/heads/master@{#27796}

[conradw, 2015-04-13 13:05:25]

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

[conradw, 2015-04-13 13:05:40]

On 2015/04/13 13:05:25, conradw wrote:
> mailto:conradw@chromium.org changed reviewers:
> + mailto:rossberg@chromium.org

PTAL

[arv (Not doing code reviews), 2015-04-13 14:14:06]

arv@chromium.org changed reviewers:
+ arv@chromium.org

[arv (Not doing code reviews), 2015-04-13 14:14:07]

LGTM

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/cla...
File test/mjsunit/strong/classes.js (left):

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/cla...
test/mjsunit/strong/classes.js:19: assertTypeError(function(){ eval("D = 0") });
This can be written as:

assertTypeError(function(){
  D = 0;
});

Which is much cleaner than the test with eval you added on line 25.

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/dec...
File test/mjsunit/strong/declaration-after-use.js (left):

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/dec...
test/mjsunit/strong/declaration-after-use.js:60: assertThrows("'use strong';
eval('if (false) { x; let x = 0;}')",
Change to use a closure?

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/use...
File test/mjsunit/strong/use-strong.js (left):

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/use...
test/mjsunit/strong/use-strong.js:13: assertThrows("'use strong'; eval('function
f() { 0 == 0 }')", SyntaxError);
Use a closure

[conradw, 2015-04-13 14:36:33]

I've been informed offline by rossberg that my change doesn't cover

(eval)();

which is also a direct call.
My approach for spotting direct eval was very optimistic and unfortunately this
case almost certainly means a completely different method is needed.

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/cla...
File test/mjsunit/strong/classes.js (left):

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/cla...
test/mjsunit/strong/classes.js:19: assertTypeError(function(){ eval("D = 0") });
On 2015/04/13 14:14:06, arv wrote:
> This can be written as:
> 
> assertTypeError(function(){
>   D = 0;
> });
> 
> Which is much cleaner than the test with eval you added on line 25. 

Unfortunately this assignment will be an early error in strong mode (once we
implement it!) and would prevent the entire test script from executing. We need
a test that checks that assignment to the identifier of a locally scoped class
is disallowed, but avoiding the early error requires some sort of "eval"
containment.

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/dec...
File test/mjsunit/strong/declaration-after-use.js (left):

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/dec...
test/mjsunit/strong/declaration-after-use.js:60: assertThrows("'use strong';
eval('if (false) { x; let x = 0;}')",
On 2015/04/13 14:14:06, arv wrote:
> Change to use a closure?

See other comment on propagating strong mode through eval.

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/use...
File test/mjsunit/strong/use-strong.js (left):

https://codereview.chromium.org/1059273004/diff/40001/test/mjsunit/strong/use...
test/mjsunit/strong/use-strong.js:13: assertThrows("'use strong'; eval('function
f() { 0 == 0 }')", SyntaxError);
On 2015/04/13 14:14:06, arv wrote:
> Use a closure

This test was checking propagation of strong mode into a direct eval, which
doesn't make sense since direct eval shouldn't be allowed in strong mode. Tests
such as the one on line 11 check the other aspects of strong mode exercised
here.

[conradw, 2015-04-13 16:24:51]

PTAL

Fixed the problematic case

[rossberg, 2015-04-13 16:32:26]

lgtm

https://codereview.chromium.org/1059273004/diff/80001/test/mjsunit/strong/cla...
File test/mjsunit/strong/classes.js (right):

https://codereview.chromium.org/1059273004/diff/80001/test/mjsunit/strong/cla...
test/mjsunit/strong/classes.js:12: let ind_eval = eval;
Nit: indirect_eval

https://codereview.chromium.org/1059273004/diff/80001/test/mjsunit/strong/cla...
test/mjsunit/strong/classes.js:25: assertTypeError("'use strong'; (function f()
{class E {}; E = 0})()");
Nit: move that inside the test function

[conradw, 2015-04-13 17:23:25]

The CQ bit was checked by conradw@chromium.org

[conradw, 2015-04-13 17:23:25]

The patchset sent to the CQ was uploaded after l-g-t-m from arv@chromium.org,
rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1059273004/#ps120001
(title: "cr feedback")

[commit-bot: I haz the power, 2015-04-13 17:23:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1059273004/120001

[commit-bot: I haz the power, 2015-04-13 17:25:25]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-13 17:25:33]

Patchset 7 (id:??) landed as
https://crrev.com/c983689d39ebd27b0be81542f0721eb5f5d5e3b8
Cr-Commit-Position: refs/heads/master@{#27796}

[marja, 2015-04-13 19:13:06]

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

[marja, 2015-04-13 19:13:07]

https://codereview.chromium.org/1059273004/diff/120001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/1059273004/diff/120001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:5923: "'use strong'; function f() {eval()}",
For this kind of a test, it would be useful to put the "use strong" into the
context_data, and then have another non-strong context too.

Then you can test that none of these is an error in the non-strong context. This
protects you from testing errors where you make random typos and other random
syntax errors here (so that the cause of the error is something else than what
you meant).

[conradw, 2015-04-16 13:37:42]

On 2015/04/13 19:13:07, marja wrote:
>
https://codereview.chromium.org/1059273004/diff/120001/test/cctest/test-parsi...
> File test/cctest/test-parsing.cc (right):
> 
>
https://codereview.chromium.org/1059273004/diff/120001/test/cctest/test-parsi...
> test/cctest/test-parsing.cc:5923: "'use strong'; function f() {eval()}",
> For this kind of a test, it would be useful to put the "use strong" into the
> context_data, and then have another non-strong context too.
> 
> Then you can test that none of these is an error in the non-strong context.
This
> protects you from testing errors where you make random typos and other random
> syntax errors here (so that the cause of the error is something else than what
> you meant).

Fixed this in another CL