Java/Cfg: Introduce new shared CFG library and replace the Java CFG.

java/ql/lib/printCfg.ql

@@ -21,7 +21,7 @@ external int selectedSourceColumn();
 
 private predicate selectedSourceColumnAlias = selectedSourceColumn/0;
 
-module ViewCfgQueryInput implements ViewCfgQueryInputSig<File> {
+module ViewCfgQueryInput implements ControlFlow::ViewCfgQueryInputSig<File> {
   predicate selectedSourceFile = selectedSourceFileAlias/0;
 
   predicate selectedSourceLine = selectedSourceLineAlias/0;
@@ -42,4 +42,4 @@ module ViewCfgQueryInput implements ViewCfgQueryInputSig<File> {
   }
 }
 
-import ViewCfgQuery<File, ViewCfgQueryInput>
+import ControlFlow::ViewCfgQuery<File, ViewCfgQueryInput>

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

@@ -1,96 +0,0 @@
-/**
- * Provides classes and predicates for representing completions.
- */
-overlay[local?]
-module;
-
-/*
- * A completion represents how a statement or expression terminates.
- *
- * There are five kinds of completions: normal completion,
- * `return` completion, `break` completion,
- * `continue` completion, and `throw` completion.
- *
- * Normal completions are further subdivided into boolean completions and all
- * other normal completions. A boolean completion adds the information that the
- * cfg node terminated with the given boolean value due to a subexpression
- * terminating with the other given boolean value. This is only
- * relevant for conditional contexts in which the value controls the
- * control-flow successor.
- */
-
-import java
-
-/**
- * A label of a `LabeledStmt`.
- */
-newtype Label = MkLabel(string l) { exists(LabeledStmt lbl | l = lbl.getLabel()) }
-
-/**
- * Either a `Label` or nothing.
- */
-newtype MaybeLabel =
-  JustLabel(Label l) or
-  NoLabel()
-
-/**
- * A completion of a statement or an expression.
- */
-newtype Completion =
-  /**
-   * The statement or expression completes normally and continues to the next statement.
-   */
-  NormalCompletion() or
-  /**
-   * The statement or expression completes by returning from the function.
-   */
-  ReturnCompletion() or
-  /**
-   * The expression completes with value `outerValue` overall and with the last control
-   * flow node having value `innerValue`.
-   */
-  BooleanCompletion(boolean outerValue, boolean innerValue) {
-    (outerValue = true or outerValue = false) and
-    (innerValue = true or innerValue = false)
-  } or
-  /**
-   * The expression or statement completes via a `break` statement.
-   */
-  BreakCompletion(MaybeLabel l) or
-  /**
-   * The expression or statement completes via a `yield` statement.
-   */
-  YieldCompletion(NormalOrBooleanCompletion c) or
-  /**
-   * The expression or statement completes via a `continue` statement.
-   */
-  ContinueCompletion(MaybeLabel l) or
-  /**
-   * The expression or statement completes by throwing a `ThrowableType`.
-   */
-  ThrowCompletion(ThrowableType tt)
-
-/** A completion that is either a `NormalCompletion` or a `BooleanCompletion`. */
-class NormalOrBooleanCompletion extends Completion {
-  NormalOrBooleanCompletion() {
-    this instanceof NormalCompletion or this instanceof BooleanCompletion
-  }
-
-  /** Gets a textual representation of this completion. */
-  string toString() { result = "completion" }
-}
-
-/** Gets the completion `ContinueCompletion(NoLabel())`. */
-ContinueCompletion anonymousContinueCompletion() { result = ContinueCompletion(NoLabel()) }
-
-/** Gets the completion `ContinueCompletion(JustLabel(l))`. */
-ContinueCompletion labelledContinueCompletion(Label l) { result = ContinueCompletion(JustLabel(l)) }
-
-/** Gets the completion `BreakCompletion(NoLabel())`. */
-BreakCompletion anonymousBreakCompletion() { result = BreakCompletion(NoLabel()) }
-
-/** Gets the completion `BreakCompletion(JustLabel(l))`. */
-BreakCompletion labelledBreakCompletion(Label l) { result = BreakCompletion(JustLabel(l)) }
-
-/** Gets the completion `BooleanCompletion(value, value)`. */
-Completion basicBooleanCompletion(boolean value) { result = BooleanCompletion(value, value) }

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

@@ -6,151 +6,8 @@ module;
 
 import java
 import Dominance
-private import codeql.controlflow.BasicBlock as BB
-private import codeql.controlflow.SuccessorType
-
-private module Input implements BB::InputSig<Location> {
-  /** Hold if `t` represents a conditional successor type. */
-  predicate successorTypeIsCondition(SuccessorType t) { none() }
-
-  /** A delineated part of the AST with its own CFG. */
-  class CfgScope = Callable;
-
-  /** The class of control flow nodes. */
-  class Node = ControlFlowNode;
-
-  /** Gets the CFG scope in which this node occurs. */
-  CfgScope nodeGetCfgScope(Node node) { node.getEnclosingCallable() = result }
-
-  /** Gets an immediate successor of this node. */
-  Node nodeGetASuccessor(Node node, SuccessorType t) { result = node.getASuccessor(t) }
-
-  /**
-   * Holds if `node` represents an entry node to be used when calculating
-   * dominance.
-   */
-  predicate nodeIsDominanceEntry(Node node) {
-    exists(Stmt entrystmt | entrystmt = node.asStmt() |
-      exists(Callable c | entrystmt = c.getBody())
-      or
-      // This disjunct is technically superfluous, but safeguards against extractor problems.
-      entrystmt instanceof BlockStmt and
-      not exists(entrystmt.getEnclosingCallable()) and
-      not entrystmt.getParent() instanceof Stmt
-    )
-  }
-
-  /**
-   * Holds if `node` represents an exit node to be used when calculating
-   * post dominance.
-   */
-  predicate nodeIsPostDominanceExit(Node node) { node instanceof ControlFlow::NormalExitNode }
-}
-
-private module BbImpl = BB::Make<Location, Input>;
-
-import BbImpl
-
-/** Holds if the dominance relation is calculated for `bb`. */
-predicate hasDominanceInformation(BasicBlock bb) {
-  exists(BasicBlock entry |
-    Input::nodeIsDominanceEntry(entry.getFirstNode()) and entry.getASuccessor*() = bb
-  )
-}
-
-/**
- * A basic block, that is, a maximal straight-line sequence of control flow nodes
- * without branches or joins.
- */
-class BasicBlock extends BbImpl::BasicBlock {
-  /** Gets the immediately enclosing callable whose body contains this node. */
-  Callable getEnclosingCallable() { result = this.getScope() }
-
-  /**
-   * Holds if this basic block dominates basic block `bb`.
-   *
-   * That is, all paths reaching `bb` from the entry point basic block must
-   * go through this basic block.
-   */
-  predicate dominates(BasicBlock bb) { super.dominates(bb) }
-
-  /**
-   * Holds if this basic block strictly dominates basic block `bb`.
-   *
-   * That is, all paths reaching `bb` from the entry point basic block must
-   * go through this basic block and this basic block is different from `bb`.
-   */
-  predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
-
-  /** Gets an immediate successor of this basic block of a given type, if any. */
-  BasicBlock getASuccessor(SuccessorType t) { result = super.getASuccessor(t) }
-
-  BasicBlock getASuccessor() { result = super.getASuccessor() }
-
-  BasicBlock getImmediateDominator() { result = super.getImmediateDominator() }
-
-  predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) }
-
-  predicate strictlyPostDominates(BasicBlock bb) { super.strictlyPostDominates(bb) }
-
-  predicate postDominates(BasicBlock bb) { super.postDominates(bb) }
-
-  /**
-   * DEPRECATED: Use `getASuccessor` instead.
-   *
-   * Gets an immediate successor of this basic block.
-   */
-  deprecated BasicBlock getABBSuccessor() { result = this.getASuccessor() }
-
-  /**
-   * DEPRECATED: Use `getAPredecessor` instead.
-   *
-   * Gets an immediate predecessor of this basic block.
-   */
-  deprecated BasicBlock getABBPredecessor() { result.getASuccessor() = this }
-
-  /**
-   * DEPRECATED: Use `strictlyDominates` instead.
-   *
-   * Holds if this basic block strictly dominates `node`.
-   */
-  deprecated predicate bbStrictlyDominates(BasicBlock node) { this.strictlyDominates(node) }
-
-  /**
-   * DEPRECATED: Use `dominates` instead.
-   *
-   * Holds if this basic block dominates `node`. (This is reflexive.)
-   */
-  deprecated predicate bbDominates(BasicBlock node) { this.dominates(node) }
-
-  /**
-   * DEPRECATED: Use `strictlyPostDominates` instead.
-   *
-   * Holds if this basic block strictly post-dominates `node`.
-   */
-  deprecated predicate bbStrictlyPostDominates(BasicBlock node) { this.strictlyPostDominates(node) }
-
-  /**
-   * DEPRECATED: Use `postDominates` instead.
-   *
-   * Holds if this basic block post-dominates `node`. (This is reflexive.)
-   */
-  deprecated predicate bbPostDominates(BasicBlock node) { this.postDominates(node) }
-}
 
 /** A basic block that ends in an exit node. */
 class ExitBlock extends BasicBlock {
   ExitBlock() { this.getLastNode() instanceof ControlFlow::ExitNode }
 }
-
-private class BasicBlockAlias = BasicBlock;
-
-module Cfg implements BB::CfgSig<Location> {
-  class ControlFlowNode = BbImpl::ControlFlowNode;
-
-  class BasicBlock = BasicBlockAlias;
-
-  class EntryBasicBlock extends BasicBlock instanceof BbImpl::EntryBasicBlock { }
-
-  predicate dominatingEdge(BasicBlock bb1, BasicBlock bb2) { BbImpl::dominatingEdge(bb1, bb2) }
-}

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

@@ -66,10 +66,6 @@ private class JoinBlock extends BasicBlock {
   JoinBlock() { 2 <= strictcount(this.getAPredecessor()) }
 }
 
-private class ReachableBlock extends BasicBlock {
-  ReachableBlock() { hasDominanceInformation(this) }
-}
-
 /**
  * Holds if `bb` is a block that is collectively dominated by a set of one or
  * more actions that individually does not dominate the exit.
@@ -78,7 +74,7 @@ private predicate postActionBlock(BasicBlock bb, ActionConfiguration conf) {
   bb = nonDominatingActionBlock(conf)
   or
   if bb instanceof JoinBlock
-  then forall(ReachableBlock pred | pred = bb.getAPredecessor() | postActionBlock(pred, conf))
+  then forall(BasicBlock pred | pred = bb.getAPredecessor() | postActionBlock(pred, conf))
   else postActionBlock(bb.getAPredecessor(), conf)
 }
 

java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll

@@ -93,8 +93,7 @@ private module BaseSsaImpl {
   /** Holds if `n` updates the local variable `v`. */
   predicate variableUpdate(BaseSsaSourceVariable v, ControlFlowNode n, BasicBlock b, int i) {
     exists(VariableUpdate a | a.getControlFlowNode() = n | getDestVar(a) = v) and
-    b.getNode(i) = n and
-    hasDominanceInformation(b)
+    b.getNode(i) = n
   }
 
   /** Gets the definition point of a nested class in the parent scope. */
@@ -178,15 +177,12 @@ private module SsaImplInput implements SsaImplCommon::InputSig<Location, BasicBl
    * Holds if the `i`th of basic block `bb` reads source variable `v`.
    */
   predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
-    hasDominanceInformation(bb) and
-    (
-      exists(VarRead use |
-        v.getAnAccess() = use and bb.getNode(i) = use.getControlFlowNode() and certain = true
-      )
-      or
-      variableCapture(v, _, bb, i) and
-      certain = false
+    exists(VarRead use |
+      v.getAnAccess() = use and bb.getNode(i) = use.getControlFlowNode() and certain = true
     )
+    or
+    variableCapture(v, _, bb, i) and
+    certain = false
   }
 }
 

java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

@@ -130,8 +130,7 @@ private predicate variableCapture(TrackedVar capturedvar, TrackedVar closurevar,
 pragma[nomagic]
 private predicate certainVariableUpdate(TrackedVar v, ControlFlowNode n, BasicBlock b, int i) {
   exists(VariableUpdate a | a.getControlFlowNode() = n | getDestVar(a) = v) and
-  b.getNode(i) = n and
-  hasDominanceInformation(b)
+  b.getNode(i) = n
   or
   certainVariableUpdate(v.getQualifier(), n, b, i)
 }
@@ -154,8 +153,7 @@ overlay[global]
 pragma[nomagic]
 private predicate uncertainVariableUpdateImpl(TrackedVar v, ControlFlowNode n, BasicBlock b, int i) {
   exists(Call c | c.getControlFlowNode() = n | updatesNamedField(c, v, _)) and
-  b.getNode(i) = n and
-  hasDominanceInformation(b)
+  b.getNode(i) = n
   or
   uncertainVariableUpdateImpl(v.getQualifier(), n, b, i)
 }
@@ -191,18 +189,15 @@ private module SsaImplInput implements SsaImplCommon::InputSig<Location, BasicBl
    * This includes implicit reads via calls.
    */
   predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
-    hasDominanceInformation(bb) and
-    (
-      exists(VarRead use |
-        v instanceof TrackedVar and
-        v.getAnAccess() = use and
-        bb.getNode(i) = use.getControlFlowNode() and
-        certain = true
-      )
-      or
-      variableCapture(v, _, bb, i) and
-      certain = false
+    exists(VarRead use |
+      v instanceof TrackedVar and
+      v.getAnAccess() = use and
+      bb.getNode(i) = use.getControlFlowNode() and
+      certain = true
     )
+    or
+    variableCapture(v, _, bb, i) and
+    certain = false
   }
 }
 

java/ql/src/Likely Bugs/Termination/ConstantLoopCondition.ql

@@ -57,13 +57,15 @@ predicate loopExitGuard(LoopStmt loop, Expr cond) {
  */
 predicate mainLoopCondition(LoopStmt loop, Expr cond) {
   loop.getCondition() = cond and
-  exists(Expr loopReentry, ControlFlowNode last |
-    if exists(loop.(ForStmt).getAnUpdate())
-    then loopReentry = loop.(ForStmt).getUpdate(0)
-    else loopReentry = cond
-  |
-    last.getEnclosingStmt().getEnclosingStmt*() = loop.getBody() and
-    last.getASuccessor().asExpr().getParent*() = loopReentry
+  exists(BasicBlock condBlock | condBlock.getANode().isBefore(cond) |
+    1 < strictcount(condBlock.getAPredecessor()) or loop instanceof DoStmt
+  )
+}
+
+predicate ssaDefinitionInLoop(LoopStmt loop, SsaDefinition ssa) {
+  exists(ControlFlowNode node | node = ssa.getControlFlowNode() |
+    node.getAstNode().(Stmt).getEnclosingStmt*() = loop or
+    node.getAstNode().(Expr).getEnclosingStmt().getEnclosingStmt*() = loop
   )
 }
 
@@ -76,7 +78,7 @@ where
   ) and
   // None of the ssa variables in `cond` are updated inside the loop.
   forex(SsaDefinition ssa, VarRead use | ssa.getARead() = use and use.getParent*() = cond |
-    not ssa.getControlFlowNode().getEnclosingStmt().getEnclosingStmt*() = loop or
+    not ssaDefinitionInLoop(loop, ssa) or
     ssa.getControlFlowNode().asExpr().getParent*() = loop.(ForStmt).getAnInit()
   ) and
   // And `cond` does not use method calls, field reads, or array reads.

java/ql/src/Violations of Best Practice/Declarations/Common.qll

@@ -1,41 +1,15 @@
 import java
 
-private Stmt getASwitchChild(SwitchStmt s) {
-  result = s.getAChild()
-  or
-  exists(Stmt mid |
-    mid = getASwitchChild(s) and not mid instanceof SwitchStmt and result = mid.getAChild()
-  )
-}
-
-private predicate blockInSwitch(SwitchStmt s, BasicBlock b) {
-  b.getFirstNode().getEnclosingStmt() = getASwitchChild(s)
-}
-
-private predicate switchCaseControlFlow(SwitchStmt switch, BasicBlock b1, BasicBlock b2) {
-  blockInSwitch(switch, b1) and
-  b1.getASuccessor() = b2 and
-  blockInSwitch(switch, b2)
-}
-
-predicate switchCaseControlFlowPlus(SwitchStmt switch, BasicBlock b1, BasicBlock b2) {
-  switchCaseControlFlow(switch, b1, b2)
-  or
-  exists(BasicBlock mid |
-    switchCaseControlFlowPlus(switch, mid, b2) and
-    switchCaseControlFlow(switch, b1, mid) and
-    not mid.getFirstNode().asStmt() = switch.getACase()
-  )
-}
-
 predicate mayDropThroughWithoutComment(SwitchStmt switch, Stmt switchCase) {
-  switchCase = switch.getACase() and
-  exists(Stmt other, BasicBlock b1, BasicBlock b2 |
-    b1.getFirstNode().asStmt() = switchCase and
-    b2.getFirstNode().asStmt() = other and
-    switchCaseControlFlowPlus(switch, b1, b2) and
-    other = switch.getACase() and
-    not fallThroughCommented(other)
+  exists(int caseIx, SwitchCase next, int nextCaseStmtIx, Stmt lastInCase, ControlFlowNode node |
+    switch.getCase(caseIx) = switchCase and
+    switch.getCase(caseIx + 1) = next and
+    switch.getStmt(nextCaseStmtIx) = next and
+    switch.getStmt(nextCaseStmtIx - 1) = lastInCase and
+    lastInCase != switchCase and
+    node.isAfter(lastInCase) and
+    node.getANormalSuccessor().asStmt() = switch.getAStmt() and
+    not fallThroughCommented(next)
   )
 }
 

java/ql/test-kotlin1/library-tests/controlflow/dominance/dominatorUnique.ql

@@ -7,5 +7,5 @@ where
   iDominates(dom1, node) and
   iDominates(dom2, node) and
   dom1 != dom2 and
-  func = node.getEnclosingStmt().getEnclosingCallable()
+  func = node.getEnclosingCallable()
 select func, node, dom1, dom2

java/ql/test-kotlin2/library-tests/controlflow/dominance/dominatorUnique.ql

@@ -7,5 +7,5 @@ where
   iDominates(dom1, node) and
   iDominates(dom2, node) and
   dom1 != dom2 and
-  func = node.getEnclosingStmt().getEnclosingCallable()
+  func = node.getEnclosingCallable()
 select func, node, dom1, dom2

java/ql/test/library-tests/controlflow/dominance/dominatorUnique.ql

@@ -7,5 +7,5 @@ where
   iDominates(dom1, node) and
   iDominates(dom2, node) and
   dom1 != dom2 and
-  func = node.getEnclosingStmt().getEnclosingCallable()
+  func = node.getEnclosingCallable()
 select func, node, dom1, dom2

java/ql/test/library-tests/java7/MultiCatch/MultiCatchControlFlow.ql

@@ -1,5 +1,5 @@
 import default
 
 from ControlFlowNode n
-where n.getEnclosingStmt().getCompilationUnit().fromSource()
+where n.getEnclosingCallable().getCompilationUnit().fromSource()
 select n, n.getASuccessor()