Changeset 211112 in webkit

Timestamp:

Jan 24, 2017, 2:40:40 PM (10 years ago)

Author:

commit-queue@webkit.org

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

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/InferredTypeTable.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-01-24  Commit Queue  <commit-queue@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  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/InferredTypeTable.cpp

@@ -58 +58 @@
     
     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();
@@ -72 +70 @@
 {
     auto iter = m_table.find(uid);
-    if (iter == m_table.end())
-        return nullptr;
-
-    auto entryValue = iter->value;
-    if (!entryValue)
+    if (iter == m_table.end() || !iter->value)
         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();
 }
 
@@ -106 +100 @@
     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;
         
@@ -125 +115 @@
         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;
     
@@ -146 +134 @@
     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;