[webkit-changes] [211112] trunk/Source/JavaScriptCore

[commit-queue]

Title: [211112] trunk/Source/_javascript_Core

Revision

211112

Author

commit-qu...@webkit.org

Date

2017-01-24 14:40:40 -0800 (Tue, 24 Jan 2017)

Log Message

Unreviewed, rolling out r211091.
https://bugs.webkit.org/show_bug.cgi?id=167384

introduces a subtle bug in InferredTypeTable, huge
Octane/deltablue regression (Requested by pizlo on #webkit).

Reverted changeset:

"InferredTypeTable entry manipulation is not TOCTOU race safe"
https://bugs.webkit.org/show_bug.cgi?id=167344
http://trac.webkit.org/changeset/211091

Modified Paths

• trunk/Source/_javascript_Core/ChangeLog
• trunk/Source/_javascript_Core/runtime/InferredTypeTable.cpp

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (211111 => 211112)

--- trunk/Source/_javascript_Core/ChangeLog	2017-01-24 22:07:34 UTC (rev 211111)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-01-24 22:40:40 UTC (rev 211112)
@@ -1,3 +1,17 @@
+2017-01-24  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, rolling out r211091.
+        https://bugs.webkit.org/show_bug.cgi?id=167384
+
+        introduces a subtle bug in InferredTypeTable, huge
+        Octane/deltablue regression (Requested by pizlo on #webkit).
+
+        Reverted changeset:
+
+        "InferredTypeTable entry manipulation is not TOCTOU race safe"
+        https://bugs.webkit.org/show_bug.cgi?id=167344
+        http://trac.webkit.org/changeset/211091
+
 2017-01-24  Filip Pizlo  <fpi...@apple.com>
 
         Enable the stochastic space-time scheduler on the larger multicores

Modified: trunk/Source/_javascript_Core/runtime/InferredTypeTable.cpp (211111 => 211112)

--- trunk/Source/_javascript_Core/runtime/InferredTypeTable.cpp	2017-01-24 22:07:34 UTC (rev 211111)
+++ trunk/Source/_javascript_Core/runtime/InferredTypeTable.cpp	2017-01-24 22:40:40 UTC (rev 211112)
@@ -57,12 +57,10 @@
     ConcurrentJSLocker locker(inferredTypeTable->m_lock);
     
     for (auto& entry : inferredTypeTable->m_table) {
-        auto entryValue = entry.value;
-
-        if (!entryValue)
+        if (!entry.value)
             continue;
-        if (entryValue->isRelevant())
-            visitor.append(entryValue);
+        if (entry.value->isRelevant())
+            visitor.append(entry.value);
         else
             entry.value.clear();
     }
@@ -71,20 +69,16 @@
 InferredType* InferredTypeTable::get(const ConcurrentJSLocker&, UniquedStringImpl* uid)
 {
     auto iter = m_table.find(uid);
-    if (iter == m_table.end())
+    if (iter == m_table.end() || !iter->value)
         return nullptr;
 
-    auto entryValue = iter->value;
-    if (!entryValue)
-        return nullptr;
-
     // Take this opportunity to prune invalidated types.
-    if (!entryValue->isRelevant()) {
+    if (!iter->value->isRelevant()) {
         iter->value.clear();
         return nullptr;
     }
 
-    return entryValue.get();
+    return iter->value.get();
 }
 
 InferredType* InferredTypeTable::get(UniquedStringImpl* uid)
@@ -105,14 +99,10 @@
     
     if (age == OldProperty) {
         TableType::iterator iter = m_table.find(propertyName.uid());
-        if (iter == m_table.end())
+        if (iter == m_table.end() || !iter->value)
             return false; // Absence on replace => top.
-
-        auto entryValue = iter->value;
-        if (!entryValue)
-            return false;
         
-        if (entryValue->willStoreValue(vm, propertyName, value))
+        if (iter->value->willStoreValue(vm, propertyName, value))
             return true;
         
         iter->value.clear();
@@ -124,16 +114,14 @@
         ConcurrentJSLocker locker(m_lock);
         result = m_table.add(propertyName.uid(), WriteBarrier<InferredType>());
     }
-    auto entryValue = result.iterator->value;
-
     if (result.isNewEntry) {
         InferredType* inferredType = InferredType::create(vm);
         WTF::storeStoreFence();
-        entryValue.set(vm, this, inferredType);
-    } else if (!entryValue)
+        result.iterator->value.set(vm, this, inferredType);
+    } else if (!result.iterator->value)
         return false;
     
-    if (entryValue->willStoreValue(vm, propertyName, value))
+    if (result.iterator->value->willStoreValue(vm, propertyName, value))
         return true;
     
     result.iterator->value.clear();
@@ -145,15 +133,10 @@
     // The algorithm here relies on the fact that only one thread modifies the hash map.
     if (age == OldProperty) {
         TableType::iterator iter = m_table.find(propertyName.uid());
-        if (iter == m_table.end())
+        if (iter == m_table.end() || !iter->value)
             return; // Absence on replace => top.
 
-        auto entryValue = iter->value;
-
-        if (!entryValue)
-            return;
-
-        entryValue->makeTop(vm, propertyName);
+        iter->value->makeTop(vm, propertyName);
         iter->value.clear();
         return;
     }

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes