- Revision
- 187767
- Author
- [email protected]
- Date
- 2015-08-03 14:39:27 -0700 (Mon, 03 Aug 2015)
Log Message
jsc-tailcall: Kraken/stanford-crypto-ccm crashes
https://bugs.webkit.org/show_bug.cgi?id=147561
Reviewed by Michael Saboff.
When performing a tail call, we assume that if the top of the tail
caller's frame is not stack-aligned, we can use the stack slot above it
in case we need to move the top of the frame. However, we don't enforce
this when actually performing a call, which leads to interesting (and
crashing) situations. We are already enforcing this in the "special
call" cases (calls from native code, inlined cache calls, etc.), so
this is just a matter of making sure the bytecode generator, DFG and
FTL ensure proper padding.
* bytecompiler/NodesCodegen.cpp:
(JSC::CallArguments::CallArguments):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::addCallWithoutSettingResult):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileCallOrConstruct):
* tests/stress/mutual-tail-call-no-stack-overflow.js:
(odd):
(even):
* tests/stress/tail-calls-dont-overwrite-live-stack.js: Added.
(tail):
(obj.method):
(obj.get fromNative):
(getThis):
Modified Paths
Added Paths
Diff
Modified: branches/jsc-tailcall/Source/_javascript_Core/ChangeLog (187766 => 187767)
--- branches/jsc-tailcall/Source/_javascript_Core/ChangeLog 2015-08-03 21:37:36 UTC (rev 187766)
+++ branches/jsc-tailcall/Source/_javascript_Core/ChangeLog 2015-08-03 21:39:27 UTC (rev 187767)
@@ -1,5 +1,36 @@
2015-08-03 Basile Clement <[email protected]>
+ jsc-tailcall: Kraken/stanford-crypto-ccm crashes
+ https://bugs.webkit.org/show_bug.cgi?id=147561
+
+ Reviewed by Michael Saboff.
+
+ When performing a tail call, we assume that if the top of the tail
+ caller's frame is not stack-aligned, we can use the stack slot above it
+ in case we need to move the top of the frame. However, we don't enforce
+ this when actually performing a call, which leads to interesting (and
+ crashing) situations. We are already enforcing this in the "special
+ call" cases (calls from native code, inlined cache calls, etc.), so
+ this is just a matter of making sure the bytecode generator, DFG and
+ FTL ensure proper padding.
+
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::CallArguments::CallArguments):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::addCallWithoutSettingResult):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileCallOrConstruct):
+ * tests/stress/mutual-tail-call-no-stack-overflow.js:
+ (odd):
+ (even):
+ * tests/stress/tail-calls-dont-overwrite-live-stack.js: Added.
+ (tail):
+ (obj.method):
+ (obj.get fromNative):
+ (getThis):
+
+2015-08-03 Basile Clement <[email protected]>
+
jsc-tailcall: Get and put inline caches are not properly restoring the stack pointer
https://bugs.webkit.org/show_bug.cgi?id=147583
Modified: branches/jsc-tailcall/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (187766 => 187767)
--- branches/jsc-tailcall/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2015-08-03 21:37:36 UTC (rev 187766)
+++ branches/jsc-tailcall/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2015-08-03 21:39:27 UTC (rev 187767)
@@ -675,6 +675,12 @@
m_argv[i] = generator.newTemporary();
ASSERT(static_cast<size_t>(i) == m_argv.size() - 1 || m_argv[i]->index() == m_argv[i + 1]->index() - 1);
}
+
+ // We need to ensure that the frame size is stack-aligned
+ while ((JSStack::CallFrameHeaderSize + m_argv.size()) % stackAlignmentRegisters()) {
+ m_argv.insert(0, generator.newTemporary());
+ m_padding++;
+ }
while (stackOffset() % stackAlignmentRegisters()) {
m_argv.insert(0, generator.newTemporary());
Modified: branches/jsc-tailcall/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (187766 => 187767)
--- branches/jsc-tailcall/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-08-03 21:37:36 UTC (rev 187766)
+++ branches/jsc-tailcall/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2015-08-03 21:39:27 UTC (rev 187767)
@@ -683,7 +683,10 @@
SpeculatedType prediction)
{
addVarArgChild(callee);
- size_t parameterSlots = JSStack::CallFrameHeaderSize - JSStack::CallerFrameAndPCSize + argCount;
+ size_t frameSize = JSStack::CallFrameHeaderSize + argCount;
+ size_t alignedFrameSize = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), frameSize);
+ size_t parameterSlots = alignedFrameSize - JSStack::CallerFrameAndPCSize;
+
if (parameterSlots > m_parameterSlots)
m_parameterSlots = parameterSlots;
Modified: branches/jsc-tailcall/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (187766 => 187767)
--- branches/jsc-tailcall/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-08-03 21:37:36 UTC (rev 187766)
+++ branches/jsc-tailcall/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-08-03 21:39:27 UTC (rev 187767)
@@ -4320,24 +4320,29 @@
void compileCallOrConstruct()
{
- int numPassedArgs = m_node->numChildren() - 1;
- int numArgs = numPassedArgs;
+ int numArgs = m_node->numChildren() - 1;
LValue jsCallee = lowJSValue(m_graph.varArgChild(m_node, 0));
unsigned stackmapID = m_stackmapIDs++;
-
+
+ unsigned frameSize = JSStack::CallFrameHeaderSize + numArgs;
+ unsigned alignedFrameSize = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), frameSize);
+ unsigned padding = alignedFrameSize - frameSize;
+
Vector<LValue> arguments;
arguments.append(m_out.constInt64(stackmapID));
arguments.append(m_out.constInt32(sizeOfCall()));
arguments.append(constNull(m_out.ref8));
- arguments.append(m_out.constInt32(1 + JSStack::CallFrameHeaderSize - JSStack::CallerFrameAndPCSize + numArgs));
+ arguments.append(m_out.constInt32(1 + alignedFrameSize - JSStack::CallerFrameAndPCSize));
arguments.append(jsCallee); // callee -> %rax
arguments.append(getUndef(m_out.int64)); // code block
arguments.append(jsCallee); // callee -> stack
arguments.append(m_out.constInt64(numArgs)); // argument count and zeros for the tag
- for (int i = 0; i < numPassedArgs; ++i)
+ for (int i = 0; i < numArgs; ++i)
arguments.append(lowJSValue(m_graph.varArgChild(m_node, 1 + i)));
+ for (unsigned i = 0; i < padding; ++i)
+ arguments.append(getUndef(m_out.int64));
callPreflight();
Modified: branches/jsc-tailcall/Source/_javascript_Core/tests/stress/mutual-tail-call-no-stack-overflow.js (187766 => 187767)
--- branches/jsc-tailcall/Source/_javascript_Core/tests/stress/mutual-tail-call-no-stack-overflow.js 2015-08-03 21:37:36 UTC (rev 187766)
+++ branches/jsc-tailcall/Source/_javascript_Core/tests/stress/mutual-tail-call-no-stack-overflow.js 2015-08-03 21:39:27 UTC (rev 187767)
@@ -55,3 +55,17 @@
shouldThrow(function () { sloppyCountdown(100000); }, "RangeError: Maximum call stack size exceeded.");
strictCountdown(100000);
+
+// Parity alterning
+function odd(n) {
+ "use strict";
+ if (n > 0)
+ return even(n, 0);
+}
+
+function even(n) {
+ "use strict";
+ return odd(n - 1);
+}
+
+odd(100000);
Added: branches/jsc-tailcall/Source/_javascript_Core/tests/stress/tail-calls-dont-overwrite-live-stack.js (0 => 187767)
--- branches/jsc-tailcall/Source/_javascript_Core/tests/stress/tail-calls-dont-overwrite-live-stack.js (rev 0)
+++ branches/jsc-tailcall/Source/_javascript_Core/tests/stress/tail-calls-dont-overwrite-live-stack.js 2015-08-03 21:39:27 UTC (rev 187767)
@@ -0,0 +1,30 @@
+"use strict";
+
+function tail(a, b) { }
+noInline(tail);
+
+var obj = {
+ method: function (x) {
+ return tail(x, x);
+ },
+
+ get fromNative() { return tail(0, 0); }
+};
+noInline(obj.method);
+
+function getThis(x) { return this; }
+noInline(getThis);
+
+for (var i = 0; i < 10000; ++i) {
+ var that = getThis(obj.method(42));
+
+ if (!Object.is(that, undefined))
+ throw new Error("Wrong 'this' value in call, expected undefined but got " + that);
+
+ that = getThis(obj.method(...[42]));
+ if (!Object.is(that, undefined))
+ throw new Error("Wrong 'this' value in varargs call, expected undefined but got " + that);
+
+ if (!Object.is(obj.fromNative, undefined))
+ throw new Error("Wrong 'fromNative' value, expected undefined but got " + obj.fromNative);
+}
