Merge pull request #14751 from owen-mc/go/feature/use-use-flow

Go: Switch from def-use flow to use-use flow

Author: owen-mc

go/ql/lib/change-notes/2025-09-19-api-changes.md

@@ -0,0 +1,5 @@
+---
+category: breaking
+---
+* The member predicate `writesField` on `DataFlow::Write` now uses the post-update node for `base` when that is the node being updated, which is in all cases except initializing a struct literal. A new member predicate `writesFieldPreUpdate` has been added for cases where this behaviour is not desired.
+* The member predicate `writesElement` on `DataFlow::Write` now uses the post-update node for `base` when that is the node being updated, which is in all cases except initializing an array/slice/map literal. A new member predicate `writesElementPreUpdate` has been added for cases where this behaviour is not desired.

go/ql/lib/change-notes/2025-09-19-use-use-flow-proper-post-update-nodes.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* The shape of the Go data-flow graph has changed. Previously for code like `x := def(); use1(x); use2(x)`, there would be edges from the definition of `x` to each use. Now there is an edge from the definition to the first use, then another from the first use to the second, and so on. This means that data-flow barriers work differently - flow will not reach any uses after the barrier node. Where this is not desired it may be be necessary to add an additional flow step to propagate the flow forward. Additionally, when a variable may be subject to a side-effect, such as updating an array, passing a pointer to a function that might write through it or writing to a field of a struct, there is now a dedicated post-update node representing the variable after this side-effect has taken place. Previously post-update nodes were aliases for either a variable's definition, or were equal to the pre-update node. This led to backwards steps in the data-flow graph, which could cause false positives. For example, in the previous code there would be an edge from `x` in `use2(x)` back to the definition of `x`. If we define our sources as any argument of `use2` and our sinks as any argument of `use1` then this would lead to a false positive path. Now there are distinct post-update nodes and no backwards edge to the definition, so we will not find this false positive path.

go/ql/lib/change-notes/2025-10-02-unvalidated-url-redirection-struct-init-fix.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* For the query `go/unvalidated-url-redirection`, when untrusted data is assigned to the `Host` field of a `url.URL` struct, we consider the whole struct untrusted. We now also include the case when this happens during struct initialization, for example `&url.URL{Host: untrustedData}`.

go/ql/lib/change-notes/2025-10-02-writenode-writescomponent-deprecated.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The member predicate `writesComponent` on `DataFlow::Write` has been deprecated. Instead, use `writesFieldPreUpdate` and `writesElementPreUpdate`, or their new versions `writesField` and `writesElement`.

go/ql/lib/semmle/go/controlflow/ControlFlowGraph.qll

@@ -118,6 +118,8 @@ module ControlFlow {
     /** Gets the left-hand side of this write. */
     IR::WriteTarget getLhs() { result = super.getLhs() }
 
+    private predicate isInitialization() { super.isInitialization() }
+
     /** Gets the right-hand side of this write. */
     DataFlow::Node getRhs() { super.getRhs() = result.asInstruction() }
 
@@ -132,49 +134,112 @@ module ControlFlow {
 
     /**
      * Holds if this node sets the value of field `f` on `base` (or its implicit dereference) to
-     * `rhs`.
+     * `rhs`, where `base` represents the post-update value.
+     *
+     * For example, for the assignment `x.width = newWidth`, `base` is the post-update node of
+     * either the data-flow node corresponding to `x` or (if `x` is a pointer) the data-flow node
+     * corresponding to the implicit dereference `*x`, `f` is the field referenced by `width`, and
+     * `rhs` is the data-flow node corresponding to `newWidth`. If this `WriteNode` is a struct
+     * initialization then there is no post-update node and `base` is the struct literal being
+     * initialized.
+     */
+    predicate writesField(DataFlow::Node base, Field f, DataFlow::Node rhs) {
+      exists(DataFlow::Node b | this.writesFieldPreUpdate(b, f, rhs) |
+        this.isInitialization() and base = b
+        or
+        not this.isInitialization() and
+        b = base.(DataFlow::PostUpdateNode).getPreUpdateNode()
+      )
+    }
+
+    /**
+     * Holds if this node sets the value of field `f` on `base` (or its implicit dereference) to
+     * `rhs`, where `base` represents the pre-update value.
      *
      * For example, for the assignment `x.width = newWidth`, `base` is either the data-flow node
      * corresponding to `x` or (if `x` is a pointer) the data-flow node corresponding to the
-     * implicit dereference `*x`, `f` is the field referenced by `width`, and `rhs` is the data-flow
-     * node corresponding to `newWidth`.
+     * implicit dereference `*x`, `f` is the field referenced by `width`, and `rhs` is the
+     * data-flow node corresponding to `newWidth`.
      */
-    predicate writesField(DataFlow::Node base, Field f, DataFlow::Node rhs) {
+    predicate writesFieldPreUpdate(DataFlow::Node base, Field f, DataFlow::Node rhs) {
+      this.writesFieldInsn(base.asInstruction(), f, rhs.asInstruction())
+    }
+
+    private predicate writesFieldInsn(IR::Instruction base, Field f, IR::Instruction rhs) {
       exists(IR::FieldTarget trg | trg = super.getLhs() |
         (
-          trg.getBase() = base.asInstruction() or
-          trg.getBase() = MkImplicitDeref(base.asExpr())
+          trg.getBase() = base or
+          trg.getBase() = MkImplicitDeref(base.(IR::EvalInstruction).getExpr())
         ) and
         trg.getField() = f and
-        super.getRhs() = rhs.asInstruction()
+        super.getRhs() = rhs
       )
     }
 
     /**
      * Holds if this node sets the value of element `index` on `base` (or its implicit dereference)
      * to `rhs`.
      *
-     * For example, for the assignment `xs[i] = v`, `base` is either the data-flow node
-     * corresponding to `xs` or (if `xs` is a pointer) the data-flow node corresponding to the
-     * implicit dereference `*xs`, `index` is the data-flow node corresponding to `i`, and `rhs`
-     * is the data-flow node corresponding to `base`.
+     * For example, for the assignment `xs[i] = v`, `base` is the post-update node of the data-flow
+     * node corresponding to `xs` or (if `xs` is a pointer) the implicit dereference `*xs`, `index`
+     * is the data-flow node corresponding to `i`, and `rhs` is the data-flow node corresponding to
+     * `base`. If this `WriteNode` corresponds to the initialization of an array/slice/map then
+     * there is no need for a post-update node and `base` is the array/slice/map literal being
+     * initialized.
      */
     predicate writesElement(DataFlow::Node base, DataFlow::Node index, DataFlow::Node rhs) {
+      exists(DataFlow::Node b | this.writesElementPreUpdate(b, index, rhs) |
+        this.isInitialization() and base = b
+        or
+        not this.isInitialization() and
+        b = base.(DataFlow::PostUpdateNode).getPreUpdateNode()
+      )
+    }
+
+    /**
+     * Holds if this node sets the value of element `index` on `base` (or its implicit dereference)
+     * to `rhs`.
+     *
+     * For example, for the assignment `xs[i] = v`, `base` is the post-update node of the data-flow
+     * node corresponding to `xs` or (if `xs` is a pointer) the implicit dereference `*xs`, `index`
+     * is the data-flow node corresponding to `i`, and `rhs` is the data-flow node corresponding to
+     * `base`. If this `WriteNode` corresponds to the initialization of an array/slice/map then
+     * there is no need for a post-update node and `base` is the array/slice/map literal being
+     * initialized.
+     */
+    predicate writesElementPreUpdate(DataFlow::Node base, DataFlow::Node index, DataFlow::Node rhs) {
+      this.writesElementInsn(base.asInstruction(), index.asInstruction(), rhs.asInstruction())
+    }
+
+    private predicate writesElementInsn(
+      IR::Instruction base, IR::Instruction index, IR::Instruction rhs
+    ) {
       exists(IR::ElementTarget trg | trg = super.getLhs() |
         (
-          trg.getBase() = base.asInstruction() or
-          trg.getBase() = MkImplicitDeref(base.asExpr())
+          trg.getBase() = base or
+          trg.getBase() = MkImplicitDeref(base.(IR::EvalInstruction).getExpr())
         ) and
-        trg.getIndex() = index.asInstruction() and
-        super.getRhs() = rhs.asInstruction()
+        trg.getIndex() = index and
+        super.getRhs() = rhs
       )
     }
 
+    /**
+     * DEPRECATED: Use the disjunct of `writesElement` and `writesField`, or `writesFieldPreUpdate`
+     * and `writesElementPreUpdate`, instead.
+     *
+     * Holds if this node sets any field or element of `base` (or its implicit dereference) to
+     * `rhs`, where `base` represents the pre-update value.
+     */
+    deprecated predicate writesComponent(DataFlow::Node base, DataFlow::Node rhs) {
+      this.writesElementPreUpdate(base, _, rhs) or this.writesFieldPreUpdate(base, _, rhs)
+    }
+
     /**
      * Holds if this node sets any field or element of `base` to `rhs`.
      */
-    predicate writesComponent(DataFlow::Node base, DataFlow::Node rhs) {
-      this.writesElement(base, _, rhs) or this.writesField(base, _, rhs)
+    predicate writesComponentInstruction(IR::Instruction base, IR::Instruction rhs) {
+      this.writesElementInsn(base, _, rhs) or this.writesFieldInsn(base, _, rhs)
     }
   }
 

go/ql/lib/semmle/go/controlflow/IR.qll

@@ -430,18 +430,25 @@ module IR {
    */
   class WriteInstruction extends Instruction {
     WriteTarget lhs;
+    Boolean initialization;
 
     WriteInstruction() {
-      lhs = MkLhs(this, _)
-      or
-      lhs = MkLiteralElementTarget(this)
+      (
+        lhs = MkLhs(this, _)
+        or
+        lhs = MkResultWriteTarget(this)
+      ) and
+      initialization = false
       or
-      lhs = MkResultWriteTarget(this)
+      lhs = MkLiteralElementTarget(this) and initialization = true
     }
 
     /** Gets the target to which this instruction writes. */
     WriteTarget getLhs() { result = lhs }
 
+    /** Holds if this instruction initializes a literal. */
+    predicate isInitialization() { initialization = true }
+
     /** Gets the instruction computing the value this instruction writes. */
     Instruction getRhs() { none() }
 

go/ql/lib/semmle/go/dataflow/SSA.qll

@@ -166,6 +166,13 @@ class SsaDefinition extends TSsaDefinition {
   ) {
     this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
   }
+
+  /**
+   * Gets the first instruction that the value of this `SsaDefinition` can
+   * reach without passing through any other instructions, but possibly through
+   * phi nodes.
+   */
+  IR::Instruction getAFirstUse() { firstUse(this, result) }
 }
 
 /**
@@ -410,3 +417,12 @@ DataFlow::Node getASimilarReadNode(DataFlow::Node node) {
     result = readFields.similar().getAUse()
   )
 }
+
+/**
+ * Gets an instruction such that  `pred` and `result` form an adjacent
+ * use-use-pair of the same`SsaSourceVariable`, that is, the value read in
+ * `pred` can reach `result` without passing through any other use or any SSA
+ * definition of the variable except for phi nodes and uncertain implicit
+ * updates.
+ */
+IR::Instruction getAnAdjacentUse(IR::Instruction pred) { adjacentUseUse(pred, result) }

go/ql/lib/semmle/go/dataflow/SsaImpl.qll

@@ -199,6 +199,8 @@ private module Internal {
   /**
    * Holds if the `i`th node of `bb` is a use or an SSA definition of variable `v`, with
    * `k` indicating whether it is the former or the latter.
+   *
+   * Note this includes phi nodes, whereas `ref` above only includes explicit writes and captures.
    */
   private predicate ssaRef(ReachableBasicBlock bb, int i, SsaSourceVariable v, RefKind k) {
     useAt(bb, i, v) and k = ReadRef()
@@ -290,6 +292,172 @@ private module Internal {
     or
     rewindReads(bb, i, v) = 1 and result = getDefReachingEndOf(bb.getImmediateDominator(), v)
   }
+
+  private module AdjacentUsesImpl {
+    /** Holds if `v` is defined or used in `b`. */
+    private predicate varOccursInBlock(SsaSourceVariable v, ReachableBasicBlock b) {
+      ssaRef(b, _, v, _)
+    }
+
+    /** Holds if `v` occurs in `b` or one of `b`'s transitive successors. */
+    private predicate blockPrecedesVar(SsaSourceVariable v, ReachableBasicBlock b) {
+      varOccursInBlock(v, b)
+      or
+      exists(getDefReachingEndOf(b, v))
+    }
+
+    /**
+     * Holds if `v` occurs in `b1` and `b2` is one of `b1`'s successors.
+     *
+     * Factored out of `varBlockReaches` to force join order compared to the larger
+     * set `blockPrecedesVar(v, b2)`.
+     */
+    pragma[noinline]
+    private predicate varBlockReachesBaseCand(
+      SsaSourceVariable v, ReachableBasicBlock b1, ReachableBasicBlock b2
+    ) {
+      varOccursInBlock(v, b1) and
+      b2 = b1.getASuccessor()
+    }
+
+    /**
+     * Holds if `b2` is a transitive successor of `b1` and `v` occurs in `b1` and
+     * in `b2` or one of its transitive successors but not in any block on the path
+     * between `b1` and `b2`. Unlike `varBlockReaches` this may include blocks `b2`
+     * where `v` is dead.
+     *
+     * Factored out of `varBlockReaches` to force join order compared to the larger
+     * set `blockPrecedesVar(v, b2)`.
+     */
+    pragma[noinline]
+    private predicate varBlockReachesRecCand(
+      SsaSourceVariable v, ReachableBasicBlock b1, ReachableBasicBlock mid, ReachableBasicBlock b2
+    ) {
+      varBlockReaches(v, b1, mid) and
+      not varOccursInBlock(v, mid) and
+      b2 = mid.getASuccessor()
+    }
+
+    /**
+     * Holds if `b2` is a transitive successor of `b1` and `v` occurs in `b1` and
+     * in `b2` or one of its transitive successors but not in any block on the path
+     * between `b1` and `b2`.
+     */
+    private predicate varBlockReaches(
+      SsaSourceVariable v, ReachableBasicBlock b1, ReachableBasicBlock b2
+    ) {
+      varBlockReachesBaseCand(v, b1, b2) and
+      blockPrecedesVar(v, b2)
+      or
+      varBlockReachesRecCand(v, b1, _, b2) and
+      blockPrecedesVar(v, b2)
+    }
+
+    /**
+     * Holds if `b2` is a transitive successor of `b1` and `v` occurs in `b1` and
+     * `b2` but not in any block on the path between `b1` and `b2`.
+     */
+    private predicate varBlockStep(
+      SsaSourceVariable v, ReachableBasicBlock b1, ReachableBasicBlock b2
+    ) {
+      varBlockReaches(v, b1, b2) and
+      varOccursInBlock(v, b2)
+    }
+
+    /**
+     * Gets the maximum rank among all SSA references to `v` in basic block `bb`.
+     */
+    private int maxSsaRefRank(ReachableBasicBlock bb, SsaSourceVariable v) {
+      result = max(ssaRefRank(bb, _, v, _))
+    }
+
+    /**
+     * Holds if `v` occurs at index `i1` in `b1` and at index `i2` in `b2` and
+     * there is a path between them without any occurrence of `v`.
+     */
+    pragma[nomagic]
+    predicate adjacentVarRefs(
+      SsaSourceVariable v, ReachableBasicBlock b1, int i1, ReachableBasicBlock b2, int i2
+    ) {
+      exists(int rankix |
+        b1 = b2 and
+        ssaRefRank(b1, i1, v, _) = rankix and
+        ssaRefRank(b2, i2, v, _) = rankix + 1
+      )
+      or
+      maxSsaRefRank(b1, v) = ssaRefRank(b1, i1, v, _) and
+      varBlockStep(v, b1, b2) and
+      ssaRefRank(b2, i2, v, _) = 1
+    }
+
+    predicate variableUse(SsaSourceVariable v, IR::Instruction use, ReachableBasicBlock bb, int i) {
+      bb.getNode(i) = use and
+      exists(SsaVariable sv |
+        sv.getSourceVariable() = v and
+        use = sv.getAUse()
+      )
+    }
+  }
+
+  private import AdjacentUsesImpl
+
+  /**
+   * Holds if the value defined at `def` can reach `use` without passing through
+   * any other uses, but possibly through phi nodes.
+   */
+  cached
+  predicate firstUse(SsaDefinition def, IR::Instruction use) {
+    exists(SsaSourceVariable v, ReachableBasicBlock b1, int i1, ReachableBasicBlock b2, int i2 |
+      adjacentVarRefs(v, b1, i1, b2, i2) and
+      def.definesAt(b1, i1, v) and
+      variableUse(v, use, b2, i2)
+    )
+    or
+    exists(
+      SsaSourceVariable v, SsaPhiNode redef, ReachableBasicBlock b1, int i1, ReachableBasicBlock b2,
+      int i2
+    |
+      adjacentVarRefs(v, b1, i1, b2, i2) and
+      def.definesAt(b1, i1, v) and
+      redef.definesAt(b2, i2, v) and
+      firstUse(redef, use)
+    )
+  }
+
+  /**
+   * Holds if `use1` and `use2` form an adjacent use-use-pair of the same SSA
+   * variable, that is, the value read in `use1` can reach `use2` without passing
+   * through any other use or any SSA definition of the variable.
+   */
+  cached
+  predicate adjacentUseUseSameVar(IR::Instruction use1, IR::Instruction use2) {
+    exists(SsaSourceVariable v, ReachableBasicBlock b1, int i1, ReachableBasicBlock b2, int i2 |
+      adjacentVarRefs(v, b1, i1, b2, i2) and
+      variableUse(v, use1, b1, i1) and
+      variableUse(v, use2, b2, i2)
+    )
+  }
+
+  /**
+   * Holds if `use1` and `use2` form an adjacent use-use-pair of the same
+   * `SsaSourceVariable`, that is, the value read in `use1` can reach `use2`
+   * without passing through any other use or any SSA definition of the variable
+   * except for phi nodes and uncertain implicit updates.
+   */
+  cached
+  predicate adjacentUseUse(IR::Instruction use1, IR::Instruction use2) {
+    adjacentUseUseSameVar(use1, use2)
+    or
+    exists(
+      SsaSourceVariable v, SsaPhiNode def, ReachableBasicBlock b1, int i1, ReachableBasicBlock b2,
+      int i2
+    |
+      adjacentVarRefs(v, b1, i1, b2, i2) and
+      variableUse(v, use1, b1, i1) and
+      def.definesAt(b2, i2, v) and
+      firstUse(def, use2)
+    )
+  }
 }
 
 import Internal