From ebeaad6cb5603774de05934eb8993627ed7b3429 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Mon, 26 Nov 2012 14:29:21 +0000 Subject: [PATCH] Ensure double arrays are filled with holes when extended from variations of empty arrays. BUG=162085 Review URL: https://chromiumcodereview.appspot.com/11414155 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13056 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/stub-cache-arm.cc | 23 ++++++++++-- src/elements.cc | 7 ++-- src/ia32/stub-cache-ia32.cc | 15 ++++++-- src/parser.cc | 9 ----- src/x64/stub-cache-x64.cc | 19 ++++++++-- test/mjsunit/array-natives-elements.js | 12 +++--- test/mjsunit/regress/regress-crbug-162085.js | 55 ++++++++++++++++++++++++++++ 7 files changed, 111 insertions(+), 29 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-162085.js diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 60a2129..866ab55 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -4783,8 +4783,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( __ 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 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( __ 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 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( __ 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. diff --git a/src/elements.cc b/src/elements.cc index eb021e5..6103da4 100644 --- a/src/elements.cc +++ b/src/elements.cc @@ -375,6 +375,9 @@ static void CopyPackedSmiToDoubleElements(FixedArray* from, 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(copy_size); } @@ -392,10 +395,6 @@ static void CopyPackedSmiToDoubleElements(FixedArray* from, ASSERT(!smi->IsTheHole()); to->set(to_start, Smi::cast(smi)->value()); } - - while (to_start < to_end) { - to->set_the_hole(to_start++); - } } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index 28a3994..c3a2654 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -4353,13 +4353,22 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( // 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 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( __ 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 diff --git a/src/parser.cc b/src/parser.cc index 75fd7b7..a7bb4e7 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3718,7 +3718,6 @@ Expression* Parser::ParseArrayLiteral(bool* ok) { isolate()->factory()->NewFixedArray(values->length(), TENURED); Handle 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 @@ Expression* Parser::ParseArrayLiteral(bool* ok) { // 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 @@ -3789,13 +3787,6 @@ Expression* Parser::ParseArrayLiteral(bool* ok) { } } - // 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. if (is_simple && depth == 1 && values->length() > 0 && diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index 26a97ab..cd0124b 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -4026,7 +4026,7 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( // -- 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 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( __ 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 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( 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 ic_miss = masm->isolate()->builtins()->KeyedStoreIC_Miss(); __ jmp(ic_miss, RelocInfo::CODE_TARGET); @@ -4107,6 +4108,16 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( __ Move(FieldOperand(rdi, FixedDoubleArray::kLengthOffset), Smi::FromInt(JSArray::kPreallocatedArrayElements)); + __ movq(r8, BitCast(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 @@ void KeyedStoreStubCompiler::GenerateStoreFastDoubleElement( // 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 diff --git a/test/mjsunit/array-natives-elements.js b/test/mjsunit/array-natives-elements.js index dde33a5..96a8cb5 100644 --- a/test/mjsunit/array-natives-elements.js +++ b/test/mjsunit/array-natives-elements.js @@ -49,7 +49,7 @@ function get(foo) { return foo; } // Used to generate dynamic values. 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 @@ function array_natives_test() { // 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 @@ function array_natives_test() { 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 @@ function array_natives_test() { 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 @@ function array_natives_test() { 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 @@ function array_natives_test() { 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); diff --git a/test/mjsunit/regress/regress-crbug-162085.js b/test/mjsunit/regress/regress-crbug-162085.js new file mode 100644 index 0000000..f26c711 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-162085.js @@ -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]); -- 2.7.4