Fix code generation for try statements

The old code was not correctly handling statements as
try { return; } finally {...}
and others. In addition it was hard to read an maintain.

We now keep a stack of try statements inside the code
generator. Loops know about their surrounding try statement.
Whenever a break, continue or return statement is encountered
we now generate code for the finally statements and exception
handlers we need to cleanup.

Change-Id: I53bcc0587f1e923be00fea9b562453ef1e96b2de
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

diff --git a/qv4codegen.cpp b/qv4codegen.cpp
index 1980ed9..c9978ed 100644 (file)
--- a/qv4codegen.cpp
+++ b/qv4codegen.cpp
@@ -364,6 +364,7 @@ Codegen::Codegen(Debugging::Debugger *debugger)
     , _env(0)
     , _loop(0)
     , _labelledStatement(0)
+    , _tryCleanup(0)
     , _debugger(debugger)
 {
 }
@@ -435,6 +436,7 @@ void Codegen::enterLoop(Statement *node, IR::BasicBlock *breakBlock, IR::BasicBl
 {
     _loop = new Loop(node, breakBlock, continueBlock, _loop);
     _loop->labelledStatement = _labelledStatement; // consume the enclosing labelled statement
+    _loop->tryCleanup = _tryCleanup;
     _labelledStatement = 0;
 }
 
@@ -1718,6 +1720,7 @@ bool Codegen::visit(Block *ast)
 
 bool Codegen::visit(BreakStatement *ast)
 {
+    unwindException(_loop->tryCleanup);
     if (ast->label.isEmpty())
         _block->JUMP(_loop->breakBlock);
     else {
@@ -1735,9 +1738,11 @@ bool Codegen::visit(BreakStatement *ast)
 
 bool Codegen::visit(ContinueStatement *ast)
 {
-    if (ast->label.isEmpty())
+    unwindException(_loop->tryCleanup);
+
+    if (ast->label.isEmpty()) {
         _block->JUMP(_loop->continueBlock);
-    else {
+    } else {
         for (Loop *loop = _loop; loop; loop = loop->parent) {
             if (loop->labelledStatement && loop->labelledStatement->label == ast->label) {
                 if (! loop->continueBlock)
@@ -1966,6 +1971,7 @@ bool Codegen::visit(LocalForStatement *ast)
 
 bool Codegen::visit(ReturnStatement *ast)
 {
+    unwindException(0);
     if (ast->expression) {
         Result expr = expression(ast->expression);
         move(_block->TEMP(_returnAddress), *expr);
@@ -2061,29 +2067,17 @@ bool Codegen::visit(TryStatement *ast)
 {
     IR::BasicBlock *tryBody = _function->newBasicBlock();
     IR::BasicBlock *catchBody = ast->catchExpression ?  _function->newBasicBlock() : 0;
-    IR::BasicBlock *finallyBody = ast->finallyExpression ? _function->newBasicBlock() : 0;
+    // We always need a finally body to clean up the exception handler
+    IR::BasicBlock *finallyBody = _function->newBasicBlock();
+
+    TryCleanup tcf(_tryCleanup, ast->finallyExpression);
+    _tryCleanup = &tcf;
+
     IR::BasicBlock *after = _function->newBasicBlock();
     assert(catchBody || finallyBody);
 
-    Loop *loop = 0;
-    if (_loop) {
-        // So there is a loop around the try statement, and now we have to setup
-        // a new loop-break-block and a new loop-continue-block so that if either
-        // a break or a continue is executed, they will go to these blocks
-        // which need to pop the exception handler. If there is a finally block,
-        // its code also gets generated into all the new break/continue blocks.
-        // And of course, as there can be a labeled break/continue, we have to
-        // do it for all parent loops as well.
-        for (Loop *it = _loop, **it2 = &loop; it; it = it->parent) {
-            *it2 = new Loop(it->node, _function->newBasicBlock(), _function->newBasicBlock(), 0);
-            (*it2)->labelledStatement = it->labelledStatement;
-            it2 = &(*it2)->parent;
-        }
-        std::swap(_loop, loop);
-    }
-
     int inCatch = 0;
-    if (catchBody && finallyBody) {
+    if (catchBody) {
         inCatch = _block->newTemp();
         move(_block->TEMP(inCatch), _block->CONST(IR::BoolType, false));
     }
@@ -2101,26 +2095,16 @@ bool Codegen::visit(TryStatement *ast)
     if (catchBody) {
         _block = catchBody;
 
-        if (finallyBody) {
-            if (inCatch != 0) {
-                // check if an exception got thrown within catch. Go to finally
-                // and then rethrow
-                IR::BasicBlock *b = _function->newBasicBlock();
-                _block->CJUMP(_block->TEMP(inCatch), finallyBody, b);
-                _block = b;
-            }
-            // if we have finally we need to clear the exception here, so we don't rethrow
-            move(_block->TEMP(inCatch), _block->CONST(IR::BoolType, true));
-            move(_block->TEMP(hasException), _block->CONST(IR::BoolType, false));
-        } else {
-            // otherwise remove the exception handler, so that throw inside catch will
-            // give the correct result
-            _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_delete_exception_handler, 0, 0), 0));
-
-            // the exception handler is deleted, so we can restore the
-            // original break/continue blocks.
-            std::swap(_loop, loop);
+        if (inCatch != 0) {
+            // check if an exception got thrown within catch. Go to finally
+            // and then rethrow
+            IR::BasicBlock *b = _function->newBasicBlock();
+            _block->CJUMP(_block->TEMP(inCatch), finallyBody, b);
+            _block = b;
         }
+        // if we have finally we need to clear the exception here, so we don't rethrow
+        move(_block->TEMP(inCatch), _block->CONST(IR::BoolType, true));
+        move(_block->TEMP(hasException), _block->CONST(IR::BoolType, false));
 
         const int exception = _block->newTemp();
         move(_block->TEMP(exception), _block->CALL(_block->NAME(IR::Name::builtin_get_exception, 0, 0), 0));
@@ -2137,30 +2121,10 @@ bool Codegen::visit(TryStatement *ast)
         _block->JUMP(finallyBody ? finallyBody : after);
     }
 
-    if (finallyBody) {
-        // the exception handler is deleted, so we can restore the
-        // original break/continue blocks.
-        std::swap(_loop, loop);
+    _tryCleanup = tcf.parent;
 
+    if (finallyBody)
         generateFinallyBlock(finallyBody, !catchBody, ast->finallyExpression, hasException, after);
-    }
-
-    if (loop) {
-        // now do the codegen for each break/continue block
-        for (Loop *it = loop, *it2 = _loop; it && it2; it = it->parent, it2 = it2->parent) {
-            // if the break/continue trampoline blocks get reached, there cannot
-            // be a pending exception, so don't bother with saving it.
-            generateFinallyBlock(it->breakBlock, false, ast->finallyExpression, hasException, it2->breakBlock);
-            generateFinallyBlock(it->continueBlock, false, ast->finallyExpression, hasException, it2->continueBlock);
-        }
-
-        // we don't need the Loop struct itself anymore.
-        while (loop) {
-            Loop *next = loop->parent;
-            delete loop;
-            loop = next;
-        }
-    }
 
     _block = after;
 
@@ -2186,9 +2150,6 @@ void Codegen::generateFinallyBlock(IR::BasicBlock *finallyBlock, bool exceptionN
         _block = realFinallyBlock;
     }
 
-    // TODO: this will not re-throw when there is a pending exception, and
-    // the finally block contains a break/continue. Should another set of
-    // break/continue trampoline blocks get inserted?
     _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_delete_exception_handler, 0, 0), 0));
     if (ast && ast->statement)
         statement(ast->statement);
@@ -2204,6 +2165,19 @@ void Codegen::generateFinallyBlock(IR::BasicBlock *finallyBlock, bool exceptionN
     }
 }
 
+void Codegen::unwindException(Codegen::TryCleanup *outest)
+{
+    TryCleanup *tryCleanup = _tryCleanup;
+    qSwap(_tryCleanup, tryCleanup);
+    while (_tryCleanup != outest) {
+        _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_delete_exception_handler, 0, 0), 0));
+        TryCleanup *tc = _tryCleanup;
+        _tryCleanup = tc->parent;
+        if (tc->finally && tc->finally->statement)
+            statement(tc->finally->statement);
+    }
+}
+
 bool Codegen::visit(VariableStatement *ast)
 {
     variableDeclarationList(ast->declarations);

diff --git a/qv4codegen_p.h b/qv4codegen_p.h
index 27c910f..a75c872 100644 (file)
--- a/qv4codegen_p.h
+++ b/qv4codegen_p.h
@@ -167,12 +167,21 @@ protected:
     struct UiMember {
     };
 
+    struct TryCleanup {
+        TryCleanup *parent;
+        AST::Finally *finally;
+
+        TryCleanup(TryCleanup *parent, AST::Finally *finally)
+            : parent(parent), finally(finally) {}
+    };
+
     struct Loop {
         AST::LabelledStatement *labelledStatement;
         AST::Statement *node;
         IR::BasicBlock *breakBlock;
         IR::BasicBlock *continueBlock;
         Loop *parent;
+        TryCleanup *tryCleanup;
 
         Loop(AST::Statement *node, IR::BasicBlock *breakBlock, IR::BasicBlock *continueBlock, Loop *parent)
             : labelledStatement(0), node(node), breakBlock(breakBlock), continueBlock(continueBlock), parent(parent) {}
@@ -184,6 +193,7 @@ protected:
     void enterLoop(AST::Statement *node, IR::BasicBlock *breakBlock, IR::BasicBlock *continueBlock);
     void leaveLoop();
 
+
     IR::Expr *member(IR::Expr *base, const QString *name);
     IR::Expr *subscript(IR::Expr *base, IR::Expr *index);
     IR::Expr *argument(IR::Expr *expr);
@@ -199,6 +209,7 @@ protected:
     int indexOfArgument(const QStringRef &string) const;
 
     void generateFinallyBlock(IR::BasicBlock *finallyBlock, bool exceptionNeedsSaving, AST::Finally *ast, int hasException, IR::BasicBlock *after);
+    void unwindException(TryCleanup *outest);
 
     void statement(AST::Statement *ast);
     void statement(AST::ExpressionNode *ast);
@@ -332,6 +343,7 @@ private:
     Environment *_env;
     Loop *_loop;
     AST::LabelledStatement *_labelledStatement;
+    TryCleanup *_tryCleanup;
     QHash<AST::Node *, Environment *> _envMap;
     QHash<AST::FunctionExpression *, int> _functionMap;
     Debugging::Debugger *_debugger;