Changeset 128706 in webkit

Timestamp:

Sep 16, 2012 12:22:46 AM (12 years ago)

Author:

fpizlo@apple.com

Message:

JSObject.cpp and JSArray.cpp have inconsistent tests for the invalid array index case
​https://bugs.webkit.org/show_bug.cgi?id=96878

Reviewed by Sam Weinig.

Removed the uses of UNLIKELY() because I don't believe they are buying us anything,
since we're already on the slow path. Also found other places where we're testing for
the invalid array index case using unusual predicates rather than just using
MAX_ARRAY_INDEX. With this change, I believe that all of our tests for invalid
array indices (i.e. indices that should be treated as non-indexed properties)
uniformly use MAX_ARRAY_INDEX and PropertyName::NotAnIndex.

• runtime/JSArray.cpp:

(JSC::JSArray::push):

• runtime/JSObject.cpp:

(JSC::JSObject::putByIndex):
(JSC::JSObject::defineOwnIndexedProperty):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSArray.cpp (modified) (1 diff)
• runtime/JSObject.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-09-16  Filip Pizlo  <fpizlo@apple.com>
+
+        JSObject.cpp and JSArray.cpp have inconsistent tests for the invalid array index case
+        https://bugs.webkit.org/show_bug.cgi?id=96878
+
+        Reviewed by Sam Weinig.
+
+        Removed the uses of UNLIKELY() because I don't believe they are buying us anything,
+        since we're already on the slow path. Also found other places where we're testing for
+        the invalid array index case using unusual predicates rather than just using
+        MAX_ARRAY_INDEX. With this change, I believe that all of our tests for invalid
+        array indices (i.e. indices that should be treated as non-indexed properties)
+        uniformly use MAX_ARRAY_INDEX and PropertyName::NotAnIndex.
+
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::push):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::putByIndex):
+        (JSC::JSObject::defineOwnIndexedProperty):
+
 2012-09-15  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSArray.cpp

@@ -478 +478 @@
         }
 
-        // Pushing to an array of length 2^32-1 stores the property, but throws a range error.
-        if (UNLIKELY(storage->length() == 0xFFFFFFFFu)) {
+        // Pushing to an array of invalid length (2^31-1) stores the property, but throws a range error.
+        if (storage->length() > MAX_ARRAY_INDEX) {
             methodTable()->putByIndex(this, exec, storage->length(), value, true);
             // Per ES5.1 15.4.4.7 step 6 & 15.4.5.1 step 3.d.

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -348 +348 @@
     thisObject->checkIndexingConsistency();
     
-    if (UNLIKELY(propertyName > MAX_ARRAY_INDEX)) {
+    if (propertyName > MAX_ARRAY_INDEX) {
         PutPropertySlot slot(shouldThrow);
         thisObject->methodTable()->put(thisObject, exec, Identifier::from(exec, propertyName), value, slot);
@@ -981 +981 @@
 bool JSObject::defineOwnIndexedProperty(ExecState* exec, unsigned index, PropertyDescriptor& descriptor, bool throwException)
 {
-    ASSERT(index != 0xFFFFFFFF);
+    ASSERT(index <= MAX_ARRAY_INDEX);
 
     if (!inSparseIndexingMode()) {