[cppia] Fix missing jit exception checks for method calls (#1212)

* [cppia] Add missing exception checks

In interp mode, we need to stop immediately if a jit exception has been
thrown.

* [cppia] Test for method on exception throwing expr

* [cppia] Add test for host class method call

This covers the CallHaxe bug

* [cppia] Fix throw/catch of seg faults in tests

The host must be compiled with HXCPP_CATCH_SEGV so that seg faults are
handled as hxcpp exceptions.

* [tests] Verify CallHaxe BCR after this check

This test verifies that the full BCR check is needed in CallHaxe::run
after runObject, otherwise the return is ignored and arguments continue
to be evaluated

* [tests] Verify CallHaxe BCR after each arg

* [tests] Verify Call with return as function

* [tests] Verify CallMemberVTable with this return

Author: tobil4sk

src/hx/cppia/Cppia.cpp

@@ -1761,6 +1761,7 @@ struct CallHaxe : public CppiaExpr
    {
       unsigned char *pointer = ctx->pointer;
       ctx->pushObject(isStatic ? 0: thisExpr ? thisExpr->runObject(ctx) : ctx->getThis(false));
+      BCR_VCHECK;
 
       const char *s = function.signature+1;
       for(int a=0;a<args.size();a++)
@@ -1776,6 +1777,7 @@ struct CallHaxe : public CppiaExpr
             case sigObject: ctx->pushObject( arg->runObject(ctx) ); break;
             default: ;// huh?
          }
+         BCR_VCHECK;
       }
 
       AutoStack a(ctx,pointer);
@@ -2165,8 +2167,9 @@ struct CallMemberVTable : public CppiaExpr
    ExprType getType() { return returnType; }
    // ScriptCallable **vtable = (ScriptCallable **)thisVal->__GetScriptVTable();
 
-   #define CALL_VTABLE_SETUP \
+   #define CALL_VTABLE_SETUP(errorValue) \
       hx::Object *thisVal = thisExpr ? thisExpr->runObject(ctx) : ctx->getThis(); \
+      BCR_CHECK_RET(errorValue); \
       CPPIA_CHECK(thisVal); \
       ScriptCallable **vtable = (!isInterfaceCall ? (*(ScriptCallable ***)((char *)thisVal +scriptVTableOffset)) : (ScriptCallable **) thisVal->__GetScriptVTable()); \
       unsigned char *pointer = ctx->pointer; \
@@ -2177,29 +2180,29 @@ struct CallMemberVTable : public CppiaExpr
 
    void runVoid(CppiaCtx *ctx)
    {
-      CALL_VTABLE_SETUP
+      CALL_VTABLE_SETUP()
       ctx->runVoid(func);
    }
    int runInt(CppiaCtx *ctx)
    {
-      CALL_VTABLE_SETUP
-      return runContextConvertInt(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func); 
+      CALL_VTABLE_SETUP(BCRReturn())
+      return runContextConvertInt(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func);
    }
- 
+
    Float runFloat(CppiaCtx *ctx)
    {
-      CALL_VTABLE_SETUP
-      return runContextConvertFloat(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func); 
+      CALL_VTABLE_SETUP(BCRReturn())
+      return runContextConvertFloat(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func);
    }
    String runString(CppiaCtx *ctx)
    {
-      CALL_VTABLE_SETUP
-      return runContextConvertString(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func); 
+      CALL_VTABLE_SETUP(BCRReturn())
+      return runContextConvertString(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func);
    }
    hx::Object *runObject(CppiaCtx *ctx)
    {
-      CALL_VTABLE_SETUP
-      return runContextConvertObject(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func); 
+      CALL_VTABLE_SETUP(BCRReturn())
+      return runContextConvertObject(ctx, checkInterfaceReturnType ? func->getReturnType() : returnType, func);
    }
 
    bool isBoolInt() { return boolResult; }
@@ -3109,6 +3112,8 @@ struct Call : public CppiaDynamicExpr
    hx::Object *runObject(CppiaCtx *ctx)
    {
       hx::Object *funcVal = func->runObject(ctx);
+      BCR_CHECK;
+
       CPPIA_CHECK_FUNC(funcVal);
 
       int size = args.size();

src/hx/cppia/Cppia.h

@@ -842,7 +842,7 @@ struct BCRReturn
 
 
 #define BCR_CHECK if (ctx->breakContReturn || ctx->exception) return BCRReturn();
-#define BCR_CHECK_RET(x) if (ctx->breakContReturn) return x;
+#define BCR_CHECK_RET(x) if (ctx->breakContReturn || ctx->exception) return x;
 #define BCR_VCHECK if (ctx->breakContReturn || ctx->exception) return;
 
 

test/cppia/Client.hx

@@ -189,6 +189,55 @@ class Client
          default:
       }
 
+      switch LocalFunctionExceptions.testObjMethodOnReturn() {
+         case Error(message):
+            Common.status = 'Failed test for running object method on returned value: ' + message;
+            return;
+         default:
+      }
+
+      switch LocalFunctionExceptions.testClassMethodOnReturn() {
+         case Error(message):
+            Common.status = 'Failed test for running class method on returned value: ' + message;
+            return;
+         default:
+      }
+
+      switch LocalFunctionExceptions.testHostClassMethodOnHostReturn() {
+         case Error(message):
+            Common.status = 'Failed test for running host class method on returned value: ' + message;
+            return;
+         default:
+      }
+
+		switch ReturnExpressions.testHostThisReturn() {
+         case Error(message):
+            Common.status = 'Failed test for host this return stopping argument evaluation: ' + message;
+            return;
+         default:
+      }
+
+		switch ReturnExpressions.testHostArgReturn() {
+			case Error(message):
+				Common.status = 'Failed test for argument return stopping argument evaluation: ' + message;
+				return;
+			default:
+		}
+
+		switch ReturnExpressions.testClientThisReturn() {
+			case Error(message):
+				Common.status = 'Failed test for client this return stopping evaluation: ' + message;
+				return;
+			default:
+		}
+
+		switch ReturnExpressions.testFuncReturn() {
+			case Error(message):
+				Common.status = 'Failed test for function value return stopping evaluation: ' + message;
+				return;
+			default:
+		}
+
       // regression test for #926
       var x:Dynamic = 3;
       x *= 5;

test/cppia/Common.hx

@@ -8,4 +8,17 @@ class Common
    public static var callbackSet:Int = 0;
    public static var callback: Void->Void;
 
+	public static var count = 0;
+	public static function incrementCount():Int {
+		return count++;
+	}
+
+   public function new() {}
+
+   public function dummyMethod() {}
+   public function dummyMethodArg(_) {}
+
+	public function instanceIncrementCount(_) {
+      count++;
+   }
 }

test/cppia/LocalFunctionExceptions.hx

@@ -3,8 +3,12 @@ enum Status {
 	Error(message:String);
 }
 
+class DummyClass {
+	public function run() {}
+}
+
 class LocalFunctionExceptions {
-	static function staticFunction() {
+	static function staticFunction():Dynamic {
 		throw 'Thrown from static';
 	}
 
@@ -65,4 +69,58 @@ class LocalFunctionExceptions {
 
 		return Error("No exception caught");
 	}
+
+	public static function testObjMethodOnReturn():Status {
+		function localFunction() {
+			(staticFunction() : Dynamic).run();
+		}
+
+		try {
+			localFunction();
+		} catch (e:Dynamic) {
+			if (e == 'Thrown from static') {
+				return Ok;
+			} else {
+				return Error("Incorrect exception caught from local function call: " + e);
+			}
+		}
+
+		return Error("No exception caught");
+	}
+
+	public static function testClassMethodOnReturn():Status {
+		function localFunction() {
+			(staticFunction() : DummyClass).run();
+		}
+
+		try {
+			localFunction();
+		} catch (e:Dynamic) {
+			if (e == 'Thrown from static') {
+				return Ok;
+			} else {
+				return Error("Incorrect exception caught from local function call: " + e);
+			}
+		}
+
+		return Error("No exception caught");
+	}
+
+	public static function testHostClassMethodOnHostReturn():Status {
+		function localFunction() {
+			(staticFunction() : Common).dummyMethod();
+		}
+
+		try {
+			localFunction();
+		} catch (e:Dynamic) {
+			if (e == 'Thrown from static') {
+				return Ok;
+			} else {
+				return Error("Incorrect exception caught from local function call: " + e);
+			}
+		}
+
+		return Error("No exception caught");
+	}
 }

test/cppia/ReturnExpressions.hx

@@ -0,0 +1,66 @@
+import LocalFunctionExceptions.Status;
+
+class ReturnExpressions {
+	// prevent the call from being optimised out due to unconditional return
+	@:analyzer(ignore)
+	static function returnAsCallHaxeThis() {
+		(cast return:Common).dummyMethodArg(Common.incrementCount());
+	}
+
+	public static function testHostThisReturn() {
+		Common.count = 0;
+		returnAsCallHaxeThis();
+		if (Common.count == 0) {
+			return Ok;
+		}
+		return Error("Host method executed after return");
+	}
+
+	@:analyzer(ignore)
+	static function returnAsCallHaxeArg() {
+		new Common().instanceIncrementCount(return);
+	}
+
+	public static function testHostArgReturn() {
+		Common.count = 0;
+		returnAsCallHaxeArg();
+		if (Common.count == 0) {
+			return Ok;
+		}
+		return Error("Host method executed after return");
+	}
+
+	@:analyzer(ignore)
+	static function returnAsFunction() {
+		// if return is not handled, there is no function to run so this will
+		// give a null function exception
+		(cast return : (Int) -> Void)(Common.incrementCount());
+	}
+
+	public static function testFuncReturn() {
+		Common.count = 0;
+		returnAsFunction();
+		if (Common.count == 0) {
+			return Ok;
+		}
+		return Error("Host method executed after return");
+	}
+
+	function vtableMethod(_) {}
+
+	@:analyzer(ignore)
+	public static function returnAsClientThis() {
+		// if return is not handled, there is no instance to run the method with
+		// so this will give a null function exception
+		(cast return:ReturnExpressions).vtableMethod(Common.incrementCount());
+	}
+
+	public static function testClientThisReturn() {
+		Common.count = 0;
+		returnAsClientThis();
+		if (Common.count == 0) {
+			return Ok;
+		}
+		return Error("Host method executed after return");
+	}
+}

test/cppia/compile-host.hxml

@@ -5,3 +5,4 @@ HostExtendedRoot
 -L utest
 --dce no
 --cpp bin
+-D HXCPP_CATCH_SEGV