Commit 32776a52 authored by fpizlo@apple.com's avatar fpizlo@apple.com

DFG OSR exit may get confused about where in the scratch buffer it stored a value

https://bugs.webkit.org/show_bug.cgi?id=74695

Source/JavaScriptCore: 

Reviewed by Oliver Hunt.
        
The code that reads from the scratch buffer now explicitly knows which locations to
read from. No new tests, since this patch covers a case so uncommon that I don't know
how to make a test for it.

* dfg/DFGOSRExitCompiler.h:
(JSC::DFG::OSRExitCompiler::badIndex):
(JSC::DFG::OSRExitCompiler::initializePoisoned):
(JSC::DFG::OSRExitCompiler::poisonIndex):
* dfg/DFGOSRExitCompiler32_64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
* dfg/DFGOSRExitCompiler64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):

LayoutTests: 

Rubber stamped by Gavin Barraclough.
        
Wrote a custom fuzzer that does 2048 different combinations of integer and float
temporaries and induces a failure whilst all of them are live. If poisoning doesn't
work correctly, a large number (>hundred) of the fuzzing cases fail.

* fast/js/dfg-poison-fuzz-expected.txt: Added.
* fast/js/dfg-poison-fuzz.html: Added.
* fast/js/script-tests/dfg-poison-fuzz.js: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@103127 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 6a261de4
2011-12-16 Filip Pizlo <fpizlo@apple.com>
DFG OSR exit may get confused about where in the scratch buffer it stored a value
https://bugs.webkit.org/show_bug.cgi?id=74695
Rubber stamped by Gavin Barraclough.
Wrote a custom fuzzer that does 2048 different combinations of integer and float
temporaries and induces a failure whilst all of them are live. If poisoning doesn't
work correctly, a large number (>hundred) of the fuzzing cases fail.
* fast/js/dfg-poison-fuzz-expected.txt: Added.
* fast/js/dfg-poison-fuzz.html: Added.
* fast/js/script-tests/dfg-poison-fuzz.js: Added.
2011-12-16 Dean Jackson <dino@apple.com>
Miscellaneous Filter updates to align with spec
This source diff could not be displayed because it is too large. You can view the blob instead.
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/dfg-poison-fuzz.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
description(
"This tests that DFG OSR exit's variable poisoning handles mixes of float and int variables correctly."
);
for (var __i = 0; __i < (1 << 11); ++__i) {
code = "function callee() {\n";
code += " var result = 0;\n";
code += " for (var i = 0; i < arguments.length; ++i)\n";
code += " result += arguments[i];\n";
code += " return result;\n";
code += "}\n";
code += "\n";
code += "function registerBomb(a, b, c, d, e, f, g, h, i, j, k, l) {\n";
code += " return callee((a + b), (b + c), (c - a), (c - b), (a - c), (k - i), (j + h), (g - f), (f - e), (d - k), l, (j - i), (h - g), (f - e + d), (a - b + c), (k - a), (l + a));\n";
code += "}\n";
code += "\n";
code += "var accumulator = 0;\n";
code += "for (var ____i = 0; ____i < 100; ++____i)\n";
code += " accumulator += registerBomb(";
for (var __j = 0; __j < 11; ++__j) {
code += __j + 1;
if ((1 << __j) & __i)
code += ".5";
code += ", ";
}
code += "12.5);\n";
code += "var o = {dummy:accumulator, result:registerBomb(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, \"12.5\")};";
code += "o";
var o = eval(code);
debug("Dummy value for __i = " + __i + " is: " + o.dummy);
shouldBe("o.result", "\"2412.511521012.51\"");
}
2011-12-16 Filip Pizlo <fpizlo@apple.com>
DFG OSR exit may get confused about where in the scratch buffer it stored a value
https://bugs.webkit.org/show_bug.cgi?id=74695
Reviewed by Oliver Hunt.
The code that reads from the scratch buffer now explicitly knows which locations to
read from. No new tests, since this patch covers a case so uncommon that I don't know
how to make a test for it.
* dfg/DFGOSRExitCompiler.h:
(JSC::DFG::OSRExitCompiler::badIndex):
(JSC::DFG::OSRExitCompiler::initializePoisoned):
(JSC::DFG::OSRExitCompiler::poisonIndex):
* dfg/DFGOSRExitCompiler32_64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
* dfg/DFGOSRExitCompiler64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
2011-12-16 Oliver Hunt <oliver@apple.com>
PutByVal[Alias] unnecessarily reloads the storage buffer
......@@ -50,7 +50,28 @@ public:
void compileExit(const OSRExit&, SpeculationRecovery*);
private:
#if !ASSERT_DISABLED
static unsigned badIndex() { return static_cast<unsigned>(-1); };
#endif
void initializePoisoned(unsigned size)
{
#if ASSERT_DISABLED
m_poisonScratchIndices.resize(size);
#else
m_poisonScratchIndices.fill(badIndex(), size);
#endif
}
unsigned poisonIndex(unsigned index)
{
unsigned result = m_poisonScratchIndices[index];
ASSERT(result != badIndex());
return result;
}
AssemblyHelpers& m_jit;
Vector<unsigned> m_poisonScratchIndices;
};
extern "C" {
......
......@@ -272,7 +272,9 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
// Note that GPRs do not have a fast change (like haveFPRs) because we expect that
// most OSR failure points will have at least one GPR that needs to be dumped.
unsigned scratchIndex = 0;
initializePoisoned(exit.m_variables.size());
unsigned currentPoisonIndex = 0;
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
int operand = exit.operandForIndex(index);
......@@ -280,9 +282,11 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
case InGPR:
case UnboxedInt32InGPR:
case UInt32InGPR:
if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)])
m_jit.storePtr(recovery.gpr(), scratchBuffer + scratchIndex++);
else
if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)]) {
m_jit.storePtr(recovery.gpr(), scratchBuffer + currentPoisonIndex);
m_poisonScratchIndices[exit.variableForIndex(index)] = currentPoisonIndex;
currentPoisonIndex++;
} else
m_jit.storePtr(recovery.gpr(), AssemblyHelpers::addressFor((VirtualRegister)operand));
break;
default:
......@@ -312,9 +316,11 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
if (recovery.technique() != InFPR)
continue;
GPRReg gpr = GPRInfo::toRegister(FPRInfo::toIndex(recovery.fpr()));
if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)])
m_jit.storePtr(gpr, scratchBuffer + scratchIndex++);
else
if (exit.isVariable(index) && poisonedVirtualRegisters[exit.variableForIndex(index)]) {
m_jit.storePtr(gpr, scratchBuffer + currentPoisonIndex);
m_poisonScratchIndices[exit.variableForIndex(index)] = currentPoisonIndex;
currentPoisonIndex++;
} else
m_jit.storePtr(gpr, AssemblyHelpers::addressFor((VirtualRegister)exit.operandForIndex(index)));
}
}
......@@ -333,7 +339,7 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
}
}
ASSERT(scratchIndex == numberOfPoisonedVirtualRegisters);
ASSERT(currentPoisonIndex == numberOfPoisonedVirtualRegisters);
// 10) Reshuffle displaced virtual registers. Optimize for the case that
// the number of displaced virtual registers is not more than the number
......@@ -403,6 +409,7 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
// location in memory, and then transferring from that scratch location
// to their new (old JIT) locations.
unsigned scratchIndex = numberOfPoisonedVirtualRegisters;
for (int index = 0; index < exit.numberOfRecoveries(); ++index) {
const ValueRecovery& recovery = exit.valueRecovery(index);
......@@ -453,7 +460,6 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
// 11) Dump all poisoned virtual registers.
scratchIndex = 0;
if (numberOfPoisonedVirtualRegisters) {
for (int virtualRegister = 0; virtualRegister < (int)exit.m_variables.size(); ++virtualRegister) {
if (!poisonedVirtualRegisters[virtualRegister])
......@@ -465,7 +471,7 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
case UnboxedInt32InGPR:
case UInt32InGPR:
case InFPR:
m_jit.loadPtr(scratchBuffer + scratchIndex++, GPRInfo::regT0);
m_jit.loadPtr(scratchBuffer + poisonIndex(virtualRegister), GPRInfo::regT0);
m_jit.storePtr(GPRInfo::regT0, AssemblyHelpers::addressFor((VirtualRegister)virtualRegister));
break;
......@@ -474,7 +480,6 @@ void OSRExitCompiler::compileExit(const OSRExit& exit, SpeculationRecovery* reco
}
}
}
ASSERT(scratchIndex == numberOfPoisonedVirtualRegisters);
// 12) Dump all constants. Optimize for Undefined, since that's a constant we see
// often.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment