Title: [100363] trunk/Source/_javascript_Core
Revision
100363
Author
fpi...@apple.com
Date
2011-11-15 16:26:30 -0800 (Tue, 15 Nov 2011)

Log Message

Crash in JSC::DFG::OSRExitCompiler::compileExit(JSC::DFG::OSRExit const&, JSC::DFG::SpeculationRecovery*)
https://bugs.webkit.org/show_bug.cgi?id=72292

Reviewed by Geoff Garen.
        
We need to be careful about how we look for the baseline CodeBlock if we're lazy-compiling
an OSR exit after our CodeBlock has been jettisoned. In short, use CodeBlock::baselineVersion()
instead of CodeBlock::alternative().
        
No performance effect.
        
No tests because all of our heuristics work very hard to make sure that this never happens in
the first place. OSR exits are rare by design, and jettisoning of CodeBlocks (i.e. recompilation)
is even rarer. Furthermore, OSR exits after a CodeBlock has been jettisoned is rarer still
because the whole point of jettisoning is to bring the probability of future OSR exits to as
close to zero as possible. But even that isn't enough to trigger this bug; it requires the OSR
exit after a jettison to be the first of its kind; our whole design tries to ensure that
CodeBlocks tend to OSR exit at a handful (i.e. 1 in most cases) of points, and since jettisoning
is triggered by OSR, in most sane cases the OSR exits after jettison will not require lazy OSR
compilation. So this is a truly evil case, and any test for it would be quite fragile.

* bytecode/CodeBlock.h:
(JSC::CodeBlock::specializationKind):
(JSC::CodeBlock::largeFailCountThreshold):
(JSC::CodeBlock::largeFailCountThresholdForLoop):
* dfg/DFGAssemblyHelpers.h:
(JSC::DFG::AssemblyHelpers::AssemblyHelpers):
(JSC::DFG::AssemblyHelpers::baselineCodeBlockFor):
(JSC::DFG::AssemblyHelpers::baselineCodeBlock):
* dfg/DFGDriver.cpp:
(JSC::DFG::compile):
* dfg/DFGOSRExitCompiler.cpp:
* dfg/DFGOSRExitCompiler64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (100362 => 100363)


--- trunk/Source/_javascript_Core/ChangeLog	2011-11-16 00:26:15 UTC (rev 100362)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-11-16 00:26:30 UTC (rev 100363)
@@ -1,3 +1,40 @@
+2011-11-15  Filip Pizlo  <fpi...@apple.com>
+
+        Crash in JSC::DFG::OSRExitCompiler::compileExit(JSC::DFG::OSRExit const&, JSC::DFG::SpeculationRecovery*)
+        https://bugs.webkit.org/show_bug.cgi?id=72292
+
+        Reviewed by Geoff Garen.
+        
+        We need to be careful about how we look for the baseline CodeBlock if we're lazy-compiling
+        an OSR exit after our CodeBlock has been jettisoned. In short, use CodeBlock::baselineVersion()
+        instead of CodeBlock::alternative().
+        
+        No performance effect.
+        
+        No tests because all of our heuristics work very hard to make sure that this never happens in
+        the first place. OSR exits are rare by design, and jettisoning of CodeBlocks (i.e. recompilation)
+        is even rarer. Furthermore, OSR exits after a CodeBlock has been jettisoned is rarer still
+        because the whole point of jettisoning is to bring the probability of future OSR exits to as
+        close to zero as possible. But even that isn't enough to trigger this bug; it requires the OSR
+        exit after a jettison to be the first of its kind; our whole design tries to ensure that
+        CodeBlocks tend to OSR exit at a handful (i.e. 1 in most cases) of points, and since jettisoning
+        is triggered by OSR, in most sane cases the OSR exits after jettison will not require lazy OSR
+        compilation. So this is a truly evil case, and any test for it would be quite fragile.
+
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::specializationKind):
+        (JSC::CodeBlock::largeFailCountThreshold):
+        (JSC::CodeBlock::largeFailCountThresholdForLoop):
+        * dfg/DFGAssemblyHelpers.h:
+        (JSC::DFG::AssemblyHelpers::AssemblyHelpers):
+        (JSC::DFG::AssemblyHelpers::baselineCodeBlockFor):
+        (JSC::DFG::AssemblyHelpers::baselineCodeBlock):
+        * dfg/DFGDriver.cpp:
+        (JSC::DFG::compile):
+        * dfg/DFGOSRExitCompiler.cpp:
+        * dfg/DFGOSRExitCompiler64.cpp:
+        (JSC::DFG::OSRExitCompiler::compileExit):
+
 2011-11-15  Geoffrey Garen  <gga...@apple.com>
 
         Use MarkedArgumentBuffer to avoid making assumptions about argument order

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (100362 => 100363)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2011-11-16 00:26:15 UTC (rev 100362)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2011-11-16 00:26:30 UTC (rev 100363)
@@ -267,7 +267,14 @@
         CodeBlock* alternative() { return m_alternative.get(); }
         PassOwnPtr<CodeBlock> releaseAlternative() { return m_alternative.release(); }
         void setAlternative(PassOwnPtr<CodeBlock> alternative) { m_alternative = alternative; }
-
+        
+        CodeSpecializationKind specializationKind()
+        {
+            if (m_isConstructor)
+                return CodeForConstruct;
+            return CodeForCall;
+        }
+        
 #if ENABLE(JIT)
         CodeBlock* baselineVersion()
         {
@@ -984,8 +991,8 @@
         static ptrdiff_t offsetOfSpeculativeFailCounter() { return OBJECT_OFFSETOF(CodeBlock, m_speculativeFailCounter); }
         
         // The number of failures that triggers the use of the ratio.
-        unsigned largeFailCountThreshold() { return Heuristics::largeFailCountThresholdBase << alternative()->reoptimizationRetryCounter(); }
-        unsigned largeFailCountThresholdForLoop() { return Heuristics::largeFailCountThresholdBaseForLoop << alternative()->reoptimizationRetryCounter(); }
+        unsigned largeFailCountThreshold() { return Heuristics::largeFailCountThresholdBase << baselineVersion()->reoptimizationRetryCounter(); }
+        unsigned largeFailCountThresholdForLoop() { return Heuristics::largeFailCountThresholdBaseForLoop << baselineVersion()->reoptimizationRetryCounter(); }
         
         bool shouldReoptimizeNow()
         {

Modified: trunk/Source/_javascript_Core/dfg/DFGAssemblyHelpers.h (100362 => 100363)


--- trunk/Source/_javascript_Core/dfg/DFGAssemblyHelpers.h	2011-11-16 00:26:15 UTC (rev 100362)
+++ trunk/Source/_javascript_Core/dfg/DFGAssemblyHelpers.h	2011-11-16 00:26:30 UTC (rev 100363)
@@ -48,7 +48,12 @@
     AssemblyHelpers(JSGlobalData* globalData, CodeBlock* codeBlock)
         : m_globalData(globalData)
         , m_codeBlock(codeBlock)
+        , m_baselineCodeBlock(codeBlock->baselineVersion())
     {
+        ASSERT(m_codeBlock);
+        ASSERT(m_baselineCodeBlock);
+        ASSERT(!m_baselineCodeBlock->alternative());
+        ASSERT(m_baselineCodeBlock->getJITType() == JITCode::BaselineJIT);
     }
     
     CodeBlock* codeBlock() { return m_codeBlock; }
@@ -299,15 +304,20 @@
             ASSERT(executable->structure()->classInfo() == &FunctionExecutable::s_info);
             return static_cast<FunctionExecutable*>(executable)->baselineCodeBlockFor(codeOrigin.inlineCallFrame->isCall ? CodeForCall : CodeForConstruct);
         }
-        ASSERT(codeBlock()->alternative() == codeBlock()->baselineVersion());
-        return codeBlock()->alternative();
+        return baselineCodeBlock();
     }
     
+    CodeBlock* baselineCodeBlock()
+    {
+        return m_baselineCodeBlock;
+    }
+    
     Vector<BytecodeAndMachineOffset>& decodedCodeMapFor(CodeBlock*);
 
 protected:
     JSGlobalData* m_globalData;
     CodeBlock* m_codeBlock;
+    CodeBlock* m_baselineCodeBlock;
 
     HashMap<CodeBlock*, Vector<BytecodeAndMachineOffset> > m_decodedCodeMaps;
 };

Modified: trunk/Source/_javascript_Core/dfg/DFGDriver.cpp (100362 => 100363)


--- trunk/Source/_javascript_Core/dfg/DFGDriver.cpp	2011-11-16 00:26:15 UTC (rev 100362)
+++ trunk/Source/_javascript_Core/dfg/DFGDriver.cpp	2011-11-16 00:26:30 UTC (rev 100363)
@@ -39,6 +39,10 @@
 {
     SamplingRegion samplingRegion("DFG Compilation (Driver)");
     
+    ASSERT(codeBlock);
+    ASSERT(codeBlock->alternative());
+    ASSERT(codeBlock->alternative()->getJITType() == JITCode::BaselineJIT);
+    
     JSGlobalData* globalData = &exec->globalData();
     Graph dfg;
     if (!parse(dfg, globalData, codeBlock))

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitCompiler.cpp (100362 => 100363)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExitCompiler.cpp	2011-11-16 00:26:15 UTC (rev 100362)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitCompiler.cpp	2011-11-16 00:26:30 UTC (rev 100363)
@@ -39,6 +39,10 @@
 void compileOSRExit(ExecState* exec)
 {
     CodeBlock* codeBlock = exec->codeBlock();
+    
+    ASSERT(codeBlock);
+    ASSERT(codeBlock->getJITType() == JITCode::DFGJIT);
+    
     JSGlobalData* globalData = &exec->globalData();
     
     uint32_t exitIndex = globalData->osrExitIndex;

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitCompiler64.cpp (100362 => 100363)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExitCompiler64.cpp	2011-11-16 00:26:15 UTC (rev 100362)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitCompiler64.cpp	2011-11-16 00:26:30 UTC (rev 100363)
@@ -470,7 +470,7 @@
     m_jit.store32(GPRInfo::regT2, AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfSpeculativeFailCounter()));
     m_jit.store32(GPRInfo::regT1, AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfSpeculativeSuccessCounter()));
     
-    m_jit.move(AssemblyHelpers::TrustedImmPtr(m_jit.codeBlock()->alternative()), GPRInfo::regT0);
+    m_jit.move(AssemblyHelpers::TrustedImmPtr(m_jit.baselineCodeBlock()), GPRInfo::regT0);
     
     AssemblyHelpers::Jump fewFails = m_jit.branch32(AssemblyHelpers::BelowOrEqual, GPRInfo::regT2, AssemblyHelpers::Imm32(m_jit.codeBlock()->largeFailCountThreshold()));
     m_jit.mul32(AssemblyHelpers::Imm32(Heuristics::desiredSpeculativeSuccessFailRatio), GPRInfo::regT2, GPRInfo::regT2);
@@ -484,7 +484,7 @@
     fewFails.link(&m_jit);
     lowFailRate.link(&m_jit);
     
-    m_jit.store32(AssemblyHelpers::Imm32(m_jit.codeBlock()->alternative()->counterValueForOptimizeAfterLongWarmUp()), AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfExecuteCounter()));
+    m_jit.store32(AssemblyHelpers::Imm32(m_jit.baselineCodeBlock()->counterValueForOptimizeAfterLongWarmUp()), AssemblyHelpers::Address(GPRInfo::regT0, CodeBlock::offsetOfExecuteCounter()));
     
     doneAdjusting.link(&m_jit);
     
@@ -495,8 +495,8 @@
     
     // 14) Fix call frame(s).
     
-    ASSERT(m_jit.codeBlock()->alternative()->getJITType() == JITCode::BaselineJIT);
-    m_jit.storePtr(AssemblyHelpers::TrustedImmPtr(m_jit.codeBlock()->alternative()), AssemblyHelpers::addressFor((VirtualRegister)RegisterFile::CodeBlock));
+    ASSERT(m_jit.baselineCodeBlock()->getJITType() == JITCode::BaselineJIT);
+    m_jit.storePtr(AssemblyHelpers::TrustedImmPtr(m_jit.baselineCodeBlock()), AssemblyHelpers::addressFor((VirtualRegister)RegisterFile::CodeBlock));
     
     for (CodeOrigin codeOrigin = exit.m_codeOrigin; codeOrigin.inlineCallFrame; codeOrigin = codeOrigin.inlineCallFrame->caller) {
         InlineCallFrame* inlineCallFrame = codeOrigin.inlineCallFrame;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to