Changeset 226010 in webkit

Timestamp:

Dec 17, 2017 2:53:51 AM (6 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] Number of SlotVisitors can increase after setting up m_visitCounters
​https://bugs.webkit.org/show_bug.cgi?id=180906

Reviewed by Filip Pizlo.

The number of SlotVisitors can increase after setting up m_visitCounters.
If it happens, our m_visitCounters misses the visit count of newly added
SlotVisitors. It accidentally decides that constraints are converged.
This leads to random assertion hits in Linux environment.

In this patch, we compare the number of SlotVisitors in didVisitSomething().
If the number of SlotVisitors is changed, we conservatively say we did
visit something.

• heap/Heap.h:
• heap/HeapInlines.h:

(JSC::Heap::numberOfSlotVisitors):

• heap/MarkingConstraintSet.h:
• heap/MarkingConstraintSolver.cpp:

(JSC::MarkingConstraintSolver::didVisitSomething const):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.h (modified) (1 diff)
• heap/HeapInlines.h (modified) (1 diff)
• heap/MarkingConstraintSet.h (modified) (1 diff)
• heap/MarkingConstraintSolver.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-12-16  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Number of SlotVisitors can increase after setting up m_visitCounters
+        https://bugs.webkit.org/show_bug.cgi?id=180906
+
+        Reviewed by Filip Pizlo.
+
+        The number of SlotVisitors can increase after setting up m_visitCounters.
+        If it happens, our m_visitCounters misses the visit count of newly added
+        SlotVisitors. It accidentally decides that constraints are converged.
+        This leads to random assertion hits in Linux environment.
+
+        In this patch, we compare the number of SlotVisitors in didVisitSomething().
+        If the number of SlotVisitors is changed, we conservatively say we did
+        visit something.
+
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::numberOfSlotVisitors):
+        * heap/MarkingConstraintSet.h:
+        * heap/MarkingConstraintSolver.cpp:
+        (JSC::MarkingConstraintSolver::didVisitSomething const):
+
 2017-12-16  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/heap/Heap.h

@@ -373 +373 @@
     template<typename Func>
     void forEachSlotVisitor(const Func&);
+    unsigned numberOfSlotVisitors();
 
 private:

trunk/Source/JavaScriptCore/heap/HeapInlines.h

@@ -270 +270 @@
 }
 
+inline unsigned Heap::numberOfSlotVisitors()
+{
+    auto locker = holdLock(m_parallelSlotVisitorLock);
+    return m_parallelSlotVisitors.size() + 2; // m_collectorSlotVisitor and m_mutatorSlotVisitor
+}
+
 } // namespace JSC

trunk/Source/JavaScriptCore/heap/MarkingConstraintSet.h

@@ -68 +68 @@
     bool executeConvergenceImpl(SlotVisitor&);
     
-    bool drain(SlotVisitor&, BitVector& unexecuted, BitVector& executed, bool& didVisitSomething);
-    
     Heap& m_heap;
     BitVector m_unexecutedRoots;

trunk/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp

@@ -52 +52 @@
             return true;
     }
+    // If the number of SlotVisitors increases after creating m_visitCounters,
+    // we conservatively say there could be something visited by added SlotVisitors.
+    if (m_heap.numberOfSlotVisitors() > m_visitCounters.size())
+        return true;
     return false;
 }