Title: [234415] releases/WebKitGTK/webkit-2.20/Source/_javascript_Core
Revision
234415
Author
[email protected]
Date
2018-07-31 02:24:56 -0700 (Tue, 31 Jul 2018)

Log Message

Merge r231518 - Deferred firing of structure transition watchpoints is racy
https://bugs.webkit.org/show_bug.cgi?id=185438

Reviewed by Saam Barati.

Changed DeferredStructureTransitionWatchpointFire to take the watchpoints to fire
and fire them in the destructor.  When the watchpoints are taken from the
original WatchpointSet, that WatchpointSet if marked invalid.

* bytecode/Watchpoint.cpp:
(JSC::WatchpointSet::fireAllSlow):
(JSC::WatchpointSet::take):
(JSC::DeferredWatchpointFire::DeferredWatchpointFire):
(JSC::DeferredWatchpointFire::~DeferredWatchpointFire):
(JSC::DeferredWatchpointFire::fireAll):
(JSC::DeferredWatchpointFire::takeWatchpointsToFire):
* bytecode/Watchpoint.h:
(JSC::WatchpointSet::fireAll):
(JSC::InlineWatchpointSet::fireAll):
* runtime/JSObject.cpp:
(JSC::JSObject::setPrototypeDirect):
(JSC::JSObject::convertToDictionary):
* runtime/JSObjectInlines.h:
(JSC::JSObject::putDirectInternal):
* runtime/Structure.cpp:
(JSC::Structure::Structure):
(JSC::DeferredStructureTransitionWatchpointFire::DeferredStructureTransitionWatchpointFire):
(JSC::DeferredStructureTransitionWatchpointFire::~DeferredStructureTransitionWatchpointFire):
(JSC::DeferredStructureTransitionWatchpointFire::dump const):
(JSC::Structure::didTransitionFromThisStructure const):
(JSC::DeferredStructureTransitionWatchpointFire::add): Deleted.
* runtime/Structure.h:
(JSC::DeferredStructureTransitionWatchpointFire::structure const):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/ChangeLog	2018-07-31 09:24:56 UTC (rev 234415)
@@ -1,3 +1,39 @@
+2018-05-08  Michael Saboff  <[email protected]>
+
+        Deferred firing of structure transition watchpoints is racy
+        https://bugs.webkit.org/show_bug.cgi?id=185438
+
+        Reviewed by Saam Barati.
+
+        Changed DeferredStructureTransitionWatchpointFire to take the watchpoints to fire
+        and fire them in the destructor.  When the watchpoints are taken from the
+        original WatchpointSet, that WatchpointSet if marked invalid.
+
+        * bytecode/Watchpoint.cpp:
+        (JSC::WatchpointSet::fireAllSlow):
+        (JSC::WatchpointSet::take):
+        (JSC::DeferredWatchpointFire::DeferredWatchpointFire):
+        (JSC::DeferredWatchpointFire::~DeferredWatchpointFire):
+        (JSC::DeferredWatchpointFire::fireAll):
+        (JSC::DeferredWatchpointFire::takeWatchpointsToFire):
+        * bytecode/Watchpoint.h:
+        (JSC::WatchpointSet::fireAll):
+        (JSC::InlineWatchpointSet::fireAll):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::setPrototypeDirect):
+        (JSC::JSObject::convertToDictionary):
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::putDirectInternal):
+        * runtime/Structure.cpp:
+        (JSC::Structure::Structure):
+        (JSC::DeferredStructureTransitionWatchpointFire::DeferredStructureTransitionWatchpointFire):
+        (JSC::DeferredStructureTransitionWatchpointFire::~DeferredStructureTransitionWatchpointFire):
+        (JSC::DeferredStructureTransitionWatchpointFire::dump const):
+        (JSC::Structure::didTransitionFromThisStructure const):
+        (JSC::DeferredStructureTransitionWatchpointFire::add): Deleted.
+        * runtime/Structure.h:
+        (JSC::DeferredStructureTransitionWatchpointFire::structure const):
+
 2018-04-28  Saam Barati  <[email protected]>
 
         We don't model regexp effects properly

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/bytecode/Watchpoint.cpp (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/bytecode/Watchpoint.cpp	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/bytecode/Watchpoint.cpp	2018-07-31 09:24:56 UTC (rev 234415)
@@ -92,6 +92,16 @@
     WTF::storeStoreFence();
 }
 
+void WatchpointSet::fireAllSlow(VM&, DeferredWatchpointFire* deferredWatchpoints)
+{
+    ASSERT(state() == IsWatched);
+
+    WTF::storeStoreFence();
+    deferredWatchpoints->takeWatchpointsToFire(this);
+    m_state = IsInvalidated; // Do after moving watchpoints to deferredWatchpoints so deferredWatchpoints gets our current state.
+    WTF::storeStoreFence();
+}
+
 void WatchpointSet::fireAllSlow(VM& vm, const char* reason)
 {
     fireAllSlow(vm, StringFireDetail(reason));
@@ -133,6 +143,15 @@
     }
 }
 
+void WatchpointSet::take(WatchpointSet* other)
+{
+    ASSERT(state() == ClearWatchpoint);
+    m_set.takeFrom(other->m_set);
+    m_setIsNotEmpty = other->m_setIsNotEmpty;
+    m_state = other->m_state;
+    other->m_setIsNotEmpty = false;
+}
+
 void InlineWatchpointSet::add(Watchpoint* watchpoint)
 {
     inflate()->add(watchpoint);
@@ -159,5 +178,28 @@
     fat()->deref();
 }
 
+DeferredWatchpointFire::DeferredWatchpointFire(VM& vm)
+    : m_vm(vm)
+    , m_watchpointsToFire(ClearWatchpoint)
+{
+}
+
+DeferredWatchpointFire::~DeferredWatchpointFire()
+{
+}
+
+void DeferredWatchpointFire::fireAll()
+{
+    if (m_watchpointsToFire.state() == IsWatched)
+        m_watchpointsToFire.fireAll(m_vm, *this);
+}
+
+void DeferredWatchpointFire::takeWatchpointsToFire(WatchpointSet* watchpointsToFire)
+{
+    ASSERT(m_watchpointsToFire.state() == ClearWatchpoint);
+    ASSERT(watchpointsToFire->state() == IsWatched);
+    m_watchpointsToFire.take(watchpointsToFire);
+}
+
 } // namespace JSC
 

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/bytecode/Watchpoint.h (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/bytecode/Watchpoint.h	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/bytecode/Watchpoint.h	2018-07-31 09:24:56 UTC (rev 234415)
@@ -89,10 +89,12 @@
 };
 
 class InlineWatchpointSet;
+class DeferredWatchpointFire;
 class VM;
 
 class WatchpointSet : public ThreadSafeRefCounted<WatchpointSet> {
     friend class LLIntOffsetsExtractor;
+    friend class DeferredWatchpointFire;
 public:
     JS_EXPORT_PRIVATE WatchpointSet(WatchpointState);
     
@@ -159,6 +161,13 @@
         fireAllSlow(vm, detail);
     }
     
+    void fireAll(VM& vm, DeferredWatchpointFire* deferredWatchpoints)
+    {
+        if (LIKELY(m_state != IsWatched))
+            return;
+        fireAllSlow(vm, deferredWatchpoints);
+    }
+
     void fireAll(VM& vm, const char* reason)
     {
         if (LIKELY(m_state != IsWatched))
@@ -201,10 +210,12 @@
     int8_t* addressOfSetIsNotEmpty() { return &m_setIsNotEmpty; }
     
     JS_EXPORT_PRIVATE void fireAllSlow(VM&, const FireDetail&); // Call only if you've checked isWatched.
+    JS_EXPORT_PRIVATE void fireAllSlow(VM&, DeferredWatchpointFire* deferredWatchpoints);
     JS_EXPORT_PRIVATE void fireAllSlow(VM&, const char* reason); // Ditto.
     
 private:
     void fireAllWatchpoints(VM&, const FireDetail&);
+    void take(WatchpointSet* other);
     
     friend class InlineWatchpointSet;
 
@@ -308,6 +319,18 @@
         WTF::storeStoreFence();
     }
     
+    void fireAll(VM& vm, DeferredWatchpointFire* deferred)
+    {
+        if (isFat()) {
+            fat()->fireAll(vm, deferred);
+            return;
+        }
+        if (decodeState(m_data) == ClearWatchpoint)
+            return;
+        m_data = encodeState(IsInvalidated);
+        WTF::storeStoreFence();
+    }
+
     void invalidate(VM& vm, const FireDetail& detail)
     {
         if (isFat())
@@ -435,4 +458,19 @@
     uintptr_t m_data;
 };
 
+class DeferredWatchpointFire : public FireDetail {
+    WTF_MAKE_NONCOPYABLE(DeferredWatchpointFire);
+public:
+    JS_EXPORT_PRIVATE DeferredWatchpointFire(VM&);
+    JS_EXPORT_PRIVATE ~DeferredWatchpointFire();
+
+    JS_EXPORT_PRIVATE void takeWatchpointsToFire(WatchpointSet*);
+    JS_EXPORT_PRIVATE void fireAll();
+
+    void dump(PrintStream& out) const override = 0;
+private:
+    VM& m_vm;
+    WatchpointSet m_watchpointsToFire;
+};
+
 } // namespace JSC

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/JSObject.cpp (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/JSObject.cpp	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/JSObject.cpp	2018-07-31 09:24:56 UTC (rev 234415)
@@ -1652,7 +1652,7 @@
         prototype.asCell()->didBecomePrototype();
     
     if (structure(vm)->hasMonoProto()) {
-        DeferredStructureTransitionWatchpointFire deferred;
+        DeferredStructureTransitionWatchpointFire deferred(vm, structure(vm));
         Structure* newStructure = Structure::changePrototypeTransition(vm, structure(vm), prototype, deferred);
         setStructure(vm, newStructure);
     } else
@@ -3543,7 +3543,7 @@
 
 void JSObject::convertToDictionary(VM& vm)
 {
-    DeferredStructureTransitionWatchpointFire deferredWatchpointFire;
+    DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure(vm));
     setStructure(
         vm, Structure::toCacheableDictionaryTransition(vm, structure(vm), &deferredWatchpointFire));
 }

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/JSObjectInlines.h (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/JSObjectInlines.h	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/JSObjectInlines.h	2018-07-31 09:24:56 UTC (rev 234415)
@@ -362,7 +362,7 @@
     // We want the structure transition watchpoint to fire after this object has switched
     // structure. This allows adaptive watchpoints to observe if the new structure is the one
     // we want.
-    DeferredStructureTransitionWatchpointFire deferredWatchpointFire;
+    DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure);
     
     newStructure = Structure::addNewPropertyTransition(
         vm, structure, propertyName, attributes, offset, slot.context(), &deferredWatchpointFire);

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Structure.cpp (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Structure.cpp	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Structure.cpp	2018-07-31 09:24:56 UTC (rev 234415)
@@ -1035,22 +1035,20 @@
     out.print("Structure transition from ", *m_structure);
 }
 
-DeferredStructureTransitionWatchpointFire::DeferredStructureTransitionWatchpointFire()
-    : m_structure(nullptr)
+DeferredStructureTransitionWatchpointFire::DeferredStructureTransitionWatchpointFire(VM& vm, Structure* structure)
+    : DeferredWatchpointFire(vm)
+    , m_structure(structure)
 {
 }
 
 DeferredStructureTransitionWatchpointFire::~DeferredStructureTransitionWatchpointFire()
 {
-    if (m_structure)
-        m_structure->transitionWatchpointSet().fireAll(*m_structure->vm(), StructureFireDetail(m_structure));
+    fireAll();
 }
 
-void DeferredStructureTransitionWatchpointFire::add(const Structure* structure)
+void DeferredStructureTransitionWatchpointFire::dump(PrintStream& out) const
 {
-    RELEASE_ASSERT(!m_structure);
-    RELEASE_ASSERT(structure);
-    m_structure = structure;
+    out.print("Structure transition from ", *m_structure);
 }
 
 void Structure::didTransitionFromThisStructure(DeferredStructureTransitionWatchpointFire* deferred) const
@@ -1060,10 +1058,11 @@
     // unwise to watch it.
     if (m_transitionWatchpointSet.isBeingWatched())
         const_cast<Structure*>(this)->setTransitionWatchpointIsLikelyToBeFired(true);
-    
-    if (deferred)
-        deferred->add(this);
-    else
+
+    if (deferred) {
+        ASSERT(deferred->structure() == this);
+        m_transitionWatchpointSet.fireAll(*vm(), deferred);
+    } else
         m_transitionWatchpointSet.fireAll(*vm(), StructureFireDetail(this));
 }
 

Modified: releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Structure.h (234414 => 234415)


--- releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Structure.h	2018-07-31 09:24:51 UTC (rev 234414)
+++ releases/WebKitGTK/webkit-2.20/Source/_javascript_Core/runtime/Structure.h	2018-07-31 09:24:56 UTC (rev 234415)
@@ -110,14 +110,16 @@
     const Structure* m_structure;
 };
 
-class DeferredStructureTransitionWatchpointFire {
+class DeferredStructureTransitionWatchpointFire : public DeferredWatchpointFire {
     WTF_MAKE_NONCOPYABLE(DeferredStructureTransitionWatchpointFire);
 public:
-    JS_EXPORT_PRIVATE DeferredStructureTransitionWatchpointFire();
+    JS_EXPORT_PRIVATE DeferredStructureTransitionWatchpointFire(VM&, Structure*);
     JS_EXPORT_PRIVATE ~DeferredStructureTransitionWatchpointFire();
     
-    void add(const Structure*);
-    
+    void dump(PrintStream& out) const override;
+
+    const Structure* structure() const { return m_structure; }
+
 private:
     const Structure* m_structure;
 };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to