Title: [225492] trunk
- Revision
- 225492
- Author
- [email protected]
- Date
- 2017-12-04 14:00:24 -0800 (Mon, 04 Dec 2017)
Log Message
We need to leave room on the top of the stack for the FTL TailCall slow path so it doesn't overwrite things we want to retrieve when doing a stack walk when throwing an exception
https://bugs.webkit.org/show_bug.cgi?id=180366
<rdar://problem/35685877>
Reviewed by Michael Saboff.
JSTests:
* stress/ftl-tail-call-throw-exception-from-slow-path-recover-stack-values.js: Added.
(theParent):
(test1.base.getParentStaticValue):
(test1.base):
(test1.__v_24888.prototype.set prop):
(test1.__v_24888):
(test2.base.getParentStaticValue):
(test2.base):
(test2.__v_24888.prototype.set prop):
(test2.__v_24888):
(test2):
Source/_javascript_Core:
On the TailCall slow path, the CallFrameShuffler will build the frame with
respect to SP instead of FP. However, this may overwrite slots on the stack
that are needed if the slow path C call does a stack walk. The slow path
C call does a stack walk when it throws an exception. This patch fixes
this bug by ensuring that the top of the stack in the FTL always has enough
space to allow CallFrameShuffler to build a frame without overwriting any
items on the stack that are needed when doing a stack walk.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileTailCall):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (225491 => 225492)
--- trunk/JSTests/ChangeLog 2017-12-04 21:56:25 UTC (rev 225491)
+++ trunk/JSTests/ChangeLog 2017-12-04 22:00:24 UTC (rev 225492)
@@ -1,3 +1,23 @@
+2017-12-04 Saam Barati <[email protected]>
+
+ We need to leave room on the top of the stack for the FTL TailCall slow path so it doesn't overwrite things we want to retrieve when doing a stack walk when throwing an exception
+ https://bugs.webkit.org/show_bug.cgi?id=180366
+ <rdar://problem/35685877>
+
+ Reviewed by Michael Saboff.
+
+ * stress/ftl-tail-call-throw-exception-from-slow-path-recover-stack-values.js: Added.
+ (theParent):
+ (test1.base.getParentStaticValue):
+ (test1.base):
+ (test1.__v_24888.prototype.set prop):
+ (test1.__v_24888):
+ (test2.base.getParentStaticValue):
+ (test2.base):
+ (test2.__v_24888.prototype.set prop):
+ (test2.__v_24888):
+ (test2):
+
2017-12-01 JF Bastien <[email protected]>
Try proxying all function arguments
Added: trunk/JSTests/stress/ftl-tail-call-throw-exception-from-slow-path-recover-stack-values.js (0 => 225492)
--- trunk/JSTests/stress/ftl-tail-call-throw-exception-from-slow-path-recover-stack-values.js (rev 0)
+++ trunk/JSTests/stress/ftl-tail-call-throw-exception-from-slow-path-recover-stack-values.js 2017-12-04 22:00:24 UTC (rev 225492)
@@ -0,0 +1,31 @@
+var theParent = function () { };
+
+function test1() {
+ var base = class C extends theParent {
+ static getParentStaticValue() {
+ let arrow = (a,b,c) => super.getStaticValue(a,b,c);
+ return arrow(1,1,1);
+ }
+ };
+
+ for (let i = 0; i < 10000; i++) {
+ try { base.getParentStaticValue() } catch (e) {}
+ try { base.getParentStaticValue() } catch (e) {}
+ }
+}
+test1();
+
+function test2() {
+ var base = class C extends theParent {
+ static getParentStaticValue() {
+ let arrow = () => super.getStaticValue();
+ return arrow();
+ }
+ };
+
+ for (let i = 0; i < 10000; i++) {
+ try { base.getParentStaticValue() } catch (e) {}
+ try { base.getParentStaticValue() } catch (e) {}
+ }
+}
+test2();
Modified: trunk/Source/_javascript_Core/ChangeLog (225491 => 225492)
--- trunk/Source/_javascript_Core/ChangeLog 2017-12-04 21:56:25 UTC (rev 225491)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-12-04 22:00:24 UTC (rev 225492)
@@ -1,3 +1,22 @@
+2017-12-04 Saam Barati <[email protected]>
+
+ We need to leave room on the top of the stack for the FTL TailCall slow path so it doesn't overwrite things we want to retrieve when doing a stack walk when throwing an exception
+ https://bugs.webkit.org/show_bug.cgi?id=180366
+ <rdar://problem/35685877>
+
+ Reviewed by Michael Saboff.
+
+ On the TailCall slow path, the CallFrameShuffler will build the frame with
+ respect to SP instead of FP. However, this may overwrite slots on the stack
+ that are needed if the slow path C call does a stack walk. The slow path
+ C call does a stack walk when it throws an exception. This patch fixes
+ this bug by ensuring that the top of the stack in the FTL always has enough
+ space to allow CallFrameShuffler to build a frame without overwriting any
+ items on the stack that are needed when doing a stack walk.
+
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileTailCall):
+
2017-12-04 Devin Rousso <[email protected]>
Web Inspector: provide method for recording CanvasRenderingContext2D from _javascript_
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (225491 => 225492)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-12-04 21:56:25 UTC (rev 225491)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2017-12-04 22:00:24 UTC (rev 225492)
@@ -6983,6 +6983,16 @@
Node* node = m_node;
unsigned numArgs = node->numChildren() - 1;
+ // It seems counterintuitive that this is needed given that tail calls don't create a new frame
+ // on the stack. However, the tail call slow path builds the frame at SP instead of FP before
+ // calling into the slow path C code. This slow path may decide to throw an exception because
+ // the callee we're trying to call is not callable. Throwing an exception will cause us to walk
+ // the stack, which may read, for the sake of the correctness of this code, arbitrary slots on the
+ // stack to recover state. This call arg area ensures the call frame shuffler does not overwrite
+ // any of the slots the stack walking code requires when on the slow path.
+ m_proc.requestCallArgAreaSizeInBytes(
+ WTF::roundUpToMultipleOf(stackAlignmentBytes(), (CallFrame::headerSizeInRegisters + numArgs) * sizeof(EncodedJSValue)));
+
LValue jsCallee = lowJSValue(m_graph.varArgChild(node, 0));
// We want B3 to give us all of the arguments using whatever mechanism it thinks is
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes