[strong] Function arity check should be based on required parameters

Also check whether the arguments count is smaller than the number of
required parameters which is the same as the SharedFunctionInfo length.

BUG=v8:4102
LOG=N
R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/1133933003

Cr-Commit-Position: refs/heads/master@{#28491}

src/arm/builtins-arm.cc

@@ -1724,12 +1724,17 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ bind(&too_few);
 
     // If the function is strong we need to throw an error.
-    Label weak_function;
+    Label no_strong_error;
     __ ldr(r4, FieldMemOperand(r1, JSFunction::kSharedFunctionInfoOffset));
-    __ ldr(r4, FieldMemOperand(r4, SharedFunctionInfo::kCompilerHintsOffset));
-    __ tst(r4, Operand(1 << (SharedFunctionInfo::kStrongModeFunction +
+    __ ldr(r5, FieldMemOperand(r4, SharedFunctionInfo::kCompilerHintsOffset));
+    __ tst(r5, Operand(1 << (SharedFunctionInfo::kStrongModeFunction +
                              kSmiTagSize)));
-    __ b(eq, &weak_function);
+    __ b(eq, &no_strong_error);
+
+    // What we really care about is the required number of arguments.
+    __ ldr(r4, FieldMemOperand(r4, SharedFunctionInfo::kLengthOffset));
+    __ cmp(r0, Operand::SmiUntag(r4));
+    __ b(ge, &no_strong_error);
 
     {
       FrameScope frame(masm, StackFrame::MANUAL);
@@ -1737,7 +1742,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
       __ CallRuntime(Runtime::kThrowStrongModeTooFewArguments, 0);
     }
 
-    __ bind(&weak_function);
+    __ bind(&no_strong_error);
     EnterArgumentsAdaptorFrame(masm);
 
     // Calculate copy start address into r0 and copy end address is fp.

src/arm64/builtins-arm64.cc

@@ -1740,14 +1740,21 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     Register scratch1 = x13, scratch2 = x14;
 
     // If the function is strong we need to throw an error.
-    Label weak_function;
+    Label no_strong_error;
     __ Ldr(scratch1,
            FieldMemOperand(function, JSFunction::kSharedFunctionInfoOffset));
     __ Ldr(scratch2.W(),
            FieldMemOperand(scratch1, SharedFunctionInfo::kCompilerHintsOffset));
     __ TestAndBranchIfAllClear(scratch2.W(),
                                (1 << SharedFunctionInfo::kStrongModeFunction),
-                               &weak_function);
+                               &no_strong_error);
+
+    // What we really care about is the required number of arguments.
+    DCHECK_EQ(kPointerSize, kInt64Size);
+    __ Ldr(scratch2.W(),
+           FieldMemOperand(scratch1, SharedFunctionInfo::kLengthOffset));
+    __ Cmp(argc_actual, Operand(scratch2, LSR, 1));
+    __ B(ge, &no_strong_error);
 
     {
       FrameScope frame(masm, StackFrame::MANUAL);
@@ -1755,7 +1762,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
       __ CallRuntime(Runtime::kThrowStrongModeTooFewArguments, 0);
     }
 
-    __ bind(&weak_function);
+    __ Bind(&no_strong_error);
     EnterArgumentsAdaptorFrame(masm);
 
     __ Lsl(argc_expected, argc_expected, kPointerSizeLog2);

src/ia32/builtins-ia32.cc

@@ -1579,11 +1579,17 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ bind(&too_few);
 
     // If the function is strong we need to throw an error.
-    Label weak_function;
+    Label no_strong_error;
     __ mov(ecx, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset));
     __ test_b(FieldOperand(ecx, SharedFunctionInfo::kStrongModeByteOffset),
               1 << SharedFunctionInfo::kStrongModeBitWithinByte);
-    __ j(equal, &weak_function, Label::kNear);
+    __ j(equal, &no_strong_error, Label::kNear);
+
+    // What we really care about is the required number of arguments.
+    __ mov(ecx, FieldOperand(ecx, SharedFunctionInfo::kLengthOffset));
+    __ SmiUntag(ecx);
+    __ cmp(eax, ecx);
+    __ j(greater_equal, &no_strong_error, Label::kNear);
 
     {
       FrameScope frame(masm, StackFrame::MANUAL);
@@ -1591,7 +1597,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
       __ CallRuntime(Runtime::kThrowStrongModeTooFewArguments, 0);
     }
 
-    __ bind(&weak_function);
+    __ bind(&no_strong_error);
     EnterArgumentsAdaptorFrame(masm);
 
     // Copy receiver and all actual arguments.

src/mips/builtins-mips.cc

@@ -1744,12 +1744,17 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ bind(&too_few);
 
     // If the function is strong we need to throw an error.
-    Label weak_function;
+    Label no_strong_error;
     __ lw(t1, FieldMemOperand(a1, JSFunction::kSharedFunctionInfoOffset));
-    __ lw(t1, FieldMemOperand(t1, SharedFunctionInfo::kCompilerHintsOffset));
-    __ And(t2, t1, Operand(1 << (SharedFunctionInfo::kStrongModeFunction +
+    __ lw(t2, FieldMemOperand(t1, SharedFunctionInfo::kCompilerHintsOffset));
+    __ And(t3, t2, Operand(1 << (SharedFunctionInfo::kStrongModeFunction +
                                  kSmiTagSize)));
-    __ Branch(&weak_function, eq, t2, Operand(zero_reg));
+    __ Branch(&no_strong_error, eq, t3, Operand(zero_reg));
+
+    // What we really care about is the required number of arguments.
+    __ lw(t2, FieldMemOperand(t1, SharedFunctionInfo::kLengthOffset));
+    __ SmiUntag(t2);
+    __ Branch(&no_strong_error, ge, a0, Operand(t2));
 
     {
       FrameScope frame(masm, StackFrame::MANUAL);
@@ -1757,7 +1762,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
       __ CallRuntime(Runtime::kThrowStrongModeTooFewArguments, 0);
     }
 
-    __ bind(&weak_function);
+    __ bind(&no_strong_error);
     EnterArgumentsAdaptorFrame(masm);
 
     // Calculate copy start address into a0 and copy end address is fp.

src/mips64/builtins-mips64.cc

@@ -1753,11 +1753,17 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ bind(&too_few);
 
     // If the function is strong we need to throw an error.
-    Label weak_function;
+    Label no_strong_error;
     __ ld(a4, FieldMemOperand(a1, JSFunction::kSharedFunctionInfoOffset));
-    __ lbu(a4, FieldMemOperand(a4, SharedFunctionInfo::kStrongModeByteOffset));
-    __ And(a4, a4, Operand(1 << SharedFunctionInfo::kStrongModeBitWithinByte));
-    __ Branch(&weak_function, eq, a4, Operand(zero_reg));
+    __ lbu(a5, FieldMemOperand(a4, SharedFunctionInfo::kStrongModeByteOffset));
+    __ And(a5, a5, Operand(1 << SharedFunctionInfo::kStrongModeBitWithinByte));
+    __ Branch(&no_strong_error, eq, a5, Operand(zero_reg));
+
+    // What we really care about is the required number of arguments.
+    DCHECK_EQ(kPointerSize, kInt64Size);
+    __ lw(a5, FieldMemOperand(a4, SharedFunctionInfo::kLengthOffset));
+    __ srl(a5, a5, 1);
+    __ Branch(&no_strong_error, ge, a0, Operand(a5));
 
     {
       FrameScope frame(masm, StackFrame::MANUAL);
@@ -1765,7 +1771,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
       __ CallRuntime(Runtime::kThrowStrongModeTooFewArguments, 0);
     }
 
-    __ bind(&weak_function);
+    __ bind(&no_strong_error);
     EnterArgumentsAdaptorFrame(masm);
 
     // Calculate copy start address into a0 and copy end address is fp.

src/x64/builtins-x64.cc

@@ -1644,13 +1644,31 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ bind(&too_few);
 
     // If the function is strong we need to throw an error.
-    Label weak_function;
+    Label no_strong_error;
     __ movp(kScratchRegister,
             FieldOperand(rdi, JSFunction::kSharedFunctionInfoOffset));
     __ testb(FieldOperand(kScratchRegister,
                           SharedFunctionInfo::kStrongModeByteOffset),
              Immediate(1 << SharedFunctionInfo::kStrongModeBitWithinByte));
-    __ j(equal, &weak_function, Label::kNear);
+    __ j(equal, &no_strong_error, Label::kNear);
+
+    // What we really care about is the required number of arguments.
+
+    if (kPointerSize == kInt32Size) {
+      __ movp(
+          kScratchRegister,
+          FieldOperand(kScratchRegister, SharedFunctionInfo::kLengthOffset));
+      __ SmiToInteger32(kScratchRegister, kScratchRegister);
+    } else {
+      // See comment near kLengthOffset in src/objects.h
+      __ movsxlq(
+          kScratchRegister,
+          FieldOperand(kScratchRegister, SharedFunctionInfo::kLengthOffset));
+      __ shrq(kScratchRegister, Immediate(1));
+    }
+
+    __ cmpp(rax, kScratchRegister);
+    __ j(greater_equal, &no_strong_error, Label::kNear);
 
     {
       FrameScope frame(masm, StackFrame::MANUAL);
@@ -1658,7 +1676,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
       __ CallRuntime(Runtime::kThrowStrongModeTooFewArguments, 0);
     }
 
-    __ bind(&weak_function);
+    __ bind(&no_strong_error);
     EnterArgumentsAdaptorFrame(masm);
 
     // Copy receiver and all actual arguments.

test/mjsunit/strong/function-arity.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 // Flags: --strong-mode --harmony-arrow-functions --harmony-reflect
-// Flags: --harmony-spreadcalls --allow-natives-syntax
+// Flags: --harmony-spreadcalls --harmony-rest-parameters --allow-natives-syntax
 
 'use strict';
 
@@ -29,6 +29,16 @@ function generateParams(n) {
   return a.join(', ');
 }
 
+function generateParamsWithRest(n) {
+  let a = [];
+  let i = 0;
+  for (; i < n; i++) {
+    a[i] = `p${i}`;
+  }
+  a.push(`...p${i}`)
+  return a.join(', ');
+}
+
 
 function generateSpread(n) {
   return `...[${generateArguments(n)}]`;
@@ -39,7 +49,9 @@ function generateSpread(n) {
   for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
     let defs = [
       `'use strong'; function f(${generateParams(parameterCount)}) {}`,
+      `'use strong'; function f(${generateParamsWithRest(parameterCount)}) {}`,
       `'use strong'; function* f(${generateParams(parameterCount)}) {}`,
+      `'use strong'; function* f(${generateParamsWithRest(parameterCount)}) {}`,
       `'use strong'; let f = (${generateParams(parameterCount)}) => {}`,
       `function f(${generateParams(parameterCount)}) { 'use strong'; }`,
       `function* f(${generateParams(parameterCount)}) { 'use strong'; }`,
@@ -90,89 +102,93 @@ function generateSpread(n) {
 
 
 (function MethodCall() {
-  for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
-    let defs = [
-      `let o = new class {
-        m(${generateParams(parameterCount)}) { 'use strong'; }
-      }`,
-      `let o = new class {
-        *m(${generateParams(parameterCount)}) { 'use strong'; }
-      }`,
-      `let o = { m(${generateParams(parameterCount)}) { 'use strong'; } }`,
-      `let o = { *m(${generateParams(parameterCount)}) { 'use strong'; } }`,
-      `'use strong';
-      let o = new class { m(${generateParams(parameterCount)}) {} }`,
-      `'use strong';
-      let o = new class { *m(${generateParams(parameterCount)}) {} }`,
-      `'use strong'; let o = { m(${generateParams(parameterCount)}) {} }`,
-      `'use strong'; let o = { *m(${generateParams(parameterCount)}) {} }`,
-    ];
-    for (let def of defs) {
-      for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
+  for (let genParams of [generateParams, generateParamsWithRest]) {
+    for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
+      let defs = [
+        `let o = new class {
+          m(${genParams(parameterCount)}) { 'use strong'; }
+        }`,
+        `let o = new class {
+          *m(${genParams(parameterCount)}) { 'use strong'; }
+        }`,
+        `let o = { m(${genParams(parameterCount)}) { 'use strong'; } }`,
+        `let o = { *m(${genParams(parameterCount)}) { 'use strong'; } }`,
+        `'use strong';
+        let o = new class { m(${genParams(parameterCount)}) {} }`,
+        `'use strong';
+        let o = new class { *m(${genParams(parameterCount)}) {} }`,
+        `'use strong'; let o = { m(${genParams(parameterCount)}) {} }`,
+        `'use strong'; let o = { *m(${genParams(parameterCount)}) {} }`,
+      ];
+      for (let def of defs) {
+        for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
+          let calls = [
+            `o.m(${generateArguments(argumentCount)})`,
+            `o.m(${generateSpread(argumentCount)})`,
+            `o.m.call(${generateArguments(argumentCount, 'o')})`,
+            `o.m.call(o, ${generateSpread(argumentCount)})`,
+            `o.m.apply(o, [${generateArguments(argumentCount)}])`,
+            `o.m.bind(o)(${generateArguments(argumentCount)})`,
+            `%_CallFunction(${generateArguments(argumentCount, 'o')}, o.m)`,
+            `%Call(${generateArguments(argumentCount, 'o')}, o.m)`,
+            `%Apply(o.m, o, [${generateArguments(argumentCount)}], 0,
+                    ${argumentCount})`,
+          ];
+
+          for (let call of calls) {
+            let code = `'use strict'; ${def}; ${call};`;
+            if (argumentCount < parameterCount) {
+              assertThrows(code, TypeError);
+            } else {
+              assertDoesNotThrow(code);
+            }
+          }
+        }
+
         let calls = [
-          `o.m(${generateArguments(argumentCount)})`,
-          `o.m(${generateSpread(argumentCount)})`,
-          `o.m.call(${generateArguments(argumentCount, 'o')})`,
-          `o.m.call(o, ${generateSpread(argumentCount)})`,
-          `o.m.apply(o, [${generateArguments(argumentCount)}])`,
-          `o.m.bind(o)(${generateArguments(argumentCount)})`,
-          `%_CallFunction(${generateArguments(argumentCount, 'o')}, o.m)`,
-          `%Call(${generateArguments(argumentCount, 'o')}, o.m)`,
-          `%Apply(o.m, o, [${generateArguments(argumentCount)}], 0,
-                  ${argumentCount})`,
+          `o.m.call()`,
+          `o.m.apply()`,
+          `o.m.apply(o)`,
         ];
-
         for (let call of calls) {
           let code = `'use strict'; ${def}; ${call};`;
-          if (argumentCount < parameterCount) {
+          if (parameterCount > 0) {
             assertThrows(code, TypeError);
           } else {
             assertDoesNotThrow(code);
           }
         }
       }
-
-      let calls = [
-        `o.m.call()`,
-        `o.m.apply()`,
-        `o.m.apply(o)`,
-      ];
-      for (let call of calls) {
-        let code = `'use strict'; ${def}; ${call};`;
-        if (parameterCount > 0) {
-          assertThrows(code, TypeError);
-        } else {
-          assertDoesNotThrow(code);
-        }
-      }
     }
   }
 })();
 
 
 (function Constructor() {
-  for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
-    for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
-      let defs = [
-        `'use strong';
-        class C { constructor(${generateParams(parameterCount)}) {} }`,
-        `'use strict';
-        class C {
-          constructor(${generateParams(parameterCount)}) { 'use strong'; }
-        }`,
-      ];
-      for (let def of defs) {
-        let calls = [
-          `new C(${generateArguments(argumentCount)})`,
-          `new C(${generateSpread(argumentCount)})`,
-          `Reflect.construct(C, [${generateArguments(argumentCount)}])`,
+  for (let genParams of [generateParams, generateParamsWithRest]) {
+    for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
+      for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
+        let defs = [
+          `'use strong';
+          class C { constructor(${genParams(parameterCount)}) {} }`,
+          `'use strict';
+          class C {
+            constructor(${genParams(parameterCount)}) { 'use strong'; }
+          }`,
         ];
-        for (let call of calls) {
-          let code = `${def}; ${call};`;
-          if (argumentCount < parameterCount) {
-            assertThrows(code, TypeError);
-          } else {
-            assertDoesNotThrow(code);
+        for (let def of defs) {
+          let calls = [
+            `new C(${generateArguments(argumentCount)})`,
+            `new C(${generateSpread(argumentCount)})`,
+            `Reflect.construct(C, [${generateArguments(argumentCount)}])`,
+          ];
+          for (let call of calls) {
+            let code = `${def}; ${call};`;
+            if (argumentCount < parameterCount) {
+              assertThrows(code, TypeError);
+            } else {
+              assertDoesNotThrow(code);
+            }
           }
         }
       }
@@ -182,35 +198,37 @@ function generateSpread(n) {
 
 
 (function DerivedConstructor() {
-  for (let genArgs of [generateArguments, generateSpread]) {
-    for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
-      for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
-        let defs = [
-          `'use strong';
-          class B {
-            constructor(${generateParams(parameterCount)}) {}
-          }
-          class C extends B {
-            constructor() {
-              super(${genArgs(argumentCount)});
+  for (let genParams of [generateParams, generateParamsWithRest]) {
+    for (let genArgs of [generateArguments, generateSpread]) {
+      for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
+        for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
+          let defs = [
+            `'use strong';
+            class B {
+              constructor(${genParams(parameterCount)}) {}
             }
-          }`,
-          `'use strict';
-          class B {
-            constructor(${generateParams(parameterCount)}) { 'use strong'; }
-          }
-          class C extends B {
-            constructor() {
-              super(${genArgs(argumentCount)});
+            class C extends B {
+              constructor() {
+                super(${genArgs(argumentCount)});
+              }
+            }`,
+            `'use strict';
+            class B {
+              constructor(${genParams(parameterCount)}) { 'use strong'; }
+            }
+            class C extends B {
+              constructor() {
+                super(${genArgs(argumentCount)});
+              }
+            }`,
+          ];
+          for (let def of defs) {
+            let code = `${def}; new C();`;
+            if (argumentCount < parameterCount) {
+              assertThrows(code, TypeError);
+            } else {
+              assertDoesNotThrow(code);
             }
-          }`,
-        ];
-        for (let def of defs) {
-          let code = `${def}; new C();`;
-          if (argumentCount < parameterCount) {
-            assertThrows(code, TypeError);
-          } else {
-            assertDoesNotThrow(code);
           }
         }
       }
@@ -220,27 +238,29 @@ function generateSpread(n) {
 
 
 (function DerivedConstructorDefaultConstructorInDerivedClass() {
-  for (let genArgs of [generateArguments, generateSpread]) {
-    for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
-      for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
-        let defs = [
-          `'use strong';
-          class B {
-            constructor(${generateParams(parameterCount)}) {}
-          }
-          class C extends B {}`,
-          `'use strict';
-          class B {
-            constructor(${generateParams(parameterCount)}) { 'use strong'; }
-          }
-          class C extends B {}`,
-        ];
-        for (let def of defs) {
-          let code = `${def}; new C(${genArgs(argumentCount)})`;
-          if (argumentCount < parameterCount) {
-            assertThrows(code, TypeError);
-          } else {
-            assertDoesNotThrow(code);
+  for (let genParams of [generateParams, generateParamsWithRest]) {
+    for (let genArgs of [generateArguments, generateSpread]) {
+      for (let argumentCount = 0; argumentCount < 3; argumentCount++) {
+        for (let parameterCount = 0; parameterCount < 3; parameterCount++) {
+          let defs = [
+            `'use strong';
+            class B {
+              constructor(${genParams(parameterCount)}) {}
+            }
+            class C extends B {}`,
+            `'use strict';
+            class B {
+              constructor(${genParams(parameterCount)}) { 'use strong'; }
+            }
+            class C extends B {}`,
+          ];
+          for (let def of defs) {
+            let code = `${def}; new C(${genArgs(argumentCount)})`;
+            if (argumentCount < parameterCount) {
+              assertThrows(code, TypeError);
+            } else {
+              assertDoesNotThrow(code);
+            }
           }
         }
       }