Changeset 165005 in webkit


Ignore:
Timestamp:
Mar 3, 2014 1:39:21 PM (10 years ago)
Author:
mark.lam@apple.com
Message:

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.
  1. 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.
  1. 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:
Location:
trunk/Source/JavaScriptCore
Files:
16 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r164991 r165005  
     12014-03-03  Mark Lam  <mark.lam@apple.com>
     2
     3        ASSERTION FAILED: m_numBreakpoints >= numBreakpoints when deleting breakpoints.
     4        <https://webkit.org/b/129393>
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        The issue manifests because the debugger will iterate all CodeBlocks in
     9        the heap when setting / clearing breakpoints, but it is possible for a
     10        CodeBlock to have been instantiate but is not yet registered with the
     11        debugger.  This can happen because of the following:
     12
     13        1. DFG worklist compilation is still in progress, and the target
     14           codeBlock is not ready for installation in its executable yet.
     15
     16        2. DFG compilation failed and we have a codeBlock that will never be
     17           installed in its executable, and the codeBlock has not been cleaned
     18           up by the GC yet.
     19
     20        The code for installing the codeBlock in its executable is the same code
     21        that registers it with the debugger.  Hence, these codeBlocks are not
     22        registered with the debugger, and any pending breakpoints that would map
     23        to that CodeBlock is as yet unset or will never be set.  As such, an
     24        attempt to remove a breakpoint in that CodeBlock will fail that assertion.
     25
     26        To fix this, we do the following:
     27
     28        1. We'll eagerly clean up any zombie CodeBlocks due to failed DFG / FTL
     29           compilation.  This is achieved by providing a
     30           DeferredCompilationCallback::compilationDidComplete() that does this
     31           clean up, and have all sub classes call it at the end of their
     32           compilationDidComplete() methods.
     33
     34        2. Before the debugger or profiler iterates CodeBlocks in the heap, they
     35           will wait for all compilations to complete before proceeding.  This
     36           ensures that:
     37           1. any zombie CodeBlocks would have been cleaned up, and won't be
     38              seen by the debugger or profiler.
     39           2. all CodeBlocks that the debugger and profiler needs to operate on
     40              will be "ready" for whatever needs to be done to them e.g.
     41              jettison'ing of DFG codeBlocks.
     42
     43        * bytecode/DeferredCompilationCallback.cpp:
     44        (JSC::DeferredCompilationCallback::compilationDidComplete):
     45        * bytecode/DeferredCompilationCallback.h:
     46        - Provide default implementation method to clean up zombie CodeBlocks.
     47
     48        * debugger/Debugger.cpp:
     49        (JSC::Debugger::forEachCodeBlock):
     50        - Utility function to iterate CodeBlocks.  It ensures that all compilations
     51          are complete before proceeding.
     52        (JSC::Debugger::setSteppingMode):
     53        (JSC::Debugger::toggleBreakpoint):
     54        (JSC::Debugger::recompileAllJSFunctions):
     55        (JSC::Debugger::clearBreakpoints):
     56        (JSC::Debugger::clearDebuggerRequests):
     57        - Use the utility iterator function.
     58
     59        * debugger/Debugger.h:
     60        * dfg/DFGOperations.cpp:
     61        - Added an assert to ensure that zombie CodeBlocks will be imminently cleaned up.
     62
     63        * dfg/DFGPlan.cpp:
     64        (JSC::DFG::Plan::finalizeWithoutNotifyingCallback):
     65        - Remove unneeded code (that was not the best solution anyway) for ensuring
     66          that we don't generate new DFG codeBlocks after enabling the debugger or
     67          profiler.  Now that we wait for compilations to complete before proceeding
     68          with debugger and profiler work, this scenario will never happen.
     69
     70        * dfg/DFGToFTLDeferredCompilationCallback.cpp:
     71        (JSC::DFG::ToFTLDeferredCompilationCallback::compilationDidComplete):
     72        - Call the super class method to clean up zombie codeBlocks.
     73
     74        * dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:
     75        (JSC::DFG::ToFTLForOSREntryDeferredCompilationCallback::compilationDidComplete):
     76        - Call the super class method to clean up zombie codeBlocks.
     77
     78        * heap/CodeBlockSet.cpp:
     79        (JSC::CodeBlockSet::remove):
     80        * heap/CodeBlockSet.h:
     81        * heap/Heap.h:
     82        (JSC::Heap::removeCodeBlock):
     83        - New method to remove a codeBlock from the codeBlock set.
     84
     85        * jit/JITOperations.cpp:
     86        - Added an assert to ensure that zombie CodeBlocks will be imminently cleaned up.
     87
     88        * jit/JITToDFGDeferredCompilationCallback.cpp:
     89        (JSC::JITToDFGDeferredCompilationCallback::compilationDidComplete):
     90        - Call the super class method to clean up zombie codeBlocks.
     91
     92        * runtime/VM.cpp:
     93        (JSC::VM::waitForCompilationsToComplete):
     94        - Renamed from prepareToDiscardCode() to be clearer about what it does.
     95
     96        (JSC::VM::discardAllCode):
     97        (JSC::VM::releaseExecutableMemory):
     98        (JSC::VM::setEnabledProfiler):
     99        - Wait for compilation to complete before enabling the profiler.
     100
     101        * runtime/VM.h:
     102
    11032014-03-03  Brian Burg  <bburg@apple.com>
    2104

  • trunk/Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp

    r154824 r165005  
    11/*
    2  * Copyright (C) 2013 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2727#include "DeferredCompilationCallback.h"
    2828
     29#include "CodeBlock.h"
     30
    2931namespace JSC {
    3032
    … …  
    3234DeferredCompilationCallback::~DeferredCompilationCallback() { }
    3335
     36void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CompilationResult result)
     37{
     38    switch (result) {
     39    case CompilationFailed:
     40    case CompilationInvalidated:
     41        codeBlock->heap()->removeCodeBlock(codeBlock);
     42        break;
     43    case CompilationSuccessful:
     44        break;
     45    case CompilationDeferred:
     46        RELEASE_ASSERT_NOT_REACHED();
     47    }
     48}
     49
    3450} // JSC
    3551

  • trunk/Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h

    r154824 r165005  
    4242
    4343    virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*) = 0;
    44     virtual void compilationDidComplete(CodeBlock*, CompilationResult) = 0;
     44    virtual void compilationDidComplete(CodeBlock*, CompilationResult);
    4545};
    4646

  • trunk/Source/JavaScriptCore/debugger/Debugger.cpp

    r163844 r165005  
    139139};
    140140
     141template<typename Functor>
     142void Debugger::forEachCodeBlock(Functor& functor)
     143{
     144    m_vm->waitForCompilationsToComplete();
     145    m_vm->heap.forEachCodeBlock(functor);
     146}
     147
    141148Debugger::Debugger(bool isInWorkerThread)
    142149    : m_vm(nullptr)
    … …  
    233240        return;
    234241    SetSteppingModeFunctor functor(this, mode);
    235     m_vm->heap.forEachCodeBlock(functor);
     242    forEachCodeBlock(functor);
    236243}
    237244
    … …  
    317324        return;
    318325    ToggleBreakpointFunctor functor(this, breakpoint, enabledOrNot);
    319     m_vm->heap.forEachCodeBlock(functor);
     326    forEachCodeBlock(functor);
    320327}
    321328
    … …  
    329336    }
    330337
    331     vm->prepareToDiscardCode();
     338    vm->waitForCompilationsToComplete();
    332339
    333340    Recompiler recompiler(this);
    … …  
    497504        return;
    498505    ClearCodeBlockDebuggerRequestsFunctor functor(this);
    499     m_vm->heap.forEachCodeBlock(functor);
     506    forEachCodeBlock(functor);
    500507}
    501508
    … …  
    522529    ASSERT(m_vm);
    523530    ClearDebuggerRequestsFunctor functor(globalObject);
    524     m_vm->heap.forEachCodeBlock(functor);
     531    forEachCodeBlock(functor);
    525532}
    526533

  • trunk/Source/JavaScriptCore/debugger/Debugger.h

    r162940 r165005  
    183183    void clearDebuggerRequests(JSGlobalObject*);
    184184
     185    template<typename Functor> inline void forEachCodeBlock(Functor&);
     186
    185187    VM* m_vm;
    186188    HashSet<JSGlobalObject*> m_globalObjects;

  • trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

    r164764 r165005  
    12541254    jitCode->reconstruct(
    12551255        exec, codeBlock, CodeOrigin(bytecodeIndex), streamIndex, mustHandleValues);
     1256    RefPtr<CodeBlock> replacementCodeBlock = codeBlock->newReplacement();
    12561257    CompilationResult forEntryResult = compile(
    1257         *vm, codeBlock->newReplacement().get(), codeBlock, FTLForOSREntryMode, bytecodeIndex,
     1258        *vm, replacementCodeBlock.get(), codeBlock, FTLForOSREntryMode, bytecodeIndex,
    12581259        mustHandleValues, ToFTLForOSREntryDeferredCompilationCallback::create(codeBlock));
    12591260   
    1260     if (forEntryResult != CompilationSuccessful)
     1261    if (forEntryResult != CompilationSuccessful) {
     1262        ASSERT(forEntryResult == CompilationDeferred || replacementCodeBlock->hasOneRef());
    12611263        return 0;
    1262    
     1264    }
     1265
    12631266    // It's possible that the for-entry compile already succeeded. In that case OSR
    12641267    // entry will succeed unless we ran out of stack. It's not clear what we should do.

  • trunk/Source/JavaScriptCore/dfg/DFGPlan.cpp

    r164459 r165005  
    392392        return CompilationInvalidated;
    393393
    394     if (vm.enabledProfiler())
    395         return CompilationInvalidated;
    396 
    397     Debugger* debugger = codeBlock->globalObject()->debugger();
    398     if (debugger && (debugger->isStepping() || codeBlock->baselineAlternative()->hasDebuggerRequests()))
    399         return CompilationInvalidated;
    400 
    401394    bool result;
    402395    if (codeBlock->codeType() == FunctionCode)

  • trunk/Source/JavaScriptCore/dfg/DFGToFTLDeferredCompilationCallback.cpp

    r164229 r165005  
    8686    m_dfgCodeBlock->jitCode()->dfg()->setOptimizationThresholdBasedOnCompilationResult(
    8787        m_dfgCodeBlock.get(), result);
     88
     89    DeferredCompilationCallback::compilationDidComplete(codeBlock, result);
    8890}
    8991

  • trunk/Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp

    r164229 r165005  
    8080    case CompilationSuccessful:
    8181        jitCode->osrEntryBlock = codeBlock;
    82         return;
     82        break;
    8383    case CompilationFailed:
    8484        jitCode->osrEntryRetry = 0;
    8585        jitCode->abandonOSREntry = true;
    86         return;
     86        break;
    8787    case CompilationDeferred:
    88         return;
     88        RELEASE_ASSERT_NOT_REACHED();
    8989    case CompilationInvalidated:
    9090        jitCode->osrEntryRetry = 0;
    91         return;
     91        break;
    9292    }
    9393   
    94     RELEASE_ASSERT_NOT_REACHED();
     94    DeferredCompilationCallback::compilationDidComplete(codeBlock, result);
    9595}
    9696

  • trunk/Source/JavaScriptCore/heap/CodeBlockSet.cpp

    r163844 r165005  
    9797}
    9898
     99void CodeBlockSet::remove(CodeBlock* codeBlock)
     100{
     101    codeBlock->deref();
     102    m_set.remove(codeBlock);
     103}
     104
    99105void CodeBlockSet::traceMarked(SlotVisitor& visitor)
    100106{

  • trunk/Source/JavaScriptCore/heap/CodeBlockSet.h

    r163691 r165005  
    6666    void deleteUnmarkedAndUnreferenced();
    6767   
     68    void remove(CodeBlock*);
     69   
    6870    // Trace all marked code blocks. The CodeBlock is free to make use of
    6971    // mayBeExecuting.

  • trunk/Source/JavaScriptCore/heap/Heap.h

    r164862 r165005  
    209209        template<typename T> void releaseSoon(RetainPtr<T>&&);
    210210#endif
     211
     212        void removeCodeBlock(CodeBlock* cb) { m_codeBlocks.remove(cb); }
    211213
    212214    private:

  • trunk/Source/JavaScriptCore/jit/JITOperations.cpp

    r164764 r165005  
    12001200        }
    12011201
     1202        RefPtr<CodeBlock> replacementCodeBlock = codeBlock->newReplacement();
    12021203        CompilationResult result = DFG::compile(
    1203             vm, codeBlock->newReplacement().get(), 0, DFG::DFGMode, bytecodeIndex,
     1204            vm, replacementCodeBlock.get(), 0, DFG::DFGMode, bytecodeIndex,
    12041205            mustHandleValues, JITToDFGDeferredCompilationCallback::create());
    12051206       
    1206         if (result != CompilationSuccessful)
     1207        if (result != CompilationSuccessful) {
     1208            ASSERT(result == CompilationDeferred || replacementCodeBlock->hasOneRef());
    12071209            return encodeResult(0, 0);
     1210        }
    12081211    }
    12091212   

  • trunk/Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.cpp

    r163844 r165005  
    6666   
    6767    codeBlock->alternative()->setOptimizationThresholdBasedOnCompilationResult(result);
     68
     69    DeferredCompilationCallback::compilationDidComplete(codeBlock, result);
    6870}
    6971

  • trunk/Source/JavaScriptCore/runtime/VM.cpp

    r164683 r165005  
    513513}
    514514
    515 void VM::prepareToDiscardCode()
     515void VM::waitForCompilationsToComplete()
    516516{
    517517#if ENABLE(DFG_JIT)
    … …  
    525525void VM::discardAllCode()
    526526{
    527     prepareToDiscardCode();
     527    waitForCompilationsToComplete();
    528528    m_codeCache->clear();
    529529    heap.deleteAllCompiledCode();
    … …  
    567567void VM::releaseExecutableMemory()
    568568{
    569     prepareToDiscardCode();
     569    waitForCompilationsToComplete();
    570570   
    571571    if (entryScope) {
    … …  
    873873    m_enabledProfiler = profiler;
    874874    if (m_enabledProfiler) {
     875        waitForCompilationsToComplete();
    875876        SetEnabledProfilerFunctor functor;
    876877        heap.forEachCodeBlock(functor);

  • trunk/Source/JavaScriptCore/runtime/VM.h

    r164683 r165005  
    11/*
    2  * Copyright (C) 2008, 2009, 2013 Apple Inc. All rights reserved.
     2 * Copyright (C) 2008, 2009, 2013, 2014 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    499499        CodeCache* codeCache() { return m_codeCache.get(); }
    500500
    501         void prepareToDiscardCode();
     501        void waitForCompilationsToComplete();
    502502       
    503503        JS_EXPORT_PRIVATE void discardAllCode();
Note: See TracChangeset for help on using the changeset viewer.