Revision: 13056
Author: [email protected]
Date: Mon Nov 26 06:29:21 2012
Log: Ensure double arrays are filled with holes when extended from
variations of empty arrays.
BUG=162085
Review URL: https://chromiumcodereview.appspot.com/11414155
http://code.google.com/p/v8/source/detail?r=13056
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-crbug-162085.js
Modified:
/branches/bleeding_edge/src/arm/stub-cache-arm.cc
/branches/bleeding_edge/src/elements.cc
/branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/x64/stub-cache-x64.cc
/branches/bleeding_edge/test/mjsunit/array-natives-elements.js
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-crbug-162085.js
Mon Nov 26 06:29:21 2012
@@ -0,0 +1,55 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Ensure extending an empty packed smi array with a double initializes the
+// array with holes.
+var a = [1,2,3];
+a.length = 0;
+a[0] = 1.4;
+assertEquals(undefined, a[1]);
+assertEquals(undefined, a[2]);
+assertEquals(undefined, a[3]);
+
+// Ensure the double array growstub initializes the array with holes.
+function grow_store(a,i,v) {
+ a[i] = v;
+}
+
+var a2 = [1.3];
+grow_store(a2,1,1.4);
+a2.length = 0;
+grow_store(a2,0,1.5);
+assertEquals(undefined, a2[1]);
+assertEquals(undefined, a2[2]);
+assertEquals(undefined, a2[3]);
+
+// Check storing objects using the double grow stub.
+var a3 = [1.3];
+var o = {};
+grow_store(a3, 1, o);
+assertEquals(1.3, a3[0]);
+assertEquals(o, a3[1]);
=======================================
--- /branches/bleeding_edge/src/arm/stub-cache-arm.cc Fri Nov 16 00:54:01
2012
+++ /branches/bleeding_edge/src/arm/stub-cache-arm.cc Mon Nov 26 06:29:21
2012
@@ -4783,8 +4783,7 @@
__ AllocateInNewSpace(size, elements_reg, scratch1, scratch2, &slow,
TAG_OBJECT);
- // Initialize the new FixedDoubleArray. Leave elements unitialized for
- // efficiency, they are guaranteed to be initialized before use.
+ // Initialize the new FixedDoubleArray.
__ LoadRoot(scratch1, Heap::kFixedDoubleArrayMapRootIndex);
__ str(scratch1, FieldMemOperand(elements_reg, JSObject::kMapOffset));
__ mov(scratch1,
@@ -4792,6 +4791,24 @@
__ str(scratch1,
FieldMemOperand(elements_reg, FixedDoubleArray::kLengthOffset));
+ __ mov(scratch1, Operand(kHoleNanLower32));
+ __ mov(scratch2, Operand(kHoleNanUpper32));
+ for (int i = 1; i < JSArray::kPreallocatedArrayElements; i++) {
+ int offset = FixedDoubleArray::OffsetOfElementAt(i);
+ __ str(scratch1, FieldMemOperand(elements_reg, offset));
+ __ str(scratch2, FieldMemOperand(elements_reg, offset +
kPointerSize));
+ }
+
+ __ StoreNumberToDoubleElements(value_reg,
+ key_reg,
+ // All registers after this are
overwritten.
+ elements_reg,
+ scratch1,
+ scratch2,
+ scratch3,
+ scratch4,
+ &transition_elements_kind);
+
// Install the new backing store in the JSArray.
__ str(elements_reg,
FieldMemOperand(receiver_reg, JSObject::kElementsOffset));
@@ -4804,7 +4821,7 @@
__ str(length_reg, FieldMemOperand(receiver_reg,
JSArray::kLengthOffset));
__ ldr(elements_reg,
FieldMemOperand(receiver_reg, JSObject::kElementsOffset));
- __ jmp(&finish_store);
+ __ Ret();
__ bind(&check_capacity);
// Make sure that the backing store can hold additional elements.
=======================================
--- /branches/bleeding_edge/src/elements.cc Mon Nov 19 07:00:34 2012
+++ /branches/bleeding_edge/src/elements.cc Mon Nov 26 06:29:21 2012
@@ -375,6 +375,9 @@
copy_size = from->length() - from_start;
if (raw_copy_size == ElementsAccessor::kCopyToEndAndInitializeToHole) {
to_end = to->length();
+ for (uint32_t i = to_start + copy_size; i < to_end; ++i) {
+ to->set_the_hole(i);
+ }
} else {
to_end = to_start + static_cast<uint32_t>(copy_size);
}
@@ -392,10 +395,6 @@
ASSERT(!smi->IsTheHole());
to->set(to_start, Smi::cast(smi)->value());
}
-
- while (to_start < to_end) {
- to->set_the_hole(to_start++);
- }
}
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Thu Nov 15 04:19:14
2012
+++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Mon Nov 26 06:29:21
2012
@@ -4353,13 +4353,22 @@
// ecx: key
// edx: receiver
// edi: elements
- // Initialize the new FixedDoubleArray. Leave elements unitialized for
- // efficiency, they are guaranteed to be initialized before use.
+ // Initialize the new FixedDoubleArray.
__ mov(FieldOperand(edi, JSObject::kMapOffset),
Immediate(masm->isolate()->factory()->fixed_double_array_map()));
__ mov(FieldOperand(edi, FixedDoubleArray::kLengthOffset),
Immediate(Smi::FromInt(JSArray::kPreallocatedArrayElements)));
+ for (int i = 1; i < JSArray::kPreallocatedArrayElements; i++) {
+ int offset = FixedDoubleArray::OffsetOfElementAt(i);
+ __ mov(FieldOperand(edi, offset), Immediate(kHoleNanLower32));
+ __ mov(FieldOperand(edi, offset + kPointerSize),
+ Immediate(kHoleNanUpper32));
+ }
+
+ __ StoreNumberToDoubleElements(eax, edi, ecx, ebx, xmm0,
+ &transition_elements_kind, true);
+
// Install the new backing store in the JSArray.
__ mov(FieldOperand(edx, JSObject::kElementsOffset), edi);
__ RecordWriteField(edx, JSObject::kElementsOffset, edi, ebx,
@@ -4369,7 +4378,7 @@
__ add(FieldOperand(edx, JSArray::kLengthOffset),
Immediate(Smi::FromInt(1)));
__ mov(edi, FieldOperand(edx, JSObject::kElementsOffset));
- __ jmp(&finish_store);
+ __ ret(0);
__ bind(&check_capacity);
// eax: value
=======================================
--- /branches/bleeding_edge/src/parser.cc Fri Nov 23 05:23:39 2012
+++ /branches/bleeding_edge/src/parser.cc Mon Nov 26 06:29:21 2012
@@ -3718,7 +3718,6 @@
isolate()->factory()->NewFixedArray(values->length(), TENURED);
Handle<FixedDoubleArray> double_literals;
ElementsKind elements_kind = FAST_SMI_ELEMENTS;
- bool has_only_undefined_values = true;
bool has_hole_values = false;
// Fill in the literals.
@@ -3750,7 +3749,6 @@
// FAST_DOUBLE_ELEMENTS and FAST_ELEMENTS as necessary. Always
remember
// the tagged value, no matter what the ElementsKind is in case we
// ultimately end up in FAST_ELEMENTS.
- has_only_undefined_values = false;
object_literals->set(i, *boilerplate_value);
if (elements_kind == FAST_SMI_ELEMENTS) {
// Smi only elements. Notice if a transition to
FAST_DOUBLE_ELEMENTS or
@@ -3788,13 +3786,6 @@
}
}
}
-
- // Very small array literals that don't have a concrete hint about their
type
- // from a constant value should default to the slow case to avoid lots of
- // elements transitions on really small objects.
- if (has_only_undefined_values && values->length() <= 2) {
- elements_kind = FAST_ELEMENTS;
- }
// Simple and shallow arrays can be lazily copied, we transform the
// elements array to a copy-on-write array.
=======================================
--- /branches/bleeding_edge/src/x64/stub-cache-x64.cc Thu Nov 15 08:16:09
2012
+++ /branches/bleeding_edge/src/x64/stub-cache-x64.cc Mon Nov 26 06:29:21
2012
@@ -4026,7 +4026,7 @@
// -- rsp[0] : return address
// -----------------------------------
Label miss_force_generic, transition_elements_kind, finish_store;
- Label grow, slow, check_capacity;
+ Label grow, slow, check_capacity, restore_key_transition_elements_kind;
// This stub is meant to be tail-jumped to, the receiver must already
// have been verified by the caller to not be a smi.
@@ -4055,7 +4055,7 @@
__ bind(&finish_store);
__ SmiToInteger32(rcx, rcx);
__ StoreNumberToDoubleElements(rax, rdi, rcx, xmm0,
- &transition_elements_kind);
+ &restore_key_transition_elements_kind);
__ ret(0);
// Handle store cache miss, replacing the ic with the generic stub.
@@ -4064,9 +4064,10 @@
masm->isolate()->builtins()->KeyedStoreIC_MissForceGeneric();
__ jmp(ic_force_generic, RelocInfo::CODE_TARGET);
- __ bind(&transition_elements_kind);
+ __ bind(&restore_key_transition_elements_kind);
// Restore smi-tagging of rcx.
__ Integer32ToSmi(rcx, rcx);
+ __ bind(&transition_elements_kind);
Handle<Code> ic_miss = masm->isolate()->builtins()->KeyedStoreIC_Miss();
__ jmp(ic_miss, RelocInfo::CODE_TARGET);
@@ -4107,6 +4108,16 @@
__ Move(FieldOperand(rdi, FixedDoubleArray::kLengthOffset),
Smi::FromInt(JSArray::kPreallocatedArrayElements));
+ __ movq(r8, BitCast<int64_t, uint64_t>(kHoleNanInt64),
RelocInfo::NONE);
+ for (int i = 1; i < JSArray::kPreallocatedArrayElements; i++) {
+ __ movq(FieldOperand(rdi, FixedDoubleArray::OffsetOfElementAt(i)),
r8);
+ }
+
+ // Increment the length of the array.
+ __ SmiToInteger32(rcx, rcx);
+ __ StoreNumberToDoubleElements(rax, rdi, rcx, xmm0,
+ &restore_key_transition_elements_kind);
+
// Install the new backing store in the JSArray.
__ movq(FieldOperand(rdx, JSObject::kElementsOffset), rdi);
__ RecordWriteField(rdx, JSObject::kElementsOffset, rdi, rbx,
@@ -4115,7 +4126,7 @@
// Increment the length of the array.
__ Move(FieldOperand(rdx, JSArray::kLengthOffset), Smi::FromInt(1));
__ movq(rdi, FieldOperand(rdx, JSObject::kElementsOffset));
- __ jmp(&finish_store);
+ __ ret(0);
__ bind(&check_capacity);
// rax: value
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-natives-elements.js Fri Nov
23 05:23:39 2012
+++ /branches/bleeding_edge/test/mjsunit/array-natives-elements.js Mon Nov
26 06:29:21 2012
@@ -49,7 +49,7 @@
function array_natives_test() {
// Ensure small array literals start in specific element kind mode.
- assertTrue(%HasFastObjectElements([]));
+ assertTrue(%HasFastSmiElements([]));
assertTrue(%HasFastSmiElements([1]));
assertTrue(%HasFastSmiElements([1,2]));
assertTrue(%HasFastDoubleElements([1.1]));
@@ -73,7 +73,7 @@
// Concat
var a1;
a1 = [1,2,3].concat([]);
- assertTrue(%HasFastObjectElements(a1));
+ assertTrue(%HasFastSmiElements(a1));
assertEquals([1,2,3], a1);
a1 = [1,2,3].concat([4,5,6]);
assertTrue(%HasFastSmiElements(a1));
@@ -82,7 +82,7 @@
assertTrue(%HasFastSmiElements(a1));
assertEquals([1,2,3,4,5,6,7,8,9], a1);
a1 = [1.1,2,3].concat([]);
- assertTrue(%HasFastObjectElements(a1));
+ assertTrue(%HasFastDoubleElements(a1));
assertEquals([1.1,2,3], a1);
a1 = [1,2,3].concat([1.1, 2]);
assertTrue(%HasFastDoubleElements(a1));
@@ -173,7 +173,7 @@
a3r = a3.splice(0, 0, 2);
// Commented out since handled in js, which takes the best fit.
// assertTrue(%HasFastDoubleElements(a3r));
- assertTrue(%HasFastObjectElements(a3r));
+ assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([], a3r);
assertEquals([2, 1.1, 2, 3], a3);
@@ -187,7 +187,7 @@
a3r = a3.splice(0, 0, 2.1);
// Commented out since handled in js, which takes the best fit.
// assertTrue(%HasFastDoubleElements(a3r));
- assertTrue(%HasFastObjectElements(a3r));
+ assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([], a3r);
assertEquals([2.1, 1.1, 2, 3], a3);
@@ -201,7 +201,7 @@
a3r = a3.splice(0, 0, 2.1);
// Commented out since handled in js, which takes the best fit.
// assertTrue(%HasFastDoubleElements(a3r));
- assertTrue(%HasFastObjectElements(a3r));
+ assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([], a3r);
assertEquals([2.1, 1, 2, 3], a3);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
