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

Wed, 01 Jun 2016 21:27:01 -0700

Title: [201590] trunk/Source/_javascript_Core
Revision
201590
Author
[email protected]
Date
2016-06-01 21:26:32 -0700 (Wed, 01 Jun 2016)

Log Message

Structure::previousID() races with Structure::allocateRareData()
https://bugs.webkit.org/show_bug.cgi?id=158280

Reviewed by Mark Lam.
        
The problem is that previousID() would test hasRareData() and then either load the
previous Structure from the rare data, or load it directly. allocateRareData() would set
the hasRareData() bit separately from moving the Structure pointer into the rare data. So
we'd have a race that would cause previousID() to sometimes return the rarae data instead
of the previous Structure.

The fix is to get rid of the hasRareData bit. We can use the structureID of the
previousOrRareData cell to determine if it's the previousID or the RareData. This fixes the
race and it's probably not any slower.

* runtime/Structure.cpp:
(JSC::Structure::Structure):
(JSC::Structure::allocateRareData):
* runtime/Structure.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (201589 => 201590)


--- trunk/Source/_javascript_Core/ChangeLog	2016-06-02 04:07:14 UTC (rev 201589)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-06-02 04:26:32 UTC (rev 201590)
@@ -1,3 +1,25 @@
+2016-06-01  Filip Pizlo  <[email protected]>
+
+        Structure::previousID() races with Structure::allocateRareData()
+        https://bugs.webkit.org/show_bug.cgi?id=158280
+
+        Reviewed by Mark Lam.
+        
+        The problem is that previousID() would test hasRareData() and then either load the
+        previous Structure from the rare data, or load it directly. allocateRareData() would set
+        the hasRareData() bit separately from moving the Structure pointer into the rare data. So
+        we'd have a race that would cause previousID() to sometimes return the rarae data instead
+        of the previous Structure.
+
+        The fix is to get rid of the hasRareData bit. We can use the structureID of the
+        previousOrRareData cell to determine if it's the previousID or the RareData. This fixes the
+        race and it's probably not any slower.
+
+        * runtime/Structure.cpp:
+        (JSC::Structure::Structure):
+        (JSC::Structure::allocateRareData):
+        * runtime/Structure.h:
+
 2016-06-01  Michael Saboff  <[email protected]>
 
         Runaway WebContent process CPU & memory @ foxnews.com

Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (201589 => 201590)


--- trunk/Source/_javascript_Core/runtime/Structure.cpp	2016-06-02 04:07:14 UTC (rev 201589)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp	2016-06-02 04:26:32 UTC (rev 201590)
@@ -206,7 +206,6 @@
     setDidPreventExtensions(false);
     setDidTransition(false);
     setStaticFunctionsReified(false);
-    setHasRareData(false);
     setTransitionWatchpointIsLikelyToBeFired(false);
     setHasBeenDictionary(false);
  
@@ -238,7 +237,6 @@
     setDidPreventExtensions(false);
     setDidTransition(false);
     setStaticFunctionsReified(false);
-    setHasRareData(false);
     setTransitionWatchpointIsLikelyToBeFired(false);
     setHasBeenDictionary(false);
  
@@ -269,7 +267,6 @@
     setDidPreventExtensions(previous->didPreventExtensions());
     setDidTransition(true);
     setStaticFunctionsReified(previous->staticFunctionsReified());
-    setHasRareData(false);
     setHasBeenDictionary(previous->hasBeenDictionary());
  
     TypeInfo typeInfo = previous->typeInfo();
@@ -823,11 +820,9 @@
 void Structure::allocateRareData(VM& vm)
 {
     ASSERT(!hasRareData());
-    StructureRareData* rareData = StructureRareData::create(vm, previous());
+    StructureRareData* rareData = StructureRareData::create(vm, previousID());
     WTF::storeStoreFence();
     m_previousOrRareData.set(vm, this, rareData);
-    WTF::storeStoreFence();
-    setHasRareData(true);
     ASSERT(hasRareData());
 }

Modified: trunk/Source/_javascript_Core/runtime/Structure.h (201589 => 201590)


--- trunk/Source/_javascript_Core/runtime/Structure.h	2016-06-02 04:07:14 UTC (rev 201589)
+++ trunk/Source/_javascript_Core/runtime/Structure.h	2016-06-02 04:26:32 UTC (rev 201590)
@@ -274,13 +274,21 @@
         
     // Will just the prototype chain intercept this property access?
     JS_EXPORT_PRIVATE bool prototypeChainMayInterceptStoreTo(VM&, PropertyName);
-        
+    
+    bool hasRareData() const
+    {
+        return isRareData(m_previousOrRareData.get());
+    }
+    
     Structure* previousID() const
     {
         ASSERT(structure()->classInfo() == info());
-        if (hasRareData())
-            return rareData()->previousID();
-        return previous();
+        // This is so written because it's used concurrently. We only load from m_previousOrRareData
+        // once, and this load is guaranteed atomic.
+        JSCell* cell = m_previousOrRareData.get();
+        if (isRareData(cell))
+            return static_cast<StructureRareData*>(cell)->previousID();
+        return static_cast<Structure*>(cell);
     }
     bool transitivelyTransitionedFrom(Structure* structureToFind);
 
@@ -602,12 +610,11 @@
     DEFINE_BITFIELD(bool, didPreventExtensions, DidPreventExtensions, 1, 20);
     DEFINE_BITFIELD(bool, didTransition, DidTransition, 1, 21);
     DEFINE_BITFIELD(bool, staticFunctionsReified, StaticFunctionsReified, 1, 22);
-    DEFINE_BITFIELD(bool, hasRareData, HasRareData, 1, 23);
-    DEFINE_BITFIELD(bool, hasBeenFlattenedBefore, HasBeenFlattenedBefore, 1, 24);
-    DEFINE_BITFIELD(bool, hasCustomGetterSetterProperties, HasCustomGetterSetterProperties, 1, 25);
-    DEFINE_BITFIELD(bool, didWatchInternalProperties, DidWatchInternalProperties, 1, 26);
-    DEFINE_BITFIELD(bool, transitionWatchpointIsLikelyToBeFired, TransitionWatchpointIsLikelyToBeFired, 1, 27);
-    DEFINE_BITFIELD(bool, hasBeenDictionary, HasBeenDictionary, 1, 28);
+    DEFINE_BITFIELD(bool, hasBeenFlattenedBefore, HasBeenFlattenedBefore, 1, 23);
+    DEFINE_BITFIELD(bool, hasCustomGetterSetterProperties, HasCustomGetterSetterProperties, 1, 24);
+    DEFINE_BITFIELD(bool, didWatchInternalProperties, DidWatchInternalProperties, 1, 25);
+    DEFINE_BITFIELD(bool, transitionWatchpointIsLikelyToBeFired, TransitionWatchpointIsLikelyToBeFired, 1, 26);
+    DEFINE_BITFIELD(bool, hasBeenDictionary, HasBeenDictionary, 1, 27);
 
 private:
     friend class LLIntOffsetsExtractor;
@@ -693,11 +700,10 @@
     bool isValid(ExecState*, StructureChain* cachedPrototypeChain) const;
         
     void pin();
-
-    Structure* previous() const
+    
+    bool isRareData(JSCell* cell) const
     {
-        ASSERT(!hasRareData());
-        return static_cast<Structure*>(m_previousOrRareData.get());
+        return cell && cell->structureID() != structureID();
     }
 
     StructureRareData* rareData() const

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to