Skip to content

Commit

Permalink
Handle the case when derived constructor is [[Call]]ed with 0 args.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
dslomov authored and Commit bot committed May 5, 2015
1 parent c37f439 commit cf53fed
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/arm/code-stubs-arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 4 additions & 0 deletions src/arm64/code-stubs-arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions src/ia32/code-stubs-ia32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions src/mips/code-stubs-mips.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions src/mips64/code-stubs-mips64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions src/x64/code-stubs-x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
24 changes: 24 additions & 0 deletions test/mjsunit/es6/regress/regress-474783.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit cf53fed

Please sign in to comment.