[webkit-changes] [158459] trunk/Source/JavaScriptCore

Fri, 01 Nov 2013 15:10:14 -0700

Title: [158459] trunk/Source/_javascript_Core
Revision
158459
Author
fpi...@apple.com
Date
2013-11-01 15:10:52 -0700 (Fri, 01 Nov 2013)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (158458 => 158459)


--- trunk/Source/_javascript_Core/ChangeLog	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-11-01 22:10:52 UTC (rev 158459)
@@ -1,3 +1,43 @@
+2013-11-01  Filip Pizlo  <fpi...@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  <oli...@apple.com>
 
         _javascript_ parser bug

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (158458 => 158459)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2013-11-01 22:10:52 UTC (rev 158459)
@@ -2469,22 +2469,33 @@
 #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 @@
         return DFG::functionForConstructCapabilityLevel(this);
     return DFG::functionForCallCapabilityLevel(this);
 }
+#endif
 
 void CodeBlock::jettison(ReoptimizationMode mode)
 {
@@ -2856,10 +2868,10 @@
     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)
 {

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (158458 => 158459)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2013-11-01 22:10:52 UTC (rev 158459)
@@ -136,6 +136,7 @@
         return specializationFromIsConstruct(m_isConstructor);
     }
     
+    CodeBlock* baselineAlternative();
     CodeBlock* baselineVersion();
 
     void visitAggregate(SlotVisitor&);
@@ -279,8 +280,6 @@
         return jitType() == JITCode::BaselineJIT;
     }
     
-    void jettison(ReoptimizationMode = DontCountReoptimization);
-    
     virtual CodeBlock* replacement() = 0;
 
     virtual DFG::CapabilityLevel capabilityLevelInternal() = 0;
@@ -296,6 +295,8 @@
     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; }

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlockJettisoningWatchpoint.cpp (158458 => 158459)


--- trunk/Source/_javascript_Core/bytecode/CodeBlockJettisoningWatchpoint.cpp	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlockJettisoningWatchpoint.cpp	2013-11-01 22:10:52 UTC (rev 158459)
@@ -36,9 +36,7 @@
     if (DFG::shouldShowDisassembly())
         dataLog("Firing watchpoint ", RawPointer(this), " on ", *m_codeBlock, "\n");
 
-#if ENABLE(JIT)
     m_codeBlock->jettison(CountReoptimization);
-#endif
 
     if (isOnList())
         remove();

Modified: trunk/Source/_javascript_Core/bytecode/ProfiledCodeBlockJettisoningWatchpoint.cpp (158458 => 158459)


--- trunk/Source/_javascript_Core/bytecode/ProfiledCodeBlockJettisoningWatchpoint.cpp	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/bytecode/ProfiledCodeBlockJettisoningWatchpoint.cpp	2013-11-01 22:10:52 UTC (rev 158459)
@@ -40,14 +40,18 @@
             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();
 }

Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.cpp (158458 => 158459)


--- trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.cpp	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExitBase.cpp	2013-11-01 22:10:52 UTC (rev 158459)
@@ -37,8 +37,12 @@
 
 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));
 }

Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.h (158458 => 158459)


--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.h	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.h	2013-11-01 22:10:52 UTC (rev 158459)
@@ -46,7 +46,7 @@
     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);

Modified: trunk/Source/_javascript_Core/runtime/Executable.cpp (158458 => 158459)


--- trunk/Source/_javascript_Core/runtime/Executable.cpp	2013-11-01 21:29:45 UTC (rev 158458)
+++ trunk/Source/_javascript_Core/runtime/Executable.cpp	2013-11-01 22:10:52 UTC (rev 158459)
@@ -510,11 +510,7 @@
     }
     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)

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to