Diff
Modified: trunk/LayoutTests/ChangeLog (93933 => 93934)
--- trunk/LayoutTests/ChangeLog 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/LayoutTests/ChangeLog 2011-08-27 04:19:22 UTC (rev 93934)
@@ -1,3 +1,17 @@
+2011-08-26 Gavin Barraclough <barraclo...@apple.com>
+
+ DFG JIT - ArithMod may clobber operands.
+ https://bugs.webkit.org/show_bug.cgi?id=67085
+
+ Reviewed by Sam Weinig.
+
+ Added test case.
+
+ * fast/js/mod-crash-expected.txt:
+ * fast/js/script-tests/mod-crash.js:
+ (nonSpeculativeModReuseInner):
+ (nonSpeculativeModReuse):
+
2011-08-26 Anna Cavender <ann...@chromium.org>
Cleanup resulting text of media/track tests.
Modified: trunk/LayoutTests/fast/js/mod-crash-expected.txt (93933 => 93934)
--- trunk/LayoutTests/fast/js/mod-crash-expected.txt 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/LayoutTests/fast/js/mod-crash-expected.txt 2011-08-27 04:19:22 UTC (rev 93934)
@@ -7,6 +7,7 @@
PASS n % 0 is NaN
PASS f() is NaN
PASS g() is NaN
+PASS nonSpeculativeModReuse(0.5) is 1
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/js/script-tests/mod-crash.js (93933 => 93934)
--- trunk/LayoutTests/fast/js/script-tests/mod-crash.js 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/LayoutTests/fast/js/script-tests/mod-crash.js 2011-08-27 04:19:22 UTC (rev 93934)
@@ -22,4 +22,20 @@
shouldBe("g()", "NaN");
+// Test that reusing a floating point value after use in a modulus works correctly.
+function nonSpeculativeModReuseInner(argument, o1, o2)
+{
+ // The + operator on objects is a reliable way to avoid the speculative JIT path for now at least.
+ o1 + o2;
+
+ var knownDouble = argument - 0;
+ return knownDouble % 1 + knownDouble;
+}
+function nonSpeculativeModReuse(argument)
+{
+ return nonSpeculativeModReuseInner(argument, {}, {});
+}
+
+shouldBe("nonSpeculativeModReuse(0.5)", "1");
+
var successfullyParsed = true;
Modified: trunk/Source/_javascript_Core/ChangeLog (93933 => 93934)
--- trunk/Source/_javascript_Core/ChangeLog 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-08-27 04:19:22 UTC (rev 93934)
@@ -1,3 +1,21 @@
+2011-08-26 Gavin Barraclough <barraclo...@apple.com>
+
+ DFG JIT - ArithMod may clobber operands.
+ https://bugs.webkit.org/show_bug.cgi?id=67085
+
+ Reviewed by Sam Weinig.
+
+ unboxDouble must be called on a temporary.
+
+ * dfg/DFGJITCodeGenerator.cpp:
+ (JSC::DFG::JITCodeGenerator::fillDouble):
+ * dfg/DFGJITCodeGenerator.h:
+ (JSC::DFG::JITCodeGenerator::boxDouble):
+ * dfg/DFGNonSpeculativeJIT.cpp:
+ (JSC::DFG::NonSpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::fillSpeculateDouble):
+
2011-08-26 Mark Hahnenberg <mhahnenb...@apple.com>
Unzip initialization lists and constructors in JSCell hierarchy (2/7)
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (93933 => 93934)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp 2011-08-27 04:19:22 UTC (rev 93934)
@@ -198,7 +198,15 @@
// Unbox the double
case DataFormatJSDouble: {
GPRReg gpr = info.gpr();
- FPRReg fpr = unboxDouble(gpr);
+ FPRReg fpr = fprAllocate();
+ if (m_gprs.isLocked(gpr)) {
+ // Make sure we don't trample gpr if it is in use.
+ GPRReg temp = allocate();
+ m_jit.move(gpr, temp);
+ unboxDouble(temp, fpr);
+ unlock(temp);
+ } else
+ unboxDouble(gpr, fpr);
m_gprs.release(gpr);
m_fprs.retain(fpr, virtualRegister, SpillOrderDouble);
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h (93933 => 93934)
--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.h 2011-08-27 04:19:22 UTC (rev 93934)
@@ -367,10 +367,6 @@
{
return boxDouble(fpr, allocate());
}
- FPRReg unboxDouble(GPRReg gpr)
- {
- return unboxDouble(gpr, fprAllocate());
- }
// Spill a VirtualRegister to the RegisterFile.
void spill(VirtualRegister spillMe)
Modified: trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp (93933 => 93934)
--- trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/Source/_javascript_Core/dfg/DFGNonSpeculativeJIT.cpp 2011-08-27 04:19:22 UTC (rev 93934)
@@ -674,12 +674,19 @@
op2.use();
GPRReg temp2 = InvalidGPRReg;
+ GPRReg unboxGPR;
if (op2GPR == X86Registers::eax || op2GPR == X86Registers::edx) {
temp2 = allocate();
m_jit.move(op2GPR, temp2);
op2GPR = temp2;
- }
-
+ unboxGPR = temp2;
+ } else if (op1GPR == X86Registers::eax)
+ unboxGPR = X86Registers::edx;
+ else
+ unboxGPR = X86Registers::eax;
+ ASSERT(unboxGPR != op1.gpr());
+ ASSERT(unboxGPR != op2.gpr());
+
JITCompiler::Jump firstOpNotInt;
JITCompiler::Jump secondOpNotInt;
JITCompiler::JumpList done;
@@ -724,12 +731,14 @@
secondOpNotInt2.link(&m_jit);
// first op is a double, second op is a double.
- unboxDouble(op2GPR, op2FPR);
+ m_jit.move(op2GPR, unboxGPR);
+ unboxDouble(unboxGPR, op2FPR);
gotSecondOp.link(&m_jit);
}
- unboxDouble(op1GPR, op1FPR);
+ m_jit.move(op1GPR, unboxGPR);
+ unboxDouble(unboxGPR, op1FPR);
gotDoubleArgs = m_jit.jump();
}
@@ -739,7 +748,8 @@
// we know that the first op is an int, and the second is a double
m_jit.convertInt32ToDouble(op1GPR, op1FPR);
- unboxDouble(op2GPR, op2FPR);
+ m_jit.move(op2GPR, unboxGPR);
+ unboxDouble(unboxGPR, op2FPR);
}
if (!isKnownInteger(node.child1()))
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (93933 => 93934)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-08-27 03:51:03 UTC (rev 93933)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2011-08-27 04:19:22 UTC (rev 93934)
@@ -271,7 +271,15 @@
// Unbox the double
case DataFormatJSDouble: {
GPRReg gpr = info.gpr();
- FPRReg fpr = unboxDouble(gpr);
+ FPRReg fpr = fprAllocate();
+ if (m_gprs.isLocked(gpr)) {
+ // Make sure we don't trample gpr if it is in use.
+ GPRReg temp = allocate();
+ m_jit.move(gpr, temp);
+ unboxDouble(temp, fpr);
+ unlock(temp);
+ } else
+ unboxDouble(gpr, fpr);
m_gprs.release(gpr);
m_fprs.retain(fpr, virtualRegister, SpillOrderDouble);
