Fix throwing an exception inside a finally block with a return in catch

When exiting a catch block with a return statement, we'll unwind the
exception handling manually and emit finally statements right before
jumping to the exit block. If we throw an exception in the final block,
we'll end up using the exception handler of the catch block that
contains the return statement, which means we'll end up popping the
exception scope one too many times, once through
ScopeAndFinally::CatchScope in unwindException() and then when executing
the exception handler block. The latter we should not be executing,
instead we should jump straight to the exit block. Therefore any
statements emitted as part of the manual exception unwinding (finally
block here) need to be part of a new basic block with no exception
handler.

This bug became visible in debug builds where the Scope destructor
compares the scope mark against the engine stack top to ensure correct
cleanup order (which was wrong). However that in turn was hidden in
debug builds again due to an accidental = instead of == in a Q_ASSERT.

With the Q_ASSERT fixed this use-case is covered by
ch12/12.14/S12.14_A13_T3

Change-Id: Id74a1b2bb3e063871b89cc05353b601dd60df08e
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

2 files changed, 10 insertions, 3 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 6bf931c882..d29b11ac84 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -2506,6 +2506,13 @@ bool Codegen::visit(ReturnStatement *ast)
         Result expr = expression(ast->expression);
         move(_block->TEMP(_returnAddress), *expr);
     }
+
+    // Since we're leaving, don't let any finally statements we emit as part of the unwinding
+    // jump to exception handlers at run-time if they throw.
+    IR::BasicBlock *unwindBlock = _function->newBasicBlock(/*no exception handler*/Q_NULLPTR);
+    _block->JUMP(unwindBlock);
+    _block = unwindBlock;
+
     unwindException(0);
 
     _block->JUMP(_exitBlock);
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index d8ae7d4e92..0a05c50432 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -1204,19 +1204,19 @@ ReturnedValue Runtime::unwindException(ExecutionEngine *engine)
 void Runtime::pushWithScope(const Value &o, ExecutionEngine *engine)
 {
     engine->pushContext(engine->currentContext->newWithContext(o.toObject(engine)));
-    Q_ASSERT(engine->jsStackTop = engine->currentContext + 2);
+    Q_ASSERT(engine->jsStackTop == engine->currentContext + 2);
 }
 
 void Runtime::pushCatchScope(NoThrowEngine *engine, int exceptionVarNameIndex)
 {
     ExecutionContext *c = engine->currentContext;
     engine->pushContext(c->newCatchContext(c->d()->compilationUnit->runtimeStrings[exceptionVarNameIndex], engine->catchException(0)));
-    Q_ASSERT(engine->jsStackTop = engine->currentContext + 2);
+    Q_ASSERT(engine->jsStackTop == engine->currentContext + 2);
 }
 
 void Runtime::popScope(ExecutionEngine *engine)
 {
-    Q_ASSERT(engine->jsStackTop = engine->currentContext + 2);
+    Q_ASSERT(engine->jsStackTop == engine->currentContext + 2);
     engine->popContext();
     engine->jsStackTop -= 2;
 }