Bug 131747

Summary: Crash in CodeBlock::setOptimizationThresholdBasedOnCompilationResult() when the debugger activates
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, mmirman, msaboff, oliver, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch
none
patch 2: with bug for fixme. fpizlo: review+

Description Mark Lam 2014-04-16 11:03:42 PDT
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::addBreakpoint() and/or CodeBlock::setSteppingMode() which may jettison the DFG codeBlock that we’re trying to install.  Thereafter, chaos ensues.
Comment 1 Mark Lam 2014-04-16 11:04:42 PDT
<rdar://problem/16278811>
Comment 2 Mark Lam 2014-04-16 14:57:45 PDT
Some notes about the fix I will post soon:

1. Debugger::registerCodeBlock() eventually calls:
    a. CodeBlock::addBreakpoint()
    b. CodeBlock::setSteppingMode().

    These 2 functions can jettison the code block, and this poses a problem if that happens during installation of said code block.

2. operationOptimize() will check if the debugger is stepping mode, or if the CodeBlock has any pending breakpoints before allowing a code block to be DFG compiled.

    If the base code block already has breakpoints enabled in it, we'll never optimize that code block, and hence, we'll never get to the scenario where we'll jettison the DFG code block for the reason of it having active breakpoints.

    If the debugger is already in stepping mode, we'll never optimize that code block, and hence, we'll never get to the scenario where we'll jettison the DFG code block for the reason of the debugger being in stepping mode.

    Which leaves ... 

3. What happens if a DFG compilation is already in progress in a compiler thread and the debugger switches to stepping mode.

    The debugger is supposed to wait for all compilations to complete before switching to stepping mode.  However, currently, the debugger is setting the stepping mode flag before compilation completes.  This is the root cause of this bug.

4. What happens if a DFG compilation is already in progress in a compiler thread and the debugger adds a new breakpoint to the function for that code block.

    The debugger is supposed to wait for all compilations to complete before it applies the new breakpoint to the code blocks.  And the debugger does behave correctly here (in Debugger::toggleBreakpoint()).
Comment 3 Mark Lam 2014-04-16 15:20:13 PDT
Created attachment 229486 [details]
the patch
Comment 4 Filip Pizlo 2014-04-16 15:43:35 PDT
Comment on attachment 229486 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229486&action=review

> Source/JavaScriptCore/debugger/Debugger.cpp:250
> +    // 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.

Can you file a bugzilla bug for this and reference it here?
Comment 5 Mark Lam 2014-04-16 15:50:31 PDT
Created attachment 229490 [details]
patch 2: with bug for fixme.
Comment 6 Mark Lam 2014-04-16 16:11:26 PDT
Landed in r167396: <http://trac.webkit.org/r167396>.