Changeset 175243 in webkit

Timestamp:

Oct 27, 2014, 10:46:52 PM (10 years ago)

Author:

mark.lam@apple.com

Message:

Crash when attempting to perform array iteration on a non-array with numeric keys not initialized.
<​https://webkit.org/b/137814>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

The arrayIteratorNextThunkGenerator() thunk was not checking for the case where
the butterfly may be NULL. This was the source of the crash, and is now fixed.

In addition, it is also not checking for the case where a property named "length"
may have been set on the iterated object. The thunk only checks the butterfly's
publicLength for its iteration operation. Array objects will work fine with this
because it always updates its butterfly's publicLength when its length changes.
In the case of iterable non-Array objects, the "length" property will require a
look up outside of the scope of this thunk. The fix is simply to limit the fast
case checks in this thunk to Array objects.

• jit/ThunkGenerators.cpp:

(JSC::arrayIteratorNextThunkGenerator):

LayoutTests:

• js/array-length-shortening-expected.txt: Added.
• js/array-length-shortening.html: Added.
• js/for-of-crash-expected.txt: Added.
• js/for-of-crash.html: Added.
• js/script-tests/array-length-shortening.js: Added.

(testLengthShortening):
(denseInt32Elements):
(denseDoubleElements):
(denseObjectElements):
(holeyInt32Elements):
(holeyDoubleElements):
(holeyObjectElements):
(arrayStorageInt32Elements):
(arrayStorageDoubleElements):
(arrayStorageObjectElements):
(sparseInt32Elements):
(sparseDoubleElements):
(sparseObjectElements):

• js/script-tests/for-of-crash.js: Added.

(foo):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/array-length-shortening-expected.txt (added)
• LayoutTests/js/array-length-shortening.html (added)
• LayoutTests/js/for-of-crash-expected.txt (added)
• LayoutTests/js/for-of-crash.html (added)
• LayoutTests/js/script-tests/array-length-shortening.js (added)
• LayoutTests/js/script-tests/for-of-crash.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/jit/ThunkGenerators.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-10-27  Mark Lam  <mark.lam@apple.com>
+
+        Crash when attempting to perform array iteration on a non-array with numeric keys not initialized.
+        <https://webkit.org/b/137814>
+
+        Reviewed by Geoffrey Garen.
+
+        * js/array-length-shortening-expected.txt: Added.
+        * js/array-length-shortening.html: Added.
+        * js/for-of-crash-expected.txt: Added.
+        * js/for-of-crash.html: Added.
+        * js/script-tests/array-length-shortening.js: Added.
+        (testLengthShortening):
+        (denseInt32Elements):
+        (denseDoubleElements):
+        (denseObjectElements):
+        (holeyInt32Elements):
+        (holeyDoubleElements):
+        (holeyObjectElements):
+        (arrayStorageInt32Elements):
+        (arrayStorageDoubleElements):
+        (arrayStorageObjectElements):
+        (sparseInt32Elements):
+        (sparseDoubleElements):
+        (sparseObjectElements):
+        * js/script-tests/for-of-crash.js: Added.
+        (foo):
+
 2014-10-27  Chris Fleizach  <cfleizach@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-10-27  Mark Lam  <mark.lam@apple.com>
+
+        Crash when attempting to perform array iteration on a non-array with numeric keys not initialized.
+        <https://webkit.org/b/137814>
+
+        Reviewed by Geoffrey Garen.
+
+        The arrayIteratorNextThunkGenerator() thunk was not checking for the case where
+        the butterfly may be NULL.  This was the source of the crash, and is now fixed.
+
+        In addition, it is also not checking for the case where a property named "length"
+        may have been set on the iterated object.  The thunk only checks the butterfly's
+        publicLength for its iteration operation.  Array objects will work fine with this
+        because it always updates its butterfly's publicLength when its length changes.
+        In the case of iterable non-Array objects, the "length" property will require a
+        look up outside of the scope of this thunk.  The fix is simply to limit the fast
+        case checks in this thunk to Array objects.
+
+        * jit/ThunkGenerators.cpp:
+        (JSC::arrayIteratorNextThunkGenerator):
+
 2014-10-27  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp

@@ -1101 +1101 @@
     jit.load8(Address(SpecializedThunkJIT::regT0, JSCell::indexingTypeOffset()), SpecializedThunkJIT::regT3);
     jit.loadPtr(Address(SpecializedThunkJIT::regT0, JSObject::butterflyOffset()), SpecializedThunkJIT::regT2);
-    
-    jit.and32(TrustedImm32(IndexingShapeMask), SpecializedThunkJIT::regT3);
-
+    Jump nullButterfly = jit.branchTestPtr(SpecializedThunkJIT::Zero, SpecializedThunkJIT::regT2);
+    
     Jump notDone = jit.branch32(SpecializedThunkJIT::Below, SpecializedThunkJIT::regT1, Address(SpecializedThunkJIT::regT2, Butterfly::offsetOfPublicLength()));
+
+    nullButterfly.link(&jit);
+
     // Return the termination signal to indicate that we've finished
     jit.move(TrustedImmPtr(vm->iterationTerminator.get()), SpecializedThunkJIT::regT0);
@@ -1123 +1125 @@
     
     // So now we perform inline loads for int32, value/undecided, and double storage
-    Jump undecidedStorage = jit.branch32(SpecializedThunkJIT::Equal, SpecializedThunkJIT::regT3, TrustedImm32(UndecidedShape));
-    Jump notContiguousStorage = jit.branch32(SpecializedThunkJIT::NotEqual, SpecializedThunkJIT::regT3, TrustedImm32(ContiguousShape));
+    Jump undecidedStorage = jit.branch32(SpecializedThunkJIT::Equal, SpecializedThunkJIT::regT3, TrustedImm32(ArrayWithUndecided));
+    Jump notContiguousStorage = jit.branch32(SpecializedThunkJIT::NotEqual, SpecializedThunkJIT::regT3, TrustedImm32(ArrayWithContiguous));
     
     undecidedStorage.link(&jit);
@@ -1152 +1154 @@
     notContiguousStorage.link(&jit);
     
-    Jump notInt32Storage = jit.branch32(SpecializedThunkJIT::NotEqual, SpecializedThunkJIT::regT3, TrustedImm32(Int32Shape));
+    Jump notInt32Storage = jit.branch32(SpecializedThunkJIT::NotEqual, SpecializedThunkJIT::regT3, TrustedImm32(ArrayWithInt32));
     jit.loadPtr(Address(SpecializedThunkJIT::regT0, JSObject::butterflyOffset()), SpecializedThunkJIT::regT2);
     jit.load32(BaseIndex(SpecializedThunkJIT::regT2, SpecializedThunkJIT::regT1, SpecializedThunkJIT::TimesEight, JSValue::offsetOfPayload()), SpecializedThunkJIT::regT0);
@@ -1159 +1161 @@
     notInt32Storage.link(&jit);
     
-    jit.appendFailure(jit.branch32(SpecializedThunkJIT::NotEqual, SpecializedThunkJIT::regT3, TrustedImm32(DoubleShape)));
+    jit.appendFailure(jit.branch32(SpecializedThunkJIT::NotEqual, SpecializedThunkJIT::regT3, TrustedImm32(ArrayWithDouble)));
     jit.loadPtr(Address(SpecializedThunkJIT::regT0, JSObject::butterflyOffset()), SpecializedThunkJIT::regT2);
     jit.loadDouble(BaseIndex(SpecializedThunkJIT::regT2, SpecializedThunkJIT::regT1, SpecializedThunkJIT::TimesEight), SpecializedThunkJIT::fpRegT0);