some Watchpoints' ::fireInternal method will call operations that might GC where...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jun 2016 21:30:20 +0000 (21:30 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jun 2016 21:30:20 +0000 (21:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159198
<rdar://problem/26302360>

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

Firing a watchpoint may cause a GC to happen. This GC could destroy various
Watchpoints themselves while they're in the process of firing. It's not safe
for most Watchpoints to be destructed while they're in the middle of firing.
This GC could also destroy the WatchpointSet itself, and it's not in a safe
state to be destroyed. WatchpointSet::fireAllWatchpoints now defers gc for a
while. This prevents a GC from destructing any Watchpoints while they're
in the process of firing. This bug was being hit by the stress GC bots
because we would destruct a particular Watchpoint while it was firing,
and then we would access its field after it had already been destroyed.
This was causing all kinds of weird symptoms. Also, this was easier to
catch when running with guard malloc because the first access after
destruction would lead to a crash.

* bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp:
(JSC::AdaptiveInferredPropertyValueWatchpointBase::fire):
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
* bytecode/VariableWriteFireDetail.cpp:
(JSC::VariableWriteFireDetail::dump):
(JSC::VariableWriteFireDetail::touch):
* bytecode/VariableWriteFireDetail.h:
* bytecode/Watchpoint.cpp:
(JSC::WatchpointSet::add):
(JSC::WatchpointSet::fireAllSlow):
(JSC::WatchpointSet::fireAllWatchpoints):
(JSC::InlineWatchpointSet::add):
(JSC::InlineWatchpointSet::fireAll):
(JSC::InlineWatchpointSet::inflateSlow):
* bytecode/Watchpoint.h:
(JSC::WatchpointSet::startWatching):
(JSC::WatchpointSet::fireAll):
(JSC::WatchpointSet::touch):
(JSC::WatchpointSet::invalidate):
(JSC::WatchpointSet::isBeingWatched):
(JSC::WatchpointSet::offsetOfState):
(JSC::WatchpointSet::addressOfSetIsNotEmpty):
(JSC::InlineWatchpointSet::startWatching):
(JSC::InlineWatchpointSet::fireAll):
(JSC::InlineWatchpointSet::invalidate):
(JSC::InlineWatchpointSet::touch):
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
* dfg/DFGOperations.cpp:
* interpreter/Interpreter.cpp:
(JSC::Interpreter::execute):
* jit/JITOperations.cpp:
* jsc.cpp:
(WTF::Masquerader::create):
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
* runtime/ArrayBufferNeuteringWatchpoint.cpp:
(JSC::ArrayBufferNeuteringWatchpoint::fireAll):
* runtime/FunctionRareData.cpp:
(JSC::FunctionRareData::clear):
* runtime/InferredType.cpp:
(JSC::InferredType::willStoreValueSlow):
(JSC::InferredType::makeTopSlow):
(JSC::InferredType::set):
(JSC::InferredType::removeStructure):
(JSC::InferredType::InferredStructureWatchpoint::fireInternal):
* runtime/InferredValue.cpp:
(JSC::InferredValue::notifyWriteSlow):
(JSC::InferredValue::ValueCleanup::finalizeUnconditionally):
* runtime/InferredValue.h:
(JSC::InferredValue::notifyWrite):
(JSC::InferredValue::invalidate):
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::haveABadTime):
* runtime/JSSymbolTableObject.h:
(JSC::symbolTablePutTouchWatchpointSet):
(JSC::symbolTablePutInvalidateWatchpointSet):
* runtime/Structure.cpp:
(JSC::Structure::didCachePropertyReplacement):
(JSC::Structure::startWatchingInternalProperties):
(JSC::DeferredStructureTransitionWatchpointFire::~DeferredStructureTransitionWatchpointFire):
(JSC::DeferredStructureTransitionWatchpointFire::add):
(JSC::Structure::didTransitionFromThisStructure):
(JSC::Structure::prototypeForLookup):
* runtime/StructureInlines.h:
(JSC::Structure::didReplaceProperty):
(JSC::Structure::propertyReplacementWatchpointSet):
* runtime/SymbolTable.h:
(JSC::SymbolTableEntry::isDontEnum):
(JSC::SymbolTableEntry::disableWatching):
* runtime/VM.cpp:
(JSC::VM::addImpureProperty):
(JSC::enableProfilerWithRespectToCount):

Source/WebCore:

* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::fireFrameClearedWatchpointsForWindow):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):
* bindings/scripts/test/JS/JSTestEventTarget.h:
(WebCore::JSTestEventTarget::create):

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

29 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/VariableWriteFireDetail.cpp
Source/JavaScriptCore/bytecode/VariableWriteFireDetail.h
Source/JavaScriptCore/bytecode/Watchpoint.cpp
Source/JavaScriptCore/bytecode/Watchpoint.h
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/heap/CopyBarrier.h
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
Source/JavaScriptCore/runtime/ArrayBufferNeuteringWatchpoint.cpp
Source/JavaScriptCore/runtime/FunctionRareData.cpp
Source/JavaScriptCore/runtime/InferredType.cpp
Source/JavaScriptCore/runtime/InferredValue.cpp
Source/JavaScriptCore/runtime/InferredValue.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSSymbolTableObject.h
Source/JavaScriptCore/runtime/Structure.cpp
Source/JavaScriptCore/runtime/StructureInlines.h
Source/JavaScriptCore/runtime/SymbolTable.h
Source/JavaScriptCore/runtime/VM.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMWindowBase.cpp
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h

index 75be523..e486b42 100644 (file)
@@ -1,3 +1,99 @@
+2016-06-28  Saam Barati  <sbarati@apple.com>
+
+        some Watchpoints' ::fireInternal method will call operations that might GC where the GC will cause the watchpoint itself to destruct
+        https://bugs.webkit.org/show_bug.cgi?id=159198
+        <rdar://problem/26302360>
+
+        Reviewed by Filip Pizlo.
+
+        Firing a watchpoint may cause a GC to happen. This GC could destroy various
+        Watchpoints themselves while they're in the process of firing. It's not safe
+        for most Watchpoints to be destructed while they're in the middle of firing.
+        This GC could also destroy the WatchpointSet itself, and it's not in a safe
+        state to be destroyed. WatchpointSet::fireAllWatchpoints now defers gc for a
+        while. This prevents a GC from destructing any Watchpoints while they're
+        in the process of firing. This bug was being hit by the stress GC bots
+        because we would destruct a particular Watchpoint while it was firing,
+        and then we would access its field after it had already been destroyed.
+        This was causing all kinds of weird symptoms. Also, this was easier to
+        catch when running with guard malloc because the first access after
+        destruction would lead to a crash.
+
+        * bytecode/AdaptiveInferredPropertyValueWatchpointBase.cpp:
+        (JSC::AdaptiveInferredPropertyValueWatchpointBase::fire):
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::finishCreation):
+        * bytecode/VariableWriteFireDetail.cpp:
+        (JSC::VariableWriteFireDetail::dump):
+        (JSC::VariableWriteFireDetail::touch):
+        * bytecode/VariableWriteFireDetail.h:
+        * bytecode/Watchpoint.cpp:
+        (JSC::WatchpointSet::add):
+        (JSC::WatchpointSet::fireAllSlow):
+        (JSC::WatchpointSet::fireAllWatchpoints):
+        (JSC::InlineWatchpointSet::add):
+        (JSC::InlineWatchpointSet::fireAll):
+        (JSC::InlineWatchpointSet::inflateSlow):
+        * bytecode/Watchpoint.h:
+        (JSC::WatchpointSet::startWatching):
+        (JSC::WatchpointSet::fireAll):
+        (JSC::WatchpointSet::touch):
+        (JSC::WatchpointSet::invalidate):
+        (JSC::WatchpointSet::isBeingWatched):
+        (JSC::WatchpointSet::offsetOfState):
+        (JSC::WatchpointSet::addressOfSetIsNotEmpty):
+        (JSC::InlineWatchpointSet::startWatching):
+        (JSC::InlineWatchpointSet::fireAll):
+        (JSC::InlineWatchpointSet::invalidate):
+        (JSC::InlineWatchpointSet::touch):
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        * dfg/DFGOperations.cpp:
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::execute):
+        * jit/JITOperations.cpp:
+        * jsc.cpp:
+        (WTF::Masquerader::create):
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+        * runtime/ArrayBufferNeuteringWatchpoint.cpp:
+        (JSC::ArrayBufferNeuteringWatchpoint::fireAll):
+        * runtime/FunctionRareData.cpp:
+        (JSC::FunctionRareData::clear):
+        * runtime/InferredType.cpp:
+        (JSC::InferredType::willStoreValueSlow):
+        (JSC::InferredType::makeTopSlow):
+        (JSC::InferredType::set):
+        (JSC::InferredType::removeStructure):
+        (JSC::InferredType::InferredStructureWatchpoint::fireInternal):
+        * runtime/InferredValue.cpp:
+        (JSC::InferredValue::notifyWriteSlow):
+        (JSC::InferredValue::ValueCleanup::finalizeUnconditionally):
+        * runtime/InferredValue.h:
+        (JSC::InferredValue::notifyWrite):
+        (JSC::InferredValue::invalidate):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::haveABadTime):
+        * runtime/JSSymbolTableObject.h:
+        (JSC::symbolTablePutTouchWatchpointSet):
+        (JSC::symbolTablePutInvalidateWatchpointSet):
+        * runtime/Structure.cpp:
+        (JSC::Structure::didCachePropertyReplacement):
+        (JSC::Structure::startWatchingInternalProperties):
+        (JSC::DeferredStructureTransitionWatchpointFire::~DeferredStructureTransitionWatchpointFire):
+        (JSC::DeferredStructureTransitionWatchpointFire::add):
+        (JSC::Structure::didTransitionFromThisStructure):
+        (JSC::Structure::prototypeForLookup):
+        * runtime/StructureInlines.h:
+        (JSC::Structure::didReplaceProperty):
+        (JSC::Structure::propertyReplacementWatchpointSet):
+        * runtime/SymbolTable.h:
+        (JSC::SymbolTableEntry::isDontEnum):
+        (JSC::SymbolTableEntry::disableWatching):
+        * runtime/VM.cpp:
+        (JSC::VM::addImpureProperty):
+        (JSC::enableProfilerWithRespectToCount):
+
 2016-06-28  Filip Pizlo  <fpizlo@apple.com>
 
         JSRopeString should use release asserts, not debug asserts, about substring bounds
index 9f02e8c..04d98f6 100644 (file)
@@ -50,10 +50,6 @@ void AdaptiveInferredPropertyValueWatchpointBase::install()
 
 void AdaptiveInferredPropertyValueWatchpointBase::fire(const FireDetail& detail)
 {
-    // We need to defer GC here otherwise we might trigger a GC that could destroy the owner
-    // CodeBlock. In particular, this can happen when we add rare data to a structure when
-    // we EnsureWatchability.
-    DeferGCForAWhile defer(*Heap::heap(m_key.object()));
     // One of the watchpoints fired, but the other one didn't. Make sure that neither of them are
     // in any set anymore. This simplifies things by allowing us to reinstall the watchpoints
     // wherever from scratch.
index d2fa017..6d5b546 100644 (file)
@@ -2240,7 +2240,7 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
                 instructions[i + 5].u.watchpointSet = op.watchpointSet;
             else if (op.type == ClosureVar || op.type == ClosureVarWithVarInjectionChecks) {
                 if (op.watchpointSet)
-                    op.watchpointSet->invalidate(PutToScopeFireDetail(this, ident));
+                    op.watchpointSet->invalidate(vm, PutToScopeFireDetail(this, ident));
             } else if (op.structure)
                 instructions[i + 5].u.structure.set(vm, this, op.structure);
             instructions[i + 6].u.pointer = reinterpret_cast<void*>(op.operand);
index b483ab2..ec61984 100644 (file)
@@ -35,9 +35,9 @@ void VariableWriteFireDetail::dump(PrintStream& out) const
     out.print("Write to ", m_name, " in ", JSValue(m_object));
 }
 
-void VariableWriteFireDetail::touch(WatchpointSet* set, JSObject* object, const PropertyName& name)
+void VariableWriteFireDetail::touch(VM& vm, WatchpointSet* set, JSObject* object, const PropertyName& name)
 {
-    set->touch(VariableWriteFireDetail(object, name));
+    set->touch(vm, VariableWriteFireDetail(object, name));
 }
 
 } // namespace JSC
index 1c79eac..de1df9e 100644 (file)
@@ -43,7 +43,7 @@ public:
     
     JS_EXPORT_PRIVATE void dump(PrintStream&) const override;
     
-    JS_EXPORT_PRIVATE static void touch(WatchpointSet*, JSObject*, const PropertyName&);
+    JS_EXPORT_PRIVATE static void touch(VM&, WatchpointSet*, JSObject*, const PropertyName&);
 
 private:
     JSObject* m_object;
index 761c067..c057e24 100644 (file)
@@ -26,6 +26,8 @@
 #include "config.h"
 #include "Watchpoint.h"
 
+#include "HeapInlines.h"
+#include "VM.h"
 #include <wtf/CompilationThread.h>
 #include <wtf/PassRefPtr.h>
 
@@ -81,26 +83,33 @@ void WatchpointSet::add(Watchpoint* watchpoint)
     m_state = IsWatched;
 }
 
-void WatchpointSet::fireAllSlow(const FireDetail& detail)
+void WatchpointSet::fireAllSlow(VM& vm, const FireDetail& detail)
 {
     ASSERT(state() == IsWatched);
     
     WTF::storeStoreFence();
     m_state = IsInvalidated; // Do this first. Needed for adaptive watchpoints.
-    fireAllWatchpoints(detail);
+    fireAllWatchpoints(vm, detail);
     WTF::storeStoreFence();
 }
 
-void WatchpointSet::fireAllSlow(const char* reason)
+void WatchpointSet::fireAllSlow(VM& vm, const char* reason)
 {
-    fireAllSlow(StringFireDetail(reason));
+    fireAllSlow(vm, StringFireDetail(reason));
 }
 
-void WatchpointSet::fireAllWatchpoints(const FireDetail& detail)
+void WatchpointSet::fireAllWatchpoints(VM& vm, const FireDetail& detail)
 {
     // In case there are any adaptive watchpoints, we need to make sure that they see that this
     // watchpoint has been already invalidated.
     RELEASE_ASSERT(hasBeenInvalidated());
+
+    // Firing a watchpoint may cause a GC to happen. This GC could destroy various
+    // Watchpoints themselves while they're in the process of firing. It's not safe
+    // for most Watchpoints to be destructed while they're in the middle of firing.
+    // This GC could also destroy us, and we're not in a safe state to be destroyed.
+    // The safest thing to do is to DeferGCForAWhile to prevent this GC from happening.
+    DeferGCForAWhile deferGC(vm.heap);
     
     while (!m_set.isEmpty()) {
         Watchpoint* watchpoint = m_set.begin();
@@ -130,9 +139,9 @@ void InlineWatchpointSet::add(Watchpoint* watchpoint)
     inflate()->add(watchpoint);
 }
 
-void InlineWatchpointSet::fireAll(const char* reason)
+void InlineWatchpointSet::fireAll(VM& vm, const char* reason)
 {
-    fireAll(StringFireDetail(reason));
+    fireAll(vm, StringFireDetail(reason));
 }
 
 WatchpointSet* InlineWatchpointSet::inflateSlow()
index d37c0f3..94b28ed 100644 (file)
@@ -90,6 +90,7 @@ enum WatchpointState {
 };
 
 class InlineWatchpointSet;
+class VM;
 
 class WatchpointSet : public ThreadSafeRefCounted<WatchpointSet> {
     friend class LLIntOffsetsExtractor;
@@ -152,43 +153,43 @@ public:
         WTF::storeStoreFence();
     }
     
-    void fireAll(const FireDetail& detail)
+    void fireAll(VM& vm, const FireDetail& detail)
     {
         if (LIKELY(m_state != IsWatched))
             return;
-        fireAllSlow(detail);
+        fireAllSlow(vm, detail);
     }
     
-    void fireAll(const char* reason)
+    void fireAll(VM& vm, const char* reason)
     {
         if (LIKELY(m_state != IsWatched))
             return;
-        fireAllSlow(reason);
+        fireAllSlow(vm, reason);
     }
     
-    void touch(const FireDetail& detail)
+    void touch(VM& vm, const FireDetail& detail)
     {
         if (state() == ClearWatchpoint)
             startWatching();
         else
-            fireAll(detail);
+            fireAll(vm, detail);
     }
     
-    void touch(const char* reason)
+    void touch(VM& vm, const char* reason)
     {
-        touch(StringFireDetail(reason));
+        touch(vm, StringFireDetail(reason));
     }
     
-    void invalidate(const FireDetail& detail)
+    void invalidate(VM& vm, const FireDetail& detail)
     {
         if (state() == IsWatched)
-            fireAll(detail);
+            fireAll(vm, detail);
         m_state = IsInvalidated;
     }
     
-    void invalidate(const char* reason)
+    void invalidate(VM& vm, const char* reason)
     {
-        invalidate(StringFireDetail(reason));
+        invalidate(vm, StringFireDetail(reason));
     }
     
     bool isBeingWatched() const
@@ -200,11 +201,11 @@ public:
     static ptrdiff_t offsetOfState() { return OBJECT_OFFSETOF(WatchpointSet, m_state); }
     int8_t* addressOfSetIsNotEmpty() { return &m_setIsNotEmpty; }
     
-    JS_EXPORT_PRIVATE void fireAllSlow(const FireDetail&); // Call only if you've checked isWatched.
-    JS_EXPORT_PRIVATE void fireAllSlow(const char* reason); // Ditto.
+    JS_EXPORT_PRIVATE void fireAllSlow(VM&, const FireDetail&); // Call only if you've checked isWatched.
+    JS_EXPORT_PRIVATE void fireAllSlow(VM&, const char* reason); // Ditto.
     
 private:
-    void fireAllWatchpoints(const FireDetail&);
+    void fireAllWatchpoints(VM&, const FireDetail&);
     
     friend class InlineWatchpointSet;
 
@@ -296,10 +297,10 @@ public:
         m_data = encodeState(IsWatched);
     }
     
-    void fireAll(const FireDetail& detail)
+    void fireAll(VM& vm, const FireDetail& detail)
     {
         if (isFat()) {
-            fat()->fireAll(detail);
+            fat()->fireAll(vm, detail);
             return;
         }
         if (decodeState(m_data) == ClearWatchpoint)
@@ -308,20 +309,20 @@ public:
         WTF::storeStoreFence();
     }
     
-    void invalidate(const FireDetail& detail)
+    void invalidate(VM& vm, const FireDetail& detail)
     {
         if (isFat())
-            fat()->invalidate(detail);
+            fat()->invalidate(vm, detail);
         else
             m_data = encodeState(IsInvalidated);
     }
     
-    JS_EXPORT_PRIVATE void fireAll(const char* reason);
+    JS_EXPORT_PRIVATE void fireAll(VM&, const char* reason);
     
-    void touch(const FireDetail& detail)
+    void touch(VM& vm, const FireDetail& detail)
     {
         if (isFat()) {
-            fat()->touch(detail);
+            fat()->touch(vm, detail);
             return;
         }
         uintptr_t data = m_data;
@@ -335,9 +336,9 @@ public:
         WTF::storeStoreFence();
     }
     
-    void touch(const char* reason)
+    void touch(VM& vm, const char* reason)
     {
-        touch(StringFireDetail(reason));
+        touch(vm, StringFireDetail(reason));
     }
 
     // Note that for any watchpoint that is visible from the DFG, it would be incorrect to write code like:
index 91f5c77..8f15dce 100644 (file)
@@ -385,7 +385,7 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
                     // notifyWrite(), since that would be cumbersome. Also, watching formal
                     // parameters when "arguments" is in play is unlikely to be super profitable.
                     // So, we just disable it.
-                    entry.disableWatching();
+                    entry.disableWatching(*m_vm);
                     functionSymbolTable->set(NoLockingNecessary, name, entry);
                 }
                 emitOpcode(op_put_to_scope);
index 0a6aaa0..632645d 100644 (file)
@@ -1445,7 +1445,7 @@ void JIT_OPERATION operationNotifyWrite(ExecState* exec, WatchpointSet* set)
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
 
-    set->touch("Executed NotifyWrite");
+    set->touch(vm, "Executed NotifyWrite");
 }
 
 void JIT_OPERATION operationThrowStackOverflowForVarargs(ExecState* exec)
index d7b0e40..42a0d8b 100644 (file)
@@ -27,6 +27,7 @@
 #define CopyBarrier_h
 
 #include "Heap.h"
+#include "VM.h"
 
 namespace JSC {
 
index 792a45e..74f8c53 100644 (file)
@@ -1206,7 +1206,7 @@ JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue
     if (numVariables || numFunctions) {
         BatchedTransitionOptimizer optimizer(vm, variableObject);
         if (variableObject->next())
-            variableObject->globalObject()->varInjectionWatchpoint()->fireAll("Executed eval, fired VarInjection watchpoint");
+            variableObject->globalObject()->varInjectionWatchpoint()->fireAll(vm, "Executed eval, fired VarInjection watchpoint");
 
         for (unsigned i = 0; i < numVariables; ++i) {
             const Identifier& ident = codeBlock->variable(i);
index c11b3a3..01f3c3d 100644 (file)
@@ -2078,7 +2078,7 @@ void JIT_OPERATION operationPutToScope(ExecState* exec, Instruction* bytecodePC)
         JSLexicalEnvironment* environment = jsCast<JSLexicalEnvironment*>(scope);
         environment->variableAt(ScopeOffset(pc[6].u.operand)).set(vm, environment, value);
         if (WatchpointSet* set = pc[5].u.watchpointSet)
-            set->touch("Executed op_put_scope<LocalClosureVar>");
+            set->touch(vm, "Executed op_put_scope<LocalClosureVar>");
         return;
     }
 
index c194f97..91991f4 100644 (file)
@@ -209,7 +209,7 @@ public:
 
     static Masquerader* create(VM& vm, JSGlobalObject* globalObject)
     {
-        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll("Masquerading object allocated");
+        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll(vm, "Masquerading object allocated");
         Structure* structure = createStructure(vm, globalObject, jsNull());
         Masquerader* result = new (NotNull, allocateCell<Masquerader>(vm.heap, sizeof(Masquerader))) Masquerader(vm, structure);
         result->finishCreation(vm);
index 335387b..130f4e8 100644 (file)
@@ -1554,7 +1554,7 @@ LLINT_SLOW_PATH_DECL(slow_path_put_to_scope)
         // to have already changed the value of the variable. Otherwise we might watch and constant-fold
         // to the Undefined value from before the assignment.
         if (WatchpointSet* set = pc[5].u.watchpointSet)
-            set->touch("Executed op_put_scope<LocalClosureVar>");
+            set->touch(vm, "Executed op_put_scope<LocalClosureVar>");
         LLINT_END();
     }
 
index a15b504..11279f5 100644 (file)
@@ -62,7 +62,7 @@ Structure* ArrayBufferNeuteringWatchpoint::createStructure(VM& vm)
 
 void ArrayBufferNeuteringWatchpoint::fireAll()
 {
-    set()->fireAll("Array buffer was neutered");
+    set()->fireAll(*vm(), "Array buffer was neutered");
 }
 
 } // namespace JSC
index 346d15e..c327c57 100644 (file)
@@ -88,7 +88,7 @@ void FunctionRareData::clear(const char* reason)
 {
     m_objectAllocationProfile.clear();
     m_internalFunctionAllocationProfile.clear();
-    m_objectAllocationProfileWatchpoint.fireAll(reason);
+    m_objectAllocationProfileWatchpoint.fireAll(*vm(), reason);
 }
 
 }
index d513522..d1a3249 100644 (file)
@@ -419,7 +419,7 @@ bool InferredType::willStoreValueSlow(VM& vm, PropertyName propertyName, JSValue
     }
     
     InferredTypeFireDetail detail(this, propertyName.uid(), oldType, myType, value);
-    m_watchpointSet.fireAll(detail);
+    m_watchpointSet.fireAll(vm, detail);
     return result;
 }
 
@@ -434,7 +434,7 @@ void InferredType::makeTopSlow(VM& vm, PropertyName propertyName)
     }
 
     InferredTypeFireDetail detail(this, propertyName.uid(), oldType, Top, JSValue());
-    m_watchpointSet.fireAll(detail);
+    m_watchpointSet.fireAll(vm, detail);
 }
 
 bool InferredType::set(const ConcurrentJITLocker& locker, VM& vm, Descriptor newDescriptor)
@@ -516,7 +516,7 @@ void InferredType::removeStructure()
     }
 
     InferredTypeFireDetail detail(this, nullptr, oldDescriptor, newDescriptor, JSValue());
-    m_watchpointSet.fireAll(detail);
+    m_watchpointSet.fireAll(vm, detail);
 }
 
 void InferredType::InferredStructureWatchpoint::fireInternal(const FireDetail&)
index 73c6bc7..028b8d7 100644 (file)
@@ -92,7 +92,7 @@ void InferredValue::notifyWriteSlow(VM& vm, JSValue value, const FireDetail& det
         ASSERT(!!m_value);
         if (m_value.get() == value)
             return;
-        invalidate(detail);
+        invalidate(vm, detail);
         return;
         
     case IsInvalidated:
@@ -125,7 +125,7 @@ void InferredValue::ValueCleanup::finalizeUnconditionally()
     if (Heap::isMarked(m_owner->m_value.get().asCell()))
         return;
     
-    m_owner->invalidate(StringFireDetail("InferredValue clean-up during GC"));
+    m_owner->invalidate(*m_owner->vm(), StringFireDetail("InferredValue clean-up during GC"));
 }
 
 } // namespace JSC
index 28318e9..2b85028 100644 (file)
@@ -88,10 +88,10 @@ public:
         notifyWriteSlow(vm, value, reason);
     }
     
-    void invalidate(const FireDetail& detail)
+    void invalidate(VM& vm, const FireDetail& detail)
     {
         m_value.clear();
-        m_set.invalidate(detail);
+        m_set.invalidate(vm, detail);
     }
     
     static const unsigned StructureFlags = StructureIsImmortal | Base::StructureFlags;
index 7c93274..9248557 100644 (file)
@@ -943,7 +943,7 @@ void JSGlobalObject::haveABadTime(VM& vm)
     // Make sure that all allocations or indexed storage transitions that are inlining
     // the assumption that it's safe to transition to a non-SlowPut array storage don't
     // do so anymore.
-    m_havingABadTimeWatchpoint->fireAll("Having a bad time");
+    m_havingABadTimeWatchpoint->fireAll(vm, "Having a bad time");
     ASSERT(isHavingABadTime()); // The watchpoint is what tells us that we're having a bad time.
     
     // Make sure that all JSArray allocations that load the appropriate structure from
index a7fd01f..c42d3f3 100644 (file)
@@ -146,7 +146,7 @@ ALWAYS_INLINE void symbolTablePutTouchWatchpointSet(VM& vm, SymbolTableObjectTyp
 {
     reg->set(vm, object, value);
     if (set)
-        VariableWriteFireDetail::touch(set, object, propertyName);
+        VariableWriteFireDetail::touch(vm, set, object, propertyName);
 }
 
 template<typename SymbolTableObjectType>
@@ -154,7 +154,7 @@ ALWAYS_INLINE void symbolTablePutInvalidateWatchpointSet(VM& vm, SymbolTableObje
 {
     reg->set(vm, object, value);
     if (set)
-        set->invalidate(VariableWriteFireDetail(object, propertyName)); // Don't mess around - if we had found this statically, we would have invalidated it.
+        set->invalidate(vm, VariableWriteFireDetail(object, propertyName)); // Don't mess around - if we had found this statically, we would have invalidated it.
 }
 
 enum class SymbolTablePutMode {
index 27ffa51..897b554 100644 (file)
@@ -858,7 +858,7 @@ void Structure::startWatchingPropertyForReplacements(VM& vm, PropertyName proper
 
 void Structure::didCachePropertyReplacement(VM& vm, PropertyOffset offset)
 {
-    ensurePropertyReplacementWatchpointSet(vm, offset)->fireAll("Did cache property replacement");
+    ensurePropertyReplacementWatchpointSet(vm, offset)->fireAll(vm, "Did cache property replacement");
 }
 
 void Structure::startWatchingInternalProperties(VM& vm)
@@ -1075,7 +1075,7 @@ DeferredStructureTransitionWatchpointFire::DeferredStructureTransitionWatchpoint
 DeferredStructureTransitionWatchpointFire::~DeferredStructureTransitionWatchpointFire()
 {
     if (m_structure)
-        m_structure->transitionWatchpointSet().fireAll(StructureFireDetail(m_structure));
+        m_structure->transitionWatchpointSet().fireAll(*m_structure->vm(), StructureFireDetail(m_structure));
 }
 
 void DeferredStructureTransitionWatchpointFire::add(const Structure* structure)
@@ -1096,7 +1096,7 @@ void Structure::didTransitionFromThisStructure(DeferredStructureTransitionWatchp
     if (deferred)
         deferred->add(this);
     else
-        m_transitionWatchpointSet.fireAll(StructureFireDetail(this));
+        m_transitionWatchpointSet.fireAll(*vm(), StructureFireDetail(this));
 }
 
 JSValue Structure::prototypeForLookup(CodeBlock* codeBlock) const
index 7e78329..b943b7e 100644 (file)
@@ -256,7 +256,7 @@ inline void Structure::didReplaceProperty(PropertyOffset offset)
     WatchpointSet* set = map->get(offset);
     if (LIKELY(!set))
         return;
-    set->fireAll("Property did get replaced");
+    set->fireAll(*vm(), "Property did get replaced");
 }
 
 inline WatchpointSet* Structure::propertyReplacementWatchpointSet(PropertyOffset offset)
index 80d1b88..e452f8d 100644 (file)
@@ -280,10 +280,10 @@ public:
         return bits() & DontEnumFlag;
     }
     
-    void disableWatching()
+    void disableWatching(VM& vm)
     {
         if (WatchpointSet* set = watchpointSet())
-            set->invalidate("Disabling watching in symbol table");
+            set->invalidate(vm, "Disabling watching in symbol table");
         if (varOffset().isScope())
             pack(varOffset(), false, isReadOnly(), isDontEnum());
     }
index 5b9bf10..d598951 100644 (file)
@@ -748,7 +748,7 @@ void VM::registerWatchpointForImpureProperty(const Identifier& propertyName, Wat
 void VM::addImpureProperty(const String& propertyName)
 {
     if (RefPtr<WatchpointSet> watchpointSet = m_impurePropertyWatchpointSets.take(propertyName))
-        watchpointSet->fireAll("Impure property added");
+        watchpointSet->fireAll(*this, "Impure property added");
 }
 
 static bool enableProfilerWithRespectToCount(unsigned& counter, std::function<void()> doEnableWork)
index 15d7b82..15180ef 100644 (file)
@@ -1,3 +1,18 @@
+2016-06-28  Saam Barati  <sbarati@apple.com>
+
+        some Watchpoints' ::fireInternal method will call operations that might GC where the GC will cause the watchpoint itself to destruct
+        https://bugs.webkit.org/show_bug.cgi?id=159198
+        <rdar://problem/26302360>
+
+        Reviewed by Filip Pizlo.
+
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::fireFrameClearedWatchpointsForWindow):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateHeader):
+        * bindings/scripts/test/JS/JSTestEventTarget.h:
+        (WebCore::JSTestEventTarget::create):
+
 2016-06-28  Anders Carlsson  <andersca@apple.com>
 
         Move the user gesture requirement to the ApplePaySession constructor
index d099349..9148d35 100644 (file)
@@ -318,7 +318,7 @@ void JSDOMWindowBase::fireFrameClearedWatchpointsForWindow(DOMWindow* window)
         if (!wrapper)
             continue;
         JSDOMWindowBase* jsWindow = JSC::jsCast<JSDOMWindowBase*>(wrapper);
-        jsWindow->m_windowCloseWatchpoints.fireAll("Frame cleared");
+        jsWindow->m_windowCloseWatchpoints.fireAll(vm, "Frame cleared");
     }
 }
 
index 9a4eb12..046e93e 100644 (file)
@@ -1126,7 +1126,7 @@ sub GenerateHeader
         AddIncludesForTypeInHeader($implType) unless $svgPropertyOrListPropertyType;
         push(@headerContent, "    static $className* create(JSC::Structure* structure, JSDOMGlobalObject* globalObject, Ref<$implType>&& impl)\n");
         push(@headerContent, "    {\n");
-        push(@headerContent, "        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll(\"Allocated masquerading object\");\n");
+        push(@headerContent, "        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll(globalObject->vm(), \"Allocated masquerading object\");\n");
         push(@headerContent, "        $className* ptr = new (NotNull, JSC::allocateCell<$className>(globalObject->vm().heap)) $className(structure, *globalObject, WTFMove(impl));\n");
         push(@headerContent, "        ptr->finishCreation(globalObject->vm());\n");
         push(@headerContent, "        return ptr;\n");
index 7cda824..28199a9 100644 (file)
@@ -31,7 +31,7 @@ public:
     typedef TestEventTarget DOMWrapped;
     static JSTestEventTarget* create(JSC::Structure* structure, JSDOMGlobalObject* globalObject, Ref<TestEventTarget>&& impl)
     {
-        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll("Allocated masquerading object");
+        globalObject->masqueradesAsUndefinedWatchpoint()->fireAll(globalObject->vm(), "Allocated masquerading object");
         JSTestEventTarget* ptr = new (NotNull, JSC::allocateCell<JSTestEventTarget>(globalObject->vm().heap)) JSTestEventTarget(structure, *globalObject, WTFMove(impl));
         ptr->finishCreation(globalObject->vm());
         return ptr;