Merge branch 'main' into brodes/overflow-buffer-fixes-upstream

cpp/ql/lib/change-notes/2024-09-04-swap-data-flow.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added a data flow model for `swap` member functions, which were previously modeled as taint tracking functions. This change improves the precision of queries where flow through `swap` member functions might affect the results.

cpp/ql/lib/semmle/code/cpp/Variable.qll

@@ -409,11 +409,18 @@ class LocalVariable extends LocalScopeVariable, @localvariable {
     exists(ConditionDeclExpr e | e.getVariable() = this and e.getEnclosingFunction() = result)
     or
     orphaned_variables(underlyingElement(this), unresolveElement(result))
+    or
+    coroutine_placeholder_variable(underlyingElement(this), _, unresolveElement(result))
   }
 
   override predicate isStatic() {
     super.isStatic() or orphaned_variables(underlyingElement(this), _)
   }
+
+  override predicate isCompilerGenerated() {
+    super.isCompilerGenerated() or
+    coroutine_placeholder_variable(underlyingElement(this), _, _)
+  }
 }
 
 /**

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

@@ -209,8 +209,13 @@ class TranslatedFunction extends TranslatedRootElement, TTranslatedFunction {
       (
         // Only generate the `Unwind` instruction if there is any exception
         // handling present in the function.
-        exists(TryStmt try | try.getEnclosingFunction() = func) or
+        exists(TryOrMicrosoftTryStmt try | try.getEnclosingFunction() = func)
+        or
         exists(ThrowExpr throw | throw.getEnclosingFunction() = func)
+        or
+        exists(FunctionCall call | call.getEnclosingFunction() = func |
+          getTranslatedExpr(call).(TranslatedCallExpr).mayThrowException()
+        )
       )
       or
       tag = AliasedUseTag() and

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

@@ -79,11 +79,6 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
     tag = TryExceptCompareOneBranch() and
     opcode instanceof Opcode::ConditionalBranch and
     resultType = getVoidType()
-    or
-    // unwind stack
-    tag = UnwindTag() and
-    opcode instanceof Opcode::Unwind and
-    resultType = getVoidType()
   }
 
   final override Instruction getInstructionRegisterOperand(InstructionTag tag, OperandTag operandTag) {
@@ -156,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.getInstruction(UnwindTag())
+      result = this.getExceptionSuccessorInstruction(any(GotoEdge edge))
       or
       kind instanceof FalseEdge and
       result = this.getInstruction(TryExceptGenerateZero())
@@ -176,7 +171,7 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
     tag = TryExceptCompareZeroBranch() and
     (
       kind instanceof TrueEdge and
-      result = this.getInstruction(UnwindTag())
+      result = this.getExceptionSuccessorInstruction(any(GotoEdge edge))
       or
       kind instanceof FalseEdge and
       result = this.getInstruction(TryExceptGenerateOne())
@@ -196,10 +191,6 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
     tag = TryExceptCompareOneBranch() and
     kind instanceof TrueEdge and
     result = this.getTranslatedHandler().getFirstInstruction(any(GotoEdge edge))
-    or
-    // Unwind -> Parent
-    tag = UnwindTag() and
-    result = this.getParent().getChildSuccessor(this, kind)
   }
 
   override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
@@ -215,8 +206,6 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
 
   override Instruction getALastInstructionInternal() {
     result = this.getTranslatedHandler().getALastInstruction()
-    or
-    result = this.getInstruction(UnwindTag())
   }
 
   private TranslatedExpr getTranslatedCondition() {
@@ -236,6 +225,12 @@ class TranslatedMicrosoftTryExceptHandler extends TranslatedElement,
   }
 
   final override Function getFunction() { result = tryExcept.getEnclosingFunction() }
+
+  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)
+  }
 }
 
 abstract class TranslatedStmt extends TranslatedElement, TTranslatedStmt {
@@ -583,7 +578,7 @@ class TranslatedNoValueReturnStmt extends TranslatedReturnStmt, TranslatedVariab
 /**
  * A C/C++ `try` statement, or a `__try __except` or `__try __finally` statement.
  */
-private class TryOrMicrosoftTryStmt extends Stmt {
+class TryOrMicrosoftTryStmt extends Stmt {
   TryOrMicrosoftTryStmt() {
     this instanceof TryStmt or
     this instanceof MicrosoftTryStmt

cpp/ql/lib/semmle/code/cpp/models/implementations/Swap.qll

@@ -26,7 +26,7 @@ private class Swap extends DataFlowFunction {
  * obj1.swap(obj2)
  * ```
  */
-private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction {
+private class MemberSwap extends DataFlowFunction, MemberFunction, AliasFunction {
   MemberSwap() {
     this.hasName("swap") and
     this.getNumberOfParameters() = 1 and
@@ -34,7 +34,7 @@ private class MemberSwap extends TaintFunction, MemberFunction, AliasFunction {
       this.getDeclaringType()
   }
 
-  override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
+  override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
     input.isQualifierObject() and
     output.isParameterDeref(0)
     or

cpp/ql/lib/semmlecode.cpp.dbscheme

@@ -384,11 +384,23 @@ function_return_type(
  */
 coroutine(
   unique int function: @function ref,
-  int traits: @type ref,
-  int handle: @variable ref,
-  int promise: @variable ref
+  int traits: @type ref
 );
 
+/*
+case @coroutine_placeholder_variable.kind of
+  1 = @handle
+| 2 = @promise
+| 3 = @init_await_resume
+;
+*/
+
+coroutine_placeholder_variable(
+  unique int placeholder_variable: @variable ref,
+  int kind: int ref,
+  int function: @function ref
+)
+
 /** The `new` function used for allocating the coroutine state, if any. */
 coroutine_new(
   unique int function: @function ref,
@@ -829,22 +841,6 @@ variable_template_argument_value(
     int arg_value: @expr ref
 );
 
-/*
-  Fixed point types
-  precision(1) = short, precision(2) = default, precision(3) = long
-  is_unsigned(1) = unsigned is_unsigned(2) = signed
-  is_fract_type(1) = declared with _Fract
-  saturating(1) = declared with _Sat
-*/
-/* TODO
-fixedpointtypes(
-    unique int id: @fixedpointtype,
-    int precision: int ref,
-    int is_unsigned: int ref,
-    int is_fract_type: int ref,
-    int saturating: int ref);
-*/
-
 routinetypes(
     unique int id: @routinetype,
     int return_type: @type ref

cpp/ql/lib/upgrades/0fea0ee7026c7c3f7d6faef4df4bf67847b67d71/upgrade.properties

@@ -0,0 +1,4 @@
+description: Improve handling of coroutine placeholder variables
+compatibility: partial
+coroutine.rel: run upgrades.qlo new_coroutine
+coroutine_placeholder_variable.rel: run upgrades.qlo new_coroutine_placeholder_variable

cpp/ql/lib/upgrades/0fea0ee7026c7c3f7d6faef4df4bf67847b67d71/upgrades.ql

@@ -0,0 +1,19 @@
+class Function extends @function {
+  string toString() { none() }
+}
+
+class Type extends @type {
+  string toString() { none() }
+}
+
+class Variable extends @variable {
+  string toString() { none() }
+}
+
+query predicate new_coroutine(Function func, Type traits) { coroutine(func, traits, _, _) }
+
+query predicate new_coroutine_placeholder_variable(Variable var, int kind, Function func) {
+  coroutine(func, _, var, _) and kind = 1
+  or
+  coroutine(func, _, _, var) and kind = 2
+}