OSR exit profiling should be robust against all code being cleared
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Nov 2013 22:10:52 +0000 (22:10 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Nov 2013 22:10:52 +0000 (22:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123629
<rdar://problem/15365476>

Reviewed by Michael Saboff.

The problem here is two-fold:

1) A watchpoint (i.e. ProfiledCodeBlockJettisoningWatchpoint) may be fired after we
have cleared the CodeBlock for all or some Executables.  This means that doing
codeBlock->baselineVersion() would either crash or return a bogus CodeBlock, since
there wasn't a baseline code block reachable from the Executable anymore.  The
solution is that we shouldn't be asking for the baseline code block reachable from
the owning executable (what baselineVersion did), but instead we should be asking
for the baseline version reachable from the code block being watchpointed (basically
what CodeBlock::alternative() did).

2) If dealing with inlined code, baselienCodeBlockForOriginAndBaselineCodeBlock()
may return null, for the same reason as above - we might have cleared the baseline
codeblock for the executable that was inlined.  The solution is to just not do
profiling if there isn't a baseline code block anymore.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::baselineAlternative):
(JSC::CodeBlock::baselineVersion):
(JSC::CodeBlock::jettison):
* bytecode/CodeBlock.h:
* bytecode/CodeBlockJettisoningWatchpoint.cpp:
(JSC::CodeBlockJettisoningWatchpoint::fireInternal):
* bytecode/ProfiledCodeBlockJettisoningWatchpoint.cpp:
(JSC::ProfiledCodeBlockJettisoningWatchpoint::fireInternal):
* dfg/DFGOSRExitBase.cpp:
(JSC::DFG::OSRExitBase::considerAddingAsFrequentExitSiteSlow):
* jit/AssemblyHelpers.h:
(JSC::AssemblyHelpers::AssemblyHelpers):
* runtime/Executable.cpp:
(JSC::FunctionExecutable::baselineCodeBlockFor):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@158459 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/bytecode/CodeBlockJettisoningWatchpoint.cpp
Source/JavaScriptCore/bytecode/ProfiledCodeBlockJettisoningWatchpoint.cpp
Source/JavaScriptCore/dfg/DFGOSRExitBase.cpp
Source/JavaScriptCore/jit/AssemblyHelpers.h
Source/JavaScriptCore/runtime/Executable.cpp

index aa85b31..c1ec09d 100644 (file)
@@ -1,3 +1,43 @@
+2013-11-01  Filip Pizlo  <fpizlo@apple.com>
+
+        OSR exit profiling should be robust against all code being cleared
+        https://bugs.webkit.org/show_bug.cgi?id=123629
+        <rdar://problem/15365476>
+
+        Reviewed by Michael Saboff.
+        
+        The problem here is two-fold:
+
+        1) A watchpoint (i.e. ProfiledCodeBlockJettisoningWatchpoint) may be fired after we
+        have cleared the CodeBlock for all or some Executables.  This means that doing
+        codeBlock->baselineVersion() would either crash or return a bogus CodeBlock, since
+        there wasn't a baseline code block reachable from the Executable anymore.  The
+        solution is that we shouldn't be asking for the baseline code block reachable from
+        the owning executable (what baselineVersion did), but instead we should be asking
+        for the baseline version reachable from the code block being watchpointed (basically
+        what CodeBlock::alternative() did).
+
+        2) If dealing with inlined code, baselienCodeBlockForOriginAndBaselineCodeBlock()
+        may return null, for the same reason as above - we might have cleared the baseline
+        codeblock for the executable that was inlined.  The solution is to just not do
+        profiling if there isn't a baseline code block anymore.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::baselineAlternative):
+        (JSC::CodeBlock::baselineVersion):
+        (JSC::CodeBlock::jettison):
+        * bytecode/CodeBlock.h:
+        * bytecode/CodeBlockJettisoningWatchpoint.cpp:
+        (JSC::CodeBlockJettisoningWatchpoint::fireInternal):
+        * bytecode/ProfiledCodeBlockJettisoningWatchpoint.cpp:
+        (JSC::ProfiledCodeBlockJettisoningWatchpoint::fireInternal):
+        * dfg/DFGOSRExitBase.cpp:
+        (JSC::DFG::OSRExitBase::considerAddingAsFrequentExitSiteSlow):
+        * jit/AssemblyHelpers.h:
+        (JSC::AssemblyHelpers::AssemblyHelpers):
+        * runtime/Executable.cpp:
+        (JSC::FunctionExecutable::baselineCodeBlockFor):
+
 2013-10-31  Oliver Hunt  <oliver@apple.com>
 
         JavaScript parser bug
index c85395e..1e5f9ed 100644 (file)
@@ -2469,22 +2469,33 @@ void CodeBlock::stronglyVisitWeakReferences(SlotVisitor& visitor)
 #endif    
 }
 
+CodeBlock* CodeBlock::baselineAlternative()
+{
+#if ENABLE(JIT)
+    CodeBlock* result = this;
+    while (result->alternative())
+        result = result->alternative();
+    RELEASE_ASSERT(result);
+    RELEASE_ASSERT(JITCode::isBaselineCode(result->jitType()) || result->jitType() == JITCode::None);
+    return result;
+#else
+    return this;
+#endif
+}
+
 CodeBlock* CodeBlock::baselineVersion()
 {
+#if ENABLE(JIT)
     if (JITCode::isBaselineCode(jitType()))
         return this;
-#if ENABLE(JIT)
     CodeBlock* result = replacement();
     if (!result) {
         // This can happen if we're creating the original CodeBlock for an executable.
         // Assume that we're the baseline CodeBlock.
-        ASSERT(jitType() == JITCode::None);
+        RELEASE_ASSERT(jitType() == JITCode::None);
         return this;
     }
-    while (result->alternative())
-        result = result->alternative();
-    RELEASE_ASSERT(result);
-    RELEASE_ASSERT(JITCode::isBaselineCode(result->jitType()));
+    result = result->baselineAlternative();
     return result;
 #else
     return this;
@@ -2806,6 +2817,7 @@ DFG::CapabilityLevel FunctionCodeBlock::capabilityLevelInternal()
         return DFG::functionForConstructCapabilityLevel(this);
     return DFG::functionForCallCapabilityLevel(this);
 }
+#endif
 
 void CodeBlock::jettison(ReoptimizationMode mode)
 {
@@ -2856,10 +2868,10 @@ void CodeBlock::jettison(ReoptimizationMode mode)
     if (DFG::shouldShowDisassembly())
         dataLog("    Did install baseline version of ", *this, "\n");
 #else // ENABLE(DFG_JIT)
+    UNUSED_PARAM(mode);
     UNREACHABLE_FOR_PLATFORM();
 #endif // ENABLE(DFG_JIT)
 }
-#endif
 
 JSGlobalObject* CodeBlock::globalObjectFor(CodeOrigin codeOrigin)
 {
index 156c25e..c21bbba 100644 (file)
@@ -136,6 +136,7 @@ public:
         return specializationFromIsConstruct(m_isConstructor);
     }
     
+    CodeBlock* baselineAlternative();
     CodeBlock* baselineVersion();
 
     void visitAggregate(SlotVisitor&);
@@ -279,8 +280,6 @@ public:
         return jitType() == JITCode::BaselineJIT;
     }
     
-    void jettison(ReoptimizationMode = DontCountReoptimization);
-    
     virtual CodeBlock* replacement() = 0;
 
     virtual DFG::CapabilityLevel capabilityLevelInternal() = 0;
@@ -296,6 +295,8 @@ public:
     bool hasOptimizedReplacement(); // the typeToReplace is my JITType
 #endif
 
+    void jettison(ReoptimizationMode = DontCountReoptimization);
+    
     ScriptExecutable* ownerExecutable() const { return m_ownerExecutable.get(); }
 
     void setVM(VM* vm) { m_vm = vm; }
index 9ff7b16..be50c97 100644 (file)
@@ -36,9 +36,7 @@ void CodeBlockJettisoningWatchpoint::fireInternal()
     if (DFG::shouldShowDisassembly())
         dataLog("Firing watchpoint ", RawPointer(this), " on ", *m_codeBlock, "\n");
 
-#if ENABLE(JIT)
     m_codeBlock->jettison(CountReoptimization);
-#endif
 
     if (isOnList())
         remove();
index 13e1bde..4bb2ba8 100644 (file)
@@ -40,14 +40,18 @@ void ProfiledCodeBlockJettisoningWatchpoint::fireInternal()
             m_exitKind, " at ", m_codeOrigin, "\n");
     }
     
-    baselineCodeBlockForOriginAndBaselineCodeBlock(
-        m_codeOrigin, m_codeBlock->baselineVersion())->addFrequentExitSite(
+    CodeBlock* machineBaselineCodeBlock = m_codeBlock->baselineAlternative();
+    CodeBlock* sourceBaselineCodeBlock =
+        baselineCodeBlockForOriginAndBaselineCodeBlock(
+            m_codeOrigin, machineBaselineCodeBlock);
+    
+    if (sourceBaselineCodeBlock) {
+        sourceBaselineCodeBlock->addFrequentExitSite(
             DFG::FrequentExitSite(m_codeOrigin.bytecodeIndex, m_exitKind));
+    }
     
-#if ENABLE(JIT)
     m_codeBlock->jettison(CountReoptimization);
-#endif
-
+    
     if (isOnList())
         remove();
 }
index 1f41f8e..762830a 100644 (file)
@@ -37,8 +37,12 @@ namespace JSC { namespace DFG {
 
 bool OSRExitBase::considerAddingAsFrequentExitSiteSlow(CodeBlock* profiledCodeBlock)
 {
-    return baselineCodeBlockForOriginAndBaselineCodeBlock(
-        m_codeOriginForExitProfile, profiledCodeBlock)->addFrequentExitSite(
+    CodeBlock* sourceProfiledCodeBlock =
+        baselineCodeBlockForOriginAndBaselineCodeBlock(
+            m_codeOriginForExitProfile, profiledCodeBlock);
+    if (!sourceProfiledCodeBlock)
+        return false;
+    return sourceProfiledCodeBlock->addFrequentExitSite(
             FrequentExitSite(m_codeOriginForExitProfile.bytecodeIndex, m_kind));
 }
 
index b81d1e4..68e1cab 100644 (file)
@@ -46,7 +46,7 @@ public:
     AssemblyHelpers(VM* vm, CodeBlock* codeBlock)
         : m_vm(vm)
         , m_codeBlock(codeBlock)
-        , m_baselineCodeBlock(codeBlock ? codeBlock->baselineVersion() : 0)
+        , m_baselineCodeBlock(codeBlock ? codeBlock->baselineAlternative() : 0)
     {
         if (m_codeBlock) {
             ASSERT(m_baselineCodeBlock);
index b3fec13..003dd43 100644 (file)
@@ -510,11 +510,7 @@ FunctionCodeBlock* FunctionExecutable::baselineCodeBlockFor(CodeSpecializationKi
     }
     if (!result)
         return 0;
-    while (result->alternative())
-        result = static_cast<FunctionCodeBlock*>(result->alternative());
-    RELEASE_ASSERT(result);
-    ASSERT(JITCode::isBaselineCode(result->jitType()));
-    return result;
+    return static_cast<FunctionCodeBlock*>(result->baselineAlternative());
 }
 
 void FunctionExecutable::visitChildren(JSCell* cell, SlotVisitor& visitor)