Merge pull request #20929 from owen-mc/go/fix-data-flow-consistency-checks

Go: fix small issues highlighted by data flow consistency checks

Author: owen-mc

go/ql/lib/change-notes/2025-11-26-unexpected-frontend-errors-query-moved.md

@@ -1,4 +1,4 @@
 ---
 category: breaking
 ---
-* The query `go/unexpected-frontend-error` has been moved from the `codeql/go-queries` query  to the `codeql-go-consistency-queries` query pack.
+* The query `go/unexpected-frontend-error` has been moved from the `codeql/go-queries` query to the `codeql-go-consistency-queries` query pack.

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

@@ -1588,4 +1588,9 @@ module IR {
    * in a field/method access, element access, or slice expression.
    */
   EvalImplicitDerefInstruction implicitDerefInstruction(Expr e) { result = MkImplicitDeref(e) }
+
+  /** Gets the base of `insn`, if `insn` is an implicit field read. */
+  Instruction lookThroughImplicitFieldRead(Instruction insn) {
+    result = insn.(ImplicitFieldReadInstruction).getBaseInstruction()
+  }
 }

go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -838,7 +838,11 @@ module Public {
       exists(IR::MethodReadInstruction mri |
         ce.getTarget() instanceof Method and
         mri = IR::evalExprInstruction(ce.getCalleeExpr()) and
-        insn = mri.getReceiver()
+        // If a.x is reading a promoted field, and it's equivalent to a.b.c.x,
+        // then mri.getReceiver() will give us the implicit field read a.b.c
+        // and we want to have post-update nodes for a, the implicit field
+        // read a.b and the implicit field read a.b.c.
+        insn = IR::lookThroughImplicitFieldRead*(mri.getReceiver())
       )
     ) and
     mutableType(insn.getResultType())

go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll

@@ -89,7 +89,8 @@ predicate basicLocalFlowStep(Node nodeFrom, Node nodeTo) {
       nodeFrom = instructionNode(pred) or
       nodeFrom.(PostUpdateNode).getPreUpdateNode() = instructionNode(pred)
     ) and
-    nodeTo = instructionNode(succ)
+    nodeTo = instructionNode(succ) and
+    nodeTo != nodeFrom
   )
   or
   // GlobalFunctionNode -> use

go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll

@@ -397,17 +397,13 @@ module SourceSinkInterpretationInput implements
   }
 
   private DataFlow::Node skipImplicitFieldReads(DataFlow::Node n) {
-    not exists(lookThroughImplicitFieldRead(n)) and result = n
+    not exists(IR::lookThroughImplicitFieldRead(n.asInstruction())) and result = n
     or
-    result = skipImplicitFieldReads(lookThroughImplicitFieldRead(n))
-  }
-
-  private DataFlow::Node lookThroughImplicitFieldRead(DataFlow::Node n) {
-    result.asInstruction() =
-      n.(DataFlow::InstructionNode)
-          .asInstruction()
-          .(IR::ImplicitFieldReadInstruction)
-          .getBaseInstruction()
+    exists(DataFlow::Node mid |
+      mid.asInstruction() = IR::lookThroughImplicitFieldRead(n.asInstruction())
+    |
+      result = skipImplicitFieldReads(mid)
+    )
   }
 
   /** Provides additional sink specification logic. */

go/ql/test/example-tests/snippets/CONSISTENCY/DataFlowConsistency.expected

@@ -34,25 +34,3 @@ reverseRead
 | test.go:92:19:92:25 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
 | test.go:93:5:93:11 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
 | test.go:94:9:94:15 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
-identityLocalStep
-| test.go:114:37:114:38 | rc | Node steps to itself |
-| test.go:198:27:198:29 | out | Node steps to itself |
-| test.go:224:27:224:29 | out | Node steps to itself |
-| test.go:249:27:249:29 | out | Node steps to itself |
-| test.go:274:27:274:29 | out | Node steps to itself |
-| test.go:299:27:299:29 | out | Node steps to itself |
-| test.go:324:26:324:28 | out | Node steps to itself |
-| test.go:349:26:349:28 | out | Node steps to itself |
-| test.go:375:28:375:30 | out | Node steps to itself |
-| test.go:403:28:403:30 | out | Node steps to itself |
-| test.go:431:24:431:26 | out | Node steps to itself |
-| test.go:463:26:463:28 | out | Node steps to itself |
-| test.go:490:26:490:28 | out | Node steps to itself |
-| test.go:517:27:517:29 | out | Node steps to itself |
-| test.go:546:26:546:28 | out | Node steps to itself |
-| test.go:571:26:571:28 | out | Node steps to itself |
-| test.go:601:24:601:26 | out | Node steps to itself |
-| test.go:614:15:614:21 | tarRead | Node steps to itself |
-| test.go:622:3:622:7 | files | Node steps to itself |
-| test.go:637:10:637:16 | tarRead | Node steps to itself |
-| test.go:647:10:647:16 | tarRead | Node steps to itself |

go/ql/test/experimental/CWE-400/CONSISTENCY/DataFlowConsistency.expected

@@ -13,6 +13,3 @@ reverseRead
 | CorsMisconfiguration.go:170:14:170:16 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
 | CorsMisconfiguration.go:194:17:194:19 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
 | CorsMisconfiguration.go:206:14:206:16 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
-identityLocalStep
-| CorsMisconfiguration.go:208:13:208:18 | origin | Node steps to itself |
-| CorsMisconfiguration.go:235:6:235:8 | try | Node steps to itself |

go/ql/test/experimental/CWE-522-DecompressionBombs/CONSISTENCY/DataFlowConsistency.expected

@@ -1,11 +1,5 @@
 reverseRead
-| pkg1/embedding.go:56:29:56:39 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
-| pkg1/embedding.go:57:33:57:46 | implicit dereference | Origin of readStep is missing a PostUpdateNode. |
-| pkg1/embedding.go:58:14:58:22 | implicit read of field embedder | Origin of readStep is missing a PostUpdateNode. |
-| pkg1/embedding.go:58:31:58:42 | implicit read of field embedder | Origin of readStep is missing a PostUpdateNode. |
 | pkg1/tst.go:43:6:43:6 | t | Origin of readStep is missing a PostUpdateNode. |
 | pkg1/tst.go:46:6:46:6 | t | Origin of readStep is missing a PostUpdateNode. |
-| pkg1/tst.go:50:2:50:2 | t | Origin of readStep is missing a PostUpdateNode. |
 | pkg1/tst.go:53:6:53:7 | t2 | Origin of readStep is missing a PostUpdateNode. |
 | pkg1/tst.go:55:6:55:7 | t2 | Origin of readStep is missing a PostUpdateNode. |
-| promoted.go:18:6:18:6 | t | Origin of readStep is missing a PostUpdateNode. |