Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (231517 => 231518)
--- trunk/Source/_javascript_Core/ChangeLog 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-05-08 23:20:33 UTC (rev 231518)
@@ -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-05-08 Eric Carlson <[email protected]>
Consecutive messages logged as JSON are coalesced
Modified: trunk/Source/_javascript_Core/bytecode/Watchpoint.cpp (231517 => 231518)
--- trunk/Source/_javascript_Core/bytecode/Watchpoint.cpp 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/bytecode/Watchpoint.cpp 2018-05-08 23:20:33 UTC (rev 231518)
@@ -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: trunk/Source/_javascript_Core/bytecode/Watchpoint.h (231517 => 231518)
--- trunk/Source/_javascript_Core/bytecode/Watchpoint.h 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/bytecode/Watchpoint.h 2018-05-08 23:20:33 UTC (rev 231518)
@@ -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: trunk/Source/_javascript_Core/runtime/JSObject.cpp (231517 => 231518)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2018-05-08 23:20:33 UTC (rev 231518)
@@ -1647,7 +1647,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
@@ -3534,7 +3534,7 @@
void JSObject::convertToDictionary(VM& vm)
{
- DeferredStructureTransitionWatchpointFire deferredWatchpointFire;
+ DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure(vm));
setStructure(
vm, Structure::toCacheableDictionaryTransition(vm, structure(vm), &deferredWatchpointFire));
}
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (231517 => 231518)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-05-08 23:20:33 UTC (rev 231518)
@@ -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: trunk/Source/_javascript_Core/runtime/Structure.cpp (231517 => 231518)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2018-05-08 23:20:33 UTC (rev 231518)
@@ -1032,22 +1032,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
@@ -1057,10 +1055,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: trunk/Source/_javascript_Core/runtime/Structure.h (231517 => 231518)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2018-05-08 23:16:33 UTC (rev 231517)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2018-05-08 23:20:33 UTC (rev 231518)
@@ -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;
};
