From ad2c3e76453a7b111af0acc39fb3b9507cf3c231 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 14:55:24 +0100
Subject: [PATCH 01/10] C++: Make 'SourceVariable' an abstract class and rename
 a few predicates.

---
 .../cpp/ir/dataflow/internal/SsaInternals.qll | 74 ++++++++++++-------
 .../dataflow/internal/SsaInternalsCommon.qll  | 14 +++-
 2 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index 30511ba12854..d6861ef2061a 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -20,46 +20,64 @@ private module SourceVariables {
       ind = [0 .. countIndirectionsForCppType(base.getLanguageType()) + 1]
     }
 
-  class SourceVariable extends TSourceVariable {
-    BaseSourceVariable base;
+  abstract private class AbstractSourceVariable extends TSourceVariable {
     int ind;
 
-    SourceVariable() { this = TMkSourceVariable(base, ind) }
-
-    /** Gets the IR variable associated with this `SourceVariable`, if any. */
-    IRVariable getIRVariable() { result = base.(BaseIRVariable).getIRVariable() }
-
-    /**
-     * Gets the base source variable (i.e., the variable without any
-     * indirections) of this source variable.
-     */
-    BaseSourceVariable getBaseVariable() { result = base }
+    bindingset[ind]
+    AbstractSourceVariable() { any() }
 
     /** Gets a textual representation of this element. */
-    string toString() { result = repeatStars(this.getIndirection()) + base.toString() }
+    abstract string toString();
 
     /**
      * Gets the number of loads performed on the base source variable
      * to reach the value of this source variable.
      */
-    int getIndirection() { result = ind }
-
-    /** Holds if this variable is a glvalue. */
-    predicate isGLValue() { ind = 0 }
+    final int getIndirection() { result = ind }
 
     /**
      * Gets the type of this source variable. If `isGLValue()` holds, then
      * the type of this source variable should be thought of as "pointer
      * to `getType()`".
      */
-    DataFlowType getType() {
+    abstract DataFlowType getType();
+
+    /** Gets the location of this variable. */
+    abstract Location getLocation();
+
+    /** Holds if this variable is a glvalue. */
+    final predicate isGLValue() { ind = 0 }
+
+    /** Gets the IR variable associated with this `SourceVariable`, if any. */
+    abstract IRVariable getIRVariable();
+
+    /**
+     * Gets the base source variable (i.e., the variable without any
+     * indirections) of this source variable.
+     */
+    abstract BaseSourceVariable getBaseVariable();
+  }
+
+  final class SourceVariable = AbstractSourceVariable;
+
+  private class SourceSsaVariable extends AbstractSourceVariable, TSourceSsaVariable {
+    BaseSourceVariable base;
+
+    SourceSsaVariable() { this = TSourceSsaVariable(base, ind) }
+
+    override IRVariable getIRVariable() { result = base.(BaseIRVariable).getIRVariable() }
+
+    override BaseSourceVariable getBaseVariable() { result = base }
+
+    override string toString() { result = repeatStars(this.getIndirection()) + base.toString() }
+
+    override DataFlowType getType() {
       if this.isGLValue()
       then result = base.getType()
       else result = getTypeImpl(base.getType(), ind - 1)
     }
 
-    /** Gets the location of this variable. */
-    Location getLocation() { result = this.getBaseVariable().getLocation() }
+    override Location getLocation() { result = this.getBaseVariable().getLocation() }
   }
 }
 
@@ -106,7 +124,7 @@ cached
 private newtype TDefImpl =
   TDefAddressImpl(BaseIRVariable v) or
   TDirectDefImpl(Operand address, int indirectionIndex) {
-    isDef(_, _, address, _, _, indirectionIndex)
+    isDef(_, _, address, _, indirectionIndex)
   } or
   TGlobalDefImpl(GlobalLikeVariable v, IRFunction f, int indirectionIndex) {
     // Represents the initial "definition" of a global variable when entering
@@ -117,8 +135,8 @@ private newtype TDefImpl =
 cached
 private newtype TUseImpl =
   TDirectUseImpl(Operand operand, int indirectionIndex) {
-    isUse(_, operand, _, _, indirectionIndex) and
-    not isDef(true, _, operand, _, _, _)
+    isUse(_, operand, _, indirectionIndex) and
+    not isDef(true, _, operand, _, _)
   } or
   TGlobalUse(GlobalLikeVariable v, IRFunction f, int indirectionIndex) {
     // Represents a final "use" of a global variable to ensure that
@@ -142,7 +160,7 @@ private predicate isGlobalUse(
     min(int cand, VariableAddressInstruction vai |
       vai.getEnclosingIRFunction() = f and
       vai.getAstVariable() = v and
-      isDef(_, _, _, vai, cand, indirectionIndex)
+      isSsaDef(_, _, _, vai, cand, indirectionIndex)
     |
       cand
     )
@@ -154,8 +172,8 @@ private predicate isGlobalDefImpl(
   exists(VariableAddressInstruction vai |
     vai.getEnclosingIRFunction() = f and
     vai.getAstVariable() = v and
-    isUse(_, _, vai, indirection, indirectionIndex) and
-    not isDef(_, _, _, vai, _, indirectionIndex)
+    isSsaUse(_, _, vai, indirection, indirectionIndex) and
+    not isSsaDef(_, _, _, vai, _, indirectionIndex)
   )
 }
 
@@ -411,7 +429,7 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
         base = this.getBase() and
         op =
           min(Operand cand, int i |
-            isUse(_, cand, base, indirection, indirectionIndex) and
+            isSsaUse(_, cand, base, indirection, indirectionIndex) and
             block.getInstruction(i) = cand.getUse()
           |
             cand order by i
@@ -433,7 +451,7 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
 
   override int getIndirection() { isUse(_, operand, _, result, indirectionIndex) }
 
-  override predicate isCertain() { isUse(true, operand, _, _, indirectionIndex) }
+  override predicate isCertain() { isUse(true, operand, _, indirectionIndex) }
 
   override Node getNode() { nodeHasOperand(result, operand, indirectionIndex) }
 }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
index 0920e5a38657..2a0a36e46b7a 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
@@ -588,7 +588,7 @@ private module Cached {
    * `indirectionIndex` specifies the number of loads required to read the variable.
    */
   cached
-  predicate isUse(
+  predicate isSsaUse(
     boolean certain, Operand op, BaseSourceVariableInstruction base, int ind, int indirectionIndex
   ) {
     not ignoreOperand(op) and
@@ -605,6 +605,11 @@ private module Cached {
     )
   }
 
+  cached
+  predicate isUse(boolean certain, Operand op, int ind, int indirectionIndex) {
+    isSsaUse(certain, op, _, ind, indirectionIndex)
+  }
+
   /**
    * Holds if the underlying IR has a suitable instruction to represent a value
    * that would otherwise need to be represented by a dedicated `OperandNode` value.
@@ -692,7 +697,7 @@ private module Cached {
    * after the write operation.
    */
   cached
-  predicate isDef(
+  predicate isSsaDef(
     boolean certain, Node0Impl value, Operand address, BaseSourceVariableInstruction base, int ind,
     int indirectionIndex
   ) {
@@ -710,6 +715,11 @@ private module Cached {
     )
   }
 
+  cached
+  predicate isDef(boolean certain, Node0Impl value, Operand address, int ind, int indirectionIndex) {
+    isSsaDef(certain, value, address, _, ind, indirectionIndex)
+  }
+
   /**
    * Holds if the address computed by `operand` is guaranteed to write
    * to a specific address.

From dab7952b209470606a73cdea7725cd6e08635cee Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 14:59:43 +0100
Subject: [PATCH 02/10] C++: Add a 'getNode' predicate on 'Def' to mirror what
 we have on 'Use'.

---
 .../ir/dataflow/internal/DataFlowPrivate.qll  |  4 ++--
 .../cpp/ir/dataflow/internal/SsaInternals.qll | 19 ++++++++++++-------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
index bc6ebf2c2958..05609d972680 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -792,7 +792,7 @@ predicate jumpStep(Node n1, Node n2) {
   exists(GlobalLikeVariable v |
     exists(Ssa::GlobalUse globalUse |
       v = globalUse.getVariable() and
-      n1.(FinalGlobalValue).getGlobalUse() = globalUse
+      n1 = globalUse.getNode()
     |
       globalUse.getIndirection() = getMinIndirectionForGlobalUse(globalUse) and
       v = n2.asVariable()
@@ -802,7 +802,7 @@ predicate jumpStep(Node n1, Node n2) {
     or
     exists(Ssa::GlobalDef globalDef |
       v = globalDef.getVariable() and
-      n2.(InitialGlobalValue).getGlobalDef() = globalDef
+      n2 = globalDef.getNode()
     |
       globalDef.getIndirection() = getMinIndirectionForGlobalDef(globalDef) and
       v = n1.asVariable()
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index d6861ef2061a..a73e0f2a860d 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -709,13 +709,7 @@ predicate outNodeHasAddressAndIndex(
  */
 predicate defToNode(Node node, Def def, SourceVariable sv, IRBlock bb, int i, boolean uncertain) {
   def.hasIndexInBlock(bb, i, sv) and
-  (
-    nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex())
-    or
-    nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex())
-    or
-    node.(InitialGlobalValue).getGlobalDef() = def
-  ) and
+  node = def.getNode() and
   if def.isCertain() then uncertain = false else uncertain = true
 }
 
@@ -1136,6 +1130,9 @@ abstract class Def extends SsaDef, TDef {
    */
   abstract int getIndirection();
 
+  /** Gets the node associated with this use. */
+  abstract Node getNode();
+
   /**
    * Gets a definition that ultimately defines this SSA definition and is not
    * itself a phi node.
@@ -1171,6 +1168,12 @@ private class NonGlobalDef extends Def {
   override int getIndirectionIndex() { result = this.getImpl().getIndirectionIndex() }
 
   override int getIndirection() { result = this.getImpl().getIndirection() }
+
+  final override Node getNode() {
+    nodeHasOperand(result, this.getValue().asOperand(), this.getIndirectionIndex())
+    or
+    nodeHasInstruction(result, this.getValue().asInstruction(), this.getIndirectionIndex())
+  }
 }
 
 class GlobalDef extends Def {
@@ -1205,6 +1208,8 @@ class GlobalDef extends Def {
   final override int getIndirectionIndex() { result = global.getIndirectionIndex() }
 
   final override int getIndirection() { result = global.getIndirection() }
+
+  final override Node getNode() { result.(InitialGlobalValue).getGlobalDef() = this }
 }
 
 class Phi extends TPhi, SsaDef {

From fd55e7ad7b0ccb7cbf12c06ad422c5898b81516c Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:11:57 +0100
Subject: [PATCH 03/10] C++: Make 'getSourceVariable' an abstract class instead
 of 'getBaseSourceVariable'.

---
 .../cpp/ir/dataflow/internal/SsaInternals.qll | 89 ++++++++++---------
 1 file changed, 45 insertions(+), 44 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index a73e0f2a860d..aa990b779394 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -229,19 +229,8 @@ abstract class DefImpl extends TDefImpl {
    */
   abstract int getIndirection();
 
-  /**
-   * Gets the base source variable (i.e., the variable without
-   * any indirection) of this definition or use.
-   */
-  abstract BaseSourceVariable getBaseSourceVariable();
-
   /** Gets the variable that is defined or used. */
-  SourceVariable getSourceVariable() {
-    exists(BaseSourceVariable v, int indirection |
-      sourceVariableHasBaseAndIndex(result, v, indirection) and
-      defHasSourceVariable(this, v, indirection)
-    )
-  }
+  abstract SourceVariable getSourceVariable();
 
   abstract predicate isCertain();
 
@@ -293,19 +282,8 @@ abstract class UseImpl extends TUseImpl {
   /** Gets the indirection index of this use. */
   final int getIndirectionIndex() { result = indirectionIndex }
 
-  /**
-   * Gets the base source variable (i.e., the variable without
-   * any indirection) of this definition or use.
-   */
-  abstract BaseSourceVariable getBaseSourceVariable();
-
   /** Gets the variable that is defined or used. */
-  SourceVariable getSourceVariable() {
-    exists(BaseSourceVariable v, int indirection |
-      sourceVariableHasBaseAndIndex(result, v, indirection) and
-      useHasSourceVariable(this, v, indirection)
-    )
-  }
+  abstract SourceVariable getSourceVariable();
 
   /**
    * Holds if this use is guaranteed to read the
@@ -374,12 +352,7 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
 
   override Cpp::Location getLocation() { result = v.getIRVariable().getLocation() }
 
-  final override SourceVariable getSourceVariable() {
-    result.getBaseVariable() = v and
-    result.getIndirection() = 0
-  }
-
-  final override BaseSourceVariable getBaseSourceVariable() { result = v }
+  final override SourceVariable getSourceVariable() { sourceVariableHasBaseAndIndex(result, v, 0) }
 }
 
 private class DirectDef extends DefImpl, TDirectDefImpl {
@@ -397,16 +370,19 @@ private class DirectDef extends DefImpl, TDirectDefImpl {
 
   override Operand getAddressOperand() { result = address }
 
-  private BaseSourceVariableInstruction getBase() {
-    isDef(_, _, address, result, _, indirectionIndex)
+  private predicate isNonVirtualDefOf(BaseSourceVariable bv, int ind) {
+    exists(BaseSourceVariableInstruction base |
+      isSsaDef(_, _, address, base, ind, indirectionIndex) and
+      bv = base.getBaseSourceVariable()
+    )
   }
 
-  override BaseSourceVariable getBaseSourceVariable() {
-    result = this.getBase().getBaseSourceVariable()
+  override SourceVariable getSourceVariable() {
+    exists(BaseSourceVariable bv, int ind | sourceVariableHasBaseAndIndex(result, bv, ind) |
+      this.isNonVirtualDefOf(bv, ind)
+    )
   }
 
-  override int getIndirection() { isDef(_, _, address, _, result, indirectionIndex) }
-
   override Node0Impl getValue() { isDef(_, result, address, _, _, _) }
 
   override predicate isCertain() { isDef(true, _, address, _, _, indirectionIndex) }
@@ -439,10 +415,21 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
     else operand.getUse() = block.getInstruction(index)
   }
 
-  private BaseSourceVariableInstruction getBase() { isUse(_, operand, result, _, indirectionIndex) }
+  override SourceVariable getSourceVariable() {
+    exists(BaseSourceVariable bv, int indirection |
+      sourceVariableHasBaseAndIndex(result, bv, indirection)
+    |
+      this.isNonVirtualUseOf(_, bv, indirection)
+    )
+  }
+
+  private BaseSourceVariableInstruction getBase() { this.isNonVirtualUseOf(result, _, _) }
 
-  override BaseSourceVariable getBaseSourceVariable() {
-    result = this.getBase().getBaseSourceVariable()
+  private predicate isNonVirtualUseOf(
+    BaseSourceVariableInstruction base, BaseSourceVariable bv, int ind
+  ) {
+    isSsaUse(_, operand, base, ind, indirectionIndex) and
+    base.getBaseSourceVariable() = bv
   }
 
   final Operand getOperand() { result = operand }
@@ -510,7 +497,13 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse {
     result instanceof UnknownDefaultLocation
   }
 
-  override BaseIRVariable getBaseSourceVariable() { result.getIRVariable().getAst() = p }
+  override SourceVariable getSourceVariable() {
+    exists(BaseIRVariable bv, int ind |
+      sourceVariableHasBaseAndIndex(result, bv, ind) and
+      bv.getIRVariable().getAst() = p and
+      ind = this.getIndirection()
+    )
+  }
 }
 
 /**
@@ -596,8 +589,12 @@ class GlobalUse extends UseImpl, TGlobalUse {
     )
   }
 
-  override BaseSourceVariable getBaseSourceVariable() {
-    baseSourceVariableIsGlobal(result, global, f)
+  override SourceVariable getSourceVariable() {
+    exists(BaseIRVariable bv, int ind |
+      sourceVariableHasBaseAndIndex(result, bv, ind) and
+      baseSourceVariableIsGlobal(bv, global, f) and
+      ind = this.getIndirection()
+    )
   }
 
   final override Cpp::Location getLocation() { result = f.getLocation() }
@@ -643,8 +640,12 @@ class GlobalDefImpl extends DefImpl, TGlobalDefImpl {
   }
 
   /** Gets the global variable associated with this definition. */
-  override BaseSourceVariable getBaseSourceVariable() {
-    baseSourceVariableIsGlobal(result, global, f)
+  override SourceVariable getSourceVariable() {
+    exists(BaseSourceVariable bv, int ind |
+      sourceVariableHasBaseAndIndex(result, bv, ind) and
+      baseSourceVariableIsGlobal(bv, global, f) and
+      ind = this.getIndirection()
+    )
   }
 
   override int getIndirection() { result = indirectionIndex }

From 6a5162a94d456aac9ac6bd93aa0b9fa517f2fcc9 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:14:40 +0100
Subject: [PATCH 04/10] C++: Rename a few more predicates.

---
 .../lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index aa990b779394..3b1fa6496ebe 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -383,9 +383,9 @@ private class DirectDef extends DefImpl, TDirectDefImpl {
     )
   }
 
-  override Node0Impl getValue() { isDef(_, result, address, _, _, _) }
+  override Node0Impl getValue() { isDef(_, result, address, _, _) }
 
-  override predicate isCertain() { isDef(true, _, address, _, _, indirectionIndex) }
+  override predicate isCertain() { isDef(true, _, address, _, indirectionIndex) }
 }
 
 private class DirectUseImpl extends UseImpl, TDirectUseImpl {

From 86a4d344ec4489c78c1fde10b1036da250536b8d Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:18:20 +0100
Subject: [PATCH 05/10] C++: Remove abstract 'getIndirection' on 'Defimpl' and
 'UseImpl'.

---
 .../cpp/ir/dataflow/internal/SsaInternals.qll | 58 +++----------------
 1 file changed, 7 insertions(+), 51 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index 3b1fa6496ebe..4ba682f055fe 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -219,16 +219,6 @@ abstract class DefImpl extends TDefImpl {
   /** Gets the indirection index of this definition. */
   final int getIndirectionIndex() { result = indirectionIndex }
 
-  /**
-   * Gets the index (i.e., the number of loads required) of this
-   * definition or use.
-   *
-   * Note that this is _not_ the definition's (or use's) index in
-   * the enclosing basic block. To obtain this index, use
-   * `DefOrUseImpl::hasIndexInBlock/2` or `DefOrUseImpl::hasIndexInBlock/3`.
-   */
-  abstract int getIndirection();
-
   /** Gets the variable that is defined or used. */
   abstract SourceVariable getSourceVariable();
 
@@ -269,16 +259,6 @@ abstract class UseImpl extends TUseImpl {
   /** Gets the location of this element. */
   abstract Cpp::Location getLocation();
 
-  /**
-   * Gets the index (i.e., the number of loads required) of this
-   * definition or use.
-   *
-   * Note that this is _not_ the definition's (or use's) index in
-   * the enclosing basic block. To obtain this index, use
-   * `DefOrUseImpl::hasIndexInBlock/2` or `DefOrUseImpl::hasIndexInBlock/3`.
-   */
-  abstract int getIndirection();
-
   /** Gets the indirection index of this use. */
   final int getIndirectionIndex() { result = indirectionIndex }
 
@@ -292,18 +272,6 @@ abstract class UseImpl extends TUseImpl {
   abstract predicate isCertain();
 }
 
-pragma[noinline]
-private predicate defHasSourceVariable(DefImpl def, BaseSourceVariable bv, int ind) {
-  bv = def.getBaseSourceVariable() and
-  ind = def.getIndirection()
-}
-
-pragma[noinline]
-private predicate useHasSourceVariable(UseImpl use, BaseSourceVariable bv, int ind) {
-  bv = use.getBaseSourceVariable() and
-  ind = use.getIndirection()
-}
-
 pragma[noinline]
 private predicate sourceVariableHasBaseAndIndex(SourceVariable v, BaseSourceVariable bv, int ind) {
   v.getBaseVariable() = bv and
@@ -332,8 +300,6 @@ private class DefAddressImpl extends DefImpl, TDefAddressImpl {
 
   override string toString() { result = "Def of &" + v.toString() }
 
-  final override int getIndirection() { result = 0 }
-
   final override predicate isCertain() { any() }
 
   final override Node0Impl getValue() { none() }
@@ -400,8 +366,9 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
     // predicate's implementation.
     if this.getBase().getAst() = any(Cpp::PostfixCrementOperation c).getOperand()
     then
-      exists(Operand op, int indirection, Instruction base |
-        indirection = this.getIndirection() and
+      exists(Operand op, int indirection, Instruction base, SourceVariable sv |
+        sv = this.getSourceVariable() and
+        indirection = sv.getIndirection() and
         base = this.getBase() and
         op =
           min(Operand cand, int i |
@@ -436,8 +403,6 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
 
   final override Cpp::Location getLocation() { result = operand.getLocation() }
 
-  override int getIndirection() { isUse(_, operand, _, result, indirectionIndex) }
-
   override predicate isCertain() { isUse(true, operand, _, indirectionIndex) }
 
   override Node getNode() { nodeHasOperand(result, operand, indirectionIndex) }
@@ -464,7 +429,7 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse {
 
   override Node getNode() { finalParameterNodeHasParameterAndIndex(result, p, indirectionIndex) }
 
-  override int getIndirection() { result = indirectionIndex + 1 }
+  int getIndirection() { result = indirectionIndex + 1 }
 
   override predicate isCertain() { any() }
 
@@ -567,7 +532,7 @@ class GlobalUse extends UseImpl, TGlobalUse {
 
   override FinalGlobalValue getNode() { result.getGlobalUse() = this }
 
-  override int getIndirection() { isGlobalUse(global, f, result, indirectionIndex) }
+  int getIndirection() { isGlobalUse(global, f, result, indirectionIndex) }
 
   /** Gets the global variable associated with this use. */
   GlobalLikeVariable getVariable() { result = global }
@@ -648,7 +613,7 @@ class GlobalDefImpl extends DefImpl, TGlobalDefImpl {
     )
   }
 
-  override int getIndirection() { result = indirectionIndex }
+  int getIndirection() { result = indirectionIndex }
 
   override Node0Impl getValue() { none() }
 
@@ -1124,13 +1089,6 @@ abstract class Def extends SsaDef, TDef {
   /** Gets the indirection index of this definition. */
   abstract int getIndirectionIndex();
 
-  /**
-   * Gets the indirection level that this definition is writing to.
-   * For instance, `x = y` is a definition of `x` at indirection level 1 and
-   * `*x = y` is a definition of `x` at indirection level 2.
-   */
-  abstract int getIndirection();
-
   /** Gets the node associated with this use. */
   abstract Node getNode();
 
@@ -1168,8 +1126,6 @@ private class NonGlobalDef extends Def {
 
   override int getIndirectionIndex() { result = this.getImpl().getIndirectionIndex() }
 
-  override int getIndirection() { result = this.getImpl().getIndirection() }
-
   final override Node getNode() {
     nodeHasOperand(result, this.getValue().asOperand(), this.getIndirectionIndex())
     or
@@ -1208,7 +1164,7 @@ class GlobalDef extends Def {
 
   final override int getIndirectionIndex() { result = global.getIndirectionIndex() }
 
-  final override int getIndirection() { result = global.getIndirection() }
+  int getIndirection() { result = global.getIndirection() }
 
   final override Node getNode() { result.(InitialGlobalValue).getGlobalDef() = this }
 }

From 7f8ef6b3febd914fa5c80ba4ada131622da293e2 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:42:53 +0100
Subject: [PATCH 06/10] C++: Use alias analysis in dataflow.

---
 .../cpp/ir/dataflow/internal/SsaInternals.qll | 55 ++++++++++++++++-
 .../dataflow/internal/SsaInternalsCommon.qll  | 61 +++++++++++++++++++
 2 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index 4ba682f055fe..23fba70fef55 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -12,12 +12,27 @@ private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
 private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedInitialization
 private import DataFlowPrivate
 import SsaInternalsCommon
+private import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasedSSA as AliasedSSA
+private import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasConfiguration
 
 private module SourceVariables {
+  private predicate isSourceVirtualVariableVariable(AliasedSSA::VariableVirtualVariable vv, int ind) {
+    ind = [1 .. countIndirectionsForCppType(vv.getType()) + 1]
+  }
+
   cached
   private newtype TSourceVariable =
-    TMkSourceVariable(BaseSourceVariable base, int ind) {
+    TSourceSsaVariable(BaseSourceVariable base, int ind) {
+      // Only create a SSA variable if it's not already created by the `TSourceVirtualVariable` branch.
+      not exists(AliasedSSA::VariableVirtualVariable vv, VariableAllocation var |
+        isSourceVirtualVariableVariable(vv, ind) and
+        var = vv.getAllocation() and
+        var.getIRVariable() = base.(BaseIRVariable).getIRVariable()
+      ) and
       ind = [0 .. countIndirectionsForCppType(base.getLanguageType()) + 1]
+    } or
+    TSourceVirtualVariable(AliasedSSA::VariableVirtualVariable vv, int ind) {
+      isSourceVirtualVariableVariable(vv, ind)
     }
 
   abstract private class AbstractSourceVariable extends TSourceVariable {
@@ -79,6 +94,26 @@ private module SourceVariables {
 
     override Location getLocation() { result = this.getBaseVariable().getLocation() }
   }
+
+  private class SourceVirtualVariable extends AbstractSourceVariable, TSourceVirtualVariable {
+    AliasedSSA::VirtualVariable vv;
+
+    SourceVirtualVariable() { this = TSourceVirtualVariable(vv, ind) }
+
+    override Location getLocation() { result = vv.getLocation() }
+
+    override BaseIRVariable getBaseVariable() { result.getIRVariable() = this.getIRVariable() }
+
+    override IRVariable getIRVariable() {
+      result = vv.getAllocation().(VariableAllocation).getIRVariable()
+    }
+
+    override DataFlowType getType() {
+      result = getTypeImpl(any(Type t | vv.getType().hasType(t, false)), ind - 1)
+    }
+
+    override string toString() { result = repeatStars(this.getIndirection()) + vv.toString() }
+  }
 }
 
 import SourceVariables
@@ -343,9 +378,18 @@ private class DirectDef extends DefImpl, TDirectDefImpl {
     )
   }
 
+  private predicate isVirtualDefOf(BaseIRVariable bv, int ind) {
+    exists(VariableAllocation v |
+      isVirtualDef(_, _, address, v, ind, indirectionIndex) and
+      bv.getIRVariable() = v.getIRVariable()
+    )
+  }
+
   override SourceVariable getSourceVariable() {
     exists(BaseSourceVariable bv, int ind | sourceVariableHasBaseAndIndex(result, bv, ind) |
       this.isNonVirtualDefOf(bv, ind)
+      or
+      this.isVirtualDefOf(bv, ind)
     )
   }
 
@@ -387,6 +431,8 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
       sourceVariableHasBaseAndIndex(result, bv, indirection)
     |
       this.isNonVirtualUseOf(_, bv, indirection)
+      or
+      this.isVirtualUseOf(bv, indirection)
     )
   }
 
@@ -399,6 +445,13 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl {
     base.getBaseSourceVariable() = bv
   }
 
+  private predicate isVirtualUseOf(BaseIRVariable bv, int ind) {
+    exists(VariableAllocation v |
+      isVirtualUse(_, operand, v, ind, indirectionIndex) and
+      bv.getIRVariable() = v.getIRVariable()
+    )
+  }
+
   final Operand getOperand() { result = operand }
 
   final override Cpp::Location getLocation() { result = operand.getLocation() }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
index 2a0a36e46b7a..0f0982bff5cb 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
@@ -8,6 +8,8 @@ private import semmle.code.cpp.models.interfaces.PointerWrapper
 private import DataFlowPrivate
 private import TypeFlow
 private import semmle.code.cpp.ir.ValueNumbering
+private import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasedSSA as AliasedSSA
+private import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasConfiguration
 
 /**
  * Holds if `operand` is an operand that is not used by the dataflow library.
@@ -605,9 +607,38 @@ private module Cached {
     )
   }
 
+  /**
+   * Holds if the IR alias anlaysis has determined that the `indirectionIndex`'th
+   * indirection of `op` points to the `ind`'th indirection of `v`.
+   *
+   * Since the IR alias analysis is a 'must' analysis `certain` is always `true`.
+   */
+  cached
+  predicate isVirtualUse(
+    boolean certain, Operand op, VariableAllocation v, int ind, int indirectionIndex
+  ) {
+    exists(
+      AliasedSSA::VariableVirtualVariable vv, MemoryOperand mem, Instruction use, CppType type,
+      int upper
+    |
+      vv = AliasedSSA::getOperandMemoryLocation(mem) and
+      v = vv.getAllocation() and
+      op = mem.getAddressOperand() and
+      type = v.getIRVariable().getLanguageType() and
+      upper = countIndirectionsForCppType(type) and
+      certain = true and
+      use = mem.getUse() and
+      not ignoreInstruction(use) and
+      ind = 1 + [0 .. upper] and
+      indirectionIndex = ind
+    )
+  }
+
   cached
   predicate isUse(boolean certain, Operand op, int ind, int indirectionIndex) {
     isSsaUse(certain, op, _, ind, indirectionIndex)
+    or
+    isVirtualUse(certain, op, _, ind, indirectionIndex)
   }
 
   /**
@@ -715,9 +746,39 @@ private module Cached {
     )
   }
 
+  /**
+   * Holds if the the IR alias analysis has determined that the `indirectionIndex`'th
+   * definition of `address` writes to the `ind`'th indirection of `v`.
+   *
+   * * Since the IR alias analysis is a 'must' analysis `certain` is always `true`.
+   */
+  cached
+  predicate isVirtualDef(
+    boolean certain, Node0Impl value, Operand address, VariableAllocation v, int ind,
+    int indirectionIndex
+  ) {
+    exists(
+      AliasedSSA::VariableVirtualVariable vv, StoreInstruction store, int upper, CppType type,
+      int lower
+    |
+      vv = AliasedSSA::getResultMemoryLocation(store) and
+      v = vv.getAllocation() and
+      address = store.getDestinationAddressOperand() and
+      value.asInstruction() = store and
+      type = v.getIRVariable().getLanguageType() and
+      upper = countIndirectionsForCppType(type) and
+      lower = getMinIndirectionsForType(any(Type t | type.hasUnspecifiedType(t, _))) and
+      ind = [lower .. upper + 1] and
+      indirectionIndex = ind - lower and
+      certain = true
+    )
+  }
+
   cached
   predicate isDef(boolean certain, Node0Impl value, Operand address, int ind, int indirectionIndex) {
     isSsaDef(certain, value, address, _, ind, indirectionIndex)
+    or
+    isVirtualDef(certain, value, address, _, ind, indirectionIndex)
   }
 
   /**

From ac8e9f7f2e9b837ae7a57351d89a798dd73a2865 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:43:27 +0100
Subject: [PATCH 07/10] C++: Accept test changes.

---
 .../dataflow-tests/test-source-sink.expected         |  1 +
 .../library-tests/dataflow/dataflow-tests/test.cpp   |  2 +-
 .../dataflow/taint-tests/arrayassignment.cpp         | 12 ++++++------
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
index e8afa785492f..966634614084 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
@@ -215,6 +215,7 @@ irFlow
 | test.cpp:83:7:83:8 | definition of u2 | test.cpp:86:8:86:9 | i1 |
 | test.cpp:89:28:89:34 | *source1 | test.cpp:90:8:90:14 | source1 |
 | test.cpp:100:13:100:18 | call to source | test.cpp:103:10:103:12 | ref |
+| test.cpp:109:9:109:14 | call to source | test.cpp:110:10:110:12 | ref |
 | test.cpp:138:27:138:32 | call to source | test.cpp:140:8:140:8 | y |
 | test.cpp:151:33:151:38 | call to source | test.cpp:144:8:144:8 | s |
 | test.cpp:151:33:151:38 | call to source | test.cpp:152:8:152:8 | y |
diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
index c1c84c71e3bf..d49bdc5ac88a 100644
--- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
+++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
@@ -107,7 +107,7 @@ void local_references(int &source1, int clean1) { // $ ast-def=source1 ir-def=*s
     int t = clean1;
     int &ref = t;
     t = source();
-    sink(ref); // $ MISSING: ast,ir
+    sink(ref); // $ ir MISSING: ast
   }
 }
 
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/arrayassignment.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/arrayassignment.cpp
index 8c73d6c552c6..3b173a6e9797 100644
--- a/cpp/ql/test/library-tests/dataflow/taint-tests/arrayassignment.cpp
+++ b/cpp/ql/test/library-tests/dataflow/taint-tests/arrayassignment.cpp
@@ -13,10 +13,10 @@ void test_pointer_deref_assignment()
 
 	*p_x = source();
 
-	sink(x); // $ MISSING: ast,ir
+	sink(x); // $ ir MISSING: ast
 	sink(*p_x); // $ ast,ir
-	sink(*p2_x); // $ MISSING: ast,ir
-	sink(r_x); // $ MISSING: ast,ir
+	sink(*p2_x); // $ ir MISSING: ast
+	sink(r_x); // $ ir MISSING: ast
 }
 
 void test_reference_deref_assignment()
@@ -28,10 +28,10 @@ void test_reference_deref_assignment()
 
 	r_x = source();
 
-	sink(x); // $ MISSING: ast,ir
-	sink(*p_x); // $ MISSING: ast,ir
+	sink(x); // $ ir MISSING: ast
+	sink(*p_x); // $ ir MISSING: ast
 	sink(r_x); // $ ast,ir
-	sink(r2_x); // $ MISSING: ast,ir
+	sink(r2_x); // $ ir MISSING: ast
 }
 
 class MyInt

From 7762acbca520dc9bf7a7d5c4c3bb86d7e1913f91 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:43:53 +0100
Subject: [PATCH 08/10] C++: Accept query test changes.

---
 .../MissingCheckScanf/MissingCheckScanf.expected   |  4 ++++
 .../NonConstantFormat/NonConstantFormat.expected   | 14 ++++++++++++--
 .../UncontrolledProcessOperation.expected          |  6 ++++++
 .../UncontrolledProcessOperation.expected          |  3 +++
 .../ArithmeticUncontrolled.expected                |  6 ++++++
 .../CWE-190/semmle/ArithmeticUncontrolled/test.c   |  2 +-
 6 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
index 8bb2c9643a9f..57625dfd5fbc 100644
--- a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
+++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
@@ -25,6 +25,7 @@ edges
 | test.cpp:271:15:271:16 | scanf output argument | test.cpp:272:7:272:7 | i | provenance |  |
 | test.cpp:279:15:279:16 | scanf output argument | test.cpp:280:7:280:7 | i | provenance |  |
 | test.cpp:291:15:291:16 | scanf output argument | test.cpp:292:7:292:7 | i | provenance |  |
+| test.cpp:301:15:301:16 | scanf output argument | test.cpp:302:7:302:12 | * ... | provenance |  |
 | test.cpp:325:34:325:35 | sscanf output argument | test.cpp:327:8:327:8 | i | provenance |  |
 | test.cpp:325:38:325:39 | sscanf output argument | test.cpp:328:8:328:8 | j | provenance |  |
 | test.cpp:335:22:335:23 | scanf output argument | test.cpp:337:8:337:8 | i | provenance |  |
@@ -90,6 +91,8 @@ nodes
 | test.cpp:280:7:280:7 | i | semmle.label | i |
 | test.cpp:291:15:291:16 | scanf output argument | semmle.label | scanf output argument |
 | test.cpp:292:7:292:7 | i | semmle.label | i |
+| test.cpp:301:15:301:16 | scanf output argument | semmle.label | scanf output argument |
+| test.cpp:302:7:302:12 | * ... | semmle.label | * ... |
 | test.cpp:325:34:325:35 | sscanf output argument | semmle.label | sscanf output argument |
 | test.cpp:325:38:325:39 | sscanf output argument | semmle.label | sscanf output argument |
 | test.cpp:327:8:327:8 | i | semmle.label | i |
@@ -129,6 +132,7 @@ subpaths
 | test.cpp:272:7:272:7 | i | test.cpp:271:15:271:16 | scanf output argument | test.cpp:272:7:272:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:271:3:271:7 | call to scanf | call to scanf |
 | test.cpp:280:7:280:7 | i | test.cpp:279:15:279:16 | scanf output argument | test.cpp:280:7:280:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:279:3:279:7 | call to scanf | call to scanf |
 | test.cpp:292:7:292:7 | i | test.cpp:291:15:291:16 | scanf output argument | test.cpp:292:7:292:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:291:3:291:7 | call to scanf | call to scanf |
+| test.cpp:302:7:302:12 | * ... | test.cpp:301:15:301:16 | scanf output argument | test.cpp:302:7:302:12 | * ... | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:301:3:301:7 | call to scanf | call to scanf |
 | test.cpp:404:25:404:25 | u | test.cpp:403:29:403:30 | sscanf output argument | test.cpp:404:18:404:25 | u | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:403:6:403:11 | call to sscanf | call to sscanf |
 | test.cpp:416:7:416:7 | i | test.cpp:413:19:413:20 | scanf output argument | test.cpp:416:7:416:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:413:7:413:11 | call to scanf | call to scanf |
 | test.cpp:423:7:423:7 | i | test.cpp:420:19:420:20 | scanf output argument | test.cpp:423:7:423:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:420:7:420:11 | call to scanf | call to scanf |
diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.expected
index 9424c731765e..eff6b548da0b 100644
--- a/cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.expected	
+++ b/cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/NonConstantFormat.expected	
@@ -29,9 +29,15 @@ edges
 | test.cpp:227:25:227:36 | *call to get_string | test.cpp:228:12:228:18 | *++ ... | provenance |  |
 | test.cpp:228:12:228:18 | *++ ... | test.cpp:228:12:228:18 | *++ ... | provenance |  |
 | test.cpp:232:25:232:36 | *call to get_string | test.cpp:232:25:232:36 | *call to get_string | provenance |  |
-| test.cpp:232:25:232:36 | *call to get_string | test.cpp:235:12:235:16 | *hello | provenance |  |
+| test.cpp:232:25:232:36 | *call to get_string | test.cpp:233:22:233:27 | **& ... | provenance |  |
+| test.cpp:232:25:232:36 | *call to get_string | test.cpp:234:5:234:10 | *... ++ | provenance |  |
+| test.cpp:233:22:233:27 | **& ... | test.cpp:234:5:234:10 | *... ++ | provenance |  |
+| test.cpp:234:5:234:10 | *... ++ | test.cpp:235:12:235:16 | *hello | provenance |  |
 | test.cpp:239:25:239:36 | *call to get_string | test.cpp:239:25:239:36 | *call to get_string | provenance |  |
-| test.cpp:239:25:239:36 | *call to get_string | test.cpp:242:12:242:16 | *hello | provenance |  |
+| test.cpp:239:25:239:36 | *call to get_string | test.cpp:240:22:240:26 | **hello | provenance |  |
+| test.cpp:239:25:239:36 | *call to get_string | test.cpp:241:5:241:7 | *... ++ | provenance |  |
+| test.cpp:240:22:240:26 | **hello | test.cpp:241:5:241:7 | *... ++ | provenance |  |
+| test.cpp:241:5:241:7 | *... ++ | test.cpp:242:12:242:16 | *hello | provenance |  |
 | test.cpp:245:25:245:36 | *call to get_string | test.cpp:245:25:245:36 | *call to get_string | provenance |  |
 | test.cpp:245:25:245:36 | *call to get_string | test.cpp:247:12:247:16 | *hello | provenance |  |
 nodes
@@ -80,9 +86,13 @@ nodes
 | test.cpp:228:12:228:18 | *++ ... | semmle.label | *++ ... |
 | test.cpp:232:25:232:36 | *call to get_string | semmle.label | *call to get_string |
 | test.cpp:232:25:232:36 | *call to get_string | semmle.label | *call to get_string |
+| test.cpp:233:22:233:27 | **& ... | semmle.label | **& ... |
+| test.cpp:234:5:234:10 | *... ++ | semmle.label | *... ++ |
 | test.cpp:235:12:235:16 | *hello | semmle.label | *hello |
 | test.cpp:239:25:239:36 | *call to get_string | semmle.label | *call to get_string |
 | test.cpp:239:25:239:36 | *call to get_string | semmle.label | *call to get_string |
+| test.cpp:240:22:240:26 | **hello | semmle.label | **hello |
+| test.cpp:241:5:241:7 | *... ++ | semmle.label | *... ++ |
 | test.cpp:242:12:242:16 | *hello | semmle.label | *hello |
 | test.cpp:245:25:245:36 | *call to get_string | semmle.label | *call to get_string |
 | test.cpp:245:25:245:36 | *call to get_string | semmle.label | *call to get_string |
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-114/SAMATE/UncontrolledProcessOperation/UncontrolledProcessOperation.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-114/SAMATE/UncontrolledProcessOperation/UncontrolledProcessOperation.expected
index f328113106e4..b15545bda524 100644
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-114/SAMATE/UncontrolledProcessOperation/UncontrolledProcessOperation.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-114/SAMATE/UncontrolledProcessOperation/UncontrolledProcessOperation.expected
@@ -3,12 +3,18 @@ edges
 | test.cpp:64:30:64:35 | *call to getenv | test.cpp:64:30:64:35 | *call to getenv | provenance |  |
 | test.cpp:64:30:64:35 | *call to getenv | test.cpp:73:24:73:27 | *data | provenance | TaintFunction |
 | test.cpp:73:24:73:27 | *data | test.cpp:37:73:37:76 | *data | provenance |  |
+| test.cpp:96:23:96:34 | fgets output argument | test.cpp:115:23:115:29 | *dataRef | provenance |  |
+| test.cpp:115:23:115:29 | *dataRef | test.cpp:120:36:120:39 | *data | provenance |  |
 nodes
 | test.cpp:37:73:37:76 | *data | semmle.label | *data |
 | test.cpp:43:32:43:35 | *data | semmle.label | *data |
 | test.cpp:64:30:64:35 | *call to getenv | semmle.label | *call to getenv |
 | test.cpp:64:30:64:35 | *call to getenv | semmle.label | *call to getenv |
 | test.cpp:73:24:73:27 | *data | semmle.label | *data |
+| test.cpp:96:23:96:34 | fgets output argument | semmle.label | fgets output argument |
+| test.cpp:115:23:115:29 | *dataRef | semmle.label | *dataRef |
+| test.cpp:120:36:120:39 | *data | semmle.label | *data |
 subpaths
 #select
 | test.cpp:43:32:43:35 | *data | test.cpp:64:30:64:35 | *call to getenv | test.cpp:43:32:43:35 | *data | The value of this argument may come from $@ and is being passed to LoadLibraryA. | test.cpp:64:30:64:35 | *call to getenv | an environment variable |
+| test.cpp:120:36:120:39 | *data | test.cpp:96:23:96:34 | fgets output argument | test.cpp:120:36:120:39 | *data | The value of this argument may come from $@ and is being passed to LoadLibraryA. | test.cpp:96:23:96:34 | fgets output argument | string read by fgets |
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/UncontrolledProcessOperation.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/UncontrolledProcessOperation.expected
index ca24075c2c34..3903d7fe7aef 100644
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/UncontrolledProcessOperation.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-114/semmle/UncontrolledProcessOperation/UncontrolledProcessOperation.expected
@@ -6,8 +6,11 @@ edges
 | test.cpp:56:12:56:17 | fgets output argument | test.cpp:58:16:58:21 | *buffer | provenance |  |
 | test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | *buffer | provenance |  |
 | test.cpp:58:16:58:21 | *buffer | test.cpp:59:20:59:23 | **data | provenance |  |
+| test.cpp:58:16:58:21 | *buffer | test.cpp:60:17:60:23 | *dataref | provenance |  |
 | test.cpp:58:16:58:21 | *buffer | test.cpp:63:10:63:13 | *data | provenance |  |
+| test.cpp:58:16:58:21 | *buffer | test.cpp:64:10:64:16 | *dataref | provenance |  |
 | test.cpp:59:20:59:23 | **data | test.cpp:60:17:60:23 | *dataref | provenance |  |
+| test.cpp:59:20:59:23 | **data | test.cpp:63:10:63:13 | *data | provenance |  |
 | test.cpp:59:20:59:23 | **data | test.cpp:64:10:64:16 | *dataref | provenance |  |
 | test.cpp:60:17:60:23 | *dataref | test.cpp:65:10:65:14 | *data2 | provenance |  |
 | test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | *buffer | provenance |  |
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected
index c21f9c38855c..86c31d11e611 100644
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/ArithmeticUncontrolled.expected
@@ -11,6 +11,8 @@ edges
 | test.c:81:13:81:29 | ... ^ ... | test.c:83:9:83:9 | r | provenance |  |
 | test.c:81:14:81:17 | call to rand | test.c:81:13:81:29 | ... ^ ... | provenance |  |
 | test.c:81:23:81:26 | call to rand | test.c:81:13:81:29 | ... ^ ... | provenance |  |
+| test.c:99:5:99:19 | ... = ... | test.c:100:5:100:5 | r | provenance |  |
+| test.c:99:14:99:19 | call to rand | test.c:99:5:99:19 | ... = ... | provenance |  |
 | test.c:125:13:125:16 | call to rand | test.c:125:13:125:16 | call to rand | provenance |  |
 | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | provenance |  |
 | test.c:131:13:131:16 | call to rand | test.c:131:13:131:16 | call to rand | provenance |  |
@@ -77,6 +79,9 @@ nodes
 | test.c:81:14:81:17 | call to rand | semmle.label | call to rand |
 | test.c:81:23:81:26 | call to rand | semmle.label | call to rand |
 | test.c:83:9:83:9 | r | semmle.label | r |
+| test.c:99:5:99:19 | ... = ... | semmle.label | ... = ... |
+| test.c:99:14:99:19 | call to rand | semmle.label | call to rand |
+| test.c:100:5:100:5 | r | semmle.label | r |
 | test.c:125:13:125:16 | call to rand | semmle.label | call to rand |
 | test.c:125:13:125:16 | call to rand | semmle.label | call to rand |
 | test.c:127:9:127:9 | r | semmle.label | r |
@@ -149,6 +154,7 @@ subpaths
 | test.c:77:9:77:9 | r | test.c:75:13:75:19 | call to rand | test.c:77:9:77:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:75:13:75:19 | call to rand | uncontrolled value |
 | test.c:83:9:83:9 | r | test.c:81:14:81:17 | call to rand | test.c:83:9:83:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:81:14:81:17 | call to rand | uncontrolled value |
 | test.c:83:9:83:9 | r | test.c:81:23:81:26 | call to rand | test.c:83:9:83:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:81:23:81:26 | call to rand | uncontrolled value |
+| test.c:100:5:100:5 | r | test.c:99:14:99:19 | call to rand | test.c:100:5:100:5 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:99:14:99:19 | call to rand | uncontrolled value |
 | test.c:127:9:127:9 | r | test.c:125:13:125:16 | call to rand | test.c:127:9:127:9 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:125:13:125:16 | call to rand | uncontrolled value |
 | test.c:133:5:133:5 | r | test.c:131:13:131:16 | call to rand | test.c:133:5:133:5 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:131:13:131:16 | call to rand | uncontrolled value |
 | test.c:139:10:139:10 | r | test.c:137:13:137:16 | call to rand | test.c:139:10:139:10 | r | This arithmetic expression depends on an $@, potentially causing an overflow. | test.c:137:13:137:16 | call to rand | uncontrolled value |
diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c
index 691801a620a5..d6eb0f3ba5da 100644
--- a/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c
+++ b/cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/ArithmeticUncontrolled/test.c
@@ -97,7 +97,7 @@ void randomTester() {
     int r = 0;
     int *ptr_r = &r;
     *ptr_r = RAND();
-    r += 100; // BAD [NOT DETECTED]
+    r += 100; // BAD
   }
 
   {

From 393c1006f3d9a362b633dfd405ec14e5dc0e9bfe Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Thu, 9 May 2024 15:55:49 +0100
Subject: [PATCH 09/10] C++: Add QLDoc.

---
 .../lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll   | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
index 23fba70fef55..db4f09e4de2b 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -585,6 +585,7 @@ class GlobalUse extends UseImpl, TGlobalUse {
 
   override FinalGlobalValue getNode() { result.getGlobalUse() = this }
 
+  /** Gets the indirection of this use. */
   int getIndirection() { isGlobalUse(global, f, result, indirectionIndex) }
 
   /** Gets the global variable associated with this use. */
@@ -1217,6 +1218,7 @@ class GlobalDef extends Def {
 
   final override int getIndirectionIndex() { result = global.getIndirectionIndex() }
 
+  /** Gets the indirection of this def. */
   int getIndirection() { result = global.getIndirection() }
 
   final override Node getNode() { result.(InitialGlobalValue).getGlobalDef() = this }

From 743c669d61f750311785e89013131c3eb36a4699 Mon Sep 17 00:00:00 2001
From: Mathias Vorreiter Pedersen <mathiasvp@github.com>
Date: Wed, 15 May 2024 23:32:13 +0100
Subject: [PATCH 10/10] C++: Better caching.

---
 cpp/ql/lib/semmle/code/cpp/Caching.qll        | 68 +++++++++++++++++++
 .../dataflow/internal/SsaInternalsCommon.qll  | 11 ++-
 .../aliased_ssa/internal/AliasedSSA.qll       |  3 +
 .../raw/internal/IRConstruction.qll           |  3 +
 4 files changed, 82 insertions(+), 3 deletions(-)
 create mode 100644 cpp/ql/lib/semmle/code/cpp/Caching.qll

diff --git a/cpp/ql/lib/semmle/code/cpp/Caching.qll b/cpp/ql/lib/semmle/code/cpp/Caching.qll
new file mode 100644
index 000000000000..0a916958f4c0
--- /dev/null
+++ b/cpp/ql/lib/semmle/code/cpp/Caching.qll
@@ -0,0 +1,68 @@
+/**
+ * INTERNAL: Do not use.
+ *
+ * The purpose of this file is to control which cached predicates belong to the same stage.
+ *
+ * Combining stages can improve performance as we are more likely to reuse shared, non-cached predicates.
+ *
+ * To make a predicate `p` belong to a stage `A`:
+ * - make `p` depend on `A::ref()`, and
+ * - make `A::backref()` depend on `p`.
+ *
+ * Since `A` is a cached module, `ref` and `backref` must be in the same stage, and the dependency
+ * chain above thus forces `p` to be in that stage as well.
+ *
+ * With these two predicates in a `cached module` we ensure that all the cached predicates will be in a single stage at runtime.
+ *
+ * Grouping stages can cause unnecessary computation, as a concrete query might not depend on
+ * all the cached predicates in a stage.
+ * Care should therefore be taken not to combine two stages, if it is likely that a query only depend
+ * on some but not all the cached predicates in the combined stage.
+ */
+
+private import cpp
+
+/**
+ * Contains a `cached module` for each stage.
+ * Each `cached module` ensures that predicates that are supposed to be in the same stage, are in the same stage.
+ *
+ * Each `cached module` contain two predicates:
+ * The first, `ref`, always holds, and is referenced from `cached` predicates.
+ * The second, `backref`, contains references to the same `cached` predicates.
+ * The `backref` predicate starts with `1 = 1 or` to ensure that the predicate will be optimized down to a constant by the optimizer.
+ */
+module Stages {
+  private import semmle.code.cpp.ir.dataflow.internal.SsaInternalsCommon as SsaInternalsCommon
+  private import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasedSSA as AliasedSSA
+  private import semmle.code.cpp.ir.implementation.raw.internal.IRConstruction as IRConstruction
+
+  /**
+   * The `IR` stage.
+   */
+  cached
+  module IR {
+    /**
+     * Always holds.
+     * Ensures that a predicate is evaluated as part of the IR stage.
+     */
+    cached
+    predicate ref() { 1 = 1 }
+
+    /**
+     * DONT USE!
+     * Contains references to each predicate that use the above `ref` predicate.
+     */
+    cached
+    predicate backref() {
+      1 = 1
+      or
+      IRConstruction::Raw::hasInstruction(_, _)
+      or
+      SsaInternalsCommon::isUse(_, _, _, _)
+      or
+      exists(AliasedSSA::getOperandMemoryLocation(_))
+      or
+      exists(AliasedSSA::getResultMemoryLocation(_))
+    }
+  }
+}
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
index 0f0982bff5cb..5645a4732636 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
@@ -573,6 +573,8 @@ private class BaseCallInstruction extends BaseSourceVariableInstruction, CallIns
 
 cached
 private module Cached {
+  private import semmle.code.cpp.Caching
+
   /** Holds if `op` is the only use of its defining instruction, and that op is used in a conversation */
   private predicate isConversion(Operand op) {
     exists(Instruction def, Operand use |
@@ -636,9 +638,12 @@ private module Cached {
 
   cached
   predicate isUse(boolean certain, Operand op, int ind, int indirectionIndex) {
-    isSsaUse(certain, op, _, ind, indirectionIndex)
-    or
-    isVirtualUse(certain, op, _, ind, indirectionIndex)
+    Stages::IR::ref() and
+    (
+      isSsaUse(certain, op, _, ind, indirectionIndex)
+      or
+      isVirtualUse(certain, op, _, ind, indirectionIndex)
+    )
   }
 
   /**
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll
index 10fddf6352ba..fcfb7c95da60 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll
@@ -8,6 +8,7 @@ private import semmle.code.cpp.ir.internal.IntegerConstant as Ints
 private import semmle.code.cpp.ir.internal.IntegerInterval as Interval
 private import semmle.code.cpp.ir.implementation.internal.OperandTag
 private import AliasConfiguration
+private import semmle.code.cpp.Caching
 
 private class IntValue = Ints::IntValue;
 
@@ -581,6 +582,7 @@ bindingset[result, b]
 private boolean unbindBool(boolean b) { result != b.booleanNot() }
 
 MemoryLocation getResultMemoryLocation(Instruction instr) {
+  Stages::IR::ref() and
   not canReuseSsaForOldResult(instr) and
   exists(MemoryAccessKind kind, boolean isMayAccess |
     kind = instr.getResultMemoryAccess() and
@@ -614,6 +616,7 @@ MemoryLocation getResultMemoryLocation(Instruction instr) {
 }
 
 MemoryLocation getOperandMemoryLocation(MemoryOperand operand) {
+  Stages::IR::ref() and
   not canReuseSsaForOldResult(operand.getAnyDef()) and
   exists(MemoryAccessKind kind, boolean isMayAccess |
     kind = operand.getMemoryAccess() and
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll
index 7bea8178d141..532819f39b47 100644
--- a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll
+++ b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll
@@ -30,6 +30,8 @@ InstructionTag getInstructionTag(Instruction instruction) {
  */
 cached
 module Raw {
+  private import semmle.code.cpp.Caching
+
   class InstructionTag1 = TranslatedElement;
 
   class InstructionTag2 = InstructionTag;
@@ -55,6 +57,7 @@ module Raw {
 
   cached
   predicate hasInstruction(TranslatedElement element, InstructionTag tag) {
+    Stages::IR::ref() and
     element.hasInstruction(_, tag, _)
   }