Merge branch 'main' into java/update-mad-decls-after-triage-2024-01-31T11-16-45

Author: max-schaefer

cpp/ql/lib/change-notes/2024-01-30-throwing-model.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Added a new `ThrowingFunction` abstract class that can be used to model an external function that may throw an exception.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -1408,7 +1408,10 @@ private class InstructionExprNode extends ExprNodeBase, InstructionNode {
   InstructionExprNode() {
     exists(Expr e, int n |
       exprNodeShouldBeInstruction(this, e, n) and
-      not exprNodeShouldBe(e, n + 1)
+      not exists(Expr conv |
+        exprNodeShouldBe(conv, n + 1) and
+        conv.getUnconverted() = e.getUnconverted()
+      )
     )
   }
 
@@ -1419,7 +1422,10 @@ private class OperandExprNode extends ExprNodeBase, OperandNode {
   OperandExprNode() {
     exists(Expr e, int n |
       exprNodeShouldBeOperand(this, e, n) and
-      not exprNodeShouldBe(e, n + 1)
+      not exists(Expr conv |
+        exprNodeShouldBe(conv, n + 1) and
+        conv.getUnconverted() = e.getUnconverted()
+      )
     )
   }
 
@@ -1484,12 +1490,17 @@ private module IndirectNodeToIndirectExpr<IndirectNodeToIndirectExprSig Sig> {
       indirectNodeHasIndirectExpr(node, e, n, indirectionIndex) and
       not exists(Expr conv, int adjustedIndirectionIndex |
         adjustForReference(e, indirectionIndex, conv, adjustedIndirectionIndex) and
-        indirectNodeHasIndirectExpr(_, conv, n + 1, adjustedIndirectionIndex)
+        indirectExprNodeShouldBe(conv, n + 1, adjustedIndirectionIndex)
       )
     )
   }
 }
 
+private predicate indirectExprNodeShouldBe(Expr e, int n, int indirectionIndex) {
+  indirectExprNodeShouldBeIndirectOperand(_, e, n, indirectionIndex) or
+  indirectExprNodeShouldBeIndirectInstruction(_, e, n, indirectionIndex)
+}
+
 private module IndirectOperandIndirectExprNodeImpl implements IndirectNodeToIndirectExprSig {
   class IndirectNode = IndirectOperand;
 

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

@@ -285,7 +285,7 @@ private predicate backEdgeCandidate(
   // is a back edge. This includes edges from `continue` and the fall-through
   // edge(s) after the last instruction(s) in the body.
   exists(TranslatedWhileStmt s |
-    targetInstruction = s.getFirstConditionInstruction() and
+    targetInstruction = s.getFirstConditionInstruction(_) and
     targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and
     requiredAncestor = s.getBody()
   )
@@ -296,7 +296,7 @@ private predicate backEdgeCandidate(
   // { ... } while (0)` statement. Note that all `continue` statements in a
   // do-while loop produce forward edges.
   exists(TranslatedDoStmt s |
-    targetInstruction = s.getBody().getFirstInstruction() and
+    targetInstruction = s.getBody().getFirstInstruction(_) and
     targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and
     requiredAncestor = s.getCondition()
   )
@@ -308,7 +308,7 @@ private predicate backEdgeCandidate(
   // last instruction(s) in the body. A for loop may not have a condition, in
   // which case `getFirstConditionInstruction` returns the body instead.
   exists(TranslatedForStmt s |
-    targetInstruction = s.getFirstConditionInstruction() and
+    targetInstruction = s.getFirstConditionInstruction(_) and
     targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and
     (
       requiredAncestor = s.getUpdate()
@@ -322,7 +322,7 @@ private predicate backEdgeCandidate(
   // Any edge from within the update of the loop to the condition of
   // the loop is a back edge.
   exists(TranslatedRangeBasedForStmt s |
-    targetInstruction = s.getCondition().getFirstInstruction() and
+    targetInstruction = s.getCondition().getFirstInstruction(_) and
     targetInstruction = sourceElement.getInstructionSuccessor(sourceTag, kind) and
     requiredAncestor = s.getUpdate()
   )

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

@@ -3,6 +3,7 @@ private import semmle.code.cpp.ir.implementation.Opcode
 private import semmle.code.cpp.ir.implementation.internal.OperandTag
 private import semmle.code.cpp.ir.internal.CppType
 private import semmle.code.cpp.models.interfaces.SideEffect
+private import semmle.code.cpp.models.interfaces.Throwing
 private import InstructionTag
 private import SideEffects
 private import TranslatedElement
@@ -40,10 +41,10 @@ abstract class TranslatedCall extends TranslatedExpr {
     id = this.getNumberOfArguments() and result = this.getSideEffects()
   }
 
-  final override Instruction getFirstInstruction() {
+  final override Instruction getFirstInstruction(EdgeKind kind) {
     if exists(this.getQualifier())
-    then result = this.getQualifier().getFirstInstruction()
-    else result = this.getFirstCallTargetInstruction()
+    then result = this.getQualifier().getFirstInstruction(kind)
+    else result = this.getFirstCallTargetInstruction(kind)
   }
 
   override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -52,34 +53,43 @@ abstract class TranslatedCall extends TranslatedExpr {
     resultType = getTypeForPRValue(this.getCallResultType())
   }
 
-  override Instruction getChildSuccessor(TranslatedElement child) {
+  override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
     child = this.getQualifier() and
-    result = this.getFirstCallTargetInstruction()
+    result = this.getFirstCallTargetInstruction(kind)
     or
     child = this.getCallTarget() and
-    result = this.getFirstArgumentOrCallInstruction()
+    result = this.getFirstArgumentOrCallInstruction(kind)
     or
     exists(int argIndex |
       child = this.getArgument(argIndex) and
       if exists(this.getArgument(argIndex + 1))
-      then result = this.getArgument(argIndex + 1).getFirstInstruction()
-      else result = this.getInstruction(CallTag())
+      then result = this.getArgument(argIndex + 1).getFirstInstruction(kind)
+      else (
+        result = this.getInstruction(CallTag()) and kind instanceof GotoEdge
+      )
     )
     or
     child = this.getSideEffects() and
     if this.isNoReturn()
     then
+      kind instanceof GotoEdge and
       result =
         any(UnreachedInstruction instr |
           this.getEnclosingFunction().getFunction() = instr.getEnclosingFunction()
         )
-    else result = this.getParent().getChildSuccessor(this)
+    else (
+      not this.mustThrowException() and
+      result = this.getParent().getChildSuccessor(this, kind)
+      or
+      this.mayThrowException() and
+      kind instanceof ExceptionEdge and
+      result = this.getParent().getExceptionSuccessorInstruction(any(GotoEdge edge))
+    )
   }
 
   override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    kind instanceof GotoEdge and
     tag = CallTag() and
-    result = this.getSideEffects().getFirstInstruction()
+    result = this.getSideEffects().getFirstInstruction(kind)
   }
 
   override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
@@ -100,6 +110,16 @@ abstract class TranslatedCall extends TranslatedExpr {
 
   final override Instruction getResult() { result = this.getInstruction(CallTag()) }
 
+  /**
+   * Holds if the evaluation of this call may throw an exception.
+   */
+  abstract predicate mayThrowException();
+
+  /**
+   * Holds if the evaluation of this call always throws an exception.
+   */
+  abstract predicate mustThrowException();
+
   /**
    * Gets the result type of the call.
    */
@@ -121,8 +141,8 @@ abstract class TranslatedCall extends TranslatedExpr {
    * it can be overridden by a subclass for cases where there is a call target
    * that is not computed from an expression (e.g. a direct call).
    */
-  Instruction getFirstCallTargetInstruction() {
-    result = this.getCallTarget().getFirstInstruction()
+  Instruction getFirstCallTargetInstruction(EdgeKind kind) {
+    result = this.getCallTarget().getFirstInstruction(kind)
   }
 
   /**
@@ -159,10 +179,12 @@ abstract class TranslatedCall extends TranslatedExpr {
    * If there are any arguments, gets the first instruction of the first
    * argument. Otherwise, returns the call instruction.
    */
-  final Instruction getFirstArgumentOrCallInstruction() {
+  final Instruction getFirstArgumentOrCallInstruction(EdgeKind kind) {
     if this.hasArguments()
-    then result = this.getArgument(0).getFirstInstruction()
-    else result = this.getInstruction(CallTag())
+    then result = this.getArgument(0).getFirstInstruction(kind)
+    else (
+      kind instanceof GotoEdge and result = this.getInstruction(CallTag())
+    )
   }
 
   /**
@@ -203,24 +225,25 @@ abstract class TranslatedSideEffects extends TranslatedElement {
       )
   }
 
-  final override Instruction getChildSuccessor(TranslatedElement te) {
+  final override Instruction getChildSuccessor(TranslatedElement te, EdgeKind kind) {
     exists(int i |
       this.getChild(i) = te and
       if exists(this.getChild(i + 1))
-      then result = this.getChild(i + 1).getFirstInstruction()
-      else result = this.getParent().getChildSuccessor(this)
+      then result = this.getChild(i + 1).getFirstInstruction(kind)
+      else result = this.getParent().getChildSuccessor(this, kind)
     )
   }
 
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) {
     none()
   }
 
-  final override Instruction getFirstInstruction() {
-    result = this.getChild(0).getFirstInstruction()
+  final override Instruction getFirstInstruction(EdgeKind kind) {
+    result = this.getChild(0).getFirstInstruction(kind)
     or
     // Some functions, like `std::move()`, have no side effects whatsoever.
-    not exists(this.getChild(0)) and result = this.getParent().getChildSuccessor(this)
+    not exists(this.getChild(0)) and
+    result = this.getParent().getChildSuccessor(this, kind)
   }
 
   final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() }
@@ -235,8 +258,9 @@ abstract class TranslatedSideEffects extends TranslatedElement {
  * (`TranslatedAllocatorCall`).
  */
 abstract class TranslatedDirectCall extends TranslatedCall {
-  final override Instruction getFirstCallTargetInstruction() {
-    result = this.getInstruction(CallTargetTag())
+  final override Instruction getFirstCallTargetInstruction(EdgeKind kind) {
+    result = this.getInstruction(CallTargetTag()) and
+    kind instanceof GotoEdge
   }
 
   final override Instruction getCallTargetResult() { result = this.getInstruction(CallTargetTag()) }
@@ -253,8 +277,7 @@ abstract class TranslatedDirectCall extends TranslatedCall {
     result = TranslatedCall.super.getInstructionSuccessor(tag, kind)
     or
     tag = CallTargetTag() and
-    kind instanceof GotoEdge and
-    result = this.getFirstArgumentOrCallInstruction()
+    result = this.getFirstArgumentOrCallInstruction(kind)
   }
 }
 
@@ -290,6 +313,15 @@ class TranslatedExprCall extends TranslatedCallExpr {
   override TranslatedExpr getCallTarget() {
     result = getTranslatedExpr(expr.getExpr().getFullyConverted())
   }
+
+  final override predicate mayThrowException() {
+    // We assume that a call to a function pointer will not throw an exception.
+    // This is not sound in general, but this will greatly reduce the number of
+    // exceptional edges.
+    none()
+  }
+
+  final override predicate mustThrowException() { none() }
 }
 
 /**
@@ -311,6 +343,14 @@ class TranslatedFunctionCall extends TranslatedCallExpr, TranslatedDirectCall {
     exists(this.getQualifier()) and
     not exists(MemberFunction func | expr.getTarget() = func and func.isStatic())
   }
+
+  final override predicate mayThrowException() {
+    expr.getTarget().(ThrowingFunction).mayThrowException(_)
+  }
+
+  final override predicate mustThrowException() {
+    expr.getTarget().(ThrowingFunction).mayThrowException(true)
+  }
 }
 
 /**
@@ -376,10 +416,11 @@ private int initializeAllocationGroup() { result = 3 }
 abstract class TranslatedSideEffect extends TranslatedElement {
   final override TranslatedElement getChild(int n) { none() }
 
-  final override Instruction getChildSuccessor(TranslatedElement child) { none() }
+  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) { none() }
 
-  final override Instruction getFirstInstruction() {
-    result = this.getInstruction(OnlyInstructionTag())
+  final override Instruction getFirstInstruction(EdgeKind kind) {
+    result = this.getInstruction(OnlyInstructionTag()) and
+    kind instanceof GotoEdge
   }
 
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) {
@@ -388,9 +429,8 @@ abstract class TranslatedSideEffect extends TranslatedElement {
   }
 
   final override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
-    result = this.getParent().getChildSuccessor(this) and
-    tag = OnlyInstructionTag() and
-    kind instanceof GotoEdge
+    result = this.getParent().getChildSuccessor(this, kind) and
+    tag = OnlyInstructionTag()
   }
 
   final override Declaration getFunction() { result = this.getParent().getFunction() }