From: erik.corry@gmail.com Date: Tue, 25 Sep 2012 13:35:42 +0000 (+0000) Subject: x64 and ARM: Fix issue 2346 (order of operations in keyed store X-Git-Tag: upstream/4.7.83~15942 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=72e9f1bea1a103f3dd1696c51a30b0f8acc2f5dc;p=platform%2Fupstream%2Fv8.git x64 and ARM: Fix issue 2346 (order of operations in keyed store on arrays) and turn get-own-property-descriptor.js test into a regression test. Review URL: https://chromiumcodereview.appspot.com/10985017 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12604 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc index 404f3c6..f2438a5 100644 --- a/src/arm/ic-arm.cc +++ b/src/arm/ic-arm.cc @@ -1301,6 +1301,144 @@ void KeyedStoreIC::GenerateRuntimeSetProperty(MacroAssembler* masm, } +static void KeyedStoreGenerateGenericHelper( + MacroAssembler* masm, + Label* fast_object, + Label* fast_double, + Label* slow, + KeyedStoreCheckMap check_map, + KeyedStoreIncrementLength increment_length, + Register value, + Register key, + Register receiver, + Register receiver_map, + Register elements_map, + Register elements) { + Label transition_smi_elements; + Label finish_object_store, non_double_value, transition_double_elements; + Label fast_double_without_map_check; + + // Fast case: Do the store, could be either Object or double. + __ bind(fast_object); + Register scratch_value = r4; + Register address = r5; + if (check_map == kCheckMap) { + __ ldr(elements_map, FieldMemOperand(elements, HeapObject::kMapOffset)); + __ cmp(elements_map, + Operand(masm->isolate()->factory()->fixed_array_map())); + __ b(ne, fast_double); + } + // Smi stores don't require further checks. + Label non_smi_value; + __ JumpIfNotSmi(value, &non_smi_value); + + if (increment_length == kIncrementLength) { + // Add 1 to receiver->length. + __ add(scratch_value, key, Operand(Smi::FromInt(1))); + __ str(scratch_value, FieldMemOperand(receiver, JSArray::kLengthOffset)); + } + // It's irrelevant whether array is smi-only or not when writing a smi. + __ add(address, elements, Operand(FixedArray::kHeaderSize - kHeapObjectTag)); + __ add(address, address, Operand(key, LSL, kPointerSizeLog2 - kSmiTagSize)); + __ str(value, MemOperand(address)); + __ Ret(); + + __ bind(&non_smi_value); + // Escape to elements kind transition case. + __ CheckFastObjectElements(receiver_map, scratch_value, + &transition_smi_elements); + + // Fast elements array, store the value to the elements backing store. + __ bind(&finish_object_store); + if (increment_length == kIncrementLength) { + // Add 1 to receiver->length. + __ add(scratch_value, key, Operand(Smi::FromInt(1))); + __ str(scratch_value, FieldMemOperand(receiver, JSArray::kLengthOffset)); + } + __ add(address, elements, Operand(FixedArray::kHeaderSize - kHeapObjectTag)); + __ add(address, address, Operand(key, LSL, kPointerSizeLog2 - kSmiTagSize)); + __ str(value, MemOperand(address)); + // Update write barrier for the elements array address. + __ mov(scratch_value, value); // Preserve the value which is returned. + __ RecordWrite(elements, + address, + scratch_value, + kLRHasNotBeenSaved, + kDontSaveFPRegs, + EMIT_REMEMBERED_SET, + OMIT_SMI_CHECK); + __ Ret(); + + __ bind(fast_double); + if (check_map == kCheckMap) { + // Check for fast double array case. If this fails, call through to the + // runtime. + __ CompareRoot(elements_map, Heap::kFixedDoubleArrayMapRootIndex); + __ b(ne, slow); + } + __ bind(&fast_double_without_map_check); + __ StoreNumberToDoubleElements(value, + key, + receiver, + elements, + r3, + r4, + r5, + r6, + &transition_double_elements); + if (increment_length == kIncrementLength) { + // Add 1 to receiver->length. + __ add(scratch_value, key, Operand(Smi::FromInt(1))); + __ str(scratch_value, FieldMemOperand(receiver, JSArray::kLengthOffset)); + } + __ Ret(); + + __ bind(&transition_smi_elements); + // Transition the array appropriately depending on the value type. + __ ldr(r4, FieldMemOperand(value, HeapObject::kMapOffset)); + __ CompareRoot(r4, Heap::kHeapNumberMapRootIndex); + __ b(ne, &non_double_value); + + // Value is a double. Transition FAST_SMI_ELEMENTS -> + // FAST_DOUBLE_ELEMENTS and complete the store. + __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, + FAST_DOUBLE_ELEMENTS, + receiver_map, + r4, + slow); + ASSERT(receiver_map.is(r3)); // Transition code expects map in r3 + ElementsTransitionGenerator::GenerateSmiToDouble(masm, slow); + __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset)); + __ jmp(&fast_double_without_map_check); + + __ bind(&non_double_value); + // Value is not a double, FAST_SMI_ELEMENTS -> FAST_ELEMENTS + __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, + FAST_ELEMENTS, + receiver_map, + r4, + slow); + ASSERT(receiver_map.is(r3)); // Transition code expects map in r3 + ElementsTransitionGenerator::GenerateMapChangeElementsTransition(masm); + __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset)); + __ jmp(&finish_object_store); + + __ bind(&transition_double_elements); + // Elements are FAST_DOUBLE_ELEMENTS, but value is an Object that's not a + // HeapNumber. Make sure that the receiver is a Array with FAST_ELEMENTS and + // transition array from FAST_DOUBLE_ELEMENTS to FAST_ELEMENTS + __ LoadTransitionedArrayMapConditional(FAST_DOUBLE_ELEMENTS, + FAST_ELEMENTS, + receiver_map, + r4, + slow); + ASSERT(receiver_map.is(r3)); // Transition code expects map in r3 + ElementsTransitionGenerator::GenerateDoubleToObject(masm, slow); + __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset)); + __ jmp(&finish_object_store); +} + + void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, StrictModeFlag strict_mode) { // ---------- S t a t e -------------- @@ -1309,11 +1447,9 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // -- r2 : receiver // -- lr : return address // ----------------------------------- - Label slow, array, extra, check_if_double_array; - Label fast_object_with_map_check, fast_object_without_map_check; - Label fast_double_with_map_check, fast_double_without_map_check; - Label transition_smi_elements, finish_object_store, non_double_value; - Label transition_double_elements; + Label slow, fast_object, fast_object_grow; + Label fast_double, fast_double_grow; + Label array, extra, check_if_double_array; // Register usage. Register value = r0; @@ -1348,7 +1484,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // Check array bounds. Both the key and the length of FixedArray are smis. __ ldr(ip, FieldMemOperand(elements, FixedArray::kLengthOffset)); __ cmp(key, Operand(ip)); - __ b(lo, &fast_object_with_map_check); + __ b(lo, &fast_object); // Slow case, handle jump to runtime. __ bind(&slow); @@ -1373,21 +1509,13 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, __ cmp(elements_map, Operand(masm->isolate()->factory()->fixed_array_map())); __ b(ne, &check_if_double_array); - // Calculate key + 1 as smi. - STATIC_ASSERT(kSmiTag == 0); - __ add(r4, key, Operand(Smi::FromInt(1))); - __ str(r4, FieldMemOperand(receiver, JSArray::kLengthOffset)); - __ b(&fast_object_without_map_check); + __ jmp(&fast_object_grow); __ bind(&check_if_double_array); __ cmp(elements_map, Operand(masm->isolate()->factory()->fixed_double_array_map())); __ b(ne, &slow); - // Add 1 to key, and go to common element store code for doubles. - STATIC_ASSERT(kSmiTag == 0); - __ add(r4, key, Operand(Smi::FromInt(1))); - __ str(r4, FieldMemOperand(receiver, JSArray::kLengthOffset)); - __ jmp(&fast_double_without_map_check); + __ jmp(&fast_double_grow); // Array case: Get the length and the elements array from the JS // array. Check that the array is in fast mode (and writable); if it @@ -1399,106 +1527,15 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, __ ldr(ip, FieldMemOperand(receiver, JSArray::kLengthOffset)); __ cmp(key, Operand(ip)); __ b(hs, &extra); - // Fall through to fast case. - __ bind(&fast_object_with_map_check); - Register scratch_value = r4; - Register address = r5; - __ ldr(elements_map, FieldMemOperand(elements, HeapObject::kMapOffset)); - __ cmp(elements_map, - Operand(masm->isolate()->factory()->fixed_array_map())); - __ b(ne, &fast_double_with_map_check); - __ bind(&fast_object_without_map_check); - // Smi stores don't require further checks. - Label non_smi_value; - __ JumpIfNotSmi(value, &non_smi_value); - // It's irrelevant whether array is smi-only or not when writing a smi. - __ add(address, elements, Operand(FixedArray::kHeaderSize - kHeapObjectTag)); - __ add(address, address, Operand(key, LSL, kPointerSizeLog2 - kSmiTagSize)); - __ str(value, MemOperand(address)); - __ Ret(); - - __ bind(&non_smi_value); - // Escape to elements kind transition case. - __ CheckFastObjectElements(receiver_map, scratch_value, - &transition_smi_elements); - // Fast elements array, store the value to the elements backing store. - __ bind(&finish_object_store); - __ add(address, elements, Operand(FixedArray::kHeaderSize - kHeapObjectTag)); - __ add(address, address, Operand(key, LSL, kPointerSizeLog2 - kSmiTagSize)); - __ str(value, MemOperand(address)); - // Update write barrier for the elements array address. - __ mov(scratch_value, value); // Preserve the value which is returned. - __ RecordWrite(elements, - address, - scratch_value, - kLRHasNotBeenSaved, - kDontSaveFPRegs, - EMIT_REMEMBERED_SET, - OMIT_SMI_CHECK); - __ Ret(); - - __ bind(&fast_double_with_map_check); - // Check for fast double array case. If this fails, call through to the - // runtime. - __ cmp(elements_map, - Operand(masm->isolate()->factory()->fixed_double_array_map())); - __ b(ne, &slow); - __ bind(&fast_double_without_map_check); - __ StoreNumberToDoubleElements(value, - key, - receiver, - elements, - r3, - r4, - r5, - r6, - &transition_double_elements); - __ Ret(); - - __ bind(&transition_smi_elements); - // Transition the array appropriately depending on the value type. - __ ldr(r4, FieldMemOperand(value, HeapObject::kMapOffset)); - __ CompareRoot(r4, Heap::kHeapNumberMapRootIndex); - __ b(ne, &non_double_value); - - // Value is a double. Transition FAST_SMI_ELEMENTS -> - // FAST_DOUBLE_ELEMENTS and complete the store. - __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, - FAST_DOUBLE_ELEMENTS, - receiver_map, - r4, - &slow); - ASSERT(receiver_map.is(r3)); // Transition code expects map in r3 - ElementsTransitionGenerator::GenerateSmiToDouble(masm, &slow); - __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset)); - __ jmp(&fast_double_without_map_check); - - __ bind(&non_double_value); - // Value is not a double, FAST_SMI_ELEMENTS -> FAST_ELEMENTS - __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, - FAST_ELEMENTS, - receiver_map, - r4, - &slow); - ASSERT(receiver_map.is(r3)); // Transition code expects map in r3 - ElementsTransitionGenerator::GenerateMapChangeElementsTransition(masm); - __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset)); - __ jmp(&finish_object_store); - - __ bind(&transition_double_elements); - // Elements are FAST_DOUBLE_ELEMENTS, but value is an Object that's not a - // HeapNumber. Make sure that the receiver is a Array with FAST_ELEMENTS and - // transition array from FAST_DOUBLE_ELEMENTS to FAST_ELEMENTS - __ LoadTransitionedArrayMapConditional(FAST_DOUBLE_ELEMENTS, - FAST_ELEMENTS, - receiver_map, - r4, - &slow); - ASSERT(receiver_map.is(r3)); // Transition code expects map in r3 - ElementsTransitionGenerator::GenerateDoubleToObject(masm, &slow); - __ ldr(elements, FieldMemOperand(receiver, JSObject::kElementsOffset)); - __ jmp(&finish_object_store); + KeyedStoreGenerateGenericHelper(masm, &fast_object, &fast_double, + &slow, kCheckMap, kDontIncrementLength, + value, key, receiver, receiver_map, + elements_map, elements); + KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow, + &slow, kDontCheckMap, kIncrementLength, + value, key, receiver, receiver_map, + elements_map, elements); } diff --git a/src/builtins.cc b/src/builtins.cc index ffaaf8b..df70cd4 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -1620,7 +1620,7 @@ void Builtins::SetUp(bool create_heap_objects) { // For now we generate builtin adaptor code into a stack-allocated // buffer, before copying it into individual code objects. Be careful // with alignment, some platforms don't like unaligned code. - union { int force_alignment; byte buffer[4*KB]; } u; + union { int force_alignment; byte buffer[8*KB]; } u; // Traverse the list of builtins and generate an adaptor in a // separate code object for each one. diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index 6945bc0..dae3bbd 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -747,12 +747,13 @@ void KeyedStoreIC::GenerateNonStrictArguments(MacroAssembler* masm) { } -static void KeyedStoreGenerateGenericHelper(MacroAssembler* masm, - Label* fast_object, - Label* fast_double, - Label* slow, - bool check_map, - bool increment_length) { +static void KeyedStoreGenerateGenericHelper( + MacroAssembler* masm, + Label* fast_object, + Label* fast_double, + Label* slow, + KeyedStoreCheckMap check_map, + KeyedStoreIncrementLength increment_length) { Label transition_smi_elements; Label finish_object_store, non_double_value, transition_double_elements; Label fast_double_without_map_check; @@ -763,7 +764,7 @@ static void KeyedStoreGenerateGenericHelper(MacroAssembler* masm, // edi: receiver map // Fast case: Do the store, could either Object or double. __ bind(fast_object); - if (check_map) { + if (check_map == kCheckMap) { __ mov(edi, FieldOperand(ebx, HeapObject::kMapOffset)); __ cmp(edi, masm->isolate()->factory()->fixed_array_map()); __ j(not_equal, fast_double); @@ -771,7 +772,7 @@ static void KeyedStoreGenerateGenericHelper(MacroAssembler* masm, // Smi stores don't require further checks. Label non_smi_value; __ JumpIfNotSmi(eax, &non_smi_value); - if (increment_length) { + if (increment_length == kIncrementLength) { // Add 1 to receiver->length. __ add(FieldOperand(edx, JSArray::kLengthOffset), Immediate(Smi::FromInt(1))); @@ -787,7 +788,7 @@ static void KeyedStoreGenerateGenericHelper(MacroAssembler* masm, // Fast elements array, store the value to the elements backing store. __ bind(&finish_object_store); - if (increment_length) { + if (increment_length == kIncrementLength) { // Add 1 to receiver->length. __ add(FieldOperand(edx, JSArray::kLengthOffset), Immediate(Smi::FromInt(1))); @@ -800,7 +801,7 @@ static void KeyedStoreGenerateGenericHelper(MacroAssembler* masm, __ ret(0); __ bind(fast_double); - if (check_map) { + if (check_map == kCheckMap) { // Check for fast double array case. If this fails, call through to the // runtime. __ cmp(edi, masm->isolate()->factory()->fixed_double_array_map()); @@ -811,7 +812,7 @@ static void KeyedStoreGenerateGenericHelper(MacroAssembler* masm, __ bind(&fast_double_without_map_check); __ StoreNumberToDoubleElements(eax, ebx, ecx, edi, xmm0, &transition_double_elements, false); - if (increment_length) { + if (increment_length == kIncrementLength) { // Add 1 to receiver->length. __ add(FieldOperand(edx, JSArray::kLengthOffset), Immediate(Smi::FromInt(1))); @@ -947,10 +948,10 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, __ cmp(ecx, FieldOperand(edx, JSArray::kLengthOffset)); // Compare smis. __ j(above_equal, &extra); - KeyedStoreGenerateGenericHelper(masm, &fast_object, &fast_double, &slow, - true, false); + KeyedStoreGenerateGenericHelper(masm, &fast_object, &fast_double, + &slow, kCheckMap, kDontIncrementLength); KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow, - &slow, false, true); + &slow, kDontCheckMap, kIncrementLength); } diff --git a/src/ic.h b/src/ic.h index c86f316..76867a9 100644 --- a/src/ic.h +++ b/src/ic.h @@ -631,6 +631,18 @@ class StoreIC: public IC { }; +enum KeyedStoreCheckMap { + kDontCheckMap, + kCheckMap +}; + + +enum KeyedStoreIncrementLength { + kDontIncrementLength, + kIncrementLength +}; + + class KeyedStoreIC: public KeyedIC { public: explicit KeyedStoreIC(Isolate* isolate) : KeyedIC(isolate) { diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc index 0fd8a40..efa07a8 100644 --- a/src/x64/ic-x64.cc +++ b/src/x64/ic-x64.cc @@ -623,6 +623,123 @@ void KeyedLoadIC::GenerateIndexedInterceptor(MacroAssembler* masm) { } +static void KeyedStoreGenerateGenericHelper( + MacroAssembler* masm, + Label* fast_object, + Label* fast_double, + Label* slow, + KeyedStoreCheckMap check_map, + KeyedStoreIncrementLength increment_length) { + Label transition_smi_elements; + Label finish_object_store, non_double_value, transition_double_elements; + Label fast_double_without_map_check; + // Fast case: Do the store, could be either Object or double. + __ bind(fast_object); + // rax: value + // rbx: receiver's elements array (a FixedArray) + // rcx: index + // rdx: receiver (a JSArray) + // r9: map of receiver + if (check_map == kCheckMap) { + __ movq(rdi, FieldOperand(rbx, HeapObject::kMapOffset)); + __ CompareRoot(rdi, Heap::kFixedArrayMapRootIndex); + __ j(not_equal, fast_double); + } + // Smi stores don't require further checks. + Label non_smi_value; + __ JumpIfNotSmi(rax, &non_smi_value); + if (increment_length == kIncrementLength) { + // Add 1 to receiver->length. + __ leal(rdi, Operand(rcx, 1)); + __ Integer32ToSmiField(FieldOperand(rdx, JSArray::kLengthOffset), rdi); + } + // It's irrelevant whether array is smi-only or not when writing a smi. + __ movq(FieldOperand(rbx, rcx, times_pointer_size, FixedArray::kHeaderSize), + rax); + __ ret(0); + + __ bind(&non_smi_value); + // Writing a non-smi, check whether array allows non-smi elements. + // r9: receiver's map + __ CheckFastObjectElements(r9, &transition_smi_elements); + + __ bind(&finish_object_store); + if (increment_length == kIncrementLength) { + // Add 1 to receiver->length. + __ leal(rdi, Operand(rcx, 1)); + __ Integer32ToSmiField(FieldOperand(rdx, JSArray::kLengthOffset), rdi); + } + __ movq(FieldOperand(rbx, rcx, times_pointer_size, FixedArray::kHeaderSize), + rax); + __ movq(rdx, rax); // Preserve the value which is returned. + __ RecordWriteArray( + rbx, rdx, rcx, kDontSaveFPRegs, EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); + __ ret(0); + + __ bind(fast_double); + if (check_map == kCheckMap) { + // Check for fast double array case. If this fails, call through to the + // runtime. + // rdi: elements array's map + __ CompareRoot(rdi, Heap::kFixedDoubleArrayMapRootIndex); + __ j(not_equal, slow); + } + __ bind(&fast_double_without_map_check); + __ StoreNumberToDoubleElements(rax, rbx, rcx, xmm0, + &transition_double_elements); + if (increment_length == kIncrementLength) { + // Add 1 to receiver->length. + __ leal(rdi, Operand(rcx, 1)); + __ Integer32ToSmiField(FieldOperand(rdx, JSArray::kLengthOffset), rdi); + } + __ ret(0); + + __ bind(&transition_smi_elements); + __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset)); + + // Transition the array appropriately depending on the value type. + __ movq(r9, FieldOperand(rax, HeapObject::kMapOffset)); + __ CompareRoot(r9, Heap::kHeapNumberMapRootIndex); + __ j(not_equal, &non_double_value); + + // Value is a double. Transition FAST_SMI_ELEMENTS -> + // FAST_DOUBLE_ELEMENTS and complete the store. + __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, + FAST_DOUBLE_ELEMENTS, + rbx, + rdi, + slow); + ElementsTransitionGenerator::GenerateSmiToDouble(masm, slow); + __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset)); + __ jmp(&fast_double_without_map_check); + + __ bind(&non_double_value); + // Value is not a double, FAST_SMI_ELEMENTS -> FAST_ELEMENTS + __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, + FAST_ELEMENTS, + rbx, + rdi, + slow); + ElementsTransitionGenerator::GenerateMapChangeElementsTransition(masm); + __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset)); + __ jmp(&finish_object_store); + + __ bind(&transition_double_elements); + // Elements are FAST_DOUBLE_ELEMENTS, but value is an Object that's not a + // HeapNumber. Make sure that the receiver is a Array with FAST_ELEMENTS and + // transition array from FAST_DOUBLE_ELEMENTS to FAST_ELEMENTS + __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset)); + __ LoadTransitionedArrayMapConditional(FAST_DOUBLE_ELEMENTS, + FAST_ELEMENTS, + rbx, + rdi, + slow); + ElementsTransitionGenerator::GenerateDoubleToObject(masm, slow); + __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset)); + __ jmp(&finish_object_store); +} + + void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, StrictModeFlag strict_mode) { // ----------- S t a t e ------------- @@ -631,11 +748,9 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // -- rdx : receiver // -- rsp[0] : return address // ----------------------------------- - Label slow, slow_with_tagged_index, fast, array, extra, check_extra_double; - Label fast_object_with_map_check, fast_object_without_map_check; - Label fast_double_with_map_check, fast_double_without_map_check; - Label transition_smi_elements, finish_object_store, non_double_value; - Label transition_double_elements; + Label slow, slow_with_tagged_index, fast_object, fast_object_grow; + Label fast_double, fast_double_grow; + Label array, extra, check_if_double_array; // Check that the object isn't a smi. __ JumpIfSmi(rdx, &slow_with_tagged_index); @@ -666,7 +781,7 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // rax: value // rbx: FixedArray // rcx: index - __ j(above, &fast_object_with_map_check); + __ j(above, &fast_object); // Slow case: call runtime. __ bind(&slow); @@ -690,18 +805,14 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, // Increment index to get new length. __ movq(rdi, FieldOperand(rbx, HeapObject::kMapOffset)); __ CompareRoot(rdi, Heap::kFixedArrayMapRootIndex); - __ j(not_equal, &check_extra_double); - __ leal(rdi, Operand(rcx, 1)); - __ Integer32ToSmiField(FieldOperand(rdx, JSArray::kLengthOffset), rdi); - __ jmp(&fast_object_without_map_check); + __ j(not_equal, &check_if_double_array); + __ jmp(&fast_object_grow); - __ bind(&check_extra_double); + __ bind(&check_if_double_array); // rdi: elements array's map __ CompareRoot(rdi, Heap::kFixedDoubleArrayMapRootIndex); __ j(not_equal, &slow); - __ leal(rdi, Operand(rcx, 1)); - __ Integer32ToSmiField(FieldOperand(rdx, JSArray::kLengthOffset), rdi); - __ jmp(&fast_double_without_map_check); + __ jmp(&fast_double_grow); // Array case: Get the length and the elements array from the JS // array. Check that the array is in fast mode (and writable); if it @@ -717,92 +828,10 @@ void KeyedStoreIC::GenerateGeneric(MacroAssembler* masm, __ SmiCompareInteger32(FieldOperand(rdx, JSArray::kLengthOffset), rcx); __ j(below_equal, &extra); - // Fast case: Do the store. - __ bind(&fast_object_with_map_check); - // rax: value - // rbx: receiver's elements array (a FixedArray) - // rcx: index - // rdx: receiver (a JSArray) - __ movq(rdi, FieldOperand(rbx, HeapObject::kMapOffset)); - __ CompareRoot(rdi, Heap::kFixedArrayMapRootIndex); - __ j(not_equal, &fast_double_with_map_check); - __ bind(&fast_object_without_map_check); - // Smi stores don't require further checks. - Label non_smi_value; - __ JumpIfNotSmi(rax, &non_smi_value); - // It's irrelevant whether array is smi-only or not when writing a smi. - __ movq(FieldOperand(rbx, rcx, times_pointer_size, FixedArray::kHeaderSize), - rax); - __ ret(0); - - __ bind(&non_smi_value); - // Writing a non-smi, check whether array allows non-smi elements. - // r9: receiver's map - __ CheckFastObjectElements(r9, &transition_smi_elements); - __ bind(&finish_object_store); - __ movq(FieldOperand(rbx, rcx, times_pointer_size, FixedArray::kHeaderSize), - rax); - __ movq(rdx, rax); // Preserve the value which is returned. - __ RecordWriteArray( - rbx, rdx, rcx, kDontSaveFPRegs, EMIT_REMEMBERED_SET, OMIT_SMI_CHECK); - __ ret(0); - - __ bind(&fast_double_with_map_check); - // Check for fast double array case. If this fails, call through to the - // runtime. - // rdi: elements array's map - __ CompareRoot(rdi, Heap::kFixedDoubleArrayMapRootIndex); - __ j(not_equal, &slow); - __ bind(&fast_double_without_map_check); - // If the value is a number, store it as a double in the FastDoubleElements - // array. - __ StoreNumberToDoubleElements(rax, rbx, rcx, xmm0, - &transition_double_elements); - __ ret(0); - - __ bind(&transition_smi_elements); - __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset)); - - // Transition the array appropriately depending on the value type. - __ movq(r9, FieldOperand(rax, HeapObject::kMapOffset)); - __ CompareRoot(r9, Heap::kHeapNumberMapRootIndex); - __ j(not_equal, &non_double_value); - - // Value is a double. Transition FAST_SMI_ELEMENTS -> - // FAST_DOUBLE_ELEMENTS and complete the store. - __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, - FAST_DOUBLE_ELEMENTS, - rbx, - rdi, - &slow); - ElementsTransitionGenerator::GenerateSmiToDouble(masm, &slow); - __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset)); - __ jmp(&fast_double_without_map_check); - - __ bind(&non_double_value); - // Value is not a double, FAST_SMI_ELEMENTS -> FAST_ELEMENTS - __ LoadTransitionedArrayMapConditional(FAST_SMI_ELEMENTS, - FAST_ELEMENTS, - rbx, - rdi, - &slow); - ElementsTransitionGenerator::GenerateMapChangeElementsTransition(masm); - __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset)); - __ jmp(&finish_object_store); - - __ bind(&transition_double_elements); - // Elements are FAST_DOUBLE_ELEMENTS, but value is an Object that's not a - // HeapNumber. Make sure that the receiver is a Array with FAST_ELEMENTS and - // transition array from FAST_DOUBLE_ELEMENTS to FAST_ELEMENTS - __ movq(rbx, FieldOperand(rdx, HeapObject::kMapOffset)); - __ LoadTransitionedArrayMapConditional(FAST_DOUBLE_ELEMENTS, - FAST_ELEMENTS, - rbx, - rdi, - &slow); - ElementsTransitionGenerator::GenerateDoubleToObject(masm, &slow); - __ movq(rbx, FieldOperand(rdx, JSObject::kElementsOffset)); - __ jmp(&finish_object_store); + KeyedStoreGenerateGenericHelper(masm, &fast_object, &fast_double, + &slow, kCheckMap, kDontIncrementLength); + KeyedStoreGenerateGenericHelper(masm, &fast_object_grow, &fast_double_grow, + &slow, kDontCheckMap, kIncrementLength); } diff --git a/test/mjsunit/regress/regress-2346.js b/test/mjsunit/regress/regress-2346.js new file mode 100644 index 0000000..4c88b3e --- /dev/null +++ b/test/mjsunit/regress/regress-2346.js @@ -0,0 +1,123 @@ +// Copyright 2010 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. + +// This file only tests very simple descriptors that always have +// configurable, enumerable, and writable set to true. +// A range of more elaborate tests are performed in +// object-define-property.js + +// Flags: --stress-runs=5 + +function get() { return x; } +function set(x) { this.x = x; } + +var obj = {x: 1}; +obj.__defineGetter__("accessor", get); +obj.__defineSetter__("accessor", set); +var a = new Array(); +a[1] = 42; +obj[1] = 42; + +var descIsData = Object.getOwnPropertyDescriptor(obj, 'x'); +assertTrue(descIsData.enumerable); +assertTrue(descIsData.writable); +assertTrue(descIsData.configurable); + +var descIsAccessor = Object.getOwnPropertyDescriptor(obj, 'accessor'); +assertTrue(descIsAccessor.enumerable); +assertTrue(descIsAccessor.configurable); +assertTrue(descIsAccessor.get == get); +assertTrue(descIsAccessor.set == set); + +var descIsNotData = Object.getOwnPropertyDescriptor(obj, 'not-x'); +assertTrue(descIsNotData == undefined); + +var descIsNotAccessor = Object.getOwnPropertyDescriptor(obj, 'not-accessor'); +assertTrue(descIsNotAccessor == undefined); + +var descArray = Object.getOwnPropertyDescriptor(a, '1'); +assertTrue(descArray.enumerable); +assertTrue(descArray.configurable); +assertTrue(descArray.writable); +assertEquals(descArray.value, 42); + +var descObjectElement = Object.getOwnPropertyDescriptor(obj, '1'); +assertTrue(descObjectElement.enumerable); +assertTrue(descObjectElement.configurable); +assertTrue(descObjectElement.writable); +assertEquals(descObjectElement.value, 42); + +// String objects. +var a = new String('foobar'); +for (var i = 0; i < a.length; i++) { + var descStringObject = Object.getOwnPropertyDescriptor(a, i); + assertTrue(descStringObject.enumerable); + assertFalse(descStringObject.configurable); + assertFalse(descStringObject.writable); + assertEquals(descStringObject.value, a.substring(i, i+1)); +} + +// Support for additional attributes on string objects. +a.x = 42; +a[10] = 'foo'; +var descStringProperty = Object.getOwnPropertyDescriptor(a, 'x'); +assertTrue(descStringProperty.enumerable); +assertTrue(descStringProperty.configurable); +assertTrue(descStringProperty.writable); +assertEquals(descStringProperty.value, 42); + +var descStringElement = Object.getOwnPropertyDescriptor(a, '10'); +assertTrue(descStringElement.enumerable); +assertTrue(descStringElement.configurable); +assertTrue(descStringElement.writable); +assertEquals(descStringElement.value, 'foo'); + +// Test that elements in the prototype chain is not returned. +var proto = {}; +proto[10] = 42; + +var objWithProto = new Array(); +objWithProto.prototype = proto; +objWithProto[0] = 'bar'; +var descWithProto = Object.getOwnPropertyDescriptor(objWithProto, '10'); +assertEquals(undefined, descWithProto); + +// Test elements on global proxy object. +var global = (function() { return this; })(); + +global[42] = 42; + +function el_getter() { return 239; }; +function el_setter() {}; +Object.defineProperty(global, '239', {get: el_getter, set: el_setter}); + +var descRegularElement = Object.getOwnPropertyDescriptor(global, '42'); +assertEquals(42, descRegularElement.value); + +var descAccessorElement = Object.getOwnPropertyDescriptor(global, '239'); +assertEquals(el_getter, descAccessorElement.get); +assertEquals(el_setter, descAccessorElement.set);