Title: [231575] branches/safari-605-branch/Source/_javascript_Core
Revision
231575
Author
[email protected]
Date
2018-05-09 11:04:44 -0700 (Wed, 09 May 2018)

Log Message

Cherry-pick r231518. rdar://problem/40096743

    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):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231518 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog	2018-05-09 18:04:44 UTC (rev 231575)
@@ -1,3 +1,80 @@
+2018-05-09  Jason Marcell  <[email protected]>
+
+        Cherry-pick r231518. rdar://problem/40096743
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231518 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-05-08  Jason Marcell  <[email protected]>
 
         Cherry-pick r231332. rdar://problem/40050814

Modified: branches/safari-605-branch/Source/_javascript_Core/bytecode/Watchpoint.cpp (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/bytecode/Watchpoint.cpp	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/bytecode/Watchpoint.cpp	2018-05-09 18:04:44 UTC (rev 231575)
@@ -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: branches/safari-605-branch/Source/_javascript_Core/bytecode/Watchpoint.h (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/bytecode/Watchpoint.h	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/bytecode/Watchpoint.h	2018-05-09 18:04:44 UTC (rev 231575)
@@ -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: branches/safari-605-branch/Source/_javascript_Core/runtime/JSObject.cpp (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSObject.cpp	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSObject.cpp	2018-05-09 18:04:44 UTC (rev 231575)
@@ -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
@@ -3549,7 +3549,7 @@
 
 void JSObject::convertToDictionary(VM& vm)
 {
-    DeferredStructureTransitionWatchpointFire deferredWatchpointFire;
+    DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure(vm));
     setStructure(
         vm, Structure::toCacheableDictionaryTransition(vm, structure(vm), &deferredWatchpointFire));
 }

Modified: branches/safari-605-branch/Source/_javascript_Core/runtime/JSObjectInlines.h (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/JSObjectInlines.h	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/JSObjectInlines.h	2018-05-09 18:04:44 UTC (rev 231575)
@@ -363,7 +363,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: branches/safari-605-branch/Source/_javascript_Core/runtime/Structure.cpp (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/Structure.cpp	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/Structure.cpp	2018-05-09 18:04:44 UTC (rev 231575)
@@ -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: branches/safari-605-branch/Source/_javascript_Core/runtime/Structure.h (231574 => 231575)


--- branches/safari-605-branch/Source/_javascript_Core/runtime/Structure.h	2018-05-09 18:02:32 UTC (rev 231574)
+++ branches/safari-605-branch/Source/_javascript_Core/runtime/Structure.h	2018-05-09 18:04:44 UTC (rev 231575)
@@ -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