Changeset 183161 in webkit


Ignore:
Timestamp:
Apr 22, 2015, 7:29:14 PM (10 years ago)
Author:
mark.lam@apple.com
Message:

Fix assertion failure and race condition in Options::dumpSourceAtDFGTime().
​https://bugs.webkit.org/show_bug.cgi?id=143898

Reviewed by Filip Pizlo.

CodeBlock::dumpSource() will access SourceCode strings in a way that requires
ref'ing of the underlying StringImpls. This is unsafe to do from arbitrary
compilation threads because StringImpls are not thread safe. As a result, we get
an assertion failure when we run with JSC_dumpSourceAtDFGTime=true on a debug
build.

This patch fixes the issue by only collecting the CodeBlock (and associated info)
into a DeferredSourceDump record while compiling, and stashing it away in a
deferredSourceDump list in the DeferredCompilationCallback object to be dumped
later.

When compilation is done, the callback object will be notified that
compilationDidComplete(). We will dump the SourceCode strings from there.
Since compilationDidComplete() is guaranteed to only be called on the thread
doing JS execution, it is safe to access the SourceCode strings there and ref
their underlying StringImpls as needed.

(JSC::DeferredCompilationCallback::compilationDidComplete):
(JSC::DeferredCompilationCallback::sourceDumpInfo):
(JSC::DeferredCompilationCallback::dumpCompiledSources):

  • bytecode/DeferredCompilationCallback.h:
  • bytecode/DeferredSourceDump.cpp: Added.

(JSC::DeferredSourceDump::DeferredSourceDump):
(JSC::DeferredSourceDump::dump):

  • bytecode/DeferredSourceDump.h: Added.
  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::parseCodeBlock):

  • dfg/DFGDriver.cpp:

(JSC::DFG::compileImpl):

Location:
trunk/Source/JavaScriptCore
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/CMakeLists.txt

    r183072 r183161  
    8181    bytecode/DFGExitProfile.cpp
    8282    bytecode/DeferredCompilationCallback.cpp
     83    bytecode/DeferredSourceDump.cpp
    8384    bytecode/ExecutionCounter.cpp
    8485    bytecode/ExitKind.cpp

  • trunk/Source/JavaScriptCore/ChangeLog

    r183141 r183161  
     12015-04-22  Mark Lam  <mark.lam@apple.com>
     2
     3        Fix assertion failure and race condition in Options::dumpSourceAtDFGTime().
     4        https://bugs.webkit.org/show_bug.cgi?id=143898
     5
     6        Reviewed by Filip Pizlo.
     7
     8        CodeBlock::dumpSource() will access SourceCode strings in a way that requires
     9        ref'ing of the underlying StringImpls. This is unsafe to do from arbitrary
     10        compilation threads because StringImpls are not thread safe. As a result, we get
     11        an assertion failure when we run with JSC_dumpSourceAtDFGTime=true on a debug
     12        build.
     13
     14        This patch fixes the issue by only collecting the CodeBlock (and associated info)
     15        into a DeferredSourceDump record while compiling, and stashing it away in a
     16        deferredSourceDump list in the DeferredCompilationCallback object to be dumped
     17        later.
     18
     19        When compilation is done, the callback object will be notified that
     20        compilationDidComplete().  We will dump the SourceCode strings from there.
     21        Since compilationDidComplete() is guaranteed to only be called on the thread
     22        doing JS execution, it is safe to access the SourceCode strings there and ref
     23        their underlying StringImpls as needed.       
     24
     25        * CMakeLists.txt:
     26        * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:
     27        * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:
     28        * JavaScriptCore.xcodeproj/project.pbxproj:
     29        * bytecode/DeferredCompilationCallback.cpp:
     30        (JSC::DeferredCompilationCallback::compilationDidComplete):
     31        (JSC::DeferredCompilationCallback::sourceDumpInfo):
     32        (JSC::DeferredCompilationCallback::dumpCompiledSources):
     33        * bytecode/DeferredCompilationCallback.h:
     34        * bytecode/DeferredSourceDump.cpp: Added.
     35        (JSC::DeferredSourceDump::DeferredSourceDump):
     36        (JSC::DeferredSourceDump::dump):
     37        * bytecode/DeferredSourceDump.h: Added.
     38        * dfg/DFGByteCodeParser.cpp:
     39        (JSC::DFG::ByteCodeParser::parseCodeBlock):
     40        * dfg/DFGDriver.cpp:
     41        (JSC::DFG::compileImpl):
     42
    1432015-04-22  Benjamin Poulain  <benjamin@webkit.org>
    244

  • trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj

    r183124 r183161  
    327327    <ClCompile Include="..\bytecode\ConstantStructureCheck.cpp" />
    328328    <ClCompile Include="..\bytecode\DeferredCompilationCallback.cpp" />
     329    <ClCompile Include="..\bytecode\DeferredSourceDump.cpp" />
    329330    <ClCompile Include="..\bytecode\DFGExitProfile.cpp" />
    330331    <ClCompile Include="..\bytecode\ExecutionCounter.cpp" />
    … …  
    978979    <ClInclude Include="..\bytecode\DataFormat.h" />
    979980    <ClInclude Include="..\bytecode\DeferredCompilationCallback.h" />
     981    <ClInclude Include="..\bytecode\DeferredSourceDump.h" />
    980982    <ClInclude Include="..\bytecode\DFGExitProfile.h" />
    981983    <ClInclude Include="..\bytecode\EvalCodeCache.h" />

  • trunk/Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters

    r183124 r183161  
    10481048      <Filter>bytecode</Filter>
    10491049    </ClCompile>
     1050    <ClCompile Include="..\bytecode\DeferredSourceDump.cpp">
     1051      <Filter>bytecode</Filter>
     1052    </ClCompile>
    10501053    <ClCompile Include="..\dfg\DFGCompilationKey.cpp">
    10511054      <Filter>dfg</Filter>
    … …  
    33173320    </ClInclude>
    33183321    <ClInclude Include="..\bytecode\DeferredCompilationCallback.h">
     3322      <Filter>bytecode</Filter>
     3323    </ClInclude>
     3324    <ClInclude Include="..\bytecode\DeferredSourceDump.h">
    33193325      <Filter>bytecode</Filter>
    33203326    </ClInclude>

  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r183124 r183161  
    16621662                FE4BFF2C1AD476E700088F87 /* FunctionOverrides.h in Headers */ = {isa = PBXBuildFile; fileRef = FE4BFF2A1AD476E700088F87 /* FunctionOverrides.h */; };
    16631663                FE4D55B81AE716CA0052E459 /* IterationStatus.h in Headers */ = {isa = PBXBuildFile; fileRef = FE4D55B71AE716CA0052E459 /* IterationStatus.h */; settings = {ATTRIBUTES = (Private, ); }; };
     1664                FE5068651AE246390009DAB7 /* DeferredSourceDump.h in Headers */ = {isa = PBXBuildFile; fileRef = FE5068641AE246390009DAB7 /* DeferredSourceDump.h */; settings = {ATTRIBUTES = (Private, ); }; };
     1665                FE5068671AE25E280009DAB7 /* DeferredSourceDump.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FE5068661AE25E280009DAB7 /* DeferredSourceDump.cpp */; };
    16641666                FE5932A7183C5A2600A1ECCC /* VMEntryScope.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FE5932A5183C5A2600A1ECCC /* VMEntryScope.cpp */; };
    16651667                FE5932A8183C5A2600A1ECCC /* VMEntryScope.h in Headers */ = {isa = PBXBuildFile; fileRef = FE5932A6183C5A2600A1ECCC /* VMEntryScope.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    31833185                A7C1EAEA17987AB600299DB2 /* CallFrameInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CallFrameInlines.h; sourceTree = "<group>"; };
    31843186                A7C1EAEB17987AB600299DB2 /* JSStackInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSStackInlines.h; sourceTree = "<group>"; };
    3185                 A7C1EAEC17987AB600299DB2 /* StackVisitor.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = StackVisitor.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
     3187                A7C1EAEC17987AB600299DB2 /* StackVisitor.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = StackVisitor.cpp; sourceTree = "<group>"; };
    31863188                A7C1EAED17987AB600299DB2 /* StackVisitor.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StackVisitor.h; sourceTree = "<group>"; };
    31873189                A7C225CC139981F100FF1662 /* KeywordLookupGenerator.py */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.script.python; path = KeywordLookupGenerator.py; sourceTree = "<group>"; };
    … …  
    34553457                FE4BFF2A1AD476E700088F87 /* FunctionOverrides.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FunctionOverrides.h; sourceTree = "<group>"; };
    34563458                FE4D55B71AE716CA0052E459 /* IterationStatus.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IterationStatus.h; sourceTree = "<group>"; };
     3459                FE5068641AE246390009DAB7 /* DeferredSourceDump.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DeferredSourceDump.h; sourceTree = "<group>"; };
     3460                FE5068661AE25E280009DAB7 /* DeferredSourceDump.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DeferredSourceDump.cpp; sourceTree = "<group>"; };
    34573461                FE5932A5183C5A2600A1ECCC /* VMEntryScope.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = VMEntryScope.cpp; sourceTree = "<group>"; };
    34583462                FE5932A6183C5A2600A1ECCC /* VMEntryScope.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = VMEntryScope.h; sourceTree = "<group>"; };
    … …  
    53075311                                1429D8830ED21C3D00B89619 /* SamplingTool.cpp */,
    53085312                                1429D8840ED21C3D00B89619 /* SamplingTool.h */,
     5313                                FE5068661AE25E280009DAB7 /* DeferredSourceDump.cpp */,
     5314                                FE5068641AE246390009DAB7 /* DeferredSourceDump.h */,
    53095315                                0F5541AF1613C1FB00CE3E25 /* SpecialPointer.cpp */,
    53105316                                0F5541B01613C1FB00CE3E25 /* SpecialPointer.h */,
    … …  
    55935599                                52C0611F1AA51E1C00B4ADBA /* RuntimeType.h in Headers */,
    55945600                                FE4D55B81AE716CA0052E459 /* IterationStatus.h in Headers */,
     5601                                FE5068651AE246390009DAB7 /* DeferredSourceDump.h in Headers */,
    55955602                                C442CB251A6CDB8C005D3D7C /* JSInputs.json in Headers */,
    55965603                                52678F911A04177C006A306D /* ControlFlowProfiler.h in Headers */,
    … …  
    71237130                                0FD82E56141DAF0800179C94 /* DFGOSREntry.cpp in Sources */,
    71247131                                0FD8A32517D51F5700CA2C40 /* DFGOSREntrypointCreationPhase.cpp in Sources */,
     7132                                FE5068671AE25E280009DAB7 /* DeferredSourceDump.cpp in Sources */,
    71257133                                0FC09791146A6F7100CF2442 /* DFGOSRExit.cpp in Sources */,
    71267134                                0F235BEB17178E7300690C7F /* DFGOSRExitBase.cpp in Sources */,

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

    r165005 r183161  
    3636void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CompilationResult result)
    3737{
     38    dumpCompiledSourcesIfNeeded();
     39
    3840    switch (result) {
    3941    case CompilationFailed:
    … …  
    4850}
    4951
     52Vector<DeferredSourceDump>& DeferredCompilationCallback::ensureDeferredSourceDump()
     53{
     54    if (!m_deferredSourceDump)
     55        m_deferredSourceDump = std::make_unique<Vector<DeferredSourceDump>>();
     56    return *m_deferredSourceDump;
     57}
     58
     59void DeferredCompilationCallback::dumpCompiledSourcesIfNeeded()
     60{
     61    if (!m_deferredSourceDump)
     62        return;
     63
     64    ASSERT(Options::dumpSourceAtDFGTime());
     65    unsigned index = 0;
     66    for (auto& info : *m_deferredSourceDump) {
     67        dataLog("[", ++index, "] ");
     68        info.dump();
     69    }
     70}
     71
    5072} // JSC
    5173

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

    r165005 r183161  
    2828
    2929#include "CompilationResult.h"
     30#include "DeferredSourceDump.h"
    3031#include <wtf/RefCounted.h>
     32#include <wtf/Vector.h>
    3133
    3234namespace JSC {
    … …  
    4345    virtual void compilationDidBecomeReadyAsynchronously(CodeBlock*) = 0;
    4446    virtual void compilationDidComplete(CodeBlock*, CompilationResult);
     47
     48    Vector<DeferredSourceDump>& ensureDeferredSourceDump();
     49
     50private:
     51    void dumpCompiledSourcesIfNeeded();
     52
     53    std::unique_ptr<Vector<DeferredSourceDump>> m_deferredSourceDump;
    4554};
    4655

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

    r183094 r183161  
    40524052    }
    40534053   
    4054     bool shouldDumpSource = Options::dumpSourceAtDFGTime();
    4055     bool shouldDumpBytecode = Options::dumpBytecodeAtDFGTime();
    4056     if (shouldDumpSource || shouldDumpBytecode) {
     4054    if (UNLIKELY(Options::dumpSourceAtDFGTime())) {
     4055        Vector<DeferredSourceDump>& deferredSourceDump = m_graph.m_plan.callback->ensureDeferredSourceDump();
     4056        if (inlineCallFrame()) {
     4057            DeferredSourceDump dump(codeBlock->baselineVersion(), m_codeBlock, JITCode::DFGJIT, inlineCallFrame()->caller);
     4058            deferredSourceDump.append(dump);
     4059        } else
     4060            deferredSourceDump.append(DeferredSourceDump(codeBlock->baselineVersion()));
     4061    }
     4062
     4063    if (Options::dumpBytecodeAtDFGTime()) {
    40574064        dataLog("Parsing ", *codeBlock);
    40584065        if (inlineCallFrame()) {
    … …  
    40644071            ": needsActivation = ", codeBlock->needsActivation(),
    40654072            ", isStrictMode = ", codeBlock->ownerExecutable()->isStrictMode(), "\n");
    4066     }
    4067 
    4068     if (shouldDumpSource) {
    4069         dataLog("==== begin source ====\n");
    4070         codeBlock->baselineVersion()->dumpSource();
    4071         dataLog("\n==== end source ====\n\n");
    4072     }
    4073    
    4074     if (shouldDumpBytecode)
    40754073        codeBlock->baselineVersion()->dumpBytecode();
     4074    }
    40764075   
    40774076    Vector<unsigned, 32> jumpTargets;

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

    r180956 r183161  
    102102        new Plan(codeBlock, profiledDFGCodeBlock, mode, osrEntryBytecodeIndex, mustHandleValues));
    103103   
     104    plan->callback = callback;
    104105    if (Options::enableConcurrentJIT()) {
    105106        Worklist* worklist = ensureGlobalWorklistFor(mode);
    106         plan->callback = callback;
    107107        if (logCompilationChanges(mode))
    108108            dataLog("Deferring DFG compilation of ", *codeBlock, " with queue length ", worklist->queueLength(), ".\n");
Note: See TracChangeset for help on using the changeset viewer.