Adding support for throw inside of microsoft try/except to simplify the IR implementation and revert consistency check issues. There is a larger issue of how to address erroneous mix and match with SEH and traditional exceptions.

Author: bdrodes

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll

@@ -91,7 +91,7 @@ abstract class TranslatedCall extends TranslatedExpr {
       exists(ExceptionEdge e | this.hasExceptionBehavior(e) |
         this.mayThrowException(e) and
         kind = e and
-        result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), kind)
+        result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge))
       )
     )
   }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -1107,8 +1107,8 @@ abstract class TranslatedElement extends TTranslatedElement {
    * nearest enclosing `try`, or the `Unwind` instruction for the function if
    * there is no enclosing `try`. The successor edge kind is specified by `kind`.
    */
-  Instruction getExceptionSuccessorInstruction(EdgeKind kind, ExceptionEdge exception) {
-    result = this.getParent().getExceptionSuccessorInstruction(kind, exception)
+  Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
+    result = this.getParent().getExceptionSuccessorInstruction(kind)
   }
 
   /**

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedExpr.qll

@@ -15,9 +15,14 @@ private import TranslatedInitialization
 private import TranslatedStmt
 private import TranslatedGlobalVar
 private import IRConstruction
-private import EdgeKind
 import TranslatedCall
 
+predicate tbd(TranslatedElement e, Instruction i, string s) {
+  e.getInstruction(_) = i and
+  not exists(i.getSuccessor(_)) and
+  s = concat(e.getAQlClass(), ",")
+}
+
 /**
  * Gets the TranslatedExpr for the specified expression. If `expr` is a load or synthesized
  * temporary object, the result is the TranslatedExpr for the load or synthetic temporary object
@@ -3045,7 +3050,7 @@ class TranslatedDestructorsAfterThrow extends TranslatedElement, TTranslatedDest
       // And otherwise, exit this element with an exceptional edge
       not exists(this.getChild(id + 1)) and
       kind instanceof CppExceptionEdge and
-      result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), kind)
+      result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge))
     )
   }
 
@@ -3084,7 +3089,7 @@ abstract class TranslatedThrowExpr extends TranslatedNonConstantExpr {
       or
       not exists(this.getDestructors()) and
       kind instanceof CppExceptionEdge and
-      result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge), kind)
+      result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge))
     )
   }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedFunction.qll

@@ -11,7 +11,6 @@ private import TranslatedExpr
 private import TranslatedInitialization
 private import TranslatedStmt
 private import VarArgs
-private import EdgeKind
 
 /**
  * Gets the `TranslatedFunction` that represents function `func`.
@@ -214,10 +213,6 @@ class TranslatedFunction extends TranslatedRootElement, TTranslatedFunction {
         or
         exists(ThrowExpr throw | throw.getEnclosingFunction() = func)
         or
-        // or
-        // exists(FunctionCall call | call.getEnclosingFunction() = func |
-        //   getTranslatedExpr(call).(TranslatedCallExpr).mustThrowException(_)
-        // )
         exists(FunctionCall call | call.getEnclosingFunction() = func |
           getTranslatedExpr(call).(TranslatedCallExpr).mayThrowException(_)
         )
@@ -233,8 +228,7 @@ class TranslatedFunction extends TranslatedRootElement, TTranslatedFunction {
     )
   }
 
-  final override Instruction getExceptionSuccessorInstruction(EdgeKind kind, ExceptionEdge exception) {
-    (exception = cppExceptionEdge() or exception = sehExceptionEdge()) and
+  final override Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
     result = this.getInstruction(UnwindTag()) and
     kind instanceof GotoEdge
   }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll

@@ -10,7 +10,6 @@ private import TranslatedElement
 private import TranslatedExpr
 private import TranslatedFunction
 private import TranslatedInitialization
-private import EdgeKind
 
 TranslatedStmt getTranslatedStmt(Stmt stmt) { result.getAst() = stmt }
 
@@ -152,7 +151,7 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
       // TODO: This is not really correct. The semantics of `EXCEPTION_CONTINUE_EXECUTION` is that
       // we should continue execution at the point where the exception occurred. But we don't have
       // any instruction to model this behavior.
-      result = this.getExceptionSuccessorInstruction(any(GotoEdge edge), sehExceptionEdge())
+      result = this.getExceptionSuccessorInstruction(any(GotoEdge edge))
       or
       kind instanceof FalseEdge and
       result = this.getInstruction(TryExceptGenerateZero())
@@ -172,7 +171,7 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
     tag = TryExceptCompareZeroBranch() and
     (
       kind instanceof TrueEdge and
-      result = this.getExceptionSuccessorInstruction(any(GotoEdge edge), sehExceptionEdge())
+      result = this.getExceptionSuccessorInstruction(any(GotoEdge edge))
       or
       kind instanceof FalseEdge and
       result = this.getInstruction(TryExceptGenerateOne())
@@ -227,10 +226,10 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
 
   final override Function getFunction() { result = tryExcept.getEnclosingFunction() }
 
-  override Instruction getExceptionSuccessorInstruction(EdgeKind kind, ExceptionEdge exception) {
+  override Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
     // A throw from within a `__except` block flows to the handler for the parent of
     // the `__try`.
-    result = this.getParent().getParent().getExceptionSuccessorInstruction(kind, exception)
+    result = this.getParent().getParent().getExceptionSuccessorInstruction(kind)
   }
 }
 
@@ -283,10 +282,10 @@ class TranslatedMicrosoftTryFinallyHandler extends TranslatedElement,
     result = getTranslatedStmt(tryFinally.getFinally())
   }
 
-  override Instruction getExceptionSuccessorInstruction(EdgeKind kind, ExceptionEdge exception) {
+  override Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
     // A throw from within a `__finally` block flows to the handler for the parent of
     // the `__try`.
-    result = this.getParent().getParent().getExceptionSuccessorInstruction(kind, exception)
+    result = this.getParent().getParent().getExceptionSuccessorInstruction(kind)
   }
 }
 
@@ -735,32 +734,14 @@ class TranslatedTryStmt extends TranslatedStmt {
     // of the `try`, because the exception successor of the `try` itself is
     // the first catch clause.
     handler = this.getHandler(stmt.getNumberOfCatchClauses() - 1) and
-    exists(ExceptionEdge exception |
-      stmt instanceof MicrosoftTryStmt and exception instanceof SehExceptionEdge
-      or
-      stmt instanceof TryStmt and exception instanceof CppExceptionEdge
-    |
-      result = this.getParent().getExceptionSuccessorInstruction(kind, exception)
-    )
+    result = this.getParent().getExceptionSuccessorInstruction(kind)
   }
 
-  final override Instruction getExceptionSuccessorInstruction(EdgeKind kind, ExceptionEdge exception) {
-    // Seh exceptions are only handled for Seh try statements and
-    // C++ exceptions for C++ try statements.
-    // I.e., we are assuming there isn't a mix and match between Seh and C++ exceptions.
-    // They are either all Seh or all C++ within a single try block depending on the
-    // try type (TryStmt vs MicrosoftTryStmt).
-    (
-      stmt instanceof TryStmt and exception instanceof CppExceptionEdge
-      or
-      stmt instanceof MicrosoftTryStmt and exception instanceof SehExceptionEdge
-    ) and
-    (
-      result = this.getHandler(0).getFirstInstruction(kind)
-      or
-      not exists(this.getHandler(_)) and
-      result = this.getFinally().getFirstInstruction(kind)
-    )
+  final override Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
+    result = this.getHandler(0).getFirstInstruction(kind)
+    or
+    not exists(this.getHandler(_)) and
+    result = this.getFinally().getFirstInstruction(kind)
   }
 
   private TranslatedElement getHandler(int index) { result = stmt.getTranslatedHandler(index) }
@@ -840,10 +821,10 @@ abstract class TranslatedHandler extends TranslatedStmt {
     child = this.getBlock() and result = this.getParent().getChildSuccessor(this, kind)
   }
 
-  override Instruction getExceptionSuccessorInstruction(EdgeKind kind, ExceptionEdge exception) {
+  override Instruction getExceptionSuccessorInstruction(EdgeKind kind) {
     // A throw from within a `catch` block flows to the handler for the parent of
     // the `try`.
-    result = this.getParent().getParent().getExceptionSuccessorInstruction(kind, exception)
+    result = this.getParent().getParent().getExceptionSuccessorInstruction(kind)
   }
 
   TranslatedStmt getBlock() { result = getTranslatedStmt(stmt.getBlock()) }