Shared: Use shared SuccessorType in shared Cfg and BasicBlock libs.

aschackmull · aschackmull · commit 144e34c669c0 · 2025-09-01T13:43:32.000+02:00
diff --git a/actions/ql/lib/codeql/actions/controlflow/internal/Cfg.qll b/actions/ql/lib/codeql/actions/controlflow/internal/Cfg.qll
@@ -101,14 +101,8 @@ private module Implementation implements CfgShared::InputSig<Location> {
     last(scope.(CompositeAction), e, c)
   }
 
-  predicate successorTypeIsSimple(SuccessorType t) { t instanceof DirectSuccessor }
-
-  predicate successorTypeIsCondition(SuccessorType t) { t instanceof BooleanSuccessor }
-
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
 
-  predicate isAbnormalExitType(SuccessorType t) { none() }
-
   int idOfAstNode(AstNode node) { none() }
 
   int idOfCfgScope(CfgScope scope) { none() }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/EdgeKind.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/EdgeKind.qll
@@ -2,6 +2,7 @@
  * Provides classes that specify the conditions under which control flows along a given edge.
  */
 
+private import codeql.controlflow.SuccessorType
 private import internal.EdgeKindInternal
 
 private newtype TEdgeKind =
@@ -28,6 +29,21 @@ abstract private class EdgeKindImpl extends TEdgeKind {
 
 final class EdgeKind = EdgeKindImpl;
 
+private SuccessorType getAMatchingSpecificSuccessorType(EdgeKind k) {
+  result.(BooleanSuccessor).getValue() = true and k instanceof TrueEdge
+  or
+  result.(BooleanSuccessor).getValue() = false and k instanceof FalseEdge
+  or
+  result instanceof ExceptionSuccessor and k instanceof ExceptionEdge
+}
+
+SuccessorType getAMatchingSuccessorType(EdgeKind k) {
+  result = getAMatchingSpecificSuccessorType(k)
+  or
+  not exists(getAMatchingSpecificSuccessorType(k)) and
+  result instanceof DirectSuccessor
+}
+
 /**
  * A "goto" edge, representing the unconditional successor of an `Instruction`
  * or `IRBlock`.
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll
@@ -265,9 +265,9 @@ private predicate isEntryBlock(TIRBlock block) {
 }
 
 module IRCfg implements BB::CfgSig<Language::Location> {
-  class ControlFlowNode = Instruction;
+  private import codeql.controlflow.SuccessorType
 
-  class SuccessorType = EdgeKind;
+  class ControlFlowNode = Instruction;
 
   final private class FinalIRBlock = IRBlock;
 
@@ -280,7 +280,12 @@ module IRCfg implements BB::CfgSig<Language::Location> {
 
     BasicBlock getASuccessor() { result = super.getASuccessor() }
 
-    BasicBlock getASuccessor(SuccessorType t) { result = super.getSuccessor(t) }
+    BasicBlock getASuccessor(SuccessorType t) {
+      exists(EdgeKind k |
+        result = super.getSuccessor(k) and
+        t = getAMatchingSuccessorType(k)
+      )
+    }
 
     predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
 
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRBlock.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRBlock.qll
@@ -265,9 +265,9 @@ private predicate isEntryBlock(TIRBlock block) {
 }
 
 module IRCfg implements BB::CfgSig<Language::Location> {
-  class ControlFlowNode = Instruction;
+  private import codeql.controlflow.SuccessorType
 
-  class SuccessorType = EdgeKind;
+  class ControlFlowNode = Instruction;
 
   final private class FinalIRBlock = IRBlock;
 
@@ -280,7 +280,12 @@ module IRCfg implements BB::CfgSig<Language::Location> {
 
     BasicBlock getASuccessor() { result = super.getASuccessor() }
 
-    BasicBlock getASuccessor(SuccessorType t) { result = super.getSuccessor(t) }
+    BasicBlock getASuccessor(SuccessorType t) {
+      exists(EdgeKind k |
+        result = super.getSuccessor(k) and
+        t = getAMatchingSuccessorType(k)
+      )
+    }
 
     predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
 
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRBlock.qll b/cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/IRBlock.qll
@@ -265,9 +265,9 @@ private predicate isEntryBlock(TIRBlock block) {
 }
 
 module IRCfg implements BB::CfgSig<Language::Location> {
-  class ControlFlowNode = Instruction;
+  private import codeql.controlflow.SuccessorType
 
-  class SuccessorType = EdgeKind;
+  class ControlFlowNode = Instruction;
 
   final private class FinalIRBlock = IRBlock;
 
@@ -280,7 +280,12 @@ module IRCfg implements BB::CfgSig<Language::Location> {
 
     BasicBlock getASuccessor() { result = super.getASuccessor() }
 
-    BasicBlock getASuccessor(SuccessorType t) { result = super.getSuccessor(t) }
+    BasicBlock getASuccessor(SuccessorType t) {
+      exists(EdgeKind k |
+        result = super.getSuccessor(k) and
+        t = getAMatchingSuccessorType(k)
+      )
+    }
 
     predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
 
diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/BasicBlocks.qll
@@ -346,8 +346,6 @@ private class EntryBasicBlockAlias = EntryBasicBlock;
 module Cfg implements BB::CfgSig<Location> {
   class ControlFlowNode = ControlFlow::Node;
 
-  class SuccessorType = ControlFlow::SuccessorType;
-
   class BasicBlock = BasicBlockAlias;
 
   class EntryBasicBlock = EntryBasicBlockAlias;
diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll
@@ -79,19 +79,10 @@ private module CfgInput implements CfgShared::InputSig<Location> {
     Impl::scopeLast(scope, last, c)
   }
 
-  class SuccessorType = ST::SuccessorType;
+  private class SuccessorType = ST::SuccessorType;
 
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
 
-  predicate successorTypeIsSimple(SuccessorType t) { t instanceof ST::DirectSuccessor }
-
-  predicate successorTypeIsCondition(SuccessorType t) { t instanceof ST::ConditionalSuccessor }
-
-  predicate isAbnormalExitType(SuccessorType t) {
-    t instanceof ST::ExceptionSuccessor or
-    t instanceof ST::ExitSuccessor
-  }
-
   int idOfAstNode(AstNode node) { result = node.getId() }
 
   int idOfCfgScope(CfgScope node) { result = idOfAstNode(node) }
diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll
@@ -163,8 +163,6 @@ class ConditionBlock extends PreBasicBlock {
 module PreCfg implements BB::CfgSig<Location> {
   class ControlFlowNode = ControlFlowElement;
 
-  class SuccessorType = Cfg::SuccessorType;
-
   class BasicBlock = PreBasicBlock;
 
   class EntryBasicBlock extends BasicBlock {
diff --git a/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll b/java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll
@@ -7,10 +7,9 @@ module;
 import java
 import Dominance
 private import codeql.controlflow.BasicBlock as BB
+private import codeql.controlflow.SuccessorType
 
 private module Input implements BB::InputSig<Location> {
-  import codeql.controlflow.SuccessorType
-
   /** Hold if `t` represents a conditional successor type. */
   predicate successorTypeIsCondition(SuccessorType t) { none() }
 
@@ -96,7 +95,7 @@ class BasicBlock extends BbImpl::BasicBlock {
   predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
 
   /** Gets an immediate successor of this basic block of a given type, if any. */
-  BasicBlock getASuccessor(Input::SuccessorType t) { result = super.getASuccessor(t) }
+  BasicBlock getASuccessor(SuccessorType t) { result = super.getASuccessor(t) }
 
   BasicBlock getASuccessor() { result = super.getASuccessor() }
 
@@ -161,8 +160,6 @@ private class BasicBlockAlias = BasicBlock;
 module Cfg implements BB::CfgSig<Location> {
   class ControlFlowNode = BbImpl::ControlFlowNode;
 
-  class SuccessorType = BbImpl::SuccessorType;
-
   class BasicBlock = BasicBlockAlias;
 
   class EntryBasicBlock extends BasicBlock instanceof BbImpl::EntryBasicBlock { }
diff --git a/java/ql/lib/semmle/code/java/controlflow/Guards.qll b/java/ql/lib/semmle/code/java/controlflow/Guards.qll
@@ -139,7 +139,7 @@ private predicate isNonFallThroughPredecessor(SwitchCase sc, ControlFlowNode pre
   )
 }
 
-private module SuccessorTypes implements SharedGuards::SuccessorTypesSig<SuccessorType> {
+private module SuccessorTypes implements SharedGuards::SuccessorTypesSig {
   import codeql.controlflow.SuccessorType
 }
 
diff --git a/javascript/ql/lib/semmle/javascript/internal/BasicBlockInternal.qll b/javascript/ql/lib/semmle/javascript/internal/BasicBlockInternal.qll
@@ -372,16 +372,28 @@ module Public {
 
   module Cfg implements BB::CfgSig<Location> {
     private import javascript as Js
-    private import codeql.util.Unit
+    private import codeql.controlflow.SuccessorType
 
     class ControlFlowNode = Js::ControlFlowNode;
 
-    class SuccessorType = Unit;
+    private predicate conditionSucc(BasicBlock bb1, BasicBlock bb2, boolean branch) {
+      exists(ConditionGuardNode g |
+        bb1 = g.getTest().getBasicBlock() and
+        bb2 = g.getBasicBlock() and
+        branch = g.getOutcome()
+      )
+    }
 
     class BasicBlock extends FinalBasicBlock {
       BasicBlock getASuccessor() { result = super.getASuccessor() }
 
-      BasicBlock getASuccessor(SuccessorType t) { result = super.getASuccessor() and exists(t) }
+      BasicBlock getASuccessor(SuccessorType t) {
+        conditionSucc(this, result, t.(BooleanSuccessor).getValue())
+        or
+        result = super.getASuccessor() and
+        t instanceof DirectSuccessor and
+        not conditionSucc(this, result, _)
+      }
 
       predicate strictlyDominates(BasicBlock bb) {
         this.(ReachableBasicBlock).strictlyDominates(bb)
diff --git a/python/ql/lib/semmle/python/Flow.qll b/python/ql/lib/semmle/python/Flow.qll
@@ -1259,9 +1259,9 @@ private class ControlFlowNodeAlias = ControlFlowNode;
 final private class FinalBasicBlock = BasicBlock;
 
 module Cfg implements BB::CfgSig<Location> {
-  class ControlFlowNode = ControlFlowNodeAlias;
+  private import codeql.controlflow.SuccessorType
 
-  class SuccessorType = Unit;
+  class ControlFlowNode = ControlFlowNodeAlias;
 
   class BasicBlock extends FinalBasicBlock {
     // Note `PY:BasicBlock` does not have a `getLocation`.
@@ -1275,7 +1275,24 @@ module Cfg implements BB::CfgSig<Location> {
 
     BasicBlock getASuccessor() { result = super.getASuccessor() }
 
-    BasicBlock getASuccessor(SuccessorType t) { result = super.getASuccessor() and exists(t) }
+    private BasicBlock getANonDirectSuccessor(SuccessorType t) {
+      result = this.getATrueSuccessor() and
+      t.(BooleanSuccessor).getValue() = true
+      or
+      result = this.getAFalseSuccessor() and
+      t.(BooleanSuccessor).getValue() = false
+      or
+      result = this.getAnExceptionalSuccessor() and
+      t instanceof ExceptionSuccessor
+    }
+
+    BasicBlock getASuccessor(SuccessorType t) {
+      result = this.getANonDirectSuccessor(t)
+      or
+      result = super.getASuccessor() and
+      t instanceof DirectSuccessor and
+      not result = this.getANonDirectSuccessor(_)
+    }
 
     predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
 
diff --git a/ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll b/ruby/ql/lib/codeql/ruby/controlflow/BasicBlocks.qll
@@ -301,13 +301,9 @@ private class BasicBlockAlias = BasicBlock;
 
 private class EntryBasicBlockAlias = EntryBasicBlock;
 
-private class SuccessorTypeAlias = SuccessorType;
-
 module Cfg implements BB::CfgSig<Location> {
   class ControlFlowNode = CfgNode;
 
-  class SuccessorType = SuccessorTypeAlias;
-
   class BasicBlock = BasicBlockAlias;
 
   class EntryBasicBlock = EntryBasicBlockAlias;
diff --git a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -46,19 +46,10 @@ private module CfgInput implements CfgShared::InputSig<Location> {
     scope.(Impl::CfgScopeImpl).exit(last, c)
   }
 
-  class SuccessorType = Cfg::SuccessorType;
+  private class SuccessorType = Cfg::SuccessorType;
 
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
 
-  predicate successorTypeIsSimple(SuccessorType t) { t instanceof Cfg::DirectSuccessor }
-
-  predicate successorTypeIsCondition(SuccessorType t) { t instanceof Cfg::ConditionalSuccessor }
-
-  predicate isAbnormalExitType(SuccessorType t) {
-    t instanceof Cfg::ExceptionSuccessor or
-    t instanceof Cfg::ExitSuccessor
-  }
-
   private predicate id(Ruby::AstNode node1, Ruby::AstNode node2) { node1 = node2 }
 
   private predicate idOf(Ruby::AstNode node, int id) = equivalenceRelation(id/2)(node, id)
diff --git a/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll b/rust/ql/lib/codeql/rust/controlflow/BasicBlocks.qll
@@ -21,8 +21,6 @@ final class JoinPredecessorBasicBlock = BasicBlocksImpl::JoinPredecessorBasicBlo
 module Cfg implements BB::CfgSig<Location> {
   class ControlFlowNode = ControlFlowGraph::CfgNode;
 
-  class SuccessorType = ControlFlowGraph::SuccessorType;
-
   class BasicBlock = BasicBlocksImpl::BasicBlock;
 
   class EntryBasicBlock = BasicBlocksImpl::EntryBasicBlock;
diff --git a/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll b/rust/ql/lib/codeql/rust/controlflow/internal/ControlFlowGraphImpl.qll
@@ -29,19 +29,11 @@ private module CfgInput implements InputSig<Location> {
     Stages::CfgStage::ref()
   }
 
-  class SuccessorType = Cfg::SuccessorType;
+  private class SuccessorType = Cfg::SuccessorType;
 
   /** Gets a successor type that matches completion `c`. */
   SuccessorType getAMatchingSuccessorType(Completion c) { result = c.getAMatchingSuccessorType() }
 
-  /**
-   * Hold if `c` represents simple (normal) evaluation of a statement or an expression.
-   */
-  predicate successorTypeIsSimple(SuccessorType t) { t instanceof Cfg::DirectSuccessor }
-
-  /** Holds if `t` is an abnormal exit type out of a CFG scope. */
-  predicate isAbnormalExitType(SuccessorType t) { none() }
-
   /** Hold if `t` represents a conditional successor type. */
   predicate successorTypeIsCondition(SuccessorType t) { t instanceof Cfg::BooleanSuccessor }
 
diff --git a/shared/controlflow/codeql/controlflow/BasicBlock.qll b/shared/controlflow/codeql/controlflow/BasicBlock.qll
@@ -9,17 +9,12 @@ overlay[local?]
 module;
 
 private import codeql.util.Location
+private import SuccessorType
 
 /** Provides the language-specific input specification. */
 signature module InputSig<LocationSig Location> {
-  /** The type of a control flow successor. */
-  class SuccessorType {
-    /** Gets a textual representation of this successor type. */
-    string toString();
-  }
-
   /** Hold if `t` represents a conditional successor type. */
-  predicate successorTypeIsCondition(SuccessorType t);
+  default predicate successorTypeIsCondition(SuccessorType t) { t instanceof ConditionalSuccessor }
 
   /** A delineated part of the AST with its own CFG. */
   class CfgScope;
@@ -61,12 +56,6 @@ signature module CfgSig<LocationSig Location> {
     Location getLocation();
   }
 
-  /** The type of a control flow successor. */
-  class SuccessorType {
-    /** Gets a textual representation of this successor type. */
-    string toString();
-  }
-
   /**
    * A basic block, that is, a maximal straight-line sequence of control flow nodes
    * without branches or joins.
@@ -180,8 +169,6 @@ module Make<LocationSig Location, InputSig<Location> Input> implements CfgSig<Lo
 
   class ControlFlowNode = Input::Node;
 
-  class SuccessorType = Input::SuccessorType;
-
   /**
    * A basic block, that is, a maximal straight-line sequence of control flow nodes
    * without branches or joins.
diff --git a/shared/controlflow/codeql/controlflow/Cfg.qll b/shared/controlflow/codeql/controlflow/Cfg.qll
diff --git a/shared/controlflow/codeql/controlflow/Guards.qll b/shared/controlflow/codeql/controlflow/Guards.qll
diff --git a/shared/controlflow/codeql/controlflow/SuccessorType.qll b/shared/controlflow/codeql/controlflow/SuccessorType.qll
diff --git a/swift/ql/lib/codeql/swift/controlflow/BasicBlocks.qll b/swift/ql/lib/codeql/swift/controlflow/BasicBlocks.qll
diff --git a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll

Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@ private predicate isNonFallThroughPredecessor(SwitchCase sc, ControlFlowNode pre`
`139`	`139`	`)`
`140`	`140`	`}`
`141`	`141`
`142`		`-private module SuccessorTypes implements SharedGuards::SuccessorTypesSig<SuccessorType> {`
	`142`	`+private module SuccessorTypes implements SharedGuards::SuccessorTypesSig {`
`143`	`143`	`import codeql.controlflow.SuccessorType`
`144`	`144`	`}`
`145`	`145`