chakra-core / ChakraCore

ChakraCore is an open source Javascript engine with a C API.
MIT License
9.12k stars 1.2k forks source link

Super property access in eval cases should throw SyntaxError not runtime error #5559

Open aneeshdk opened 6 years ago

aneeshdk commented 6 years ago

Following up on the discussion in this PR https://github.com/Microsoft/ChakraCore/pull/5471. Right now for eval cases we always assume superpopertyallowed status and then throw a runtime error for error cases. Ideally is should be a Syntax Error.

function foo() {
    print(eval("print('Inside eval');super.a"));
}
foo();

This shouldn't print 'Inside eval' at all. We should only get a SyntaxError.

edit:formatting

zenparsing commented 6 years ago
#### ch
Inside evalReferenceError: Missing or invalid 'super' binding

#### v8
SyntaxError: 'super' keyword unexpected here

#### sm
SyntaxError: use of super property accesses only valid within methods or eval code within methods:
leobalter commented 6 years ago

@zenparsing:

Inside evalReferenceError: Missing or invalid 'super' binding

This gets even worse if foo is a property (non method):

var obj = {
    foo: function foo() {
        print(eval("print('Inside eval');super.a"));
    }
};

Object.setPrototypeOf(obj, {a: 42});
obj.foo();

but I guess it might be fixed in the #5471.


@aneeshdk to support your case, this is well defined in the specs:

https://tc39.github.io/ecma262/#sec-performeval

Runtime Semantics: PerformEval ( x, evalRealm, strictCaller, direct )

....
4. If thisEnvRec is a function Environment Record, then
  ...
  c. Let inMethod be thisEnvRec.HasSuperBinding().
  ...
5. Else,
  ...
  b. Let inMethod be false.
  ...
6. Let script be the ECMAScript code that is the result of parsing x, interpreted as UTF-16 encoded Unicode text as described in 6.1.4, for the goal symbol Script. If inFunction is false, additional early error rules from 18.2.1.1.1 are applied. If inMethod is false, additional early error rules from 18.2.1.1.2 are applied. If inDerivedConstructor is false, additional early error rules from 18.2.1.1.3 are applied. If the parse fails, throw a SyntaxError exception. If any early errors are detected, throw a SyntaxError or a ReferenceError exception, depending on the type of the error (but see also clause 16). Parsing and early error detection may be interweaved in an implementation-dependent manner.
...

18.2.1.1.2 Additional Early Error Rules for Eval Outside Methods

These static semantics are applied by PerformEval when a direct eval call occurs outside of a MethodDefinition.

ScriptBody : StatementList
- It is a Syntax Error if StatementList Contains SuperProperty.

So your thoughts are correct and ChakraCore should print a parse error evaluating the statements in the eval call.

leobalter commented 6 years ago

Btw, these tests from Test262 can help supporting a fix for this:

git grep -l 'Additional Early Error Rules for Eval Outside Methods' test/language/
test/language/eval-code/direct/super-call-fn.js
test/language/eval-code/direct/super-call-method.js
test/language/eval-code/direct/super-prop-dot-no-home.js
test/language/eval-code/direct/super-prop-expr-no-home-no-eval.js
test/language/eval-code/direct/super-prop-expr-no-home.js
test/language/expressions/class/fields-derived-cls-direct-eval-err-contains-superproperty-1.js
test/language/expressions/class/fields-derived-cls-direct-eval-err-contains-superproperty-2.js
test/language/expressions/class/fields-derived-cls-indirect-eval-err-contains-superproperty-1.js
test/language/expressions/class/fields-derived-cls-indirect-eval-err-contains-superproperty-2.js
test/language/expressions/class/fields-private-derived-cls-direct-eval-err-contains-superproperty-1.js
test/language/expressions/class/fields-private-derived-cls-direct-eval-err-contains-superproperty-2.js
test/language/expressions/class/fields-private-derived-cls-indirect-eval-err-contains-superproperty-1.js
test/language/expressions/class/fields-private-derived-cls-indirect-eval-err-contains-superproperty-2.js
test/language/statements/class/fields-derived-cls-direct-eval-err-contains-superproperty-1.js
test/language/statements/class/fields-derived-cls-direct-eval-err-contains-superproperty-2.js
test/language/statements/class/fields-derived-cls-indirect-eval-err-contains-superproperty-1.js
test/language/statements/class/fields-derived-cls-indirect-eval-err-contains-superproperty-2.js
test/language/statements/class/fields-private-derived-cls-direct-eval-err-contains-superproperty-1.js
test/language/statements/class/fields-private-derived-cls-direct-eval-err-contains-superproperty-2.js
test/language/statements/class/fields-private-derived-cls-indirect-eval-err-contains-superproperty-1.js
test/language/statements/class/fields-private-derived-cls-indirect-eval-err-contains-superproperty-2.js
zenparsing commented 6 years ago

Thanks, @leobalter!