Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (165004 => 165005)
--- trunk/Source/_javascript_Core/ChangeLog 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-03-03 21:39:21 UTC (rev 165005)
@@ -1,3 +1,105 @@
+2014-03-03 Mark Lam <mark....@apple.com>
+
+ ASSERTION FAILED: m_numBreakpoints >= numBreakpoints when deleting breakpoints.
+ <https://webkit.org/b/129393>
+
+ Reviewed by Geoffrey Garen.
+
+ The issue manifests because the debugger will iterate all CodeBlocks in
+ the heap when setting / clearing breakpoints, but it is possible for a
+ CodeBlock to have been instantiate but is not yet registered with the
+ debugger. This can happen because of the following:
+
+ 1. DFG worklist compilation is still in progress, and the target
+ codeBlock is not ready for installation in its executable yet.
+
+ 2. DFG compilation failed and we have a codeBlock that will never be
+ installed in its executable, and the codeBlock has not been cleaned
+ up by the GC yet.
+
+ The code for installing the codeBlock in its executable is the same code
+ that registers it with the debugger. Hence, these codeBlocks are not
+ registered with the debugger, and any pending breakpoints that would map
+ to that CodeBlock is as yet unset or will never be set. As such, an
+ attempt to remove a breakpoint in that CodeBlock will fail that assertion.
+
+ To fix this, we do the following:
+
+ 1. We'll eagerly clean up any zombie CodeBlocks due to failed DFG / FTL
+ compilation. This is achieved by providing a
+ DeferredCompilationCallback::compilationDidComplete() that does this
+ clean up, and have all sub classes call it at the end of their
+ compilationDidComplete() methods.
+
+ 2. Before the debugger or profiler iterates CodeBlocks in the heap, they
+ will wait for all compilations to complete before proceeding. This
+ ensures that:
+ 1. any zombie CodeBlocks would have been cleaned up, and won't be
+ seen by the debugger or profiler.
+ 2. all CodeBlocks that the debugger and profiler needs to operate on
+ will be "ready" for whatever needs to be done to them e.g.
+ jettison'ing of DFG codeBlocks.
+
+ * bytecode/DeferredCompilationCallback.cpp:
+ (JSC::DeferredCompilationCallback::compilationDidComplete):
+ * bytecode/DeferredCompilationCallback.h:
+ - Provide default implementation method to clean up zombie CodeBlocks.
+
+ * debugger/Debugger.cpp:
+ (JSC::Debugger::forEachCodeBlock):
+ - Utility function to iterate CodeBlocks. It ensures that all compilations
+ are complete before proceeding.
+ (JSC::Debugger::setSteppingMode):
+ (JSC::Debugger::toggleBreakpoint):
+ (JSC::Debugger::recompileAllJSFunctions):
+ (JSC::Debugger::clearBreakpoints):
+ (JSC::Debugger::clearDebuggerRequests):
+ - Use the utility iterator function.
+
+ * debugger/Debugger.h:
+ * dfg/DFGOperations.cpp:
+ - Added an assert to ensure that zombie CodeBlocks will be imminently cleaned up.
+
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::finalizeWithoutNotifyingCallback):
+ - Remove unneeded code (that was not the best solution anyway) for ensuring
+ that we don't generate new DFG codeBlocks after enabling the debugger or
+ profiler. Now that we wait for compilations to complete before proceeding
+ with debugger and profiler work, this scenario will never happen.
+
+ * dfg/DFGToFTLDeferredCompilationCallback.cpp:
+ (JSC::DFG::ToFTLDeferredCompilationCallback::compilationDidComplete):
+ - Call the super class method to clean up zombie codeBlocks.
+
+ * dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:
+ (JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidComplete):
+ - Call the super class method to clean up zombie codeBlocks.
+
+ * heap/CodeBlockSet.cpp:
+ (JSC::CodeBlockSet::remove):
+ * heap/CodeBlockSet.h:
+ * heap/Heap.h:
+ (JSC::Heap::removeCodeBlock):
+ - New method to remove a codeBlock from the codeBlock set.
+
+ * jit/JITOperations.cpp:
+ - Added an assert to ensure that zombie CodeBlocks will be imminently cleaned up.
+
+ * jit/JITToDFGDeferredCompilationCallback.cpp:
+ (JSC::JITToDFGDeferredCompilationCallback::compilationDidComplete):
+ - Call the super class method to clean up zombie codeBlocks.
+
+ * runtime/VM.cpp:
+ (JSC::VM::waitForCompilationsToComplete):
+ - Renamed from prepareToDiscardCode() to be clearer about what it does.
+
+ (JSC::VM::discardAllCode):
+ (JSC::VM::releaseExecutableMemory):
+ (JSC::VM::setEnabledProfiler):
+ - Wait for compilation to complete before enabling the profiler.
+
+ * runtime/VM.h:
+
2014-03-03 Brian Burg <bb...@apple.com>
Another unreviewed build fix attempt for Windows after r164986.
Modified: trunk/Source/_javascript_Core/bytecode/DeferredCompilationCallback.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/bytecode/DeferredCompilationCallback.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/bytecode/DeferredCompilationCallback.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -26,10 +26,26 @@
#include "config.h"
#include "DeferredCompilationCallback.h"
+#include "CodeBlock.h"
+
namespace JSC {
DeferredCompilationCallback::DeferredCompilationCallback() { }
DeferredCompilationCallback::~DeferredCompilationCallback() { }
+void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CompilationResult result)
+{
+ switch (result) {
+ case CompilationFailed:
+ case CompilationInvalidated:
+ codeBlock->heap()->removeCodeBlock(codeBlock);
+ break;
+ case CompilationSuccessful:
+ break;
+ case CompilationDeferred:
+ RELEASE_ASSERT_NOT_REACHED();
+ }
+}
+
} // JSC
Modified: trunk/Source/_javascript_Core/bytecode/DeferredCompilationCallback.h (165004 => 165005)
--- trunk/Source/_javascript_Core/bytecode/DeferredCompilationCallback.h 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/bytecode/DeferredCompilationCallback.h 2014-03-03 21:39:21 UTC (rev 165005)
@@ -41,7 +41,7 @@
virtual ~DeferredCompilationCallback();
virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*) = 0;
- virtual void compilationDidComplete(CodeBlock*, CompilationResult) = 0;
+ virtual void compilationDidComplete(CodeBlock*, CompilationResult);
};
} // namespace JSC
Modified: trunk/Source/_javascript_Core/debugger/Debugger.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/debugger/Debugger.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/debugger/Debugger.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -138,6 +138,13 @@
Debugger& m_debugger;
};
+template<typename Functor>
+void Debugger::forEachCodeBlock(Functor& functor)
+{
+ m_vm->waitForCompilationsToComplete();
+ m_vm->heap.forEachCodeBlock(functor);
+}
+
Debugger::Debugger(bool isInWorkerThread)
: m_vm(nullptr)
, m_pauseOnExceptionsState(DontPauseOnExceptions)
@@ -232,7 +239,7 @@
if (!m_vm)
return;
SetSteppingModeFunctor functor(this, mode);
- m_vm->heap.forEachCodeBlock(functor);
+ forEachCodeBlock(functor);
}
void Debugger::registerCodeBlock(CodeBlock* codeBlock)
@@ -316,7 +323,7 @@
if (!m_vm)
return;
ToggleBreakpointFunctor functor(this, breakpoint, enabledOrNot);
- m_vm->heap.forEachCodeBlock(functor);
+ forEachCodeBlock(functor);
}
void Debugger::recompileAllJSFunctions(VM* vm)
@@ -328,7 +335,7 @@
return;
}
- vm->prepareToDiscardCode();
+ vm->waitForCompilationsToComplete();
Recompiler recompiler(this);
HeapIterationScope iterationScope(vm->heap);
@@ -496,7 +503,7 @@
if (!m_vm)
return;
ClearCodeBlockDebuggerRequestsFunctor functor(this);
- m_vm->heap.forEachCodeBlock(functor);
+ forEachCodeBlock(functor);
}
class Debugger::ClearDebuggerRequestsFunctor {
@@ -521,7 +528,7 @@
{
ASSERT(m_vm);
ClearDebuggerRequestsFunctor functor(globalObject);
- m_vm->heap.forEachCodeBlock(functor);
+ forEachCodeBlock(functor);
}
void Debugger::setBreakpointsActivated(bool activated)
Modified: trunk/Source/_javascript_Core/debugger/Debugger.h (165004 => 165005)
--- trunk/Source/_javascript_Core/debugger/Debugger.h 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/debugger/Debugger.h 2014-03-03 21:39:21 UTC (rev 165005)
@@ -182,6 +182,8 @@
void clearDebuggerRequests(JSGlobalObject*);
+ template<typename Functor> inline void forEachCodeBlock(Functor&);
+
VM* m_vm;
HashSet<JSGlobalObject*> m_globalObjects;
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -1253,13 +1253,16 @@
Operands<JSValue> mustHandleValues;
jitCode->reconstruct(
exec, codeBlock, CodeOrigin(bytecodeIndex), streamIndex, mustHandleValues);
+ RefPtr<CodeBlock> replacementCodeBlock = codeBlock->newReplacement();
CompilationResult forEntryResult = compile(
- *vm, codeBlock->newReplacement().get(), codeBlock, FTLForOSREntryMode, bytecodeIndex,
+ *vm, replacementCodeBlock.get(), codeBlock, FTLForOSREntryMode, bytecodeIndex,
mustHandleValues, ToFTLForOSREntryDeferredCompilationCallback::create(codeBlock));
- if (forEntryResult != CompilationSuccessful)
+ if (forEntryResult != CompilationSuccessful) {
+ ASSERT(forEntryResult == CompilationDeferred || replacementCodeBlock->hasOneRef());
return 0;
-
+ }
+
// It's possible that the for-entry compile already succeeded. In that case OSR
// entry will succeed unless we ran out of stack. It's not clear what we should do.
// We signal to try again after a while if that happens.
Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -391,13 +391,6 @@
if (!isStillValid())
return CompilationInvalidated;
- if (vm.enabledProfiler())
- return CompilationInvalidated;
-
- Debugger* debugger = codeBlock->globalObject()->debugger();
- if (debugger && (debugger->isStepping() || codeBlock->baselineAlternative()->hasDebuggerRequests()))
- return CompilationInvalidated;
-
bool result;
if (codeBlock->codeType() == FunctionCode)
result = finalizer->finalizeFunction();
Modified: trunk/Source/_javascript_Core/dfg/DFGToFTLDeferredCompilationCallback.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/dfg/DFGToFTLDeferredCompilationCallback.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/dfg/DFGToFTLDeferredCompilationCallback.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -85,6 +85,8 @@
m_dfgCodeBlock->jitCode()->dfg()->setOptimizationThresholdBasedOnCompilationResult(
m_dfgCodeBlock.get(), result);
+
+ DeferredCompilationCallback::compilationDidComplete(codeBlock, result);
}
} } // JSC::DFG
Modified: trunk/Source/_javascript_Core/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -79,19 +79,19 @@
switch (result) {
case CompilationSuccessful:
jitCode->osrEntryBlock = codeBlock;
- return;
+ break;
case CompilationFailed:
jitCode->osrEntryRetry = 0;
jitCode->abandonOSREntry = true;
- return;
+ break;
case CompilationDeferred:
- return;
+ RELEASE_ASSERT_NOT_REACHED();
case CompilationInvalidated:
jitCode->osrEntryRetry = 0;
- return;
+ break;
}
- RELEASE_ASSERT_NOT_REACHED();
+ DeferredCompilationCallback::compilationDidComplete(codeBlock, result);
}
} } // JSC::DFG
Modified: trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -96,6 +96,12 @@
}
}
+void CodeBlockSet::remove(CodeBlock* codeBlock)
+{
+ codeBlock->deref();
+ m_set.remove(codeBlock);
+}
+
void CodeBlockSet::traceMarked(SlotVisitor& visitor)
{
if (verbose)
Modified: trunk/Source/_javascript_Core/heap/CodeBlockSet.h (165004 => 165005)
--- trunk/Source/_javascript_Core/heap/CodeBlockSet.h 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/heap/CodeBlockSet.h 2014-03-03 21:39:21 UTC (rev 165005)
@@ -65,6 +65,8 @@
// by this set), and that have not been marked.
void deleteUnmarkedAndUnreferenced();
+ void remove(CodeBlock*);
+
// Trace all marked code blocks. The CodeBlock is free to make use of
// mayBeExecuting.
void traceMarked(SlotVisitor&);
Modified: trunk/Source/_javascript_Core/heap/Heap.h (165004 => 165005)
--- trunk/Source/_javascript_Core/heap/Heap.h 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2014-03-03 21:39:21 UTC (rev 165005)
@@ -209,6 +209,8 @@
template<typename T> void releaseSoon(RetainPtr<T>&&);
#endif
+ void removeCodeBlock(CodeBlock* cb) { m_codeBlocks.remove(cb); }
+
private:
friend class CodeBlock;
friend class CopiedBlock;
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -1199,12 +1199,15 @@
mustHandleValues[i] = exec->uncheckedR(operand).jsValue();
}
+ RefPtr<CodeBlock> replacementCodeBlock = codeBlock->newReplacement();
CompilationResult result = DFG::compile(
- vm, codeBlock->newReplacement().get(), 0, DFG::DFGMode, bytecodeIndex,
+ vm, replacementCodeBlock.get(), 0, DFG::DFGMode, bytecodeIndex,
mustHandleValues, JITToDFGDeferredCompilationCallback::create());
- if (result != CompilationSuccessful)
+ if (result != CompilationSuccessful) {
+ ASSERT(result == CompilationDeferred || replacementCodeBlock->hasOneRef());
return encodeResult(0, 0);
+ }
}
CodeBlock* optimizedCodeBlock = codeBlock->replacement();
Modified: trunk/Source/_javascript_Core/jit/JITToDFGDeferredCompilationCallback.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/jit/JITToDFGDeferredCompilationCallback.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/jit/JITToDFGDeferredCompilationCallback.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -65,6 +65,8 @@
codeBlock->install();
codeBlock->alternative()->setOptimizationThresholdBasedOnCompilationResult(result);
+
+ DeferredCompilationCallback::compilationDidComplete(codeBlock, result);
}
} // JSC
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (165004 => 165005)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2014-03-03 21:39:21 UTC (rev 165005)
@@ -512,7 +512,7 @@
interpreter->stopSampling();
}
-void VM::prepareToDiscardCode()
+void VM::waitForCompilationsToComplete()
{
#if ENABLE(DFG_JIT)
for (unsigned i = DFG::numberOfWorklists(); i--;) {
@@ -524,7 +524,7 @@
void VM::discardAllCode()
{
- prepareToDiscardCode();
+ waitForCompilationsToComplete();
m_codeCache->clear();
heap.deleteAllCompiledCode();
heap.reportAbandonedObjectGraph();
@@ -566,7 +566,7 @@
void VM::releaseExecutableMemory()
{
- prepareToDiscardCode();
+ waitForCompilationsToComplete();
if (entryScope) {
StackPreservingRecompiler recompiler;
@@ -872,6 +872,7 @@
{
m_enabledProfiler = profiler;
if (m_enabledProfiler) {
+ waitForCompilationsToComplete();
SetEnabledProfilerFunctor functor;
heap.forEachCodeBlock(functor);
}
Modified: trunk/Source/_javascript_Core/runtime/VM.h (165004 => 165005)
--- trunk/Source/_javascript_Core/runtime/VM.h 2014-03-03 21:33:17 UTC (rev 165004)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2014-03-03 21:39:21 UTC (rev 165005)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008, 2009, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2009, 2013, 2014 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -498,7 +498,7 @@
JSLock& apiLock() { return *m_apiLock; }
CodeCache* codeCache() { return m_codeCache.get(); }
- void prepareToDiscardCode();
+ void waitForCompilationsToComplete();
JS_EXPORT_PRIVATE void discardAllCode();
