Log Message
Exception stack unwinding doesn't handle inline callframes correctly https://bugs.webkit.org/show_bug.cgi?id=113952
Reviewed by Geoffrey Garen. Source/_javascript_Core: The basic problem here is that the exception stack unwinding was attempting to be "clever" and avoid doing a correct stack walk as it "knew" inline callframes couldn't have exception handlers. This used to be safe as the exception handling machinery was designed to fail gently and just claim that no handler existed. This was "safe" and even "correct" inasmuch as we currently don't run any code with exception handlers through the dfg. This patch fixes the logic by simply making everything uniformly use the safe stack walking machinery, and making the correct boundary checks occur everywhere that they should. * bytecode/CodeBlock.cpp: (JSC::CodeBlock::findClosureCallForReturnPC): (JSC::CodeBlock::bytecodeOffset): * interpreter/Interpreter.cpp: (JSC): (JSC::Interpreter::dumpRegisters): (JSC::Interpreter::unwindCallFrame): (JSC::getCallerInfo): (JSC::Interpreter::getStackTrace): (JSC::Interpreter::retrieveCallerFromVMCode): LayoutTests: Yay tests! * fast/js/js-correct-exception-handler-expected.txt: Added. * fast/js/js-correct-exception-handler.html: Added. * fast/js/script-tests/js-correct-exception-handler.js: Added. (throwEventually): (f.g): (f): (test):
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp
- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (147669 => 147670)
--- trunk/LayoutTests/ChangeLog 2013-04-04 21:16:43 UTC (rev 147669)
+++ trunk/LayoutTests/ChangeLog 2013-04-04 21:25:26 UTC (rev 147670)
@@ -1,3 +1,20 @@
+2013-04-04 Oliver Hunt <[email protected]>
+
+ Exception stack unwinding doesn't handle inline callframes correctly
+ https://bugs.webkit.org/show_bug.cgi?id=113952
+
+ Reviewed by Geoffrey Garen.
+
+ Yay tests!
+
+ * fast/js/js-correct-exception-handler-expected.txt: Added.
+ * fast/js/js-correct-exception-handler.html: Added.
+ * fast/js/script-tests/js-correct-exception-handler.js: Added.
+ (throwEventually):
+ (f.g):
+ (f):
+ (test):
+
2013-04-04 Glenn Adams <[email protected]>
[EFL] Unreviewed gardening. Rebaseline after r147588.
Added: trunk/LayoutTests/fast/js/js-correct-exception-handler-expected.txt (0 => 147670)
--- trunk/LayoutTests/fast/js/js-correct-exception-handler-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/js-correct-exception-handler-expected.txt 2013-04-04 21:25:26 UTC (rev 147670)
@@ -0,0 +1,10 @@
+Verify that handle exceptions flowing throw inline callframes correctly
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS test() threw exception Error.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/js-correct-exception-handler.html (0 => 147670)
--- trunk/LayoutTests/fast/js/js-correct-exception-handler.html (rev 0)
+++ trunk/LayoutTests/fast/js/js-correct-exception-handler.html 2013-04-04 21:25:26 UTC (rev 147670)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/script-tests/js-correct-exception-handler.js (0 => 147670)
--- trunk/LayoutTests/fast/js/script-tests/js-correct-exception-handler.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/js-correct-exception-handler.js 2013-04-04 21:25:26 UTC (rev 147670)
@@ -0,0 +1,26 @@
+description("Verify that handle exceptions flowing throw inline callframes correctly");
+
+function throwEventually() {
+ if (value++ > 10000)
+ throw new Error;
+ return 5;
+}
+
+var value = 0;
+function f(x) {
+ var result = 0;
+ function g(a) {
+ return a * throwEventually();
+ }
+ for (var i = 0; i < 3; i++)
+ i += i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * i * g(x);
+ return i;
+}
+
+function test() {
+ while (true)
+ (function() {f(5)})()
+}
+
+shouldThrow("test()");
+
Modified: trunk/Source/_javascript_Core/ChangeLog (147669 => 147670)
--- trunk/Source/_javascript_Core/ChangeLog 2013-04-04 21:16:43 UTC (rev 147669)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-04-04 21:25:26 UTC (rev 147670)
@@ -1,3 +1,34 @@
+2013-04-04 Oliver Hunt <[email protected]>
+
+ Exception stack unwinding doesn't handle inline callframes correctly
+ https://bugs.webkit.org/show_bug.cgi?id=113952
+
+ Reviewed by Geoffrey Garen.
+
+ The basic problem here is that the exception stack unwinding was
+ attempting to be "clever" and avoid doing a correct stack walk
+ as it "knew" inline callframes couldn't have exception handlers.
+
+ This used to be safe as the exception handling machinery was
+ designed to fail gently and just claim that no handler existed.
+ This was "safe" and even "correct" inasmuch as we currently
+ don't run any code with exception handlers through the dfg.
+
+ This patch fixes the logic by simply making everything uniformly
+ use the safe stack walking machinery, and making the correct
+ boundary checks occur everywhere that they should.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::findClosureCallForReturnPC):
+ (JSC::CodeBlock::bytecodeOffset):
+ * interpreter/Interpreter.cpp:
+ (JSC):
+ (JSC::Interpreter::dumpRegisters):
+ (JSC::Interpreter::unwindCallFrame):
+ (JSC::getCallerInfo):
+ (JSC::Interpreter::getStackTrace):
+ (JSC::Interpreter::retrieveCallerFromVMCode):
+
2013-04-04 Geoffrey Garen <[email protected]>
Removed a defunct comment
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (147669 => 147670)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2013-04-04 21:16:43 UTC (rev 147669)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2013-04-04 21:25:26 UTC (rev 147670)
@@ -2462,7 +2462,7 @@
HandlerInfo* CodeBlock::handlerForBytecodeOffset(unsigned bytecodeOffset)
{
- ASSERT(bytecodeOffset < instructions().size());
+ RELEASE_ASSERT(bytecodeOffset < instructions().size());
if (!m_rareData)
return 0;
@@ -2660,7 +2660,8 @@
continue;
if (!info.stub->code().executableMemory()->contains(returnAddress.value()))
continue;
-
+
+ RELEASE_ASSERT(info.stub->codeOrigin().bytecodeIndex < info.stub->codeOrigin().maximumBytecodeIndex);
return info.stub.get();
}
@@ -2673,6 +2674,7 @@
ClosureCallStubRoutine* stub = static_cast<ClosureCallStubRoutine*>(genericStub);
if (!stub->code().executableMemory()->contains(returnAddress.value()))
continue;
+ RELEASE_ASSERT(stub->codeOrigin().bytecodeIndex < stub->codeOrigin().maximumBytecodeIndex);
return stub;
}
@@ -2724,10 +2726,21 @@
binarySearch<CallReturnOffsetToBytecodeOffset, unsigned>(
callIndices, callIndices.size(), callReturnOffset, getCallReturnOffset);
RELEASE_ASSERT(result->callReturnOffset == callReturnOffset);
+ RELEASE_ASSERT(result->bytecodeOffset < instructionCount());
return result->bytecodeOffset;
}
-
- return findClosureCallForReturnPC(returnAddress)->codeOrigin().bytecodeIndex;
+ ClosureCallStubRoutine* closureInfo = findClosureCallForReturnPC(returnAddress);
+ CodeOrigin origin = closureInfo->codeOrigin();
+ while (InlineCallFrame* inlineCallFrame = origin.inlineCallFrame) {
+ if (inlineCallFrame->baselineCodeBlock() == this)
+ break;
+ origin = inlineCallFrame->caller;
+ RELEASE_ASSERT(origin.bytecodeIndex < origin.maximumBytecodeIndex);
+ }
+ RELEASE_ASSERT(origin.bytecodeIndex < origin.maximumBytecodeIndex);
+ unsigned bytecodeIndex = origin.bytecodeIndex;
+ RELEASE_ASSERT(bytecodeIndex < instructionCount());
+ return bytecodeIndex;
#endif // ENABLE(JIT)
#if !ENABLE(LLINT) && !ENABLE(JIT)
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (147669 => 147670)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2013-04-04 21:16:43 UTC (rev 147669)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2013-04-04 21:25:26 UTC (rev 147670)
@@ -199,7 +199,7 @@
}
-static CallFrame* getCallerInfo(JSGlobalData*, CallFrame*, int& lineNumber, unsigned& bytecodeOffset);
+static CallFrame* getCallerInfo(JSGlobalData*, CallFrame*, int& lineNumber, unsigned& bytecodeOffset, CodeBlock*& callerOut);
// Returns the depth of the scope chain within a given call frame.
static int depth(CodeBlock* codeBlock, JSScope* sc)
@@ -422,7 +422,8 @@
#endif
unsigned bytecodeOffset = 0;
int line = 0;
- getCallerInfo(&callFrame->globalData(), callFrame, line, bytecodeOffset);
+ CodeBlock* unusedCallerCodeBlock = 0;
+ getCallerInfo(&callFrame->globalData(), callFrame, line, bytecodeOffset, unusedCallerCodeBlock);
dataLogF("[ReturnVPC] | %10p | %d (line %d)\n", it, bytecodeOffset, line);
++it;
dataLogF("[CodeBlock] | %10p | %p \n", it, callFrame->codeBlock());
@@ -506,17 +507,8 @@
callFrame->globalData().topCallFrame = callerFrame;
if (callerFrame->hasHostCallFrameFlag())
return false;
-
- codeBlock = callerFrame->codeBlock();
-
- // Because of how the JIT records call site->bytecode offset
- // information the JIT reports the bytecodeOffset for the returnPC
- // to be at the beginning of the opcode that has caused the call.
-#if ENABLE(JIT) || ENABLE(LLINT)
- bytecodeOffset = codeBlock->bytecodeOffset(callerFrame, callFrame->returnPC());
-#endif
-
- callFrame = callerFrame;
+ int unusedLineNumber = 0;
+ callFrame = getCallerInfo(&callFrame->globalData(), callFrame, unusedLineNumber, bytecodeOffset, codeBlock);
return true;
}
@@ -588,9 +580,9 @@
#endif
}
-static CallFrame* getCallerInfo(JSGlobalData* globalData, CallFrame* callFrame, int& lineNumber, unsigned& bytecodeOffset)
+static CallFrame* getCallerInfo(JSGlobalData* globalData, CallFrame* callFrame, int& lineNumber, unsigned& bytecodeOffset, CodeBlock*& caller)
{
- UNUSED_PARAM(globalData);
+ ASSERT_UNUSED(globalData, globalData);
bytecodeOffset = 0;
lineNumber = -1;
ASSERT(!callFrame->hasHostCallFrameFlag());
@@ -598,8 +590,10 @@
bool callframeIsHost = callerFrame->addHostCallFrameFlag() == callFrame->callerFrame();
ASSERT(!callerFrame->hasHostCallFrameFlag());
- if (callerFrame == CallFrame::noCaller() || !callerFrame || !callerFrame->codeBlock())
+ if (callerFrame == CallFrame::noCaller() || !callerFrame || !callerFrame->codeBlock()) {
+ caller = 0;
return callerFrame;
+ }
CodeBlock* callerCodeBlock = callerFrame->codeBlock();
@@ -659,6 +653,7 @@
}
RELEASE_ASSERT(callerCodeBlock);
+ caller = callerCodeBlock;
lineNumber = callerCodeBlock->lineNumberForBytecodeOffset(bytecodeOffset);
return callerFrame;
}
@@ -705,7 +700,8 @@
results.append(s);
}
unsigned unusedBytecodeOffset = 0;
- callFrame = getCallerInfo(globalData, callFrame, line, unusedBytecodeOffset);
+ CodeBlock* unusedCallerCodeBlock = 0;
+ callFrame = getCallerInfo(globalData, callFrame, line, unusedBytecodeOffset, unusedCallerCodeBlock);
}
}
@@ -1381,7 +1377,8 @@
int lineNumber;
unsigned bytecodeOffset;
- CallFrame* callerFrame = getCallerInfo(&callFrame->globalData(), functionCallFrame, lineNumber, bytecodeOffset);
+ CodeBlock* unusedCallerCodeBlock = 0;
+ CallFrame* callerFrame = getCallerInfo(&callFrame->globalData(), functionCallFrame, lineNumber, bytecodeOffset, unusedCallerCodeBlock);
if (!callerFrame)
return jsNull();
JSValue caller = callerFrame->callee();
@@ -1391,7 +1388,7 @@
// Skip over function bindings.
ASSERT(caller.isObject());
while (asObject(caller)->inherits(&JSBoundFunction::s_info)) {
- callerFrame = getCallerInfo(&callFrame->globalData(), callerFrame, lineNumber, bytecodeOffset);
+ callerFrame = getCallerInfo(&callFrame->globalData(), callerFrame, lineNumber, bytecodeOffset, unusedCallerCodeBlock);
if (!callerFrame)
return jsNull();
caller = callerFrame->callee();
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
