Handle the case when derived constructor is [[Call]]ed with 0 args.
authordslomov <dslomov@chromium.org>
Tue, 5 May 2015 19:56:56 +0000 (12:56 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 5 May 2015 19:57:04 +0000 (19:57 +0000)
ArgumentsAdaptorStub for derived constructor (the one that needs
new.target) works in this way:
 - If the constructor is invoked via the Construct stub, we know that
   actual arguments always include new.target. ``arguments`` object
   however should not include a new.target, therefore we remove it.
   We achieve this by decrementing the argument count.
 - If the constructor is invoked as a call, we do not care for a correct
   ``arguments`` array since the constructor will immediately throw on
   entrance.
The bug is that the call could actually pass 0 actual arguments, but I
decrement unconditionally :(. The fix is to detect this case and avoid
decrementing. ``arguments`` is bogus, but it is ok as constructor
throws.

Long-term we should just remove mucking about with arguments for
new.target and just get it from the stack.

R=arv@chromium.org,rossberg@chromium.org
BUG=chromium:474783
LOG=Y

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

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

src/arm/code-stubs-arm.cc
src/arm64/code-stubs-arm64.cc
src/ia32/code-stubs-ia32.cc
src/mips/code-stubs-mips.cc
src/mips64/code-stubs-mips64.cc
src/x64/code-stubs-x64.cc
test/mjsunit/es6/regress/regress-474783.js [new file with mode: 0644]

index 9d72959..6ec8145 100644 (file)
@@ -1848,8 +1848,12 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
   __ bind(&adaptor_frame);
   __ ldr(r1, MemOperand(r2, ArgumentsAdaptorFrameConstants::kLengthOffset));
   if (has_new_target()) {
+    __ cmp(r1, Operand(Smi::FromInt(0)));
+    Label skip_decrement;
+    __ b(eq, &skip_decrement);
     // Subtract 1 from smi-tagged arguments count.
     __ sub(r1, r1, Operand(2));
+    __ bind(&skip_decrement);
   }
   __ str(r1, MemOperand(sp, 0));
   __ add(r3, r2, Operand::PointerOffsetFromSmiKey(r1));
index df0ecfe..b9dd63f 100644 (file)
@@ -2084,9 +2084,13 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
                     ArgumentsAdaptorFrameConstants::kLengthOffset));
   __ SmiUntag(param_count, param_count_smi);
   if (has_new_target()) {
+    __ Cmp(param_count, Operand(0));
+    Label skip_decrement;
+    __ B(eq, &skip_decrement);
     // Skip new.target: it is not a part of arguments.
     __ Sub(param_count, param_count, Operand(1));
     __ SmiTag(param_count_smi, param_count);
+    __ Bind(&skip_decrement);
   }
   __ Add(x10, caller_fp, Operand(param_count, LSL, kPointerSizeLog2));
   __ Add(params, x10, StandardFrameConstants::kCallerSPOffset);
index 2f77d07..15e7110 100644 (file)
@@ -1079,8 +1079,15 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
   __ mov(ecx, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));
 
   if (has_new_target()) {
+    // If the constructor was [[Call]]ed, the call will not push a new.target
+    // onto the stack. In that case the arguments array we construct is bogus,
+    // bu we do not care as the constructor throws immediately.
+    __ cmp(ecx, Immediate(Smi::FromInt(0)));
+    Label skip_decrement;
+    __ j(equal, &skip_decrement);
     // Subtract 1 from smi-tagged arguments count.
     __ sub(ecx, Immediate(2));
+    __ bind(&skip_decrement);
   }
 
   __ lea(edx, Operand(edx, ecx, times_2,
index 582520d..0f7700a 100644 (file)
@@ -1961,8 +1961,11 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
   __ bind(&adaptor_frame);
   __ lw(a1, MemOperand(a2, ArgumentsAdaptorFrameConstants::kLengthOffset));
   if (has_new_target()) {
+    Label skip_decrement;
+    __ Branch(&skip_decrement, eq, a1, Operand(Smi::FromInt(0)));
     // Subtract 1 from smi-tagged arguments count.
     __ Subu(a1, a1, Operand(2));
+    __ bind(&skip_decrement);
   }
   __ sw(a1, MemOperand(sp, 0));
   __ sll(at, a1, kPointerSizeLog2 - kSmiTagSize);
index 584ab90..4d2400f 100644 (file)
@@ -1960,10 +1960,13 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
   __ bind(&adaptor_frame);
   __ ld(a1, MemOperand(a2, ArgumentsAdaptorFrameConstants::kLengthOffset));
   if (has_new_target()) {
+    Label skip_decrement;
+    __ Branch(&skip_decrement, eq, a1, Operand(Smi::FromInt(0)));
     // Subtract 1 from smi-tagged arguments count.
     __ SmiUntag(a1);
     __ Daddu(a1, a1, Operand(-1));
     __ SmiTag(a1);
+    __ bind(&skip_decrement);
   }
   __ sd(a1, MemOperand(sp, 0));
   __ SmiScale(at, a1, kPointerSizeLog2);
index dd7346e..11cceb7 100644 (file)
@@ -962,10 +962,17 @@ void ArgumentsAccessStub::GenerateNewStrict(MacroAssembler* masm) {
   __ movp(rcx, Operand(rdx, ArgumentsAdaptorFrameConstants::kLengthOffset));
 
   if (has_new_target()) {
+    // If the constructor was [[Call]]ed, the call will not push a new.target
+    // onto the stack. In that case the arguments array we construct is bogus,
+    // bu we do not care as the constructor throws immediately.
+    __ Cmp(rcx, Smi::FromInt(0));
+    Label skip_decrement;
+    __ j(equal, &skip_decrement);
     // Subtract 1 from smi-tagged arguments count.
     __ SmiToInteger32(rcx, rcx);
     __ decl(rcx);
     __ Integer32ToSmi(rcx, rcx);
+    __ bind(&skip_decrement);
   }
   __ movp(args.GetArgumentOperand(2), rcx);
   __ SmiToInteger64(rcx, rcx);
diff --git a/test/mjsunit/es6/regress/regress-474783.js b/test/mjsunit/es6/regress/regress-474783.js
new file mode 100644 (file)
index 0000000..e258dcb
--- /dev/null
@@ -0,0 +1,24 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+
+"use strict";
+class Base {
+}
+class Subclass extends Base {
+  constructor(a,b,c) {
+    arguments[1];
+  }
+}
+assertThrows(function() { Subclass(); }, TypeError);
+assertThrows(function() { Subclass(1); }, TypeError);
+assertThrows(function() { Subclass(1, 2); }, TypeError);
+assertThrows(function() { Subclass(1, 2, 3); }, TypeError);
+assertThrows(function() { Subclass(1, 2, 3, 4); }, TypeError);
+
+assertThrows(function() { Subclass.call(); }, TypeError);
+assertThrows(function() { Subclass.call({}); }, TypeError);
+assertThrows(function() { Subclass.call({}, 1); }, TypeError);
+assertThrows(function() { Subclass.call({}, 1, 2); }, TypeError);
+assertThrows(function() { Subclass.call({}, 1, 2, 3, 4); }, TypeError);