op_in should mark if it sees out of bounds accesses
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 May 2018 23:16:09 +0000 (23:16 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 May 2018 23:16:09 +0000 (23:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185792

Reviewed by Filip Pizlo.

JSTests:

* stress/has-indexed-property-array-storage-ftl.js:
(test2):
* stress/has-indexed-property-slow-put-array-storage-ftl.js:
(test2):

Source/JavaScriptCore:

This would used to cause us to OSR loop since we would always speculate
we were in bounds in HasIndexedProperty.

* bytecode/ArrayProfile.cpp:
(JSC::ArrayProfile::observeIndexedRead):
* bytecode/ArrayProfile.h:
* runtime/CommonSlowPaths.h:
(JSC::CommonSlowPaths::opIn):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231990 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/has-indexed-property-array-storage-ftl.js
JSTests/stress/has-indexed-property-slow-put-array-storage-ftl.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/ArrayProfile.cpp
Source/JavaScriptCore/bytecode/ArrayProfile.h
Source/JavaScriptCore/runtime/CommonSlowPaths.h

index 5983914..b6ad046 100644 (file)
@@ -1,3 +1,15 @@
+2018-05-18  Keith Miller  <keith_miller@apple.com>
+
+        op_in should mark if it sees out of bounds accesses
+        https://bugs.webkit.org/show_bug.cgi?id=185792
+
+        Reviewed by Filip Pizlo.
+
+        * stress/has-indexed-property-array-storage-ftl.js:
+        (test2):
+        * stress/has-indexed-property-slow-put-array-storage-ftl.js:
+        (test2):
+
 2018-05-18  Mark Lam  <mark.lam@apple.com>
 
         Add missing exception check.
index b967c3d..6d937d6 100644 (file)
@@ -32,11 +32,11 @@ shouldBe(test1(array), false);
 function test2(array)
 {
     didFTLCompile = ftlTrue();
-    return 2 in array;
+    return 13 in array;
 }
 noInline(test2);
 
-var array1 = [1, 2, 3, 4];
+var array1 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14];
 ensureArrayStorage(array1);
 var array2 = [1, 2];
 ensureArrayStorage(array2);
index 9fedf2d..828afdb 100644 (file)
@@ -43,11 +43,11 @@ shouldBe(test1(array), false);
 function test2(array)
 {
     didFTLCompile = ftlTrue();
-    return 2 in array;
+    return 9 in array;
 }
 noInline(test2);
 
-var array1 = [1, 2, 3, 4];
+var array1 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
 array1.__proto__ = object;
 ensureArrayStorage(array1);
 var array2 = [1, 2];
index 71e84aa..c1c8c85 100644 (file)
@@ -1,3 +1,19 @@
+2018-05-18  Keith Miller  <keith_miller@apple.com>
+
+        op_in should mark if it sees out of bounds accesses
+        https://bugs.webkit.org/show_bug.cgi?id=185792
+
+        Reviewed by Filip Pizlo.
+
+        This would used to cause us to OSR loop since we would always speculate
+        we were in bounds in HasIndexedProperty.
+
+        * bytecode/ArrayProfile.cpp:
+        (JSC::ArrayProfile::observeIndexedRead):
+        * bytecode/ArrayProfile.h:
+        * runtime/CommonSlowPaths.h:
+        (JSC::CommonSlowPaths::opIn):
+
 2018-05-18  Mark Lam  <mark.lam@apple.com>
 
         Add missing exception check.
index cb1b758..c1c7dbd 100644 (file)
@@ -121,6 +121,23 @@ void ArrayProfile::computeUpdatedPrediction(const ConcurrentJSLocker&, CodeBlock
         m_usesOriginalArrayStructures = false;
 }
 
+void ArrayProfile::observeIndexedRead(VM& vm, JSCell* cell, unsigned index)
+{
+    m_lastSeenStructureID = cell->structureID();
+
+    if (JSObject* object = jsDynamicCast<JSObject*>(vm, cell)) {
+        if (hasAnyArrayStorage(object->indexingType()) && index >= object->getVectorLength())
+            setOutOfBounds();
+        else if (index >= object->getArrayLength())
+            setOutOfBounds();
+    }
+
+    if (JSString* string = jsDynamicCast<JSString*>(vm, cell)) {
+        if (index >= string->length())
+            setOutOfBounds();
+    }
+}
+
 CString ArrayProfile::briefDescription(const ConcurrentJSLocker& locker, CodeBlock* codeBlock)
 {
     computeUpdatedPrediction(locker, codeBlock);
index c10c5e2..73eb882 100644 (file)
@@ -214,11 +214,13 @@ public:
     {
         m_lastSeenStructureID = structure->id();
     }
-    
+
     void computeUpdatedPrediction(const ConcurrentJSLocker&, CodeBlock*);
     void computeUpdatedPrediction(const ConcurrentJSLocker&, CodeBlock*, Structure* lastSeenStructure);
     
     void observeArrayMode(ArrayModes mode) { m_observedArrayModes |= mode; }
+    void observeIndexedRead(VM&, JSCell*, unsigned index);
+
     ArrayModes observedArrayModes(const ConcurrentJSLocker&) const { return m_observedArrayModes; }
     bool mayInterceptIndexedAccesses(const ConcurrentJSLocker&) const { return m_mayInterceptIndexedAccesses; }
     
index db1eea9..c285135 100644 (file)
@@ -100,6 +100,8 @@ inline bool opIn(ExecState* exec, JSValue baseVal, JSValue propName, ArrayProfil
 
     uint32_t i;
     if (propName.getUInt32(i)) {
+        if (arrayProfile)
+            arrayProfile->observeIndexedRead(vm, baseObj, i);
         scope.release();
         return baseObj->hasProperty(exec, i);
     }