Reviewers: Jakob,
Message:
PTAL
Description:
Ensure large Smi-only arrays don't transition to FAST_DOUBLE_ARRAY
BUG=v8:1849
TEST=test/mjsunit/regress/regress-1849.js
Please review this at http://codereview.chromium.org/8968028/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/objects-inl.h
M src/objects.h
M src/objects.cc
A test/mjsunit/regress/regress-1849.js
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
7c7f0940067ad7500c80d1ed416f38c818e7f192..1abe9a5c760f4ea52c7d1551e28ea2f9820231b9
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1219,7 +1219,7 @@ void JSObject::ValidateSmiOnlyElements() {
map != heap->free_space_map()) {
for (int i = 0; i < fixed_array->length(); i++) {
Object* current = fixed_array->get(i);
- ASSERT(current->IsSmi() || current == heap->the_hole_value());
+ ASSERT(current->IsSmi() || current-IsTheHole());
}
}
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
ae5aa78cae57fc078a87ed41d290a3c9b10a1597..2323484ee565a2bd0b2dbef150c82a1625e71710
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -8188,10 +8188,13 @@ MaybeObject*
JSObject::SetFastElementsCapacityAndLength(
Map* new_map = NULL;
if (elements()->map() != heap->non_strict_arguments_elements_map()) {
Object* object;
+ // The resized array has FAST_SMI_ONLY_ELEMENTS if the capacity mode
forces
+ // it, or if it's allowed and the old elements array contained only
SMIs.
bool has_fast_smi_only_elements =
- (set_capacity_mode == kAllowSmiOnlyElements) &&
- (elements()->map()->has_fast_smi_only_elements() ||
- elements() == heap->empty_fixed_array());
+ (set_capacity_mode == kForceSmiOnlyElements) ||
+ ((set_capacity_mode == kAllowSmiOnlyElements) &&
+ (elements()->map()->has_fast_smi_only_elements() ||
+ elements() == heap->empty_fixed_array()));
ElementsKind elements_kind = has_fast_smi_only_elements
? FAST_SMI_ONLY_ELEMENTS
: FAST_ELEMENTS;
@@ -9241,11 +9244,20 @@ MaybeObject*
JSObject::SetDictionaryElement(uint32_t index,
} else {
new_length = dictionary->max_number_key() + 1;
}
- MaybeObject* result = CanConvertToFastDoubleElements()
+ SetFastElementsCapacityMode set_capacity_mode = FLAG_smi_only_arrays
+ ? kAllowSmiOnlyElements
+ : kDontAllowSmiOnlyElements;
+ bool has_smi_only_elements = false;
+ bool should_convert_to_fast_double_elements =
+ ShouldConvertToFastDoubleElements(&has_smi_only_elements);
+ if (has_smi_only_elements) {
+ set_capacity_mode = kForceSmiOnlyElements;
+ }
+ MaybeObject* result = should_convert_to_fast_double_elements
? SetFastDoubleElementsCapacityAndLength(new_length, new_length)
: SetFastElementsCapacityAndLength(new_length,
new_length,
- kDontAllowSmiOnlyElements);
+ set_capacity_mode);
if (result->IsFailure()) return result;
#ifdef DEBUG
if (FLAG_trace_normalization) {
@@ -9724,17 +9736,25 @@ bool JSObject::ShouldConvertToFastElements() {
}
-bool JSObject::CanConvertToFastDoubleElements() {
+bool JSObject::ShouldConvertToFastDoubleElements(
+ bool* has_smi_only_elements) {
+ *has_smi_only_elements = false;
if (FLAG_unbox_double_arrays) {
ASSERT(HasDictionaryElements());
NumberDictionary* dictionary = NumberDictionary::cast(elements());
+ bool found_double = false;
for (int i = 0; i < dictionary->Capacity(); i++) {
Object* key = dictionary->KeyAt(i);
if (key->IsNumber()) {
- if (!dictionary->ValueAt(i)->IsNumber()) return false;
+ Object* value = dictionary->ValueAt(i);
+ if (!value->IsNumber()) return false;
+ if (!value->IsSmi()) {
+ found_double = true;
+ }
}
}
- return true;
+ *has_smi_only_elements = !found_double;
+ return found_double;
} else {
return false;
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
5346585bd3e0aad0e7bcf6b2b49cdc6680ecb315..912cd21ab2abf47d3cf95ad8cbaea0213ec8a1ff
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -1644,8 +1644,9 @@ class JSObject: public JSReceiver {
// elements.
bool ShouldConvertToFastElements();
// Returns true if the elements of JSObject contains only values that
can be
- // represented in a FixedDoubleArray.
- bool CanConvertToFastDoubleElements();
+ // represented in a FixedDoubleArray and has at least one value that can
only
+ // be represented as a double and not a Smi.
+ bool ShouldConvertToFastDoubleElements(bool* has_smi_only_elements);
// Tells whether the index'th element is present.
bool HasElementWithReceiver(JSReceiver* receiver, uint32_t index);
@@ -1708,6 +1709,7 @@ class JSObject: public JSReceiver {
enum SetFastElementsCapacityMode {
kAllowSmiOnlyElements,
+ kForceSmiOnlyElements,
kDontAllowSmiOnlyElements
};
Index: test/mjsunit/regress/regress-1849.js
diff --git a/test/mjsunit/regress/regress-1849.js
b/test/mjsunit/regress/regress-1849.js
new file mode 100644
index
0000000000000000000000000000000000000000..72b2c87ed80a19cd4a864a6405150f65b96a2b48
--- /dev/null
+++ b/test/mjsunit/regress/regress-1849.js
@@ -0,0 +1,39 @@
+// Copyright 2009 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.
+
+// See: http://code.google.com/p/v8/issues/detail?id=1878
+
+// Flags: --allow-natives-syntax
+
+var count = 1e5;
+var arr = new Array(count);
+assertFalse(%HasFastDoubleElements(arr));
+for (var i = 0; i < count; i++) {
+ arr[i] = 0;
+}
+assertFalse(%HasFastDoubleElements(arr));
+assertTrue(%HasFastSmiOnlyElements(arr));
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
