Adding support for throw inside of microsoft try/except to simplify t…

…he 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.
github · bdrodes · Dec 9, 2024 · Dec 9, 2024 · Dec 9, 2024 · Dec 9, 2024
commit b2871962da728e78ac939ea5fa0e930cc99faeea
@@ -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))
      )
    )
  }

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/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)
  }

  /**

@@ -15,9 +15,14 @@
 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 @@
      // 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 @@
      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))
    )
  }


@@ -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
  }

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll b/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()) }