diff --git a/src/objects.cc b/src/objects.cc index 6242198ec3..4e0416dd12 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3068,7 +3068,9 @@ MaybeObject* JSObject::DeleteDictionaryElement(uint32_t index, Isolate* isolate = GetIsolate(); Heap* heap = isolate->heap(); FixedArray* backing_store = FixedArray::cast(elements()); - if (backing_store->map() == heap->non_strict_arguments_elements_map()) { + bool is_arguments = + (GetElementsKind() == JSObject::NON_STRICT_ARGUMENTS_ELEMENTS); + if (is_arguments) { backing_store = FixedArray::cast(backing_store->get(1)); } NumberDictionary* dictionary = NumberDictionary::cast(backing_store); @@ -3081,7 +3083,11 @@ MaybeObject* JSObject::DeleteDictionaryElement(uint32_t index, if (!maybe_elements->To(&new_elements)) { return maybe_elements; } - set_elements(new_elements); + if (is_arguments) { + FixedArray::cast(elements())->set(1, new_elements); + } else { + set_elements(new_elements); + } } if (mode == STRICT_DELETION && result == heap->false_value()) { // In strict mode, attempting to delete a non-configurable property @@ -9428,7 +9434,7 @@ void JSObject::GetLocalPropertyNames(FixedArray* storage, int index) { } ASSERT(storage->length() >= index); } else { - property_dictionary()->CopyKeysTo(storage); + property_dictionary()->CopyKeysTo(storage, StringDictionary::UNSORTED); } } @@ -9505,33 +9511,49 @@ int JSObject::GetLocalElementKeys(FixedArray* storage, break; case DICTIONARY_ELEMENTS: { if (storage != NULL) { - element_dictionary()->CopyKeysTo(storage, filter); + element_dictionary()->CopyKeysTo(storage, + filter, + NumberDictionary::SORTED); } counter += element_dictionary()->NumberOfElementsFilterAttributes(filter); break; } case NON_STRICT_ARGUMENTS_ELEMENTS: { FixedArray* parameter_map = FixedArray::cast(elements()); - int length = parameter_map->length(); - for (int i = 2; i < length; ++i) { - if (!parameter_map->get(i)->IsTheHole()) { - if (storage != NULL) storage->set(i - 2, Smi::FromInt(i - 2)); - ++counter; - } - } + int mapped_length = parameter_map->length() - 2; FixedArray* arguments = FixedArray::cast(parameter_map->get(1)); if (arguments->IsDictionary()) { + // Copy the keys from arguments first, because Dictionary::CopyKeysTo + // will insert in storage starting at index 0. NumberDictionary* dictionary = NumberDictionary::cast(arguments); - if (storage != NULL) dictionary->CopyKeysTo(storage, filter); + if (storage != NULL) { + dictionary->CopyKeysTo(storage, filter, NumberDictionary::UNSORTED); + } counter += dictionary->NumberOfElementsFilterAttributes(filter); - } else { - int length = arguments->length(); - for (int i = 0; i < length; ++i) { - if (!arguments->get(i)->IsTheHole()) { - if (storage != NULL) storage->set(i, Smi::FromInt(i)); + for (int i = 0; i < mapped_length; ++i) { + if (!parameter_map->get(i + 2)->IsTheHole()) { + if (storage != NULL) storage->set(counter, Smi::FromInt(i)); ++counter; } } + if (storage != NULL) storage->SortPairs(storage, counter); + + } else { + int backing_length = arguments->length(); + int i = 0; + for (; i < mapped_length; ++i) { + if (!parameter_map->get(i + 2)->IsTheHole()) { + if (storage != NULL) storage->set(counter, Smi::FromInt(i)); + ++counter; + } else if (i < backing_length && !arguments->get(i)->IsTheHole()) { + if (storage != NULL) storage->set(counter, Smi::FromInt(i)); + ++counter; + } + } + for (; i < backing_length; ++i) { + if (storage != NULL) storage->set(counter, Smi::FromInt(i)); + ++counter; + } } break; } @@ -10132,7 +10154,9 @@ template Object* Dictionary::SlowReverseLookup( Object*); template void Dictionary::CopyKeysTo( - FixedArray*, PropertyAttributes); + FixedArray*, + PropertyAttributes, + Dictionary::SortMode); template Object* Dictionary::DeleteProperty( int, JSObject::DeleteMode); @@ -10147,7 +10171,8 @@ template MaybeObject* Dictionary::Shrink( uint32_t); template void Dictionary::CopyKeysTo( - FixedArray*); + FixedArray*, + Dictionary::SortMode); template int Dictionary::NumberOfElementsFilterAttributes( @@ -11199,8 +11224,10 @@ int Dictionary::NumberOfEnumElements() { template -void Dictionary::CopyKeysTo(FixedArray* storage, - PropertyAttributes filter) { +void Dictionary::CopyKeysTo( + FixedArray* storage, + PropertyAttributes filter, + Dictionary::SortMode sort_mode) { ASSERT(storage->length() >= NumberOfEnumElements()); int capacity = HashTable::Capacity(); int index = 0; @@ -11213,7 +11240,9 @@ void Dictionary::CopyKeysTo(FixedArray* storage, if ((attr & filter) == 0) storage->set(index++, k); } } - storage->SortPairs(storage, index); + if (sort_mode == Dictionary::SORTED) { + storage->SortPairs(storage, index); + } ASSERT(storage->length() >= index); } @@ -11239,7 +11268,9 @@ void StringDictionary::CopyEnumKeysTo(FixedArray* storage, template -void Dictionary::CopyKeysTo(FixedArray* storage) { +void Dictionary::CopyKeysTo( + FixedArray* storage, + Dictionary::SortMode sort_mode) { ASSERT(storage->length() >= NumberOfElementsFilterAttributes( static_cast(NONE))); int capacity = HashTable::Capacity(); @@ -11252,6 +11283,9 @@ void Dictionary::CopyKeysTo(FixedArray* storage) { storage->set(index++, k); } } + if (sort_mode == Dictionary::SORTED) { + storage->SortPairs(storage, index); + } ASSERT(storage->length() >= index); } diff --git a/src/objects.h b/src/objects.h index c34efdd4c7..9765fe2a0a 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2770,10 +2770,13 @@ class Dictionary: public HashTable { // Returns the number of enumerable elements in the dictionary. int NumberOfEnumElements(); + enum SortMode { UNSORTED, SORTED }; // Copies keys to preallocated fixed array. - void CopyKeysTo(FixedArray* storage, PropertyAttributes filter); + void CopyKeysTo(FixedArray* storage, + PropertyAttributes filter, + SortMode sort_mode); // Fill in details for properties into storage. - void CopyKeysTo(FixedArray* storage); + void CopyKeysTo(FixedArray* storage, SortMode sort_mode); // Accessors for next enumeration index. void SetNextEnumerationIndex(int index) { diff --git a/test/mjsunit/regress/regress-1531.js b/test/mjsunit/regress/regress-1531.js new file mode 100644 index 0000000000..09e61a6040 --- /dev/null +++ b/test/mjsunit/regress/regress-1531.js @@ -0,0 +1,49 @@ +// Copyright 2011 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. + +// Regression test for computing elements keys of arguments object. Should +// not crash or assert. +function test(x) { + arguments[10] = 0; + var arr = []; + for (var p in arguments) arr.push(p); + return arr; +} +assertEquals(["0", "10"], test(0)); + +// Regression test for lookup after delete of a dictionary-mode arguments +// backing store. Should not crash or assert. +function test1(x, y, z) { + // Put into dictionary mode. + arguments.__defineGetter__("5", function () { return 0; }); + // Delete a property from the dictionary. + delete arguments[5]; + // Look up a property in the dictionary. + return arguments[2]; +} + +assertEquals(void 0, test1(0));