Java: IPA the CFG (second try)

java/ql/consistency-queries/cfgDeadEnds.ql

@@ -1,7 +1,6 @@
 import java
-import semmle.code.java.ControlFlowGraph
 
-predicate shouldBeDeadEnd(ControlFlowNode n) {
+predicate shouldBeDeadEnd(ExprParent n) {
   n instanceof BreakStmt and n.getFile().isKotlinSourceFile() // TODO
   or
   n instanceof Interface // TODO
@@ -55,8 +54,11 @@ predicate shouldBeDeadEnd(ControlFlowNode n) {
   n = any(ConstCase c).getValue(_) // TODO
 }
 
-from ControlFlowNode n, string s
+from ControlFlowNode n, ExprParent astnode, string s
 where
-  // TODO: exists(n.getASuccessor()) and shouldBeDeadEnd(n) and s = "expected dead end"
-  not exists(n.getASuccessor()) and not shouldBeDeadEnd(n) and s = "unexpected dead end"
-select n, n.getPrimaryQlClasses(), s
+  astnode = n.getAstNode() and
+  // TODO: exists(n.getASuccessor()) and shouldBeDeadEnd(n.getAstNode()) and s = "expected dead end"
+  not exists(n.getASuccessor()) and
+  not shouldBeDeadEnd(astnode) and
+  s = "unexpected dead end"
+select n, astnode.getPrimaryQlClasses(), s

java/ql/lib/semmle/code/java/Expr.qll

@@ -61,10 +61,10 @@ class Expr extends ExprParent, @expr {
   Expr getAChildExpr() { exprs(result, _, _, this, _) }
 
   /** Gets the basic block in which this expression occurs, if any. */
-  BasicBlock getBasicBlock() { result.getANode() = this }
+  BasicBlock getBasicBlock() { result.getANode().asExpr() = this }
 
   /** Gets the `ControlFlowNode` corresponding to this expression. */
-  ControlFlowNode getControlFlowNode() { result = this }
+  ControlFlowNode getControlFlowNode() { result.asExpr() = this }
 
   /** This statement's Halstead ID (used to compute Halstead metrics). */
   string getHalsteadID() { result = this.toString() }

java/ql/lib/semmle/code/java/Statement.qll

@@ -45,10 +45,10 @@ class Stmt extends StmtParent, ExprParent, @stmt {
   Stmt getAChild() { result.getParent() = this }
 
   /** Gets the basic block in which this statement occurs. */
-  BasicBlock getBasicBlock() { result.getANode() = this }
+  BasicBlock getBasicBlock() { result.getANode().asStmt() = this }
 
   /** Gets the `ControlFlowNode` corresponding to this statement. */
-  ControlFlowNode getControlFlowNode() { result = this }
+  ControlFlowNode getControlFlowNode() { result.asStmt() = this }
 
   /** Cast this statement to a class that provides access to metrics information. */
   MetricStmt getMetrics() { result = this }

java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll

@@ -66,3 +66,7 @@
   /** Holds if this basic block post-dominates `node`. (This is reflexive.) */
   predicate bbPostDominates(BasicBlock node) { bbPostDominates(this, node) }
 }
+
+class ExitBlock extends BasicBlock {
+  ExitBlock() { this.getLastNode() instanceof ControlFlow::ExitNode }
+}

java/ql/lib/semmle/code/java/controlflow/Dominance.qll

@@ -9,13 +9,15 @@ import java
  */
 
 /** Entry points for control-flow. */
-private predicate flowEntry(Stmt entry) {
-  exists(Callable c | entry = c.getBody())
-  or
-  // This disjunct is technically superfluous, but safeguards against extractor problems.
-  entry instanceof BlockStmt and
-  not exists(entry.getEnclosingCallable()) and
-  not entry.getParent() instanceof Stmt
+private predicate flowEntry(BasicBlock entry) {
+  exists(Stmt entrystmt | entrystmt = entry.getFirstNode().asStmt() |
+    exists(Callable c | entrystmt = c.getBody())
+    or
+    // This disjunct is technically superfluous, but safeguards against extractor problems.
+    entrystmt instanceof BlockStmt and
+    not exists(entry.getEnclosingCallable()) and
+    not entrystmt.getParent() instanceof Stmt
+  )
 }
 
 /** The successor relation for basic blocks. */
@@ -31,11 +33,8 @@ predicate hasDominanceInformation(BasicBlock bb) {
   exists(BasicBlock entry | flowEntry(entry) and bbSucc*(entry, bb))
 }
 
-/** Exit points for control-flow. */
-private predicate flowExit(Callable exit) { exists(ControlFlowNode s | s.getASuccessor() = exit) }
-
 /** Exit points for basic-block control-flow. */
-private predicate bbSink(BasicBlock exit) { flowExit(exit.getLastNode()) }
+private predicate bbSink(BasicBlock exit) { exit.getLastNode() instanceof ControlFlow::ExitNode }
 
 /** Reversed `bbSucc`. */
 private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getABBSuccessor() }

java/ql/lib/semmle/code/java/controlflow/Guards.qll

@@ -113,7 +113,7 @@ private PatternCase getClosestPrecedingPatternCase(SwitchCase case) {
 private predicate isNonFallThroughPredecessor(SwitchCase sc, ControlFlowNode pred) {
   pred = sc.getControlFlowNode().getAPredecessor() and
   (
-    pred.(Expr).getParent*() = sc.getSelectorExpr()
+    pred.asExpr().getParent*() = sc.getSelectorExpr()
     or
     // Ambiguous: in the case of `case String _ when x: case "SomeConstant":`, the guard `x`
     // passing edge will fall through into the constant case, and the guard failing edge
@@ -122,7 +122,7 @@ private predicate isNonFallThroughPredecessor(SwitchCase sc, ControlFlowNode pre
     exists(PatternCase previousPatternCase |
       previousPatternCase = getClosestPrecedingPatternCase(sc)
     |
-      pred.(Expr).getParent*() = previousPatternCase.getGuard() and
+      pred.asExpr().getParent*() = previousPatternCase.getGuard() and
       // Check there is any statement in between the previous pattern case and this one,
       // or the case is a rule, so there is no chance of a fall-through.
       (
@@ -133,7 +133,7 @@ private predicate isNonFallThroughPredecessor(SwitchCase sc, ControlFlowNode pre
     or
     // Unambigious: on the test-passing edge there must be at least one intervening
     // declaration node, including anonymous `_` declarations.
-    pred = getClosestPrecedingPatternCase(sc)
+    pred.asStmt() = getClosestPrecedingPatternCase(sc)
   )
 }
 

java/ql/lib/semmle/code/java/controlflow/Paths.qll

@@ -32,7 +32,7 @@ abstract class ActionConfiguration extends string {
 private BasicBlock actionBlock(ActionConfiguration conf) {
   exists(ControlFlowNode node | result = node.getBasicBlock() |
     conf.isAction(node) or
-    callAlwaysPerformsAction(node, conf)
+    callAlwaysPerformsAction(node.asCall(), conf)
   )
 }
 
@@ -45,17 +45,17 @@ private predicate callAlwaysPerformsAction(Call call, ActionConfiguration conf)
 
 /** Holds if an action dominates the exit of the callable. */
 private predicate actionDominatesExit(Callable callable, ActionConfiguration conf) {
-  exists(BasicBlock exit |
-    exit.getLastNode() = callable and
+  exists(ExitBlock exit |
+    exit.getEnclosingCallable() = callable and
     actionBlock(conf).bbDominates(exit)
   )
 }
 
 /** Gets a `BasicBlock` that contains an action that does not dominate the exit. */
 private BasicBlock nonDominatingActionBlock(ActionConfiguration conf) {
-  exists(BasicBlock exit |
+  exists(ExitBlock exit |
     result = actionBlock(conf) and
-    exit.getLastNode() = result.getEnclosingCallable() and
+    exit.getEnclosingCallable() = result.getEnclosingCallable() and
     not result.bbDominates(exit)
   )
 }
@@ -80,8 +80,8 @@ private predicate postActionBlock(BasicBlock bb, ActionConfiguration conf) {
 private predicate callableAlwaysPerformsAction(Callable callable, ActionConfiguration conf) {
   actionDominatesExit(callable, conf)
   or
-  exists(BasicBlock exit |
-    exit.getLastNode() = callable and
+  exists(ExitBlock exit |
+    exit.getEnclosingCallable() = callable and
     postActionBlock(exit, conf)
   )
 }

java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll

@@ -207,14 +207,12 @@
       conditionBlock.controls(this, constant.booleanNot())
     )
     or
-    // This block is not reachable in the CFG, and is not a callable, a body of a callable, an
-    // expression in an annotation, an expression in an assert statement, or a catch clause.
+    // This block is not reachable in the CFG, and is not the entrypoint in a callable, an
+    // expression in an assert statement, or a catch clause.
     forall(BasicBlock bb | bb = this.getABBPredecessor() | bb instanceof UnreachableBasicBlock) and
-    not exists(Callable c | c.getBody() = this) and
-    not this instanceof Callable and
-    not exists(Annotation a | a.getAChildExpr*() = this) and
-    not this.(Expr).getEnclosingStmt() instanceof AssertStmt and
-    not this instanceof CatchClause
+    not exists(Callable c | c.getBody().getControlFlowNode() = this.getFirstNode()) and
+    not exists(AssertStmt a | a = this.getFirstNode().asExpr().getEnclosingStmt()) and
+    not this.getFirstNode().asStmt() instanceof CatchClause
     or
     // Switch statements with a constant comparison expression may have unreachable cases.
     exists(ConstSwitchStmt constSwitchStmt, BasicBlock failingCaseBlock |
@@ -223,7 +221,7 @@
       // Not accessible from the successful case
       not constSwitchStmt.getMatchingCase().getBasicBlock().getABBSuccessor*() = failingCaseBlock and
       // Blocks dominated by the failing case block are unreachable
-      constSwitchStmt.getAFailingCase().getBasicBlock().bbDominates(this)
+      failingCaseBlock.bbDominates(this)
     )
   }
 }

java/ql/lib/semmle/code/java/dataflow/InstanceAccess.qll

@@ -227,12 +227,14 @@ class InstanceAccessExt extends TInstanceAccessExt {
   /** Gets the control flow node associated with this instance access. */
   ControlFlowNode getCfgNode() {
     exists(ExprParent e | e = this.getAssociatedExprOrStmt() |
-      e instanceof Call and result = e
+      result.asCall() = e
       or
-      e instanceof InstanceAccess and result = e
+      e.(InstanceAccess).getControlFlowNode() = result
       or
       exists(FieldAccess fa | fa = e |
-        if fa instanceof VarRead then fa = result else result.(AssignExpr).getDest() = fa
+        if fa instanceof VarRead
+        then fa.getControlFlowNode() = result
+        else result.asExpr().(AssignExpr).getDest() = fa
       )
     )
   }

java/ql/lib/semmle/code/java/dataflow/Nullness.qll

@@ -130,8 +130,8 @@ predicate dereference(Expr e) {
  * The `VarAccess` is included for nicer error reporting.
  */
 private ControlFlowNode varDereference(SsaVariable v, VarAccess va) {
-  dereference(result) and
-  result = sameValue(v, va)
+  dereference(result.asExpr()) and
+  result.asExpr() = sameValue(v, va)
 }
 
 /**
@@ -141,16 +141,16 @@ private ControlFlowNode varDereference(SsaVariable v, VarAccess va) {
 private ControlFlowNode ensureNotNull(SsaVariable v) {
   result = varDereference(v, _)
   or
-  result.(AssertStmt).getExpr() = nullGuard(v, true, false)
+  result.asStmt().(AssertStmt).getExpr() = nullGuard(v, true, false)
   or
-  exists(AssertTrueMethod m | result = m.getACheck(nullGuard(v, true, false)))
+  exists(AssertTrueMethod m | result.asCall() = m.getACheck(nullGuard(v, true, false)))
   or
-  exists(AssertFalseMethod m | result = m.getACheck(nullGuard(v, false, false)))
+  exists(AssertFalseMethod m | result.asCall() = m.getACheck(nullGuard(v, false, false)))
   or
-  exists(AssertNotNullMethod m | result = m.getACheck(v.getAUse()))
+  exists(AssertNotNullMethod m | result.asCall() = m.getACheck(v.getAUse()))
   or
   exists(AssertThatMethod m, MethodCall ma |
-    result = m.getACheck(v.getAUse()) and ma.getControlFlowNode() = result
+    result.asCall() = m.getACheck(v.getAUse()) and ma.getControlFlowNode() = result
   |
     ma.getAnArgument().(MethodCall).getMethod().getName() = "notNullValue"
   )
@@ -279,10 +279,10 @@ private predicate enhancedForEarlyExit(EnhancedForStmt for, ControlFlowNode n1,
   exists(Expr forExpr |
     n1.getANormalSuccessor() = n2 and
     for.getExpr() = forExpr and
-    forExpr.getAChildExpr*() = n1 and
-    not forExpr.getAChildExpr*() = n2 and
-    n1.getANormalSuccessor() = for.getVariable() and
-    not n2 = for.getVariable()
+    forExpr.getAChildExpr*() = n1.asExpr() and
+    not forExpr.getAChildExpr*() = n2.asExpr() and
+    n1.getANormalSuccessor().asExpr() = for.getVariable() and
+    not n2.asExpr() = for.getVariable()
   )
 }
 
@@ -343,7 +343,7 @@ private predicate nullVarStep(
   not impossibleEdge(mid, bb) and
   not exists(boolean branch | nullGuard(midssa, branch, false).hasBranchEdge(mid, bb, branch)) and
   not (leavingFinally(mid, bb, true) and midstoredcompletion = true) and
-  if bb.getFirstNode() = any(TryStmt try | | try.getFinally())
+  if bb.getFirstNode().asStmt() = any(TryStmt try | | try.getFinally())
   then
     if bb.getFirstNode() = mid.getLastNode().getANormalSuccessor()
     then storedcompletion = false

java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll

@@ -211,9 +211,11 @@ module Sem implements Semantic {
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getABBSuccessor() }
 
-  private predicate id(BasicBlock x, BasicBlock y) { x = y }
+  private predicate id(ExprParent x, ExprParent y) { x = y }
 
-  private predicate idOf(BasicBlock x, int y) = equivalenceRelation(id/2)(x, y)
+  private predicate idOfAst(ExprParent x, int y) = equivalenceRelation(id/2)(x, y)
+
+  private predicate idOf(BasicBlock x, int y) { idOfAst(x.getAstNode(), y) }
 
   int getBlockId1(BasicBlock bb) { idOf(bb, result) }