Diff
Modified: trunk/LayoutTests/ChangeLog (136926 => 136927)
--- trunk/LayoutTests/ChangeLog 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/LayoutTests/ChangeLog 2012-12-07 07:44:01 UTC (rev 136927)
@@ -1,3 +1,17 @@
+2012-12-06 Filip Pizlo <[email protected]>
+
+ Incorrect inequality for checking whether a statement is within bounds of a handler
+ https://bugs.webkit.org/show_bug.cgi?id=104313
+ <rdar://problem/12808934>
+
+ Reviewed by Geoffrey Garen.
+
+ * fast/js/jsc-test-list:
+ * fast/js/script-tests/try-catch-try-try-catch-try-finally-return-catch-finally.js: Added.
+ (foo):
+ * fast/js/try-catch-try-try-catch-try-finally-return-catch-finally-expected.txt: Added.
+ * fast/js/try-catch-try-try-catch-try-finally-return-catch-finally.html: Added.
+
2012-12-06 Noel Gordon <[email protected]>
Unreviewed Gardening. More http://trac.webkit.org/changeset/136910. Add Chromium Linux results.
Modified: trunk/LayoutTests/fast/js/jsc-test-list (136926 => 136927)
--- trunk/LayoutTests/fast/js/jsc-test-list 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/LayoutTests/fast/js/jsc-test-list 2012-12-07 07:44:01 UTC (rev 136927)
@@ -338,6 +338,7 @@
fast/js/toString-number-dot-expr
fast/js/toString-prefix-postfix-preserve-parens
fast/js/toString-recursion
+fast/js/try-catch-try-try-catch-try-finally-return-catch-finally
fast/js/try-try-return-finally-finally
fast/js/typeof-codegen-crash
fast/js/typeof-constant-string
Added: trunk/LayoutTests/fast/js/script-tests/try-catch-try-try-catch-try-finally-return-catch-finally.js (0 => 136927)
--- trunk/LayoutTests/fast/js/script-tests/try-catch-try-try-catch-try-finally-return-catch-finally.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/try-catch-try-try-catch-try-finally-return-catch-finally.js 2012-12-07 07:44:01 UTC (rev 136927)
@@ -0,0 +1,30 @@
+description(
+"Tests what would happen if you a throwing operation at the beginning of a finally blow that gets inlined inside a complicated catch/finally stack. The correct outcome is for this test to not crash during exception throwing."
+);
+
+function foo() {
+ try{
+ N
+ } catch(x) {
+ try {
+ try {
+ w
+ } catch(x) {
+ try {
+ } finally {
+ return
+ }
+ }
+ } catch (a) {
+ }
+ } finally {
+ z
+ }
+}
+
+try {
+ foo();
+} catch (e) {
+ testPassed("It worked.");
+}
+
Added: trunk/LayoutTests/fast/js/try-catch-try-try-catch-try-finally-return-catch-finally-expected.txt (0 => 136927)
--- trunk/LayoutTests/fast/js/try-catch-try-try-catch-try-finally-return-catch-finally-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/try-catch-try-try-catch-try-finally-return-catch-finally-expected.txt 2012-12-07 07:44:01 UTC (rev 136927)
@@ -0,0 +1,10 @@
+Tests what would happen if you a throwing operation at the beginning of a finally blow that gets inlined inside a complicated catch/finally stack. The correct outcome is for this test to not crash during exception throwing.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS It worked.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/try-catch-try-try-catch-try-finally-return-catch-finally.html (0 => 136927)
--- trunk/LayoutTests/fast/js/try-catch-try-try-catch-try-finally-return-catch-finally.html (rev 0)
+++ trunk/LayoutTests/fast/js/try-catch-try-try-catch-try-finally-return-catch-finally.html 2012-12-07 07:44:01 UTC (rev 136927)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Modified: trunk/Source/_javascript_Core/ChangeLog (136926 => 136927)
--- trunk/Source/_javascript_Core/ChangeLog 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-12-07 07:44:01 UTC (rev 136927)
@@ -1,3 +1,40 @@
+2012-12-06 Filip Pizlo <[email protected]>
+
+ Incorrect inequality for checking whether a statement is within bounds of a handler
+ https://bugs.webkit.org/show_bug.cgi?id=104313
+ <rdar://problem/12808934>
+
+ Reviewed by Geoffrey Garen.
+
+ The most relevant change is in handlerForBytecodeOffset(), which fixes the inequality
+ used for checking whether a handler is pertinent to the current instruction. '<' is
+ correct, but '<=' isn't, since the 'end' is not inclusive.
+
+ Also found, and addressed, a benign goof in how the finally inliner works: sometimes
+ we will have end > start. This falls out naturally from how the inliner works and how
+ we pop scopes in the bytecompiler, but it's sufficiently surprising that, to avoid any
+ future confusion, I added a comment and some code to prune those handlers out. Because
+ of how the handler resolution works, these handlers would have been skipped anyway.
+
+ Also made various fixes to debugging code, which was necessary for tracking this down.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::dumpBytecode):
+ (JSC::CodeBlock::handlerForBytecodeOffset):
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::generate):
+ * bytecompiler/Label.h:
+ (JSC::Label::bind):
+ * interpreter/Interpreter.cpp:
+ (JSC::Interpreter::throwException):
+ * llint/LLIntExceptions.cpp:
+ (JSC::LLInt::interpreterThrowInCaller):
+ (JSC::LLInt::returnToThrow):
+ (JSC::LLInt::callToThrow):
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ (JSC::LLInt::handleHostCall):
+
2012-12-06 Rick Byers <[email protected]>
CSS cursor property should support webkit-image-set
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (136926 => 136927)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2012-12-07 07:44:01 UTC (rev 136927)
@@ -570,7 +570,7 @@
out.printf("\nException Handlers:\n");
unsigned i = 0;
do {
- out.printf("\t %d: { start: [%4d] end: [%4d] target: [%4d] }\n", i + 1, m_rareData->m_exceptionHandlers[i].start, m_rareData->m_exceptionHandlers[i].end, m_rareData->m_exceptionHandlers[i].target);
+ out.printf("\t %d: { start: [%4d] end: [%4d] target: [%4d] depth: [%4d] }\n", i + 1, m_rareData->m_exceptionHandlers[i].start, m_rareData->m_exceptionHandlers[i].end, m_rareData->m_exceptionHandlers[i].target, m_rareData->m_exceptionHandlers[i].scopeDepth);
++i;
} while (i < m_rareData->m_exceptionHandlers.size());
}
@@ -2476,7 +2476,7 @@
for (size_t i = 0; i < exceptionHandlers.size(); ++i) {
// Handlers are ordered innermost first, so the first handler we encounter
// that contains the source address is the correct handler to use.
- if (exceptionHandlers[i].start <= bytecodeOffset && exceptionHandlers[i].end >= bytecodeOffset)
+ if (exceptionHandlers[i].start <= bytecodeOffset && exceptionHandlers[i].end > bytecodeOffset)
return &exceptionHandlers[i];
}
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (136926 => 136927)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2012-12-07 07:44:01 UTC (rev 136927)
@@ -157,10 +157,38 @@
for (unsigned i = 0; i < m_tryRanges.size(); ++i) {
TryRange& range = m_tryRanges[i];
+ int start = range.start->bind();
+ int end = range.end->bind();
+
+ // This will happen for empty try blocks and for some cases of finally blocks:
+ //
+ // try {
+ // try {
+ // } finally {
+ // return 42;
+ // // *HERE*
+ // }
+ // } finally {
+ // print("things");
+ // }
+ //
+ // The return will pop scopes to execute the outer finally block. But this includes
+ // popping the try context for the inner try. The try context is live in the fall-through
+ // part of the finally block not because we will emit a handler that overlaps the finally,
+ // but because we haven't yet had a chance to plant the catch target. Then when we finish
+ // emitting code for the outer finally block, we repush the try contex, this time with a
+ // new start index. But that means that the start index for the try range corresponding
+ // to the inner-finally-following-the-return (marked as "*HERE*" above) will be greater
+ // than the end index of the try block. This is harmless since end < start handlers will
+ // never get matched in our logic, but we do the runtime a favor and choose to not emit
+ // such handlers at all.
+ if (end <= start)
+ continue;
+
ASSERT(range.tryData->targetScopeDepth != UINT_MAX);
UnlinkedHandlerInfo info = {
- static_cast<uint32_t>(range.start->bind(0, 0)), static_cast<uint32_t>(range.end->bind(0, 0)),
- static_cast<uint32_t>(range.tryData->target->bind(0, 0)),
+ static_cast<uint32_t>(start), static_cast<uint32_t>(end),
+ static_cast<uint32_t>(range.tryData->target->bind()),
range.tryData->targetScopeDepth
};
m_codeBlock->addExceptionHandler(info);
Modified: trunk/Source/_javascript_Core/bytecompiler/Label.h (136926 => 136927)
--- trunk/Source/_javascript_Core/bytecompiler/Label.h 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/bytecompiler/Label.h 2012-12-07 07:44:01 UTC (rev 136927)
@@ -66,6 +66,12 @@
int refCount() const { return m_refCount; }
bool isForward() const { return m_location == invalidLocation; }
+
+ int bind()
+ {
+ ASSERT(!isForward());
+ return bind(0, 0);
+ }
private:
typedef Vector<std::pair<int, int>, 8> JumpVector;
Modified: trunk/Source/_javascript_Core/interpreter/Interpreter.cpp (136926 => 136927)
--- trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/interpreter/Interpreter.cpp 2012-12-07 07:44:01 UTC (rev 136927)
@@ -786,9 +786,12 @@
JSScope* scope = callFrame->scope();
int scopeDelta = 0;
if (!codeBlock->needsFullScopeChain() || codeBlock->codeType() != FunctionCode
- || callFrame->uncheckedR(codeBlock->activationRegister()).jsValue())
- scopeDelta = depth(codeBlock, scope) - handler->scopeDepth;
- ASSERT(scopeDelta >= 0);
+ || callFrame->uncheckedR(codeBlock->activationRegister()).jsValue()) {
+ int currentDepth = depth(codeBlock, scope);
+ int targetDepth = handler->scopeDepth;
+ scopeDelta = currentDepth - targetDepth;
+ ASSERT(scopeDelta >= 0);
+ }
while (scopeDelta--)
scope = scope->next();
callFrame->setScope(scope);
Modified: trunk/Source/_javascript_Core/llint/LLIntExceptions.cpp (136926 => 136927)
--- trunk/Source/_javascript_Core/llint/LLIntExceptions.cpp 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/llint/LLIntExceptions.cpp 2012-12-07 07:44:01 UTC (rev 136927)
@@ -50,7 +50,7 @@
JSGlobalData* globalData = &exec->globalData();
NativeCallFrameTracer tracer(globalData, exec);
#if LLINT_SLOW_PATH_TRACING
- dataLogF("Throwing exception %s.\n", globalData->exception.description());
+ dataLog("Throwing exception ", globalData->exception, ".\n");
#endif
fixupPCforExceptionIfNeeded(exec);
genericThrow(
@@ -69,7 +69,7 @@
JSGlobalData* globalData = &exec->globalData();
NativeCallFrameTracer tracer(globalData, exec);
#if LLINT_SLOW_PATH_TRACING
- dataLogF("Throwing exception %s (returnToThrow).\n", globalData->exception.description());
+ dataLog("Throwing exception ", globalData->exception, " (returnToThrow).\n");
#endif
fixupPCforExceptionIfNeeded(exec);
genericThrow(globalData, exec, globalData->exception, pc - exec->codeBlock()->instructions().begin());
@@ -82,7 +82,7 @@
JSGlobalData* globalData = &exec->globalData();
NativeCallFrameTracer tracer(globalData, exec);
#if LLINT_SLOW_PATH_TRACING
- dataLogF("Throwing exception %s (callToThrow).\n", globalData->exception.description());
+ dataLog("Throwing exception ", globalData->exception, " (callToThrow).\n");
#endif
fixupPCforExceptionIfNeeded(exec);
genericThrow(globalData, exec, globalData->exception, pc - exec->codeBlock()->instructions().begin());
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (136926 => 136927)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2012-12-07 07:24:23 UTC (rev 136926)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2012-12-07 07:44:01 UTC (rev 136927)
@@ -637,8 +637,7 @@
JSValue v2 = LLINT_OP_C(3).jsValue();
#if LLINT_SLOW_PATH_TRACING
- dataLogF("Trying to add %s", v1.description());
- dataLogF(" to %s.\n", v2.description());
+ dataLog("Trying to add ", v1, " to ", v2, ".\n");
#endif
if (v1.isString() && !v2.isObject())
@@ -1369,7 +1368,7 @@
}
#if LLINT_SLOW_PATH_TRACING
- dataLogF("Call callee is not a function: %s\n", callee.description());
+ dataLog("Call callee is not a function: ", callee, "\n");
#endif
ASSERT(callType == CallTypeNone);
@@ -1392,7 +1391,7 @@
}
#if LLINT_SLOW_PATH_TRACING
- dataLogF("Constructor callee is not a function: %s\n", callee.description());
+ dataLog("Constructor callee is not a function: ", callee, "\n");
#endif
ASSERT(constructType == ConstructTypeNone);
