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 [email protected]

Author: galpeter

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. */

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;

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)');
+}

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);

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);