V4: Fix JavaScript finally-block execution

After moving all runtime functions into the Runtime class and doing
indirect function calls, the code generation would always emit code
to check for an exception after a call. This is problematic for methods
that do not throw, but might be called when an exception is thrown. I.e.
in a finally block. This is especially problematic for methods like
popScope, the very first runtime method that is called in a finally
block. The result was that after popScope, execution was passed over to
the exception handler block for that finally block (meaning: the body
of the finally block was never executed).

The fix is to declare an enumerator in an anonymous enum for each
runtime method that indicates if an exception check is needed. The
existing ExceptionCheck templates are used to set the value.

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

7 files changed, 84 insertions, 67 deletions

diff --git a/src/qml/jit/qv4assembler_p.h b/src/qml/jit/qv4assembler_p.h
index e29b165c2d..2ef0db78c0 100644
--- a/src/qml/jit/qv4assembler_p.h
+++ b/src/qml/jit/qv4assembler_p.h
@@ -108,36 +108,6 @@ struct RuntimeCall {
     bool isValid() const { return addr.offset >= 0; }
 };
 
-template <typename T>
-struct ExceptionCheck {
-    enum { NeedsCheck = 1 };
-};
-// push_catch and pop context methods shouldn't check for exceptions
-template <>
-struct ExceptionCheck<void (*)(QV4::ExecutionEngine *)> {
-    enum { NeedsCheck = 0 };
-};
-template <typename A>
-struct ExceptionCheck<void (*)(A, QV4::NoThrowEngine)> {
-    enum { NeedsCheck = 0 };
-};
-template <>
-struct ExceptionCheck<QV4::ReturnedValue (*)(QV4::NoThrowEngine *)> {
-    enum { NeedsCheck = 0 };
-};
-template <typename A>
-struct ExceptionCheck<QV4::ReturnedValue (*)(QV4::NoThrowEngine *, A)> {
-    enum { NeedsCheck = 0 };
-};
-template <typename A, typename B>
-struct ExceptionCheck<QV4::ReturnedValue (*)(QV4::NoThrowEngine *, A, B)> {
-    enum { NeedsCheck = 0 };
-};
-template <typename A, typename B, typename C>
-struct ExceptionCheck<void (*)(QV4::NoThrowEngine *, A, B, C)> {
-    enum { NeedsCheck = 0 };
-};
-
 class Assembler : public JSC::MacroAssembler, public TargetPlatform
 {
     Q_DISABLE_COPY(Assembler)
@@ -809,7 +779,7 @@ public:
     };
 
     template <typename ArgRet, typename Callable, typename Arg1, typename Arg2, typename Arg3, typename Arg4, typename Arg5, typename Arg6>
-    void generateFunctionCallImp(ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3, Arg4 arg4, Arg5 arg5, Arg6 arg6)
+    void generateFunctionCallImp(bool needsExceptionCheck, ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3, Arg4 arg4, Arg5 arg5, Arg6 arg6)
     {
         int stackSpaceNeeded =   SizeOnStack<0, Arg1>::Size
                                + SizeOnStack<1, Arg2>::Size
@@ -852,7 +822,7 @@ public:
         if (stackSpaceNeeded)
             addPtr(TrustedImm32(stackSpaceNeeded), StackPointerRegister);
 
-        if (ExceptionCheck<Callable>::NeedsCheck) {
+        if (needsExceptionCheck) {
             checkException();
         }
 
@@ -861,33 +831,33 @@ public:
     }
 
     template <typename ArgRet, typename Callable, typename Arg1, typename Arg2, typename Arg3, typename Arg4, typename Arg5>
-    void generateFunctionCallImp(ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3, Arg4 arg4, Arg5 arg5)
+    void generateFunctionCallImp(bool needsExceptionCheck, ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3, Arg4 arg4, Arg5 arg5)
     {
-        generateFunctionCallImp(r, functionName, function, arg1, arg2, arg3, arg4, arg5, VoidType());
+        generateFunctionCallImp(needsExceptionCheck, r, functionName, function, arg1, arg2, arg3, arg4, arg5, VoidType());
     }
 
     template <typename ArgRet, typename Callable, typename Arg1, typename Arg2, typename Arg3, typename Arg4>
-    void generateFunctionCallImp(ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3, Arg4 arg4)
+    void generateFunctionCallImp(bool needsExceptionCheck, ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3, Arg4 arg4)
     {
-        generateFunctionCallImp(r, functionName, function, arg1, arg2, arg3, arg4, VoidType(), VoidType());
+        generateFunctionCallImp(needsExceptionCheck, r, functionName, function, arg1, arg2, arg3, arg4, VoidType(), VoidType());
     }
 
     template <typename ArgRet, typename Callable, typename Arg1, typename Arg2, typename Arg3>
-    void generateFunctionCallImp(ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3)
+    void generateFunctionCallImp(bool needsExceptionCheck, ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2, Arg3 arg3)
     {
-        generateFunctionCallImp(r, functionName, function, arg1, arg2, arg3, VoidType(), VoidType(), VoidType());
+        generateFunctionCallImp(needsExceptionCheck, r, functionName, function, arg1, arg2, arg3, VoidType(), VoidType(), VoidType());
     }
 
     template <typename ArgRet, typename Callable, typename Arg1, typename Arg2>
-    void generateFunctionCallImp(ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2)
+    void generateFunctionCallImp(bool needsExceptionCheck, ArgRet r, const char* functionName, Callable function, Arg1 arg1, Arg2 arg2)
     {
-        generateFunctionCallImp(r, functionName, function, arg1, arg2, VoidType(), VoidType(), VoidType(), VoidType());
+        generateFunctionCallImp(needsExceptionCheck, r, functionName, function, arg1, arg2, VoidType(), VoidType(), VoidType(), VoidType());
     }
 
     template <typename ArgRet, typename Callable, typename Arg1>
-    void generateFunctionCallImp(ArgRet r, const char* functionName, Callable function, Arg1 arg1)
+    void generateFunctionCallImp(bool needsExceptionCheck, ArgRet r, const char* functionName, Callable function, Arg1 arg1)
     {
-        generateFunctionCallImp(r, functionName, function, arg1, VoidType(), VoidType(), VoidType(), VoidType(), VoidType());
+        generateFunctionCallImp(needsExceptionCheck, r, functionName, function, arg1, VoidType(), VoidType(), VoidType(), VoidType(), VoidType());
     }
 
     Pointer toAddress(RegisterID tmpReg, IR::Expr *e, int offset)
diff --git a/src/qml/jit/qv4binop.cpp b/src/qml/jit/qv4binop.cpp
index 45cc9259c3..9c535bb0bb 100644
--- a/src/qml/jit/qv4binop.cpp
+++ b/src/qml/jit/qv4binop.cpp
@@ -45,17 +45,17 @@ using namespace QV4;
 using namespace JIT;
 
 #define OP(op) \
-    { "Runtime::" isel_stringIfy(op), offsetof(QV4::Runtime, op), INT_MIN, 0, 0 }
+    { "Runtime::" isel_stringIfy(op), offsetof(QV4::Runtime, op), INT_MIN, 0, 0, QV4::Runtime::Method_##op##_NeedsExceptionCheck }
 #define OPCONTEXT(op) \
-    { "Runtime::" isel_stringIfy(op), INT_MIN, offsetof(QV4::Runtime, op), 0, 0 }
+    { "Runtime::" isel_stringIfy(op), INT_MIN, offsetof(QV4::Runtime, op), 0, 0, QV4::Runtime::Method_##op##_NeedsExceptionCheck }
 
 #define INLINE_OP(op, memOp, immOp) \
-    { "Runtime::" isel_stringIfy(op), offsetof(QV4::Runtime, op), INT_MIN, memOp, immOp }
+    { "Runtime::" isel_stringIfy(op), offsetof(QV4::Runtime, op), INT_MIN, memOp, immOp, QV4::Runtime::Method_##op##_NeedsExceptionCheck }
 #define INLINE_OPCONTEXT(op, memOp, immOp) \
-    { "Runtime::" isel_stringIfy(op), INT_MIN, offsetof(QV4::Runtime, op), memOp, immOp }
+    { "Runtime::" isel_stringIfy(op), INT_MIN, offsetof(QV4::Runtime, op), memOp, immOp, QV4::Runtime::Method_##op##_NeedsExceptionCheck }
 
 #define NULL_OP \
-    { 0, 0, 0, 0, 0 }
+    { 0, 0, 0, 0, 0, false }
 
 const Binop::OpInfo Binop::operations[IR::LastAluOp + 1] = {
     NULL_OP, // OpInvalid
@@ -128,11 +128,11 @@ void Binop::generate(IR::Expr *lhs, IR::Expr *rhs, IR::Expr *target)
     RuntimeCall fallBack(info.fallbackImplementation);
     RuntimeCall context(info.contextImplementation);
     if (fallBack.isValid()) {
-        as->generateFunctionCallImp(target, info.name, fallBack,
+        as->generateFunctionCallImp(info.needsExceptionCheck, target, info.name, fallBack,
                                      Assembler::PointerToValue(lhs),
                                      Assembler::PointerToValue(rhs));
     } else if (context.isValid()) {
-        as->generateFunctionCallImp(target, info.name, context,
+        as->generateFunctionCallImp(info.needsExceptionCheck, target, info.name, context,
                                      Assembler::EngineRegister,
                                      Assembler::PointerToValue(lhs),
                                      Assembler::PointerToValue(rhs));
diff --git a/src/qml/jit/qv4binop_p.h b/src/qml/jit/qv4binop_p.h
index c246ee43b0..37601f54ba 100644
--- a/src/qml/jit/qv4binop_p.h
+++ b/src/qml/jit/qv4binop_p.h
@@ -81,6 +81,7 @@ struct Binop {
         int contextImplementation; // offsetOf(Runtime,...)
         MemRegOp inlineMemRegOp;
         ImmRegOp inlineImmRegOp;
+        bool needsExceptionCheck;
     };
 
     static const OpInfo operations[IR::LastAluOp + 1];
diff --git a/src/qml/jit/qv4isel_masm.cpp b/src/qml/jit/qv4isel_masm.cpp
index da28df817d..c1c42f876c 100644
--- a/src/qml/jit/qv4isel_masm.cpp
+++ b/src/qml/jit/qv4isel_masm.cpp
@@ -970,9 +970,15 @@ void InstructionSelection::swapValues(IR::Expr *source, IR::Expr *target)
 }
 
 #define setOp(op, opName, operation) \
-    do { op = RuntimeCall(qOffsetOf(QV4::Runtime, operation)); opName = "Runtime::" isel_stringIfy(operation); } while (0)
+    do { \
+        op = RuntimeCall(qOffsetOf(QV4::Runtime, operation)); opName = "Runtime::" isel_stringIfy(operation); \
+        needsExceptionCheck = QV4::Runtime::Method_##operation##_NeedsExceptionCheck; \
+    } while (0)
 #define setOpContext(op, opName, operation) \
-    do { opContext = RuntimeCall(qOffsetOf(QV4::Runtime, operation)); opName = "Runtime::" isel_stringIfy(operation); } while (0)
+    do { \
+        opContext = RuntimeCall(qOffsetOf(QV4::Runtime, operation)); opName = "Runtime::" isel_stringIfy(operation); \
+        needsExceptionCheck = QV4::Runtime::Method_##operation##_NeedsExceptionCheck; \
+    } while (0)
 
 void InstructionSelection::unop(IR::AluOp oper, IR::Expr *source, IR::Expr *target)
 {
@@ -1446,18 +1452,19 @@ void InstructionSelection::visitCJump(IR::CJump *s)
         RuntimeCall op;
         RuntimeCall opContext;
         const char *opName = 0;
+        bool needsExceptionCheck;
         switch (b->op) {
         default: Q_UNREACHABLE(); Q_ASSERT(!"todo"); break;
-        case IR::OpGt: setOp(op, opName, Runtime::compareGreaterThan); break;
-        case IR::OpLt: setOp(op, opName, Runtime::compareLessThan); break;
-        case IR::OpGe: setOp(op, opName, Runtime::compareGreaterEqual); break;
-        case IR::OpLe: setOp(op, opName, Runtime::compareLessEqual); break;
-        case IR::OpEqual: setOp(op, opName, Runtime::compareEqual); break;
-        case IR::OpNotEqual: setOp(op, opName, Runtime::compareNotEqual); break;
-        case IR::OpStrictEqual: setOp(op, opName, Runtime::compareStrictEqual); break;
-        case IR::OpStrictNotEqual: setOp(op, opName, Runtime::compareStrictNotEqual); break;
-        case IR::OpInstanceof: setOpContext(op, opName, Runtime::compareInstanceof); break;
-        case IR::OpIn: setOpContext(op, opName, Runtime::compareIn); break;
+        case IR::OpGt: setOp(op, opName, compareGreaterThan); break;
+        case IR::OpLt: setOp(op, opName, compareLessThan); break;
+        case IR::OpGe: setOp(op, opName, compareGreaterEqual); break;
+        case IR::OpLe: setOp(op, opName, compareLessEqual); break;
+        case IR::OpEqual: setOp(op, opName, compareEqual); break;
+        case IR::OpNotEqual: setOp(op, opName, compareNotEqual); break;
+        case IR::OpStrictEqual: setOp(op, opName, compareStrictEqual); break;
+        case IR::OpStrictNotEqual: setOp(op, opName, compareStrictNotEqual); break;
+        case IR::OpInstanceof: setOpContext(op, opName, compareInstanceof); break;
+        case IR::OpIn: setOpContext(op, opName, compareIn); break;
         } // switch
 
         // TODO: in SSA optimization, do constant expression evaluation.
@@ -1466,12 +1473,14 @@ void InstructionSelection::visitCJump(IR::CJump *s)
         // Of course, after folding the CJUMP to a JUMP, dead-code (dead-basic-block)
         // elimination (which isn't there either) would remove the whole else block.
         if (opContext.isValid())
-            _as->generateFunctionCallImp(Assembler::ReturnValueRegister, opName, opContext,
+            _as->generateFunctionCallImp(needsExceptionCheck,
+                                         Assembler::ReturnValueRegister, opName, opContext,
                                          Assembler::EngineRegister,
                                          Assembler::PointerToValue(b->left),
                                          Assembler::PointerToValue(b->right));
         else
-            _as->generateFunctionCallImp(Assembler::ReturnValueRegister, opName, op,
+            _as->generateFunctionCallImp(needsExceptionCheck,
+                                         Assembler::ReturnValueRegister, opName, op,
                                          Assembler::PointerToValue(b->left),
                                          Assembler::PointerToValue(b->right));
 
diff --git a/src/qml/jit/qv4isel_masm_p.h b/src/qml/jit/qv4isel_masm_p.h
index 93453f71be..5bca879a77 100644
--- a/src/qml/jit/qv4isel_masm_p.h
+++ b/src/qml/jit/qv4isel_masm_p.h
@@ -244,7 +244,7 @@ private:
     #define isel_stringIfy(s) isel_stringIfyx(s)
 
     #define generateRuntimeCall(t, function, ...) \
-        _as->generateFunctionCallImp(t, "Runtime::" isel_stringIfy(function), RuntimeCall(qOffsetOf(QV4::Runtime, function)), __VA_ARGS__)
+        _as->generateFunctionCallImp(Runtime::Method_##function##_NeedsExceptionCheck, t, "Runtime::" isel_stringIfy(function), RuntimeCall(qOffsetOf(QV4::Runtime, function)), __VA_ARGS__)
 
     int prepareVariableArguments(IR::ExprList* args);
     int prepareCallData(IR::ExprList* args, IR::Expr *thisObject);
@@ -260,7 +260,7 @@ private:
         // address.
         Assembler::Pointer lookupAddr(Assembler::ReturnValueRegister, index * sizeof(QV4::Lookup));
 
-         _as->generateFunctionCallImp(retval, "lookup getter/setter",
+         _as->generateFunctionCallImp(true, retval, "lookup getter/setter",
                                       LookupCall(lookupAddr, getterSetterOffset), lookupAddr,
                                       arg1, arg2, arg3);
     }
diff --git a/src/qml/jit/qv4unop.cpp b/src/qml/jit/qv4unop.cpp
index 6a32069ac4..799103849b 100644
--- a/src/qml/jit/qv4unop.cpp
+++ b/src/qml/jit/qv4unop.cpp
@@ -47,10 +47,14 @@ using namespace JIT;
 #define stringIfyx(s) #s
 #define stringIfy(s) stringIfyx(s)
 #define setOp(operation) \
-    do { call = RuntimeCall(qOffsetOf(QV4::Runtime, operation)); name = "Runtime::" stringIfy(operation); } while (0)
+    do { \
+        call = RuntimeCall(qOffsetOf(QV4::Runtime, operation)); name = "Runtime::" stringIfy(operation); \
+        needsExceptionCheck = Runtime::Method_##operation##_NeedsExceptionCheck; \
+    } while (0)
 
 void Unop::generate(IR::Expr *source, IR::Expr *target)
 {
+    bool needsExceptionCheck;
     RuntimeCall call;
     const char *name = 0;
     switch (op) {
@@ -71,7 +75,7 @@ void Unop::generate(IR::Expr *source, IR::Expr *target)
     } // switch
 
     Q_ASSERT(call.isValid());
-    _as->generateFunctionCallImp(target, name, call, Assembler::PointerToValue(source));
+    _as->generateFunctionCallImp(needsExceptionCheck, target, name, call, Assembler::PointerToValue(source));
 }
 
 void Unop::generateUMinus(IR::Expr *source, IR::Expr *target)
diff --git a/src/qml/jsruntime/qv4runtimeapi_p.h b/src/qml/jsruntime/qv4runtimeapi_p.h
index cbc7a2ddc1..582cdcf4e9 100644
--- a/src/qml/jsruntime/qv4runtimeapi_p.h
+++ b/src/qml/jsruntime/qv4runtimeapi_p.h
@@ -58,8 +58,41 @@ namespace QV4 {
 
 struct NoThrowEngine;
 
+namespace {
+template <typename T>
+struct ExceptionCheck {
+    enum { NeedsCheck = 1 };
+};
+// push_catch and pop context methods shouldn't check for exceptions
+template <>
+struct ExceptionCheck<void (*)(QV4::ExecutionEngine *)> {
+    enum { NeedsCheck = 0 };
+};
+template <typename A>
+struct ExceptionCheck<void (*)(A, QV4::NoThrowEngine)> {
+    enum { NeedsCheck = 0 };
+};
+template <>
+struct ExceptionCheck<QV4::ReturnedValue (*)(QV4::NoThrowEngine *)> {
+    enum { NeedsCheck = 0 };
+};
+template <typename A>
+struct ExceptionCheck<QV4::ReturnedValue (*)(QV4::NoThrowEngine *, A)> {
+    enum { NeedsCheck = 0 };
+};
+template <typename A, typename B>
+struct ExceptionCheck<QV4::ReturnedValue (*)(QV4::NoThrowEngine *, A, B)> {
+    enum { NeedsCheck = 0 };
+};
+template <typename A, typename B, typename C>
+struct ExceptionCheck<void (*)(QV4::NoThrowEngine *, A, B, C)> {
+    enum { NeedsCheck = 0 };
+};
+} // anonymous namespace
+
 #define RUNTIME_METHOD(returnvalue, name, args) \
     typedef returnvalue (*Method_##name)args; \
+    enum { Method_##name##_NeedsExceptionCheck = ExceptionCheck<Method_##name>::NeedsCheck }; \
     static returnvalue method_##name args; \
     const Method_##name name