Fix overzealous assert in CallOrConstructVarArgs

For spread calls with arrays with double elements but zero length,
we skip the box-as-heapnumber step; so in this corner case the
Call builtin sees a FixedDoubleArray, which is fine because it
doesn't read any of the raw double values from it.
This patch doesn't change the implementation, it only updates the
assert to match reality.

Bug: chromium:856095
Change-Id: I0227f4ccbc6c61c8f5f7669a266ef7a64c6a9a43
Reviewed-on: https://chromium-review.googlesource.com/1117922
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54149}
This commit is contained in:
Jakob Kummerow 2018-06-28 10:37:52 -07:00 committed by Commit Bot
parent 55930aadc7
commit 34225a6afb
19 changed files with 115 additions and 95 deletions

View File

@ -2008,18 +2008,6 @@ void MacroAssembler::AssertSmi(Register object) {
}
}
void MacroAssembler::AssertFixedArray(Register object) {
if (emit_debug_code()) {
STATIC_ASSERT(kSmiTag == 0);
tst(object, Operand(kSmiTagMask));
Check(ne, AbortReason::kOperandIsASmiAndNotAFixedArray);
push(object);
CompareObjectType(object, object, object, FIXED_ARRAY_TYPE);
pop(object);
Check(eq, AbortReason::kOperandIsNotAFixedArray);
}
}
void MacroAssembler::AssertConstructor(Register object) {
if (emit_debug_code()) {
STATIC_ASSERT(kSmiTag == 0);

View File

@ -849,9 +849,6 @@ class MacroAssembler : public TurboAssembler {
void AssertNotSmi(Register object);
void AssertSmi(Register object);
// Abort execution if argument is not a FixedArray, enabled via --debug-code.
void AssertFixedArray(Register object);
// Abort execution if argument is not a Constructor, enabled via --debug-code.
void AssertConstructor(Register object);

View File

@ -1626,18 +1626,6 @@ void MacroAssembler::AssertNotSmi(Register object, AbortReason reason) {
}
}
void MacroAssembler::AssertFixedArray(Register object) {
if (emit_debug_code()) {
AssertNotSmi(object, AbortReason::kOperandIsASmiAndNotAFixedArray);
UseScratchRegisterScope temps(this);
Register temp = temps.AcquireX();
CompareObjectType(object, temp, temp, FIXED_ARRAY_TYPE);
Check(eq, AbortReason::kOperandIsNotAFixedArray);
}
}
void MacroAssembler::AssertConstructor(Register object) {
if (emit_debug_code()) {
AssertNotSmi(object, AbortReason::kOperandIsASmiAndNotAConstructor);

View File

@ -1732,9 +1732,6 @@ class MacroAssembler : public TurboAssembler {
inline void ObjectTag(Register tagged_obj, Register obj);
inline void ObjectUntag(Register untagged_obj, Register obj);
// Abort execution if argument is not a FixedArray, enabled via --debug-code.
void AssertFixedArray(Register object);
// Abort execution if argument is not a Constructor, enabled via --debug-code.
void AssertConstructor(Register object);

View File

@ -1577,10 +1577,27 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
// -- r4 : len (number of elements to push from args)
// -- r3 : new.target (for [[Construct]])
// -----------------------------------
__ AssertFixedArray(r2);
Register scratch = r8;
if (masm->emit_debug_code()) {
// Allow r2 to be a FixedArray, or a FixedDoubleArray if r4 == 0.
Label ok, fail;
__ AssertNotSmi(r2);
__ ldr(scratch, FieldMemOperand(r2, HeapObject::kMapOffset));
__ ldrh(r6, FieldMemOperand(scratch, Map::kInstanceTypeOffset));
__ cmp(r6, Operand(FIXED_ARRAY_TYPE));
__ b(eq, &ok);
__ cmp(r6, Operand(FIXED_DOUBLE_ARRAY_TYPE));
__ b(ne, &fail);
__ cmp(r4, Operand(0));
__ b(eq, &ok);
// Fall through.
__ bind(&fail);
__ Abort(AbortReason::kOperandIsNotAFixedArray);
__ bind(&ok);
}
// Check for stack overflow.
{
// Check the stack for overflow. We are not trying to catch interruptions

View File

@ -1908,7 +1908,24 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
// -- x4 : len (number of elements to push from args)
// -- x3 : new.target (for [[Construct]])
// -----------------------------------
__ AssertFixedArray(x2);
if (masm->emit_debug_code()) {
// Allow x2 to be a FixedArray, or a FixedDoubleArray if x4 == 0.
Label ok, fail;
__ AssertNotSmi(x2, AbortReason::kOperandIsNotAFixedArray);
__ Ldr(x10, FieldMemOperand(x2, HeapObject::kMapOffset));
__ Ldrh(x13, FieldMemOperand(x10, Map::kInstanceTypeOffset));
__ Cmp(x13, FIXED_ARRAY_TYPE);
__ B(eq, &ok);
__ Cmp(x13, FIXED_DOUBLE_ARRAY_TYPE);
__ B(ne, &fail);
__ Cmp(x4, 0);
__ B(eq, &ok);
// Fall through.
__ bind(&fail);
__ Abort(AbortReason::kOperandIsNotAFixedArray);
__ bind(&ok);
}
Register arguments_list = x2;
Register argc = x0;

View File

@ -1657,13 +1657,30 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
// -- edx : new.target (checked to be constructor or undefined)
// -- esp[0] : return address.
// -----------------------------------
__ AssertFixedArray(ebx);
// We need to preserve eax, edi and ebx.
__ movd(xmm0, edx);
__ movd(xmm1, edi);
__ movd(xmm2, eax);
if (masm->emit_debug_code()) {
// Allow ebx to be a FixedArray, or a FixedDoubleArray if ecx == 0.
Label ok, fail;
__ AssertNotSmi(ebx);
__ mov(edx, FieldOperand(ebx, HeapObject::kMapOffset));
__ CmpInstanceType(edx, FIXED_ARRAY_TYPE);
__ j(equal, &ok);
__ CmpInstanceType(edx, FIXED_DOUBLE_ARRAY_TYPE);
__ j(not_equal, &fail);
__ cmp(ecx, 0);
__ j(equal, &ok);
// Fall through.
__ bind(&fail);
__ Abort(AbortReason::kOperandIsNotAFixedArray);
__ bind(&ok);
}
// Check for stack overflow.
{
// Check the stack for overflow. We are not trying to catch interruptions

View File

@ -1585,7 +1585,20 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
// -- t0 : len (number of elements to push from args)
// -- a3 : new.target (for [[Construct]])
// -----------------------------------
__ AssertFixedArray(a2);
if (masm->emit_debug_code()) {
// Allow a2 to be a FixedArray, or a FixedDoubleArray if t0 == 0.
Label ok, fail;
__ AssertNotSmi(a2);
__ GetObjectType(a2, t8, t8);
__ Branch(&ok, eq, t8, Operand(FIXED_ARRAY_TYPE));
__ Branch(&fail, ne, t8, Operand(FIXED_DOUBLE_ARRAY_TYPE));
__ Branch(&ok, eq, t0, Operand(0));
// Fall through.
__ bind(&fail);
__ Abort(AbortReason::kOperandIsNotAFixedArray);
__ bind(&ok);
}
// Check for stack overflow.
{

View File

@ -1599,7 +1599,20 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
// -- a4 : len (number of elements to push from args)
// -- a3 : new.target (for [[Construct]])
// -----------------------------------
__ AssertFixedArray(a2);
if (masm->emit_debug_code()) {
// Allow a2 to be a FixedArray, or a FixedDoubleArray if a4 == 0.
Label ok, fail;
__ AssertNotSmi(a2);
__ GetObjectType(a2, t8, t8);
__ Branch(&ok, eq, t8, Operand(FIXED_ARRAY_TYPE));
__ Branch(&fail, ne, t8, Operand(FIXED_DOUBLE_ARRAY_TYPE));
__ Branch(&ok, eq, a4, Operand(zero_reg));
// Fall through.
__ bind(&fail);
__ Abort(AbortReason::kOperandIsNotAFixedArray);
__ bind(&ok);
}
Register args = a2;
Register len = a4;

View File

@ -1756,7 +1756,24 @@ void Builtins::Generate_CallOrConstructVarargs(MacroAssembler* masm,
// -- rdx : new.target (for [[Construct]])
// -- rsp[0] : return address
// -----------------------------------
__ AssertFixedArray(rbx);
if (masm->emit_debug_code()) {
// Allow rbx to be a FixedArray, or a FixedDoubleArray if rcx == 0.
Label ok, fail;
__ AssertNotSmi(rbx);
Register map = r9;
__ movp(map, FieldOperand(rbx, HeapObject::kMapOffset));
__ CmpInstanceType(map, FIXED_ARRAY_TYPE);
__ j(equal, &ok);
__ CmpInstanceType(map, FIXED_DOUBLE_ARRAY_TYPE);
__ j(not_equal, &fail);
__ cmpl(rcx, Immediate(0));
__ j(equal, &ok);
// Fall through.
__ bind(&fail);
__ Abort(AbortReason::kOperandIsNotAFixedArray);
__ bind(&ok);
}
// Check for stack overflow.
{

View File

@ -501,17 +501,6 @@ void MacroAssembler::AssertSmi(Register object) {
}
}
void MacroAssembler::AssertFixedArray(Register object) {
if (emit_debug_code()) {
test(object, Immediate(kSmiTagMask));
Check(not_equal, AbortReason::kOperandIsASmiAndNotAFixedArray);
Push(object);
CmpObjectType(object, FIXED_ARRAY_TYPE, object);
Pop(object);
Check(equal, AbortReason::kOperandIsNotAFixedArray);
}
}
void MacroAssembler::AssertConstructor(Register object) {
if (emit_debug_code()) {
test(object, Immediate(kSmiTagMask));

View File

@ -608,9 +608,6 @@ class MacroAssembler : public TurboAssembler {
// Abort execution if argument is a smi, enabled via --debug-code.
void AssertNotSmi(Register object);
// Abort execution if argument is not a FixedArray, enabled via --debug-code.
void AssertFixedArray(Register object);
// Abort execution if argument is not a JSFunction, enabled via --debug-code.
void AssertFunction(Register object);

View File

@ -5036,18 +5036,6 @@ void MacroAssembler::AssertSmi(Register object) {
}
}
void MacroAssembler::AssertFixedArray(Register object) {
if (emit_debug_code()) {
STATIC_ASSERT(kSmiTag == 0);
SmiTst(object, t8);
Check(ne, AbortReason::kOperandIsASmiAndNotAFixedArray, t8,
Operand(zero_reg));
GetObjectType(object, t8, t8);
Check(eq, AbortReason::kOperandIsNotAFixedArray, t8,
Operand(FIXED_ARRAY_TYPE));
}
}
void MacroAssembler::AssertConstructor(Register object) {
if (emit_debug_code()) {
STATIC_ASSERT(kSmiTag == 0);

View File

@ -1146,9 +1146,6 @@ const Operand& rt = Operand(zero_reg), BranchDelaySlot bd = PROTECT
void AssertNotSmi(Register object);
void AssertSmi(Register object);
// Abort execution if argument is not a FixedArray, enabled via --debug-code.
void AssertFixedArray(Register object);
// Abort execution if argument is not a Constructor, enabled via --debug-code.
void AssertConstructor(Register object);

View File

@ -5361,18 +5361,6 @@ void MacroAssembler::AssertSmi(Register object) {
}
}
void MacroAssembler::AssertFixedArray(Register object) {
if (emit_debug_code()) {
STATIC_ASSERT(kSmiTag == 0);
SmiTst(object, t8);
Check(ne, AbortReason::kOperandIsASmiAndNotAFixedArray, t8,
Operand(zero_reg));
GetObjectType(object, t8, t8);
Check(eq, AbortReason::kOperandIsNotAFixedArray, t8,
Operand(FIXED_ARRAY_TYPE));
}
}
void MacroAssembler::AssertConstructor(Register object) {
if (emit_debug_code()) {
STATIC_ASSERT(kSmiTag == 0);

View File

@ -1216,9 +1216,6 @@ const Operand& rt = Operand(zero_reg), BranchDelaySlot bd = PROTECT
void AssertNotSmi(Register object);
void AssertSmi(Register object);
// Abort execution if argument is not a FixedArray, enabled via --debug-code.
void AssertFixedArray(Register object);
// Abort execution if argument is not a Constructor, enabled via --debug-code.
void AssertConstructor(Register object);

View File

@ -1978,17 +1978,6 @@ void MacroAssembler::AssertSmi(Operand object) {
}
}
void MacroAssembler::AssertFixedArray(Register object) {
if (emit_debug_code()) {
testb(object, Immediate(kSmiTagMask));
Check(not_equal, AbortReason::kOperandIsASmiAndNotAFixedArray);
Push(object);
CmpObjectType(object, FIXED_ARRAY_TYPE, object);
Pop(object);
Check(equal, AbortReason::kOperandIsNotAFixedArray);
}
}
void TurboAssembler::AssertZeroExtended(Register int32_register) {
if (emit_debug_code()) {
DCHECK_NE(int32_register, kScratchRegister);

View File

@ -804,9 +804,6 @@ class MacroAssembler : public TurboAssembler {
void AssertSmi(Register object);
void AssertSmi(Operand object);
// Abort execution if argument is not a FixedArray, enabled via --debug-code.
void AssertFixedArray(Register object);
// Abort execution if argument is not a Constructor, enabled via --debug-code.
void AssertConstructor(Register object);

View File

@ -0,0 +1,14 @@
// Copyright 2018 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.
function f(a, b, c) { }
function a() {
let o1;
o1 = new Array();
f(...o1);
o1[1000] = Infinity;
}
a();
a();