Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (202396 => 202397)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-23 20:55:41 UTC (rev 202397)
@@ -1,3 +1,50 @@
+2016-06-23 Filip Pizlo <[email protected]>
+
+ Failing baseline JIT compilation of a code block and then trying to compile it from OSR from DFG/FTL will corrupt the CodeBlock
+ https://bugs.webkit.org/show_bug.cgi?id=158806
+
+ Reviewed by Saam Barati.
+
+ If we try to compile a CodeBlock that we already tried compiling in the past then we need
+ to clean up the data structures that were partly filled in by the failed compile. That
+ causes some races, since the DFG may be trying to parse those data structures while we are
+ clearing them. This patch introduces such a clean-up (CodeBlock::resetJITData()) and fixes
+ the races.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::dumpBytecode):
+ (JSC::CodeBlock::getStubInfoMap):
+ (JSC::CodeBlock::getCallLinkInfoMap):
+ (JSC::CodeBlock::getByValInfoMap):
+ (JSC::CodeBlock::getCallLinkInfoForBytecodeIndex):
+ (JSC::CodeBlock::resetJITData):
+ (JSC::CodeBlock::visitOSRExitTargets):
+ (JSC::CodeBlock::setSteppingMode):
+ (JSC::CodeBlock::addRareCaseProfile):
+ (JSC::CodeBlock::rareCaseProfileForBytecodeOffset):
+ (JSC::CodeBlock::rareCaseProfileCountForBytecodeOffset):
+ (JSC::CodeBlock::resultProfileForBytecodeOffset):
+ (JSC::CodeBlock::specialFastCaseProfileCountForBytecodeOffset):
+ (JSC::CodeBlock::couldTakeSpecialFastCase):
+ (JSC::CodeBlock::ensureResultProfile):
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlock::getFromAllValueProfiles):
+ (JSC::CodeBlock::numberOfRareCaseProfiles):
+ (JSC::CodeBlock::numberOfResultProfiles):
+ (JSC::CodeBlock::numberOfArrayProfiles):
+ (JSC::CodeBlock::arrayProfiles):
+ (JSC::CodeBlock::addRareCaseProfile): Deleted.
+ (JSC::CodeBlock::specialFastCaseProfileCountForBytecodeOffset): Deleted.
+ (JSC::CodeBlock::couldTakeSpecialFastCase): Deleted.
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::makeSafe):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::methodOfGettingAValueProfileFor):
+ * jit/JIT.cpp:
+ (JSC::JIT::link):
+ * jit/JITWorklist.cpp:
+ (JSC::JITWorklist::compileNow):
+
2016-06-23 Joseph Pecoraro <[email protected]>
Web Inspector: Memory Timeline sometimes shows impossible value for bmalloc size (underflowed)
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (202396 => 202397)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-06-23 20:55:41 UTC (rev 202397)
@@ -1760,7 +1760,10 @@
}
dumpRareCaseProfile(out, "rare case: ", rareCaseProfileForBytecodeOffset(location), hasPrintedProfiling);
- dumpResultProfile(out, resultProfileForBytecodeOffset(location), hasPrintedProfiling);
+ {
+ ConcurrentJITLocker locker(m_lock);
+ dumpResultProfile(out, resultProfileForBytecodeOffset(locker, location), hasPrintedProfiling);
+ }
#if ENABLE(DFG_JIT)
Vector<DFG::FrequentExitSite> exitSites = exitProfile().exitSitesFor(location);
@@ -2932,7 +2935,8 @@
void CodeBlock::getStubInfoMap(const ConcurrentJITLocker&, StubInfoMap& result)
{
#if ENABLE(JIT)
- toHashMap(m_stubInfos, getStructureStubInfoCodeOrigin, result);
+ if (JITCode::isJIT(jitType()))
+ toHashMap(m_stubInfos, getStructureStubInfoCodeOrigin, result);
#else
UNUSED_PARAM(result);
#endif
@@ -2947,7 +2951,8 @@
void CodeBlock::getCallLinkInfoMap(const ConcurrentJITLocker&, CallLinkInfoMap& result)
{
#if ENABLE(JIT)
- toHashMap(m_callLinkInfos, getCallLinkInfoCodeOrigin, result);
+ if (JITCode::isJIT(jitType()))
+ toHashMap(m_callLinkInfos, getCallLinkInfoCodeOrigin, result);
#else
UNUSED_PARAM(result);
#endif
@@ -2962,8 +2967,10 @@
void CodeBlock::getByValInfoMap(const ConcurrentJITLocker&, ByValInfoMap& result)
{
#if ENABLE(JIT)
- for (auto* byValInfo : m_byValInfos)
- result.add(CodeOrigin(byValInfo->bytecodeIndex), byValInfo);
+ if (JITCode::isJIT(jitType())) {
+ for (auto* byValInfo : m_byValInfos)
+ result.add(CodeOrigin(byValInfo->bytecodeIndex), byValInfo);
+ }
#else
UNUSED_PARAM(result);
#endif
@@ -3011,6 +3018,29 @@
}
return nullptr;
}
+
+void CodeBlock::resetJITData()
+{
+ RELEASE_ASSERT(!JITCode::isJIT(jitType()));
+ ConcurrentJITLocker locker(m_lock);
+
+ // We can clear these because no other thread will have references to any stub infos, call
+ // link infos, or by val infos if we don't have JIT code. Attempts to query these data
+ // structures using the concurrent API (getStubInfoMap and friends) will return nothing if we
+ // don't have JIT code.
+ m_stubInfos.clear();
+ m_callLinkInfos.clear();
+ m_byValInfos.clear();
+
+ // We can clear this because the DFG's queries to these data structures are guarded by whether
+ // there is JIT code.
+ m_rareCaseProfiles.clear();
+
+ // We can clear these because the DFG only accesses members of this data structure when
+ // holding the lock or after querying whether we have JIT code.
+ m_resultProfiles.clear();
+ m_bytecodeOffsetToResultProfileIndexMap = nullptr;
+}
#endif
void CodeBlock::visitOSRExitTargets(SlotVisitor& visitor)
@@ -4296,6 +4326,12 @@
jettison(Profiler::JettisonDueToDebuggerStepping);
}
+RareCaseProfile* CodeBlock::addRareCaseProfile(int bytecodeOffset)
+{
+ m_rareCaseProfiles.append(RareCaseProfile(bytecodeOffset));
+ return &m_rareCaseProfiles.last();
+}
+
RareCaseProfile* CodeBlock::rareCaseProfileForBytecodeOffset(int bytecodeOffset)
{
return tryBinarySearch<RareCaseProfile, int>(
@@ -4311,12 +4347,6 @@
return 0;
}
-ResultProfile* CodeBlock::resultProfileForBytecodeOffset(int bytecodeOffset)
-{
- ConcurrentJITLocker locker(m_lock);
- return resultProfileForBytecodeOffset(locker, bytecodeOffset);
-}
-
ResultProfile* CodeBlock::resultProfileForBytecodeOffset(const ConcurrentJITLocker&, int bytecodeOffset)
{
if (!m_bytecodeOffsetToResultProfileIndexMap)
@@ -4327,7 +4357,28 @@
return &m_resultProfiles[iterator->value];
}
+unsigned CodeBlock::specialFastCaseProfileCountForBytecodeOffset(int bytecodeOffset)
+{
+ ConcurrentJITLocker locker(m_lock);
+ return specialFastCaseProfileCountForBytecodeOffset(locker, bytecodeOffset);
+}
+unsigned CodeBlock::specialFastCaseProfileCountForBytecodeOffset(const ConcurrentJITLocker& locker, int bytecodeOffset)
+{
+ ResultProfile* profile = "" bytecodeOffset);
+ if (!profile)
+ return 0;
+ return profile->specialFastPathCount();
+}
+
+bool CodeBlock::couldTakeSpecialFastCase(int bytecodeOffset)
+{
+ if (!hasBaselineJITProfiling())
+ return false;
+ unsigned specialFastCaseCount = specialFastCaseProfileCountForBytecodeOffset(bytecodeOffset);
+ return specialFastCaseCount >= Options::couldTakeSlowCaseMinimumCount();
+}
+
ResultProfile* CodeBlock::ensureResultProfile(int bytecodeOffset)
{
ConcurrentJITLocker locker(m_lock);
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (202396 => 202397)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2016-06-23 20:55:41 UTC (rev 202397)
@@ -262,6 +262,13 @@
// that there had been inlining. Chances are if you want to use this, you're really
// looking for a CallLinkInfoMap to amortize the cost of calling this.
CallLinkInfo* getCallLinkInfoForBytecodeIndex(unsigned bytecodeIndex);
+
+ // We call this when we want to reattempt compiling something with the baseline JIT. Ideally
+ // the baseline JIT would not add data to CodeBlock, but instead it would put its data into
+ // a newly created JITCode, which could be thrown away if we bail on JIT compilation. Then we
+ // would be able to get rid of this silly function.
+ // FIXME: https://bugs.webkit.org/show_bug.cgi?id=159061
+ void resetJITData();
#endif // ENABLE(JIT)
void unlinkIncomingCalls();
@@ -412,11 +419,7 @@
return valueProfile(index - numberOfArgumentValueProfiles());
}
- RareCaseProfile* addRareCaseProfile(int bytecodeOffset)
- {
- m_rareCaseProfiles.append(RareCaseProfile(bytecodeOffset));
- return &m_rareCaseProfiles.last();
- }
+ RareCaseProfile* addRareCaseProfile(int bytecodeOffset);
unsigned numberOfRareCaseProfiles() { return m_rareCaseProfiles.size(); }
RareCaseProfile* rareCaseProfileForBytecodeOffset(int bytecodeOffset);
unsigned rareCaseProfileCountForBytecodeOffset(int bytecodeOffset);
@@ -440,24 +443,12 @@
ResultProfile* ensureResultProfile(int bytecodeOffset);
ResultProfile* ensureResultProfile(const ConcurrentJITLocker&, int bytecodeOffset);
unsigned numberOfResultProfiles() { return m_resultProfiles.size(); }
- ResultProfile* resultProfileForBytecodeOffset(int bytecodeOffset);
ResultProfile* resultProfileForBytecodeOffset(const ConcurrentJITLocker&, int bytecodeOffset);
- unsigned specialFastCaseProfileCountForBytecodeOffset(int bytecodeOffset)
- {
- ResultProfile* profile = ""
- if (!profile)
- return 0;
- return profile->specialFastPathCount();
- }
+ unsigned specialFastCaseProfileCountForBytecodeOffset(const ConcurrentJITLocker&, int bytecodeOffset);
+ unsigned specialFastCaseProfileCountForBytecodeOffset(int bytecodeOffset);
- bool couldTakeSpecialFastCase(int bytecodeOffset)
- {
- if (!hasBaselineJITProfiling())
- return false;
- unsigned specialFastCaseCount = specialFastCaseProfileCountForBytecodeOffset(bytecodeOffset);
- return specialFastCaseCount >= Options::couldTakeSlowCaseMinimumCount();
- }
+ bool couldTakeSpecialFastCase(int bytecodeOffset);
unsigned numberOfArrayProfiles() const { return m_arrayProfiles.size(); }
const ArrayProfileVector& arrayProfiles() { return m_arrayProfiles; }
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (202396 => 202397)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2016-06-23 20:55:41 UTC (rev 202397)
@@ -908,34 +908,37 @@
if (!isX86() && node->op() == ArithMod)
return node;
- ResultProfile* resultProfile = m_inlineStackTop->m_profiledBlock->resultProfileForBytecodeOffset(m_currentIndex);
- if (resultProfile) {
- switch (node->op()) {
- case ArithAdd:
- case ArithSub:
- case ValueAdd:
- if (resultProfile->didObserveDouble())
- node->mergeFlags(NodeMayHaveDoubleResult);
- if (resultProfile->didObserveNonNumber())
- node->mergeFlags(NodeMayHaveNonNumberResult);
- break;
+ {
+ ConcurrentJITLocker locker(m_inlineStackTop->m_profiledBlock->m_lock);
+ ResultProfile* resultProfile = m_inlineStackTop->m_profiledBlock->resultProfileForBytecodeOffset(locker, m_currentIndex);
+ if (resultProfile) {
+ switch (node->op()) {
+ case ArithAdd:
+ case ArithSub:
+ case ValueAdd:
+ if (resultProfile->didObserveDouble())
+ node->mergeFlags(NodeMayHaveDoubleResult);
+ if (resultProfile->didObserveNonNumber())
+ node->mergeFlags(NodeMayHaveNonNumberResult);
+ break;
- case ArithMul: {
- if (resultProfile->didObserveInt52Overflow())
- node->mergeFlags(NodeMayOverflowInt52);
- if (resultProfile->didObserveInt32Overflow() || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Overflow))
- node->mergeFlags(NodeMayOverflowInt32InBaseline);
- if (resultProfile->didObserveNegZeroDouble() || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, NegativeZero))
- node->mergeFlags(NodeMayNegZeroInBaseline);
- if (resultProfile->didObserveDouble())
- node->mergeFlags(NodeMayHaveDoubleResult);
- if (resultProfile->didObserveNonNumber())
- node->mergeFlags(NodeMayHaveNonNumberResult);
- break;
- }
+ case ArithMul: {
+ if (resultProfile->didObserveInt52Overflow())
+ node->mergeFlags(NodeMayOverflowInt52);
+ if (resultProfile->didObserveInt32Overflow() || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Overflow))
+ node->mergeFlags(NodeMayOverflowInt32InBaseline);
+ if (resultProfile->didObserveNegZeroDouble() || m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, NegativeZero))
+ node->mergeFlags(NodeMayNegZeroInBaseline);
+ if (resultProfile->didObserveDouble())
+ node->mergeFlags(NodeMayHaveDoubleResult);
+ if (resultProfile->didObserveNonNumber())
+ node->mergeFlags(NodeMayHaveNonNumberResult);
+ break;
+ }
- default:
- break;
+ default:
+ break;
+ }
}
}
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (202396 => 202397)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2016-06-23 20:55:41 UTC (rev 202397)
@@ -1544,8 +1544,13 @@
if (node->hasHeapPrediction())
return profiledBlock->valueProfileForBytecodeOffset(node->origin.semantic.bytecodeIndex);
- if (ResultProfile* result = profiledBlock->resultProfileForBytecodeOffset(node->origin.semantic.bytecodeIndex))
- return result;
+ {
+ ConcurrentJITLocker locker(profiledBlock->m_lock);
+ if (profiledBlock->hasBaselineJITProfiling()) {
+ if (ResultProfile* result = profiledBlock->resultProfileForBytecodeOffset(locker, node->origin.semantic.bytecodeIndex))
+ return result;
+ }
+ }
switch (node->op()) {
case Identity:
Modified: trunk/Source/_javascript_Core/jit/JIT.cpp (202396 => 202397)
--- trunk/Source/_javascript_Core/jit/JIT.cpp 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/jit/JIT.cpp 2016-06-23 20:55:41 UTC (rev 202397)
@@ -681,8 +681,6 @@
if (patchBuffer.didFailToAllocate())
return CompilationFailed;
- m_codeBlock->setCalleeSaveRegisters(RegisterSet::llintBaselineCalleeSaveRegisters()); // Might be able to remove as this is probably already set to this value.
-
// Translate vPC offsets into addresses in JIT generated code, for switch tables.
for (unsigned i = 0; i < m_switches.size(); ++i) {
SwitchRecord record = m_switches[i];
Modified: trunk/Source/_javascript_Core/jit/JITWorklist.cpp (202396 => 202397)
--- trunk/Source/_javascript_Core/jit/JITWorklist.cpp 2016-06-23 20:21:34 UTC (rev 202396)
+++ trunk/Source/_javascript_Core/jit/JITWorklist.cpp 2016-06-23 20:55:41 UTC (rev 202397)
@@ -219,11 +219,6 @@
void JITWorklist::compileNow(CodeBlock* codeBlock)
{
- // If this ever happens, we'll have a bad time because the baseline JIT does not clean up its
- // changes to the CodeBlock after a failed compilation.
- // FIXME: https://bugs.webkit.org/show_bug.cgi?id=158806
- RELEASE_ASSERT(!codeBlock->m_didFailJITCompilation);
-
DeferGC deferGC(codeBlock->vm()->heap);
if (codeBlock->jitType() != JITCode::InterpreterThunk)
return;
@@ -244,6 +239,10 @@
if (codeBlock->jitType() != JITCode::InterpreterThunk)
return;
+ // We do this in case we had previously attempted, and then failed, to compile with the
+ // baseline JIT.
+ codeBlock->resetJITData();
+
// OK, just compile it.
JIT::compile(codeBlock->vm(), codeBlock, JITCompilationMustSucceed);
codeBlock->ownerScriptExecutable()->installCode(codeBlock);
