From c90306449e51e6f1665b34735cb388b4a38f763e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20G=C3=A1l?= Date: Wed, 31 Jul 2019 15:13:06 +0200 Subject: [PATCH] Correctly handle call arguments when using 'super' calls (#2996) There are two interconnected changes here: * Added missing argument number handling for `super.propname(..)` calls. * The `super.propname.call(...)` is does not need extra 'this' binding helper. Fixes #2990. JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com --- jerry-core/parser/js/js-parser-expr.c | 22 ++++ jerry-core/vm/vm.c | 43 ++++++- .../jerry/es2015/class-super-access-direct.js | 102 ++++++++++++++++ .../es2015/class-super-access-indirect.js | 114 ++++++++++++++++++ .../es2015/regression-test-issue-2990.js | 52 ++++++++ 5 files changed, 332 insertions(+), 1 deletion(-) create mode 100644 tests/jerry/es2015/class-super-access-direct.js create mode 100644 tests/jerry/es2015/class-super-access-indirect.js create mode 100644 tests/jerry/es2015/regression-test-issue-2990.js diff --git a/jerry-core/parser/js/js-parser-expr.c b/jerry-core/parser/js/js-parser-expr.c index de4567106..2ef267a08 100644 --- a/jerry-core/parser/js/js-parser-expr.c +++ b/jerry-core/parser/js/js-parser-expr.c @@ -1532,9 +1532,31 @@ parser_parse_unary_expression (parser_context_t *context_p, /**< context */ static void parser_process_unary_expression (parser_context_t *context_p) /**< context */ { +#if ENABLED (JERRY_ES2015_CLASS) + /* Track to see if a property was accessed or not */ + bool property_accessed = false; +#endif /* ENABLED (JERRY_ES2015_CLASS) */ + /* Parse postfix part of a primary expression. */ while (true) { +#if ENABLED (JERRY_ES2015_CLASS) + if (context_p->token.type == LEXER_DOT || context_p->token.type == LEXER_LEFT_SQUARE) + { + if (property_accessed) + { + /** + * In the case of "super.prop1.prop2(...)" the second property access should not + * generate a super prop call thus the 'PARSER_CLASS_SUPER_PROP_REFERENCE' flags should be removed. + * + * Similar case: "super[propname].prop2(...)" + */ + context_p->status_flags &= (uint32_t) ~PARSER_CLASS_SUPER_PROP_REFERENCE; + } + property_accessed = true; + } +#endif /* ENABLED (JERRY_ES2015_CLASS) */ + /* Since break would only break the switch, we use * continue to continue this loop. Without continue, * the code abandons the loop. */ diff --git a/jerry-core/vm/vm.c b/jerry-core/vm/vm.c index d81bf3d2a..28949b771 100644 --- a/jerry-core/vm/vm.c +++ b/jerry-core/vm/vm.c @@ -1566,7 +1566,48 @@ vm_loop (vm_frame_ctx_t *frame_ctx_p) /**< frame context */ } case VM_OC_SUPER_PROP_REFERENCE: { - const int index = (byte_code_start_p[1] == CBC_EXT_SUPER_PROP_ASSIGN) ? -1 : -3; + /** + * In case of this VM_OC_SUPER_PROP_REFERENCE the previously pushed 'super' must be replaced + * with the current 'this' value to correctly access the properties. + */ + int index = -1; /* -1 in case of CBC_EXT_SUPER_PROP_ASSIGN */ + + if (JERRY_LIKELY (byte_code_start_p[1] == CBC_EXT_SUPER_PROP_CALL)) + { + cbc_opcode_t next_call_opcode = (cbc_opcode_t) byte_code_start_p[2]; + /* The next opcode must be a call opcode */ + JERRY_ASSERT (CBC_CALL <= next_call_opcode && next_call_opcode <= CBC_CALL2_PROP_BLOCK); + + int arguments_list_len; + if (next_call_opcode >= CBC_CALL0) + { + /* CBC_CALL{0,1,2}* have their arg count encoded, extract it from there */ + arguments_list_len = (int) ((next_call_opcode - CBC_CALL0) / 6); + } + else + { + /** + * In this case the arguments are coded into the byte code stream as a byte argument + * following the call opcode. + */ + arguments_list_len = (int) byte_code_start_p[3]; + } + /* The old 'super' value is at least '-3' element away from the current position on the stack. */ + index = -3 - arguments_list_len; + } + else + { + /** + * The bytecode order for super assignment should be one of this: + * - CBC_EXT_PUSH_SUPER, CBC_EXT_SUPER_PROP_ASSIGN. + * - CBC_EXT_PUSH_CONSTRUCTOR_SUPER_PROP, CBC_EXT_SUPER_PROP_ASSIGN. + * That is one ext opcode back (-1). + */ + JERRY_ASSERT (byte_code_start_p[-1] == CBC_EXT_PUSH_SUPER + || byte_code_start_p[-1] == CBC_EXT_PUSH_CONSTRUCTOR_SUPER_PROP); + } + + /* Replace the old 'super' value with the correct 'this' binding */ ecma_free_value (stack_top_p[index]); stack_top_p[index] = ecma_copy_value (frame_ctx_p->this_binding); continue; diff --git a/tests/jerry/es2015/class-super-access-direct.js b/tests/jerry/es2015/class-super-access-direct.js new file mode 100644 index 000000000..99bff5a7c --- /dev/null +++ b/tests/jerry/es2015/class-super-access-direct.js @@ -0,0 +1,102 @@ +// Copyright JS Foundation and other contributors, http://js.foundation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +class Base { + constructor () { + this.parent_value = 100; + } + + parent_value () { + return this.parent_value; + } + + parent_value_arg (a, b, c) { + if (c) { + return this.parent_value + a + b + c; + } else if (b) { + return this.parent_value + a + b; + } else { + return this.parent_value + a; + } + } + + method () { + return { + method: function (a, b, c, d, e) { return -50 + a + b + c + d + e; } + } + } +} + +class Target extends Base { + constructor () { + super (); + this.parent_value = -10; + } + + parent_value () { + throw new Error ('(parent_value)'); + } + + parent_value_direct () { + return super.parent_value (); + } + + parent_value_direct_arg (a, b, c) { + if (c) { + return super.parent_value_arg (a, b, c); + } else if (b) { + return super.parent_value_arg (a, b); + } else { + return super.parent_value_arg (a); + } + } + + method () { + throw new Error ("(method)"); + } + + parent_method_dot () { + return super.method ().method (1, 2, 3, 4, 5) + } + + parent_method_index () { + return super['method']()['method'](1, 2, 3, 4, 5); + } +} + + +var obj = new Target (); + +assert (obj.parent_value_direct () === -10); +assert (obj.parent_value_direct_arg (1) === -9); +assert (obj.parent_value_direct_arg (1, 2) === -7); +assert (obj.parent_value_direct_arg (1, 2, 3) === -4); + +try { + obj.parent_value(); + assert (false) +} catch (ex) { + /* 'obj.parent_value is a number! */ + assert (ex instanceof TypeError); +} + +assert (obj.parent_method_dot () === -35); +assert (obj.parent_method_index () === -35); + +try { + obj.method (); + assert (false); +} catch (ex) { + assert (ex.message === '(method)'); +} diff --git a/tests/jerry/es2015/class-super-access-indirect.js b/tests/jerry/es2015/class-super-access-indirect.js new file mode 100644 index 000000000..01926b034 --- /dev/null +++ b/tests/jerry/es2015/class-super-access-indirect.js @@ -0,0 +1,114 @@ +// Copyright JS Foundation and other contributors, http://js.foundation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +class Base { + constructor () { + this.parent_value = 100; + } + + parent_value () { + return this.parent_value; + } + + parent_value_arg (a, b, c) { + if (c) { + return this.parent_value + a + b + c; + } else if (b) { + return this.parent_value + a + b; + } else { + return this.parent_value + a; + } + } + + method () { + return { + method: function (a, b, c, d, e) { return -50 + a + b + c + d + e; } + } + } +} + +class Target extends Base { + constructor () { + super (); + this.parent_value = -10; + } + + parent_value () { + throw new Error ('(parent_value)'); + } + + parent_value_indirect () { + return super.parent_value.call (this); + } + + parent_value_indirect_arg (a, b, c) { + if (c) { + return super.parent_value_arg.call (this, a, b, c); + } else if (b) { + return super.parent_value_arg.call (this, a, b); + } else { + return super.parent_value_arg.call (this, a); + } + } + + method () { + throw new Error ("(method)"); + } + + parent_method_dot () { + return super.method.call (this).method (1, 2, 3, 4, 5) + } + + parent_method_index() { + return super['method'].call (this)['method'] (1, 2, 3, 4, 5); + } +} + + +var obj = new Target(); + +assert (obj.parent_value_indirect () === -10); +assert (obj.parent_value_indirect_arg (1) === -9); +assert (obj.parent_value_indirect_arg (1, 2) === -7); +assert (obj.parent_value_indirect_arg (1, 2, 3) === -4); + +try { + obj.parent_value (); + assert (false); +} catch (ex) { + /* 'obj.parent_value is a number! */ + assert (ex instanceof TypeError); +} + +assert (obj.parent_method_dot () === -35); +assert (obj.parent_method_index () === -35); + +try { + obj.method(); + assert (false); +} catch (ex) { + assert (ex.message === '(method)'); +} + +var demo_object = { + parent_value: 1000, + method: function () { + throw new Error ('Very bad!'); + } +} + +assert (obj.parent_value_indirect_arg.call (demo_object, 1) === 1001); +assert (obj.parent_value_indirect_arg.call (demo_object, 1, 2) === 1003); + +assert (obj.parent_method_dot.call (demo_object) === -35); diff --git a/tests/jerry/es2015/regression-test-issue-2990.js b/tests/jerry/es2015/regression-test-issue-2990.js new file mode 100644 index 000000000..75b5c554c --- /dev/null +++ b/tests/jerry/es2015/regression-test-issue-2990.js @@ -0,0 +1,52 @@ +// Copyright JS Foundation and other contributors, http://js.foundation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +class A { + constructor() { + this.id = 50000; + } + + getid(a) { + if (typeof a === 'number') { + this.id += a; + } + return this.id; + } +} + +class B extends A { + constructor() { + super(); + this.id = 100; + } + + getid_A() { + return super.getid.call(this); + } + + getid_B(a) { + return super.getid.call(this, a); + } + getid_C(a) { + var fn = super.getid.bind(this); + return fn(a); + } +} + +var obj = new B(); + +assert (obj.getid_A() === 100); +assert (obj.getid_B(1) === 101); +assert (obj.getid_C(1) === 102); +assert (obj.id === 102);