fold implicit-one operand into incdec-rhs instruction

Increment/decrement statements previously produced three control-flow nodes: an 'implicit-one' node for the constant 1, an 'incdec-rhs' node for the 'operand + 1' value, and the write node. The implicit-one node existed only to serve as the right operand of the increment's binary operation model. Remove the implicit-one node, modelling the implicit constant 1 directly on the incdec-rhs instruction. This saves one CFG node per inc/dec. The inc/dec disjunct of BinaryOperationNode is dropped as well; it only fed string-concatenation taint (impossible for ++/--) and GVN of a synthetic node, so no data-flow results are lost.
2026-07-31 23:42:59 +02:00 · 2026-07-10 01:00:55 +01:00
parent c08b03fa97
commit 2bde73cd2d
5 changed files with 12 additions and 51 deletions
--- a/go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll
+++ b/go/ql/lib/semmle/go/controlflow/ControlFlowGraphShared.qll
@@ -537,9 +537,7 @@ module GoCfg {
          tag = "zero-init:" + i.toString()
        )
        or
-        // Increment/decrement implicit operations
-        n instanceof Go::IncDecStmt and tag = "implicit-one"
-        or
+        // Increment/decrement implicit right-hand side (the `operand + 1` value)
        n instanceof Go::IncDecStmt and tag = "incdec-rhs"
        or
        // Result write nodes in return statements
@@ -1184,19 +1182,20 @@ module GoCfg {
    }

    /**
-     * Increment/decrement: operand → implicit-one → incdec-rhs → In(stmt)
+     * Increment/decrement: operand → incdec-rhs → In(stmt)
     * (IncDecStmt is in postOrInOrder, so In(stmt) is its evaluation point)
+     *
+     * The implicit constant `1` operand of the `operand + 1` computation is
+     * modelled directly on the `incdec-rhs` instruction rather than as its own
+     * control-flow node.
     */
    private predicate incDecStep(PreControlFlowNode n1, PreControlFlowNode n2) {
      exists(Go::IncDecStmt s |
        // Before → Before operand
        n1.isBefore(s) and n2.isBefore(s.getOperand())
        or
-        // After operand → implicit-one
-        n1.isAfter(s.getOperand()) and n2.isAdditional(s, "implicit-one")
-        or
-        // implicit-one → incdec-rhs
-        n1.isAdditional(s, "implicit-one") and n2.isAdditional(s, "incdec-rhs")
+        // After operand → incdec-rhs
+        n1.isAfter(s.getOperand()) and n2.isAdditional(s, "incdec-rhs")
        or
        // incdec-rhs → In(stmt) (the assignment itself)
        n1.isAdditional(s, "incdec-rhs") and n2.isIn(s)
--- a/go/ql/lib/semmle/go/controlflow/IR.qll
+++ b/go/ql/lib/semmle/go/controlflow/IR.qll
@@ -183,8 +183,6 @@ module IR {
      this instanceof EvalIncDecRhsInstruction and
      result = "right-hand side of increment/decrement"
      or
-      this instanceof EvalImplicitOneInstruction and result = "implicit 1"
-      or
      this instanceof ReturnInstruction and result = "return"
      or
      this instanceof WriteResultInstruction and result = "result write"
@@ -948,28 +946,6 @@ module IR {
    override ControlFlow::Root getRoot() { result.isRootOf(ids) }
  }

-  /** An instruction computing the implicit operand `1` in an increment or decrement statement. */
-  class EvalImplicitOneInstruction extends Instruction {
-    IncDecStmt ids;
-
-    EvalImplicitOneInstruction() { this.isAdditional(ids, "implicit-one") }
-
-    /** Gets the corresponding increment or decrement statement. */
-    IncDecStmt getStmt() { result = ids }
-
-    override Type getResultType() { result = ids.getOperand().getType() }
-
-    override ControlFlow::Root getRoot() { result.isRootOf(ids) }
-
-    override int getIntValue() { result = 1 }
-
-    override string getExactValue() { result = "1" }
-
-    override predicate isConst() { any() }
-
-    override predicate isPlatformIndependentConstant() { any() }
-  }
-
  /** An instruction corresponding to a return from a function. */
  class ReturnInstruction extends Instruction {
    ReturnStmt ret;
--- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll
+++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll
@@ -1127,15 +1127,6 @@ module Public {
        right = DataFlow::exprNode(assgn.getRhs()) and
        op = o.substring(0, o.length() - 1)
      )
-      or
-      exists(IR::EvalIncDecRhsInstruction rhs, IncDecStmt ids |
-        rhs = this.asInstruction() and ids = rhs.getStmt()
-      |
-        left = DataFlow::exprNode(ids.getOperand()) and
-        right =
-          DataFlow::instructionNode(any(IR::EvalImplicitOneInstruction one | one.getStmt() = ids)) and
-        op = ids.getOperator().charAt(0)
-      )
    }

    /** Holds if this operation may have observable side effects. */
--- a/go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected
+++ b/go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected
@@ -1361,12 +1361,11 @@
 | exprs.go:50:22:50:23 | After xs | exprs.go:50:18:50:24 | call to len |
 | exprs.go:50:22:50:23 | Before xs | exprs.go:50:22:50:23 | xs |
 | exprs.go:50:22:50:23 | xs | exprs.go:50:22:50:23 | After xs |
-| exprs.go:50:27:50:27 | After i | exprs.go:50:27:50:29 | implicit-one increment statement |
+| exprs.go:50:27:50:27 | After i | exprs.go:50:27:50:29 | incdec-rhs increment statement |
 | exprs.go:50:27:50:27 | Before i | exprs.go:50:27:50:27 | i |
 | exprs.go:50:27:50:27 | i | exprs.go:50:27:50:27 | After i |
 | exprs.go:50:27:50:29 | After increment statement | exprs.go:50:14:50:24 | Before ...<... |
 | exprs.go:50:27:50:29 | Before increment statement | exprs.go:50:27:50:27 | Before i |
-| exprs.go:50:27:50:29 | implicit-one increment statement | exprs.go:50:27:50:29 | incdec-rhs increment statement |
 | exprs.go:50:27:50:29 | incdec-rhs increment statement | exprs.go:50:27:50:29 | increment statement |
 | exprs.go:50:27:50:29 | increment statement | exprs.go:50:27:50:29 | After increment statement |
 | exprs.go:50:31:52:2 | After block statement | exprs.go:50:2:52:2 | [LoopHeader] for statement |
@@ -2338,12 +2337,11 @@
 | main.go:74:11:74:11 | 0 | main.go:74:11:74:11 | After 0 |
 | main.go:74:11:74:11 | After 0 | main.go:74:6:74:11 | assign:0 ... := ... |
 | main.go:74:11:74:11 | Before 0 | main.go:74:11:74:11 | 0 |
-| main.go:74:16:74:16 | After i | main.go:74:16:74:18 | implicit-one increment statement |
+| main.go:74:16:74:16 | After i | main.go:74:16:74:18 | incdec-rhs increment statement |
 | main.go:74:16:74:16 | Before i | main.go:74:16:74:16 | i |
 | main.go:74:16:74:16 | i | main.go:74:16:74:16 | After i |
 | main.go:74:16:74:18 | After increment statement | main.go:74:20:79:2 | block statement |
 | main.go:74:16:74:18 | Before increment statement | main.go:74:16:74:16 | Before i |
-| main.go:74:16:74:18 | implicit-one increment statement | main.go:74:16:74:18 | incdec-rhs increment statement |
 | main.go:74:16:74:18 | incdec-rhs increment statement | main.go:74:16:74:18 | increment statement |
 | main.go:74:16:74:18 | increment statement | main.go:74:16:74:18 | After increment statement |
 | main.go:74:20:79:2 | After block statement | main.go:74:2:79:2 | [LoopHeader] for statement |
@@ -2395,12 +2393,11 @@
 | main.go:83:11:83:11 | 0 | main.go:83:11:83:11 | After 0 |
 | main.go:83:11:83:11 | After 0 | main.go:83:6:83:11 | assign:0 ... := ... |
 | main.go:83:11:83:11 | Before 0 | main.go:83:11:83:11 | 0 |
-| main.go:83:16:83:16 | After i | main.go:83:16:83:18 | implicit-one increment statement |
+| main.go:83:16:83:16 | After i | main.go:83:16:83:18 | incdec-rhs increment statement |
 | main.go:83:16:83:16 | Before i | main.go:83:16:83:16 | i |
 | main.go:83:16:83:16 | i | main.go:83:16:83:16 | After i |
 | main.go:83:16:83:18 | After increment statement | main.go:83:20:88:2 | block statement |
 | main.go:83:16:83:18 | Before increment statement | main.go:83:16:83:16 | Before i |
-| main.go:83:16:83:18 | implicit-one increment statement | main.go:83:16:83:18 | incdec-rhs increment statement |
 | main.go:83:16:83:18 | incdec-rhs increment statement | main.go:83:16:83:18 | increment statement |
 | main.go:83:16:83:18 | increment statement | main.go:83:16:83:18 | After increment statement |
 | main.go:83:20:88:2 | After block statement | main.go:83:2:88:2 | [LoopHeader] for statement |
@@ -3297,12 +3294,11 @@
 | stmts.go:24:19:24:20 | 10 | stmts.go:24:19:24:20 | After 10 |
 | stmts.go:24:19:24:20 | After 10 | stmts.go:24:15:24:20 | ...<... |
 | stmts.go:24:19:24:20 | Before 10 | stmts.go:24:19:24:20 | 10 |
-| stmts.go:24:23:24:23 | After i | stmts.go:24:23:24:25 | implicit-one increment statement |
+| stmts.go:24:23:24:23 | After i | stmts.go:24:23:24:25 | incdec-rhs increment statement |
 | stmts.go:24:23:24:23 | Before i | stmts.go:24:23:24:23 | i |
 | stmts.go:24:23:24:23 | i | stmts.go:24:23:24:23 | After i |
 | stmts.go:24:23:24:25 | After increment statement | stmts.go:24:15:24:20 | Before ...<... |
 | stmts.go:24:23:24:25 | Before increment statement | stmts.go:24:23:24:23 | Before i |
-| stmts.go:24:23:24:25 | implicit-one increment statement | stmts.go:24:23:24:25 | incdec-rhs increment statement |
 | stmts.go:24:23:24:25 | incdec-rhs increment statement | stmts.go:24:23:24:25 | increment statement |
 | stmts.go:24:23:24:25 | increment statement | stmts.go:24:23:24:25 | After increment statement |
 | stmts.go:24:27:36:3 | block statement | stmts.go:25:4:35:4 | if statement |
--- a/go/ql/test/library-tests/semmle/go/dataflow/GlobalValueNumbering/GlobalValueNumber.expected
+++ b/go/ql/test/library-tests/semmle/go/dataflow/GlobalValueNumbering/GlobalValueNumber.expected
@@ -1,4 +1,3 @@
-| main.go:6:2:6:5 | implicit-one increment statement | main.go:14:7:14:7 | 1 |
 | main.go:10:2:10:7 | SSA def(x) | main.go:10:7:10:7 | 0 |
 | main.go:10:7:10:7 | 0 | main.go:10:7:10:7 | 0 |
 | main.go:11:6:11:10 | SSA def(y) | main.go:10:7:10:7 | 0 |