Java: Fix RangeAnalysis/ModulusAnalysis.

aschackmull · aschackmull · commit 1e9dcea88bc1 · 2026-02-23T15:09:51.000+01:00
diff --git a/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll b/java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll
@@ -220,6 +220,8 @@ module Sem implements Semantic<Location> {
 
   int getBlockId1(BasicBlock bb) { idOf(bb, result) }
 
+  string getBlockId2(BasicBlock bb) { bb.getFirstNode().getIdTag() = result }
+
   class Guard extends G::Guards_v2::Guard {
     Expr asExpr() { result = this }
   }
diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll b/java/ql/lib/semmle/code/java/dataflow/internal/rangeanalysis/SsaReadPositionSpecific.qll
@@ -23,7 +23,9 @@ private predicate idOfAst(BB::ExprParent x, int y) = equivalenceRelation(id/2)(x
 
 private predicate idOf(BasicBlock x, int y) { idOfAst(x.getFirstNode().getAstNode(), y) }
 
-private int getId(BasicBlock bb) { idOf(bb, result) }
+private int getId1(BasicBlock bb) { idOf(bb, result) }
+
+private string getId2(BasicBlock bb) { bb.getFirstNode().getIdTag() = result }
 
 /**
  * Declarations to be exposed to users of SsaReadPositionCommon
@@ -39,7 +41,7 @@ module Public {
       rank[r](SsaReadPositionPhiInputEdge e |
         e.phiInput(phi, _)
       |
-        e order by getId(e.getOrigBlock())
+        e order by getId1(e.getOrigBlock()), getId2(e.getOrigBlock())
       )
   }
 }
diff --git a/shared/controlflow/codeql/controlflow/ControlFlowGraph.qll b/shared/controlflow/codeql/controlflow/ControlFlowGraph.qll
@@ -706,6 +706,14 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
        */
       abstract AstNode getAstNode();
 
+      /**
+       * INTERNAL: Do not use.
+       *
+       * Gets a tag such that the pair `(getAstNode(), getIdTag())` uniquely
+       * identifies this node.
+       */
+      abstract string getIdTag();
+
       /** Gets a textual representation of this node. */
       abstract string toString();
 
@@ -727,6 +735,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = n }
 
+      override string getIdTag() { result = "before" }
+
       override string toString() {
         if postOrInOrder(n) then result = "Before " + n.toString() else result = n.toString()
       }
@@ -739,6 +749,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = n }
 
+      override string getIdTag() { result = "ast" }
+
       override string toString() { result = n.toString() }
     }
 
@@ -750,6 +762,12 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = n }
 
+      override string getIdTag() {
+        t.getValue() = true and result = "after-true"
+        or
+        t.getValue() = false and result = "after-false"
+      }
+
       override string toString() { result = "After " + n.toString() + " [" + t.toString() + "]" }
     }
 
@@ -760,6 +778,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = n }
 
+      override string getIdTag() { result = "after" }
+
       override string toString() { result = "After " + n.toString() }
     }
 
@@ -773,6 +793,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       NormalSuccessor getSuccessorType() { additionalNode(n, tag, result) }
 
+      override string getIdTag() { result = "add. " + tag }
+
       override string toString() { result = tag + " " + n.toString() }
     }
 
@@ -785,6 +807,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = c }
 
+      override string getIdTag() { result = "entry" }
+
       override string toString() { result = "Entry" }
     }
 
@@ -799,6 +823,12 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = c }
 
+      override string getIdTag() {
+        normal = true and result = "exit-normal"
+        or
+        normal = false and result = "exit-exc"
+      }
+
       override string toString() {
         normal = true and result = "Normal Exit"
         or
@@ -826,6 +856,8 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
 
       override AstNode getAstNode() { result = c }
 
+      override string getIdTag() { result = "exit" }
+
       override string toString() { result = "Exit" }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -220,6 +220,8 @@ module Sem implements Semantic<Location> {`
`220`	`220`
`221`	`221`	`int getBlockId1(BasicBlock bb) { idOf(bb, result) }`
`222`	`222`
	`223`	`+ string getBlockId2(BasicBlock bb) { bb.getFirstNode().getIdTag() = result }`
	`224`	`+`
`223`	`225`	`class Guard extends G::Guards_v2::Guard {`
`224`	`226`	`Expr asExpr() { result = this }`
`225`	`227`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,9 @@ private predicate idOfAst(BB::ExprParent x, int y) = equivalenceRelation(id/2)(x`
`23`	`23`
`24`	`24`	`private predicate idOf(BasicBlock x, int y) { idOfAst(x.getFirstNode().getAstNode(), y) }`
`25`	`25`
`26`		`-private int getId(BasicBlock bb) { idOf(bb, result) }`
	`26`	`+private int getId1(BasicBlock bb) { idOf(bb, result) }`
	`27`	`+`
	`28`	`+private string getId2(BasicBlock bb) { bb.getFirstNode().getIdTag() = result }`
`27`	`29`
`28`	`30`	`/**`
`29`	`31`	`* Declarations to be exposed to users of SsaReadPositionCommon`
`@@ -39,7 +41,7 @@ module Public {`
`39`	`41`	`rank[r](SsaReadPositionPhiInputEdge e \|`
`40`	`42`	`e.phiInput(phi, _)`
`41`	`43`	`\|`
`42`		`- e order by getId(e.getOrigBlock())`
	`44`	`+ e order by getId1(e.getOrigBlock()), getId2(e.getOrigBlock())`
`43`	`45`	`)`
`44`	`46`	`}`
`45`	`47`	`}`