Diff
Modified: trunk/LayoutTests/ChangeLog (126717 => 126718)
--- trunk/LayoutTests/ChangeLog 2012-08-26 23:48:39 UTC (rev 126717)
+++ trunk/LayoutTests/ChangeLog 2012-08-27 00:49:25 UTC (rev 126718)
@@ -1,3 +1,16 @@
+2012-08-26 Filip Pizlo <[email protected]>
+
+ Finally inlining should correctly track the catch context
+ https://bugs.webkit.org/show_bug.cgi?id=94986
+ <rdar://problem/11753784>
+
+ Reviewed by Sam Weinig.
+
+ * fast/js/jsc-test-list:
+ * fast/js/script-tests/throw-from-finally.js: Added.
+ * fast/js/throw-from-finally.html: Added.
+ * fast/js/throw-from-finally-expected.txt: Added.
+
2012-08-26 Robert Hogan <[email protected]>
Suppress Mac test requiring rebaseline after r126683.
Modified: trunk/LayoutTests/fast/js/jsc-test-list (126717 => 126718)
--- trunk/LayoutTests/fast/js/jsc-test-list 2012-08-26 23:48:39 UTC (rev 126717)
+++ trunk/LayoutTests/fast/js/jsc-test-list 2012-08-27 00:49:25 UTC (rev 126718)
@@ -302,6 +302,7 @@
fast/js/string-trim
fast/js/string_replace
fast/js/this-non-object-proto
+fast/js/throw-from-finally
fast/js/ToNumber
fast/js/toString-elision-trailing-comma
fast/js/tostring-exception-in-property-access
Added: trunk/LayoutTests/fast/js/script-tests/throw-from-finally.js (0 => 126718)
--- trunk/LayoutTests/fast/js/script-tests/throw-from-finally.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/throw-from-finally.js 2012-08-27 00:49:25 UTC (rev 126718)
@@ -0,0 +1,58 @@
+description(
+"This tests that throwing from a finally block has the expected effect."
+);
+
+var events = [];
+
+try {
+ events.push("1:try");
+} finally {
+ events.push("1:finally");
+}
+
+try {
+ try {
+ throw "2:thingy";
+ } finally {
+ events.push("2:finally");
+ }
+} catch (e) {
+ events.push(e);
+}
+
+try {
+ throw "3:thingy";
+} catch (e) {
+ events.push(e);
+} finally {
+ events.push("3:finally");
+}
+
+try {
+ try {
+ throw "4:thingy";
+ } catch (e) {
+ events.push(e);
+ } finally {
+ events.push("4:finally");
+ throw "4:another thingy";
+ }
+} catch (e) {
+ events.push(e);
+}
+
+try {
+ for (;;) {
+ try {
+ continue;
+ } finally {
+ events.push("5:hi");
+ throw "5:wat";
+ }
+ }
+} catch (e) {
+ events.push(e);
+}
+
+shouldBe("\"\" + events", "\"1:try,1:finally,2:finally,2:thingy,3:thingy,3:finally,4:thingy,4:finally,4:another thingy,5:hi,5:wat\"");
+
Added: trunk/LayoutTests/fast/js/throw-from-finally-expected.txt (0 => 126718)
--- trunk/LayoutTests/fast/js/throw-from-finally-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/throw-from-finally-expected.txt 2012-08-27 00:49:25 UTC (rev 126718)
@@ -0,0 +1,10 @@
+This tests that throwing from a finally block has the expected effect.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS "" + events is "1:try,1:finally,2:finally,2:thingy,3:thingy,3:finally,4:thingy,4:finally,4:another thingy,5:hi,5:wat"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/throw-from-finally.html (0 => 126718)
--- trunk/LayoutTests/fast/js/throw-from-finally.html (rev 0)
+++ trunk/LayoutTests/fast/js/throw-from-finally.html 2012-08-27 00:49:25 UTC (rev 126718)
@@ -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 (126717 => 126718)
--- trunk/Source/_javascript_Core/ChangeLog 2012-08-26 23:48:39 UTC (rev 126717)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-08-27 00:49:25 UTC (rev 126718)
@@ -1,3 +1,38 @@
+2012-08-24 Filip Pizlo <[email protected]>
+
+ Finally inlining should correctly track the catch context
+ https://bugs.webkit.org/show_bug.cgi?id=94986
+ <rdar://problem/11753784>
+
+ Reviewed by Sam Weinig.
+
+ This fixes two behaviors:
+
+ 1) Throwing from a finally block. Previously, we would seem to reenter the finally
+ block - though only once.
+
+ 2) Executing a finally block from some nested context, for example due to a
+ 'continue', 'break', or 'return' in the try. This would execute the finally
+ block in the context of of the try block, which could lead to either scope depth
+ mismatches or reexecutions of the finally block on throw, similarly to (1) but
+ for different reasons.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC):
+ (JSC::BytecodeGenerator::pushFinallyContext):
+ (JSC::BytecodeGenerator::emitComplexJumpScopes):
+ (JSC::BytecodeGenerator::pushTry):
+ (JSC::BytecodeGenerator::popTryAndEmitCatch):
+ * bytecompiler/BytecodeGenerator.h:
+ (FinallyContext):
+ (TryData):
+ (JSC):
+ (TryContext):
+ (TryRange):
+ (BytecodeGenerator):
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::TryNode::emitBytecode):
+
2012-08-26 Filip Pizlo <[email protected]>
Array type checks and storage accesses should be uniformly represented and available to CSE
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (126717 => 126718)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2012-08-26 23:48:39 UTC (rev 126717)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2012-08-27 00:49:25 UTC (rev 126718)
@@ -188,6 +188,24 @@
m_scopeNode->emitBytecode(*this);
+ for (unsigned i = 0; i < m_tryRanges.size(); ++i) {
+ TryRange& range = m_tryRanges[i];
+ ASSERT(range.tryData->targetScopeDepth != UINT_MAX);
+ HandlerInfo info = {
+ range.start->bind(0, 0), range.end->bind(0, 0),
+ range.tryData->target->bind(0, 0), range.tryData->targetScopeDepth
+#if ENABLE(JIT)
+ ,
+#if ENABLE(LLINT)
+ CodeLocationLabel(MacroAssemblerCodePtr::createFromExecutableAddress(bitwise_cast<void*>(&llint_op_catch)))
+#else
+ CodeLocationLabel()
+#endif
+#endif
+ };
+ m_codeBlock->addExceptionHandler(info);
+ }
+
m_codeBlock->instructions() = RefCountedArray<Instruction>(m_instructions);
if (s_dumpsGeneratedCode)
@@ -2122,6 +2140,7 @@
m_scopeContextStack.size(),
m_switchContextStack.size(),
m_forInContextStack.size(),
+ m_tryContextStack.size(),
m_labelScopes.size(),
m_finallyDepth,
m_dynamicScopeDepth
@@ -2255,14 +2274,18 @@
Vector<ControlFlowContext> savedScopeContextStack;
Vector<SwitchInfo> savedSwitchContextStack;
Vector<ForInContext> savedForInContextStack;
+ Vector<TryContext> poppedTryContexts;
SegmentedVector<LabelScope, 8> savedLabelScopes;
while (topScope > bottomScope && topScope->isFinallyBlock) {
+ RefPtr<Label> beforeFinally = emitLabel(newLabel().get());
+
// Save the current state of the world while instating the state of the world
// for the finally block.
FinallyContext finallyContext = topScope->finallyContext;
bool flipScopes = finallyContext.scopeContextStackSize != m_scopeContextStack.size();
bool flipSwitches = finallyContext.switchContextStackSize != m_switchContextStack.size();
bool flipForIns = finallyContext.forInContextStackSize != m_forInContextStack.size();
+ bool flipTries = finallyContext.tryContextStackSize != m_tryContextStack.size();
bool flipLabelScopes = finallyContext.labelScopesSize != m_labelScopes.size();
int topScopeIndex = -1;
int bottomScopeIndex = -1;
@@ -2280,6 +2303,19 @@
savedForInContextStack = m_forInContextStack;
m_forInContextStack.shrink(finallyContext.forInContextStackSize);
}
+ if (flipTries) {
+ while (m_tryContextStack.size() != finallyContext.tryContextStackSize) {
+ ASSERT(m_tryContextStack.size() > finallyContext.tryContextStackSize);
+ TryContext context = m_tryContextStack.last();
+ m_tryContextStack.removeLast();
+ TryRange range;
+ range.start = context.start;
+ range.end = beforeFinally;
+ range.tryData = context.tryData;
+ m_tryRanges.append(range);
+ poppedTryContexts.append(context);
+ }
+ }
if (flipLabelScopes) {
savedLabelScopes = m_labelScopes;
while (m_labelScopes.size() > finallyContext.labelScopesSize)
@@ -2293,6 +2329,8 @@
// Emit the finally block.
emitNode(finallyContext.finallyBlock);
+ RefPtr<Label> afterFinally = emitLabel(newLabel().get());
+
// Restore the state of the world.
if (flipScopes) {
m_scopeContextStack = savedScopeContextStack;
@@ -2303,6 +2341,14 @@
m_switchContextStack = savedSwitchContextStack;
if (flipForIns)
m_forInContextStack = savedForInContextStack;
+ if (flipTries) {
+ ASSERT(m_tryContextStack.size() == finallyContext.tryContextStackSize);
+ for (unsigned i = poppedTryContexts.size(); i--;) {
+ TryContext context = poppedTryContexts[i];
+ context.start = afterFinally;
+ m_tryContextStack.append(context);
+ }
+ }
if (flipLabelScopes)
m_labelScopes = savedLabelScopes;
m_finallyDepth = savedFinallyDepth;
@@ -2362,20 +2408,39 @@
return dst;
}
-RegisterID* BytecodeGenerator::emitCatch(RegisterID* targetRegister, Label* start, Label* end)
+TryData* BytecodeGenerator::pushTry(Label* start)
{
+ TryData tryData;
+ tryData.target = newLabel();
+ tryData.targetScopeDepth = UINT_MAX;
+ m_tryData.append(tryData);
+ TryData* result = &m_tryData.last();
+
+ TryContext tryContext;
+ tryContext.start = start;
+ tryContext.tryData = result;
+
+ m_tryContextStack.append(tryContext);
+
+ return result;
+}
+
+RegisterID* BytecodeGenerator::popTryAndEmitCatch(TryData* tryData, RegisterID* targetRegister, Label* end)
+{
m_usesExceptions = true;
-#if ENABLE(JIT)
-#if ENABLE(LLINT)
- HandlerInfo info = { start->bind(0, 0), end->bind(0, 0), instructions().size(), m_dynamicScopeDepth + m_baseScopeDepth, CodeLocationLabel(MacroAssemblerCodePtr::createFromExecutableAddress(bitwise_cast<void*>(&llint_op_catch))) };
-#else
- HandlerInfo info = { start->bind(0, 0), end->bind(0, 0), instructions().size(), m_dynamicScopeDepth + m_baseScopeDepth, CodeLocationLabel() };
-#endif
-#else
- HandlerInfo info = { start->bind(0, 0), end->bind(0, 0), instructions().size(), m_dynamicScopeDepth + m_baseScopeDepth };
-#endif
+
+ ASSERT_UNUSED(tryData, m_tryContextStack.last().tryData == tryData);
+
+ TryRange tryRange;
+ tryRange.start = m_tryContextStack.last().start;
+ tryRange.end = end;
+ tryRange.tryData = m_tryContextStack.last().tryData;
+ m_tryRanges.append(tryRange);
+ m_tryContextStack.removeLast();
+
+ emitLabel(tryRange.tryData->target.get());
+ tryRange.tryData->targetScopeDepth = m_dynamicScopeDepth + m_baseScopeDepth;
- m_codeBlock->addExceptionHandler(info);
emitOpcode(op_catch);
instructions().append(targetRegister->index());
return targetRegister;
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (126717 => 126718)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2012-08-26 23:48:39 UTC (rev 126717)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2012-08-27 00:49:25 UTC (rev 126718)
@@ -75,6 +75,7 @@
unsigned scopeContextStackSize;
unsigned switchContextStackSize;
unsigned forInContextStackSize;
+ unsigned tryContextStackSize;
unsigned labelScopesSize;
int finallyDepth;
int dynamicScopeDepth;
@@ -92,6 +93,22 @@
RefPtr<RegisterID> propertyRegister;
};
+ struct TryData {
+ RefPtr<Label> target;
+ unsigned targetScopeDepth;
+ };
+
+ struct TryContext {
+ RefPtr<Label> start;
+ TryData* tryData;
+ };
+
+ struct TryRange {
+ RefPtr<Label> start;
+ RefPtr<Label> end;
+ TryData* tryData;
+ };
+
class ResolveResult {
public:
enum Flags {
@@ -492,7 +509,11 @@
RegisterID* emitGetPropertyNames(RegisterID* dst, RegisterID* base, RegisterID* i, RegisterID* size, Label* breakTarget);
RegisterID* emitNextPropertyName(RegisterID* dst, RegisterID* base, RegisterID* i, RegisterID* size, RegisterID* iter, Label* target);
- RegisterID* emitCatch(RegisterID*, Label* start, Label* end);
+ // Start a try block. 'start' must have been emitted.
+ TryData* pushTry(Label* start);
+ // End a try block. 'end' must have been emitted.
+ RegisterID* popTryAndEmitCatch(TryData*, RegisterID* targetRegister, Label* end);
+
void emitThrow(RegisterID* exc)
{
m_usesExceptions = true;
@@ -634,7 +655,9 @@
RegisterID* emitInitLazyRegister(RegisterID*);
+ public:
Vector<Instruction>& instructions() { return m_instructions; }
+
SymbolTable& symbolTable() { return *m_symbolTable; }
#if ENABLE(BYTECODE_COMMENTS)
Vector<Comment>& comments() { return m_comments; }
@@ -708,6 +731,10 @@
Vector<ControlFlowContext> m_scopeContextStack;
Vector<SwitchInfo> m_switchContextStack;
Vector<ForInContext> m_forInContextStack;
+ Vector<TryContext> m_tryContextStack;
+
+ Vector<TryRange> m_tryRanges;
+ SegmentedVector<TryData, 8> m_tryData;
int m_firstConstantIndex;
int m_nextConstantOffset;
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (126717 => 126718)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2012-08-26 23:48:39 UTC (rev 126717)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2012-08-27 00:49:25 UTC (rev 126718)
@@ -1970,11 +1970,15 @@
generator.emitDebugHook(WillExecuteStatement, firstLine(), lastLine(), column());
+ ASSERT(m_catchBlock || m_finallyBlock);
+
RefPtr<Label> tryStartLabel = generator.newLabel();
+ generator.emitLabel(tryStartLabel.get());
+
if (m_finallyBlock)
generator.pushFinallyContext(m_finallyBlock);
+ TryData* tryData = generator.pushTry(tryStartLabel.get());
- generator.emitLabel(tryStartLabel.get());
generator.emitNode(dst, m_tryBlock);
if (m_catchBlock) {
@@ -1985,7 +1989,14 @@
// Uncaught exception path: the catch block.
RefPtr<Label> here = generator.emitLabel(generator.newLabel().get());
- RefPtr<RegisterID> exceptionRegister = generator.emitCatch(generator.newTemporary(), tryStartLabel.get(), here.get());
+ RefPtr<RegisterID> exceptionRegister = generator.popTryAndEmitCatch(tryData, generator.newTemporary(), here.get());
+
+ if (m_finallyBlock) {
+ // If the catch block throws an exception and we have a finally block, then the finally
+ // block should "catch" that exception.
+ tryData = generator.pushTry(here.get());
+ }
+
generator.emitPushNewScope(exceptionRegister.get(), m_exceptionIdent, exceptionRegister.get());
generator.emitNode(dst, m_catchBlock);
generator.emitPopScope();
@@ -1993,6 +2004,8 @@
}
if (m_finallyBlock) {
+ RefPtr<Label> preFinallyLabel = generator.emitLabel(generator.newLabel().get());
+
generator.popFinallyContext();
RefPtr<Label> finallyEndLabel = generator.newLabel();
@@ -2002,8 +2015,7 @@
generator.emitJump(finallyEndLabel.get());
// Uncaught exception path: invoke the finally block, then re-throw the exception.
- RefPtr<Label> here = generator.emitLabel(generator.newLabel().get());
- RefPtr<RegisterID> tempExceptionRegister = generator.emitCatch(generator.newTemporary(), tryStartLabel.get(), here.get());
+ RefPtr<RegisterID> tempExceptionRegister = generator.popTryAndEmitCatch(tryData, generator.newTemporary(), preFinallyLabel.get());
generator.emitNode(dst, m_finallyBlock);
generator.emitThrow(tempExceptionRegister.get());
