From c09d79451e4a2aec6c39f1333cc74a78a285257c Mon Sep 17 00:00:00 2001 From: kpreisser Date: Sun, 6 Jan 2019 14:01:58 +0100 Subject: [PATCH] Change the type of the exception variable in the catch block to 'object', and use a temporary variable for "skipFinallyBlock". This ensures the finally block is also correctly skipped if objects are thrown that do not derive from Exception (which is possible in the CLR). --- Jurassic/Compiler/Emit/ReflectionHelpers.cs | 2 +- .../Statements/TryCatchFinallyStatement.cs | 29 ++++++++++++------- Jurassic/Core/ScriptEngine.cs | 10 +++---- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/Jurassic/Compiler/Emit/ReflectionHelpers.cs b/Jurassic/Compiler/Emit/ReflectionHelpers.cs index 903b6169..28f615c9 100644 --- a/Jurassic/Compiler/Emit/ReflectionHelpers.cs +++ b/Jurassic/Compiler/Emit/ReflectionHelpers.cs @@ -199,7 +199,7 @@ static ReflectionHelpers() ScriptEngine_RegExp = GetInstanceMethod(typeof(ScriptEngine), "get_RegExp"); ScriptEngine_Array = GetInstanceMethod(typeof(ScriptEngine), "get_Array"); ScriptEngine_Object = GetInstanceMethod(typeof(ScriptEngine), "get_Object"); - ScriptEngine_CanCatchException = GetInstanceMethod(typeof(ScriptEngine), "CanCatchException", typeof(Exception)); + ScriptEngine_CanCatchException = GetInstanceMethod(typeof(ScriptEngine), "CanCatchException", typeof(object)); Global_Eval = GetStaticMethod(typeof(GlobalObject), "Eval", typeof(ScriptEngine), typeof(object), typeof(Scope), typeof(object), typeof(bool)); String_Constructor_Char_Int = GetConstructor(typeof(string), typeof(char), typeof(int)); diff --git a/Jurassic/Compiler/Statements/TryCatchFinallyStatement.cs b/Jurassic/Compiler/Statements/TryCatchFinallyStatement.cs index e397e9ec..40df7764 100644 --- a/Jurassic/Compiler/Statements/TryCatchFinallyStatement.cs +++ b/Jurassic/Compiler/Statements/TryCatchFinallyStatement.cs @@ -86,22 +86,31 @@ public override void GenerateCode(ILGenerator generator, OptimizationInfo optimi var previousInsideTryCatchOrFinally = optimizationInfo.InsideTryCatchOrFinally; optimizationInfo.InsideTryCatchOrFinally = true; - // Finally requires two exception nested blocks. + // When we have a finally block, use a temporary variable that stores if the + // finally block should be skipped. This will be set to true when an exception was + // caught but ScriptEngine.CanCatchException() returns false. + // returns true. + ILLocalVariable skipFinallyBlock = null; if (this.FinallyBlock != null) + { + // Finally requires two exception nested blocks. generator.BeginExceptionBlock(); + skipFinallyBlock = generator.CreateTemporaryVariable(typeof(bool)); + generator.LoadBoolean(false); + generator.StoreVariable(skipFinallyBlock); + } + // Begin the exception block. generator.BeginExceptionBlock(); // Generate code for the try block. this.TryBlock.GenerateCode(generator, optimizationInfo); - // Generate code for the catch block. - ILLocalVariable skipFinallyBlock = null; - + // Generate code for the catch block. // Begin a catch block. The exception is on the top of the stack. - generator.BeginCatchBlock(typeof(Exception)); + generator.BeginCatchBlock(typeof(object)); // Check the exception is catchable by calling CanCatchException(ex). // We need to handle the case where JS code calls into .NET code which then throws @@ -109,7 +118,7 @@ public override void GenerateCode(ILGenerator generator, OptimizationInfo optimi // If CatchBlock is null, we need to rethrow the exception in every case. var endOfIfLabel = generator.CreateLabel(); generator.Duplicate(); // ex - var exceptionTemporary = generator.CreateTemporaryVariable(typeof(Exception)); + var exceptionTemporary = generator.CreateTemporaryVariable(typeof(object)); generator.StoreVariable(exceptionTemporary); EmitHelpers.LoadScriptEngine(generator); generator.LoadVariable(exceptionTemporary); @@ -119,7 +128,6 @@ public override void GenerateCode(ILGenerator generator, OptimizationInfo optimi if (this.FinallyBlock != null) { generator.LoadBoolean(true); - skipFinallyBlock = generator.DeclareVariable(typeof(bool), "skipFinallyBlock"); generator.StoreVariable(skipFinallyBlock); } if (this.CatchBlock == null) @@ -154,11 +162,12 @@ public override void GenerateCode(ILGenerator generator, OptimizationInfo optimi { generator.BeginFinallyBlock(); - // If an exception was thrown that wasn't handled by the catch block, then don't - // run the finally block either. This prevents user code from being run when a - // ThreadAbortException is thrown. + // If an exception was thrown that isn't determined as catchable by the ScriptEngine, + // then don't run the finally block either. This prevents user code from being run + // when a non-JavaScriptException is thrown (e.g. to cancel script execution). var endOfFinallyBlock = generator.CreateLabel(); generator.LoadVariable(skipFinallyBlock); + generator.ReleaseTemporaryVariable(skipFinallyBlock); generator.BranchIfTrue(endOfFinallyBlock); var branches = new List(); diff --git a/Jurassic/Core/ScriptEngine.cs b/Jurassic/Core/ScriptEngine.cs index de8d2111..e0e9851b 100644 --- a/Jurassic/Core/ScriptEngine.cs +++ b/Jurassic/Core/ScriptEngine.cs @@ -1326,16 +1326,16 @@ internal void PopStackFrame() } /// - /// Checks if the given is catchable by JavaScript code with a + /// Checks if the given is catchable by JavaScript code with a /// catch clause. - /// Note: This property is public for technical reasons only and should not be used by user code. + /// Note: This method is public for technical reasons only and should not be used by user code. /// - /// The exception to check. + /// The exception to check. /// true if the is catchable, false otherwise [EditorBrowsable(EditorBrowsableState.Never)] - public bool CanCatchException(Exception ex) + public bool CanCatchException(object exception) { - var jsException = ex as JavaScriptException; + var jsException = exception as JavaScriptException; if (jsException == null) return false; return jsException.Engine == this || jsException.Engine == null;