Title: [202847] trunk/Source/_javascript_Core
- Revision
- 202847
- Author
- [email protected]
- Date
- 2016-07-05 22:25:06 -0700 (Tue, 05 Jul 2016)
Log Message
StackVisitor::unwindToMachineCodeBlockFrame() may unwind past a VM entry frame when catching an exception and the frame has inlined tail calls
https://bugs.webkit.org/show_bug.cgi?id=159448
<rdar://problem/27084459>
Reviewed by Mark Lam.
Consider the following stack trace:
(machine) foo -> VM entry frame -> (machine) bar -> (inlined tailcall) baz
If an exception is thrown at 'baz', we will do exception unwinding,
which will eventually call unwindToMachineCodeBlockFrame() which will call
gotoNextFrame() on the 'baz' frame. The next logical frame for 'baz' is 'foo' because
'bar' tail called 'baz' even though there is a machine frame for 'bar' on the stack.
This is a bug. unwindToMachineCodeBlockFrame() should not care about the next
logical frame, it just wants to move StackVisitor's state to the current machine
frame. The bug here is that we would end up unwinding past the VM entry frame
which can have all kinds of terrible consequences.
This bug fixes unwindToMachineCodeBlockFrame() by having it not rely
on gotoNextFrame() and instead using its own mechanism for setting
the StackVisotor's state to the current machine frame.
* interpreter/StackVisitor.cpp:
(JSC::StackVisitor::unwindToMachineCodeBlockFrame):
* tests/stress/dont-unwind-past-vm-entry-frame.js: Added.
(let.p.new.Proxy):
(let.p.new.Proxy.apply):
(bar):
(let.good):
(getItem):
(start):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (202846 => 202847)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-06 05:04:05 UTC (rev 202846)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-06 05:25:06 UTC (rev 202847)
@@ -1,3 +1,37 @@
+2016-07-05 Saam Barati <[email protected]>
+
+ StackVisitor::unwindToMachineCodeBlockFrame() may unwind past a VM entry frame when catching an exception and the frame has inlined tail calls
+ https://bugs.webkit.org/show_bug.cgi?id=159448
+ <rdar://problem/27084459>
+
+ Reviewed by Mark Lam.
+
+ Consider the following stack trace:
+ (machine) foo -> VM entry frame -> (machine) bar -> (inlined tailcall) baz
+
+ If an exception is thrown at 'baz', we will do exception unwinding,
+ which will eventually call unwindToMachineCodeBlockFrame() which will call
+ gotoNextFrame() on the 'baz' frame. The next logical frame for 'baz' is 'foo' because
+ 'bar' tail called 'baz' even though there is a machine frame for 'bar' on the stack.
+ This is a bug. unwindToMachineCodeBlockFrame() should not care about the next
+ logical frame, it just wants to move StackVisitor's state to the current machine
+ frame. The bug here is that we would end up unwinding past the VM entry frame
+ which can have all kinds of terrible consequences.
+
+ This bug fixes unwindToMachineCodeBlockFrame() by having it not rely
+ on gotoNextFrame() and instead using its own mechanism for setting
+ the StackVisotor's state to the current machine frame.
+
+ * interpreter/StackVisitor.cpp:
+ (JSC::StackVisitor::unwindToMachineCodeBlockFrame):
+ * tests/stress/dont-unwind-past-vm-entry-frame.js: Added.
+ (let.p.new.Proxy):
+ (let.p.new.Proxy.apply):
+ (bar):
+ (let.good):
+ (getItem):
+ (start):
+
2016-07-05 Joseph Pecoraro <[email protected]>
RELEASE_ASSERT(!thisObject) in ObjCCallbackFunctionImpl::call when calling JSExport ObjC Constructor without operator new
Modified: trunk/Source/_javascript_Core/interpreter/StackVisitor.cpp (202846 => 202847)
--- trunk/Source/_javascript_Core/interpreter/StackVisitor.cpp 2016-07-06 05:04:05 UTC (rev 202846)
+++ trunk/Source/_javascript_Core/interpreter/StackVisitor.cpp 2016-07-06 05:25:06 UTC (rev 202847)
@@ -81,8 +81,12 @@
void StackVisitor::unwindToMachineCodeBlockFrame()
{
#if ENABLE(DFG_JIT)
- while (m_frame.isInlinedFrame())
- gotoNextFrame();
+ if (m_frame.isInlinedFrame()) {
+ CodeOrigin codeOrigin = m_frame.inlineCallFrame()->directCaller;
+ while (codeOrigin.inlineCallFrame)
+ codeOrigin = codeOrigin.inlineCallFrame->directCaller;
+ readNonInlinedFrame(m_frame.callFrame(), &codeOrigin);
+ }
#endif
}
Modified: trunk/Source/_javascript_Core/jit/JITExceptions.cpp (202846 => 202847)
--- trunk/Source/_javascript_Core/jit/JITExceptions.cpp 2016-07-06 05:04:05 UTC (rev 202846)
+++ trunk/Source/_javascript_Core/jit/JITExceptions.cpp 2016-07-06 05:25:06 UTC (rev 202847)
@@ -81,6 +81,8 @@
} else
catchRoutine = LLInt::getCodePtr(handleUncaughtException);
+ ASSERT(bitwise_cast<uintptr_t>(callFrame) < bitwise_cast<uintptr_t>(vm->topVMEntryFrame));
+
vm->callFrameForCatch = callFrame;
vm->targetMachinePCForThrow = catchRoutine;
vm->targetInterpreterPCForThrow = catchPCForInterpreter;
Added: trunk/Source/_javascript_Core/tests/stress/dont-unwind-past-vm-entry-frame.js (0 => 202847)
--- trunk/Source/_javascript_Core/tests/stress/dont-unwind-past-vm-entry-frame.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/dont-unwind-past-vm-entry-frame.js 2016-07-06 05:25:06 UTC (rev 202847)
@@ -0,0 +1,39 @@
+"use strict";
+
+// This test passes when JSC doesn't crash.
+
+let p = new Proxy(function() { }, {
+ apply: function() {
+ return bar();
+ }
+});
+
+function bar() {
+ let item = getItem();
+ return item.foo;
+}
+
+let i;
+let shouldReturnBad = false;
+let good = [function() {return 1}, {b: 20}, {c: 40}, {d:50}]
+let bad = [{asdfhasf: 20}, {e:50}, {j:70}, {k:100}, null];
+function getItem() {
+ if (shouldReturnBad)
+ return bad[i % bad.length];
+ return good[i % good.length];
+}
+noInline(getItem);
+
+function start() {
+ for (i = 0; i < 1000; i++) {
+ p();
+ }
+
+ shouldReturnBad = true;
+ for (i = 0; i < 10000; i++) {
+ try {
+ p();
+ } catch(e) { }
+ }
+}
+start();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
