Changeset 167396 in webkit

Timestamp:

Apr 16, 2014 4:07:49 PM (10 years ago)

Author:

mark.lam@apple.com

Message:

Crash in CodeBlock::setOptimizationThresholdBasedOnCompilationResult() when the debugger activates.
<​https://webkit.org/b/131747>

Reviewed by Filip Pizlo.

When the debugger is about to activate (e.g. enter stepping mode), it first
waits for all DFG compilations to complete. However, when the DFG completes,
if compilation is successful, it will install a new DFG codeBlock. The
CodeBlock installation process is required to register codeBlocks with the
debugger. Debugger::registerCodeBlock() will eventually call
CodeBlock::setSteppingMode() which may jettison the DFG codeBlock that we're
trying to install. Thereafter, chaos ensues.

This jettison'ing only happens because the debugger currently set its
m_steppingMode flag before waiting for compilation to complete. The fix is
simply to set that flag only after compilation is complete.

• debugger/Debugger.cpp:

(JSC::Debugger::setSteppingMode):
(JSC::Debugger::registerCodeBlock):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• debugger/Debugger.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-04-16  Mark Lam  <mark.lam@apple.com>
+
+        Crash in CodeBlock::setOptimizationThresholdBasedOnCompilationResult() when the debugger activates.
+        <https://webkit.org/b/131747>
+
+        Reviewed by Filip Pizlo.
+
+        When the debugger is about to activate (e.g. enter stepping mode), it first
+        waits for all DFG compilations to complete.  However, when the DFG completes,
+        if compilation is successful, it will install a new DFG codeBlock.  The
+        CodeBlock installation process is required to register codeBlocks with the
+        debugger.  Debugger::registerCodeBlock() will eventually call
+        CodeBlock::setSteppingMode() which may jettison the DFG codeBlock that we're
+        trying to install.  Thereafter, chaos ensues.
+
+        This jettison'ing only happens because the debugger currently set its
+        m_steppingMode flag before waiting for compilation to complete.  The fix is
+        simply to set that flag only after compilation is complete.
+
+        * debugger/Debugger.cpp:
+        (JSC::Debugger::setSteppingMode):
+        (JSC::Debugger::registerCodeBlock):
+
 2014-04-16  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/debugger/Debugger.cpp

@@ -233 +233 @@
 void Debugger::setSteppingMode(SteppingMode mode)
 {
-    if (mode == m_steppingMode)
-        return;
+    if (mode == m_steppingMode || !m_vm)
+        return;
+
+    m_vm->waitForCompilationsToComplete();
+
     m_steppingMode = mode;
-
-    if (!m_vm)
-        return;
     SetSteppingModeFunctor functor(this, mode);
-    forEachCodeBlock(functor);
+    m_vm->heap.forEachCodeBlock(functor);
 }
 
 void Debugger::registerCodeBlock(CodeBlock* codeBlock)
 {
+    // FIXME: We should never have to jettison a code block (due to pending breakpoints
+    // or stepping mode) that is being registered. operationOptimize() should have
+    // prevented the optimizing of such code blocks in the first place. Find a way to
+    // express this with greater clarity in the code. See <https://webkit.org/b131771>.
     applyBreakpoints(codeBlock);
     if (isStepping())