C++: Also handle '__finally' blocks.

MathiasVP · MathiasVP · commit d2964a7d4a33 · 2022-12-20T13:28:33.000Z
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
@@ -263,6 +263,9 @@ private class TryOrMicrosoftTryStmt extends Stmt {
     or
     this instanceof MicrosoftTryExceptStmt and
     result = 1
+    or
+    this instanceof MicrosoftTryFinallyStmt and
+    result = 0
   }
 
   /** Gets the `body` statement of this statement. */
@@ -279,6 +282,9 @@ private class TryOrMicrosoftTryStmt extends Stmt {
     i = 0 and
     result = this.(MicrosoftTryExceptStmt).getExcept()
   }
+
+  /** Gets the `finally` statement (usually a BlockStmt), if any. */
+  Stmt getFinally() { result = this.(MicrosoftTryFinallyStmt).getFinally() }
 }
 
 /**
@@ -291,6 +297,9 @@ class TranslatedTryStmt extends TranslatedStmt {
     id = 0 and result = getBody()
     or
     result = getHandler(id - 1)
+    or
+    id = stmt.getNumberOfCatchClauses() + 1 and
+    result = this.getFinally()
   }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -302,8 +311,20 @@ class TranslatedTryStmt extends TranslatedStmt {
   override Instruction getFirstInstruction() { result = getBody().getFirstInstruction() }
 
   override Instruction getChildSuccessor(TranslatedElement child) {
-    // All children go to the successor of the `try`.
-    child = getAChild() and result = getParent().getChildSuccessor(this)
+    // All non-finally children go to the successor of the `try` if
+    // there is no finally block, but if there is a finally block
+    // then we go to that one.
+    child = [getBody(), getHandler(_)] and
+    (
+      not exists(this.getFinally()) and
+      result = getParent().getChildSuccessor(this)
+      or
+      result = this.getFinally().getFirstInstruction()
+    )
+    or
+    // And after the finally block we go to the successor of the `try`.
+    child = this.getFinally() and
+    result = getParent().getChildSuccessor(this)
   }
 
   final Instruction getNextHandler(TranslatedHandler handler) {
@@ -327,6 +348,8 @@ class TranslatedTryStmt extends TranslatedStmt {
     result = getTranslatedStmt(stmt.getHandlerStmt(index))
   }
 
+  private TranslatedStmt getFinally() { result = getTranslatedStmt(stmt.getFinally()) }
+
   private TranslatedStmt getBody() { result = getTranslatedStmt(stmt.getStmt()) }
 }
 
diff --git a/cpp/ql/test/library-tests/ir/ir/operand_locations.expected b/cpp/ql/test/library-tests/ir/ir/operand_locations.expected
@@ -9162,7 +9162,7 @@
 | try_except.c:11:19:11:19 | Arg(0) | 0:r11_2 |
 | try_except.c:18:6:18:6 | ChiPartial | partial:m18_3 |
 | try_except.c:18:6:18:6 | ChiTotal | total:m18_2 |
-| try_except.c:18:6:18:6 | SideEffect | ~m23_5 |
+| try_except.c:18:6:18:6 | SideEffect | ~m26_6 |
 | try_except.c:19:7:19:7 | Address | &:r19_1 |
 | try_except.c:19:10:19:10 | Address | &:r19_3 |
 | try_except.c:19:13:19:14 | StoreValue | r19_4 |
@@ -9180,3 +9180,10 @@
 | try_except.c:23:5:23:17 | ChiTotal | total:m21_5 |
 | try_except.c:23:5:23:17 | SideEffect | ~m21_5 |
 | try_except.c:23:19:23:19 | Arg(0) | 0:r23_2 |
+| try_except.c:26:5:26:8 | CallTarget | func:r26_1 |
+| try_except.c:26:5:26:8 | ChiPartial | partial:m26_5 |
+| try_except.c:26:5:26:8 | ChiTotal | total:m23_5 |
+| try_except.c:26:5:26:8 | SideEffect | ~m23_5 |
+| try_except.c:26:10:26:10 | Address | &:r26_2 |
+| try_except.c:26:10:26:10 | Arg(0) | 0:r26_3 |
+| try_except.c:26:10:26:10 | Load | m22_4 |
diff --git a/cpp/ql/test/library-tests/ir/ir/raw_consistency.expected b/cpp/ql/test/library-tests/ir/ir/raw_consistency.expected
@@ -8,7 +8,6 @@ sideEffectWithoutPrimary
 instructionWithoutSuccessor
 | try_except.c:13:13:13:13 | Constant: 0 | Instruction 'Constant: 0' has no successors in function '$@'. | try_except.c:6:6:6:6 | void f() | void f() |
 | try_except.c:14:5:14:8 | CallSideEffect: call to sink | Instruction 'CallSideEffect: call to sink' has no successors in function '$@'. | try_except.c:6:6:6:6 | void f() | void f() |
-| try_except.c:26:5:26:8 | CallSideEffect: call to sink | Instruction 'CallSideEffect: call to sink' has no successors in function '$@'. | try_except.c:18:6:18:6 | void g() | void g() |
 ambiguousSuccessors
 unexplainedLoop
 unnecessaryPhiInstruction
diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected
@@ -10471,14 +10471,12 @@ try_except.c:
 #   23|     r23_2(int)            = Constant[0]                    : 
 #   23|     v23_3(void)           = Call[ProbeFunction]            : func:r23_1, 0:r23_2
 #   23|     mu23_4(unknown)       = ^CallSideEffect                : ~m?
+#   26|     r26_1(glval<unknown>) = FunctionAddress[sink]          : 
+#   26|     r26_2(glval<int>)     = VariableAddress[x]             : 
+#   26|     r26_3(int)            = Load[x]                        : &:r26_2, ~m?
+#   26|     v26_4(void)           = Call[sink]                     : func:r26_1, 0:r26_3
+#   26|     mu26_5(unknown)       = ^CallSideEffect                : ~m?
 #   28|     v28_1(void)           = NoOp                           : 
 #   18|     v18_4(void)           = ReturnVoid                     : 
 #   18|     v18_5(void)           = AliasedUse                     : ~m?
 #   18|     v18_6(void)           = ExitFunction                   : 
-
-#   26|   Block 1
-#   26|     r26_1(glval<unknown>) = FunctionAddress[sink] : 
-#   26|     r26_2(glval<int>)     = VariableAddress[x]    : 
-#   26|     r26_3(int)            = Load[x]               : &:r26_2, ~m?
-#   26|     v26_4(void)           = Call[sink]            : func:r26_1, 0:r26_3
-#   26|     mu26_5(unknown)       = ^CallSideEffect       : ~m?
diff --git a/cpp/ql/test/library-tests/syntax-zoo/raw_consistency.expected b/cpp/ql/test/library-tests/syntax-zoo/raw_consistency.expected
@@ -33,16 +33,13 @@ instructionWithoutSuccessor
 | misc.c:174:55:174:60 | Store: (char ****)... | Instruction 'Store: (char ****)...' has no successors in function '$@'. | misc.c:168:6:168:8 | void vla() | void vla() |
 | ms_try_except.cpp:9:19:9:19 | Load: j | Instruction 'Load: j' has no successors in function '$@'. | ms_try_except.cpp:2:6:2:18 | void ms_try_except(int) | void ms_try_except(int) |
 | ms_try_except.cpp:10:13:10:17 | Store: ... = ... | Instruction 'Store: ... = ...' has no successors in function '$@'. | ms_try_except.cpp:2:6:2:18 | void ms_try_except(int) | void ms_try_except(int) |
-| ms_try_except.cpp:17:13:17:17 | Store: ... = ... | Instruction 'Store: ... = ...' has no successors in function '$@'. | ms_try_except.cpp:2:6:2:18 | void ms_try_except(int) | void ms_try_except(int) |
 | ms_try_except.cpp:19:17:19:21 | Sub: ... - ... | Instruction 'Sub: ... - ...' has no successors in function '$@'. | ms_try_except.cpp:2:6:2:18 | void ms_try_except(int) | void ms_try_except(int) |
 | ms_try_except.cpp:20:9:20:13 | Store: ... = ... | Instruction 'Store: ... = ...' has no successors in function '$@'. | ms_try_except.cpp:2:6:2:18 | void ms_try_except(int) | void ms_try_except(int) |
 | ms_try_mix.cpp:16:13:16:19 | ThrowValue: throw ... | Instruction 'ThrowValue: throw ...' has no successors in function '$@'. | ms_try_mix.cpp:10:6:10:18 | void ms_except_mix(int) | void ms_except_mix(int) |
 | ms_try_mix.cpp:20:15:20:39 | Constant: 1 | Instruction 'Constant: 1' has no successors in function '$@'. | ms_try_mix.cpp:10:6:10:18 | void ms_except_mix(int) | void ms_except_mix(int) |
 | ms_try_mix.cpp:21:16:21:19 | IndirectMayWriteSideEffect: call to C | Instruction 'IndirectMayWriteSideEffect: call to C' has no successors in function '$@'. | ms_try_mix.cpp:10:6:10:18 | void ms_except_mix(int) | void ms_except_mix(int) |
 | ms_try_mix.cpp:33:13:33:19 | ThrowValue: throw ... | Instruction 'ThrowValue: throw ...' has no successors in function '$@'. | ms_try_mix.cpp:27:6:27:19 | void ms_finally_mix(int) | void ms_finally_mix(int) |
-| ms_try_mix.cpp:38:16:38:19 | IndirectMayWriteSideEffect: call to C | Instruction 'IndirectMayWriteSideEffect: call to C' has no successors in function '$@'. | ms_try_mix.cpp:27:6:27:19 | void ms_finally_mix(int) | void ms_finally_mix(int) |
 | ms_try_mix.cpp:51:5:51:11 | ThrowValue: throw ... | Instruction 'ThrowValue: throw ...' has no successors in function '$@'. | ms_try_mix.cpp:47:6:47:28 | void ms_empty_finally_at_end() | void ms_empty_finally_at_end() |
-| ms_try_mix.cpp:53:13:54:3 | NoOp: { ... } | Instruction 'NoOp: { ... }' has no successors in function '$@'. | ms_try_mix.cpp:47:6:47:28 | void ms_empty_finally_at_end() | void ms_empty_finally_at_end() |
 | stmt_expr.cpp:27:5:27:15 | Store: ... = ... | Instruction 'Store: ... = ...' has no successors in function '$@'. | stmt_expr.cpp:21:6:21:6 | void stmtexpr::g(int) | void stmtexpr::g(int) |
 | stmt_expr.cpp:29:11:32:11 | CopyValue: (statement expression) | Instruction 'CopyValue: (statement expression)' has no successors in function '$@'. | stmt_expr.cpp:21:6:21:6 | void stmtexpr::g(int) | void stmtexpr::g(int) |
 | stmt_in_type.cpp:5:53:5:53 | Constant: 1 | Instruction 'Constant: 1' has no successors in function '$@'. | stmt_in_type.cpp:2:6:2:12 | void cpp_fun() | void cpp_fun() |
