- Revision
- 159064
- Author
- [email protected]
- Date
- 2013-11-11 12:57:51 -0800 (Mon, 11 Nov 2013)
Log Message
DFG Int52 boxing code may clobber the source without telling anyone
https://bugs.webkit.org/show_bug.cgi?id=124137
Source/_javascript_Core:
Reviewed by Mark Hahnenberg.
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::boxInt52): This is called in places where source is expected to be unchanged. We never call this expecting super-amazing codegen. So, preserve the source's value the dumb way (by recovering it mathematically).
* jit/AssemblyHelpers.h: Document the invariant for boxInt52.
* jsc.cpp:
(GlobalObject::finishCreation): It's been super annoying that sometimes we say noInline() and sometimes we say neverInlineFunction(). The LayoutTests harnesses ensure that we have something called noInline(), but it's great to also ensure that the shell has it.
LayoutTests:
Reviewed by Mark Hahnenberg.
Write the test as a JSRegress test because we currently need a couple
recompiles to get the bug. JSRegress tests are meant to be longer-running
stress tests and they are usually run with different compilation thresholds, so
that ensures that we will actually hit the relevant code path.
* js/regress/int52-spill-expected.txt: Added.
* js/regress/int52-spill.html: Added.
* js/regress/script-tests/int52-spill.js: Added.
(bar):
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (159063 => 159064)
--- trunk/LayoutTests/ChangeLog 2013-11-11 20:53:13 UTC (rev 159063)
+++ trunk/LayoutTests/ChangeLog 2013-11-11 20:57:51 UTC (rev 159064)
@@ -1,3 +1,21 @@
+2013-11-10 Filip Pizlo <[email protected]>
+
+ DFG Int52 boxing code may clobber the source without telling anyone
+ https://bugs.webkit.org/show_bug.cgi?id=124137
+
+ Reviewed by Mark Hahnenberg.
+
+ Write the test as a JSRegress test because we currently need a couple
+ recompiles to get the bug. JSRegress tests are meant to be longer-running
+ stress tests and they are usually run with different compilation thresholds, so
+ that ensures that we will actually hit the relevant code path.
+
+ * js/regress/int52-spill-expected.txt: Added.
+ * js/regress/int52-spill.html: Added.
+ * js/regress/script-tests/int52-spill.js: Added.
+ (bar):
+ (foo):
+
2013-11-11 Oliver Hunt <[email protected]>
ExtJS breaks with modern Array.prototype.values API due to use of with()
Added: trunk/LayoutTests/js/regress/int52-spill-expected.txt (0 => 159064)
--- trunk/LayoutTests/js/regress/int52-spill-expected.txt (rev 0)
+++ trunk/LayoutTests/js/regress/int52-spill-expected.txt 2013-11-11 20:57:51 UTC (rev 159064)
@@ -0,0 +1,10 @@
+JSRegress/int52-spill
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/js/regress/int52-spill.html (0 => 159064)
--- trunk/LayoutTests/js/regress/int52-spill.html (rev 0)
+++ trunk/LayoutTests/js/regress/int52-spill.html 2013-11-11 20:57:51 UTC (rev 159064)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/js/regress/script-tests/int52-spill.js (0 => 159064)
--- trunk/LayoutTests/js/regress/script-tests/int52-spill.js (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/int52-spill.js 2013-11-11 20:57:51 UTC (rev 159064)
@@ -0,0 +1,24 @@
+var array;
+
+function bar(x) {
+ array.push(x);
+}
+
+function foo(n) {
+ var x = n + 1000;
+ bar(x);
+ x += 1000;
+ bar(x);
+ x += 1000;
+ bar(x);
+}
+
+noInline(bar);
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ array = [];
+ foo(2147483647);
+ if ("" + array != "2147484647,2147485647,2147486647")
+ throw "Error: bad array: " + array;
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (159063 => 159064)
--- trunk/Source/_javascript_Core/ChangeLog 2013-11-11 20:53:13 UTC (rev 159063)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-11-11 20:57:51 UTC (rev 159064)
@@ -1,3 +1,16 @@
+2013-11-10 Filip Pizlo <[email protected]>
+
+ DFG Int52 boxing code may clobber the source without telling anyone
+ https://bugs.webkit.org/show_bug.cgi?id=124137
+
+ Reviewed by Mark Hahnenberg.
+
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::boxInt52): This is called in places where source is expected to be unchanged. We never call this expecting super-amazing codegen. So, preserve the source's value the dumb way (by recovering it mathematically).
+ * jit/AssemblyHelpers.h: Document the invariant for boxInt52.
+ * jsc.cpp:
+ (GlobalObject::finishCreation): It's been super annoying that sometimes we say noInline() and sometimes we say neverInlineFunction(). The LayoutTests harnesses ensure that we have something called noInline(), but it's great to also ensure that the shell has it.
+
2013-11-11 Oliver Hunt <[email protected]>
ExtJS breaks with modern Array.prototype.values API due to use of with()
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (159063 => 159064)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2013-11-11 20:53:13 UTC (rev 159063)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2013-11-11 20:57:51 UTC (rev 159064)
@@ -58,6 +58,9 @@
m_jit.boxInt52(sourceGPR, targetGPR, tempGPR, fpr);
+ if (format == DataFormatInt52 && sourceGPR != targetGPR)
+ m_jit.lshift64(TrustedImm32(JSValue::int52ShiftAmount), sourceGPR);
+
if (tempGPR != targetGPR)
unlock(tempGPR);
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.h (159063 => 159064)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2013-11-11 20:53:13 UTC (rev 159063)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2013-11-11 20:57:51 UTC (rev 159064)
@@ -325,6 +325,10 @@
return fpr;
}
+ // Here are possible arrangements of source, target, scratch:
+ // - source, target, scratch can all be separate registers.
+ // - source and target can be the same but scratch is separate.
+ // - target and scratch can be the same but source is separate.
void boxInt52(GPRReg source, GPRReg target, GPRReg scratch, FPRReg fpScratch)
{
// Is it an int32?
Modified: trunk/Source/_javascript_Core/jsc.cpp (159063 => 159064)
--- trunk/Source/_javascript_Core/jsc.cpp 2013-11-11 20:53:13 UTC (rev 159063)
+++ trunk/Source/_javascript_Core/jsc.cpp 2013-11-11 20:57:51 UTC (rev 159064)
@@ -231,6 +231,7 @@
addFunction(vm, "readline", functionReadline, 0);
addFunction(vm, "preciseTime", functionPreciseTime, 0);
addFunction(vm, "neverInlineFunction", functionNeverInlineFunction, 1);
+ addFunction(vm, "noInline", functionNeverInlineFunction, 1);
addFunction(vm, "numberOfDFGCompiles", functionNumberOfDFGCompiles, 1);
#if ENABLE(SAMPLING_FLAGS)
addFunction(vm, "setSamplingFlags", functionSetSamplingFlags, 1);
