Merge pull request #15219 from rdmarsh2/rdmarsh2/swift/parameterized-cfg-library

Swift: switch to shared, parameterized CFG library

Author: MathiasVP

swift/ql/consistency-queries/CfgConsistency.ql

@@ -1 +1 @@
-import codeql.swift.controlflow.internal.ControlFlowGraphImplShared::Consistency
+import codeql.swift.controlflow.internal.ControlFlowGraphImplSpecific::CfgImpl::Consistency

swift/ql/lib/change-notes/2024-01-05-parameterized-cfg-library.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The control flow graph library (`codeql.swift.controlflow`) has been transitioned to use the shared implementation from the `codeql/controlflow` qlpack. No result changes are expected due to this change.

swift/ql/lib/codeql/swift/controlflow/BasicBlocks.qll

@@ -2,7 +2,7 @@
 
 private import swift
 private import ControlFlowGraph
-private import internal.ControlFlowGraphImpl
+private import internal.ControlFlowGraphImpl as Impl
 private import internal.ControlFlowElements
 private import CfgNodes
 private import SuccessorTypes
@@ -133,7 +133,9 @@ private module Cached {
   private predicate predBB(BasicBlock succ, BasicBlock pred) { succBB(pred, succ) }
 
   /** Holds if `bb` is an exit basic block that represents normal exit. */
-  private predicate normalExitBB(BasicBlock bb) { bb.getANode().(AnnotatedExitNode).isNormal() }
+  private predicate normalExitBB(BasicBlock bb) {
+    bb.getANode().(Impl::AnnotatedExitNode).isNormal()
+  }
 
   /** Holds if `dom` is an immediate post-dominator of `bb`. */
   cached
@@ -167,7 +169,9 @@ private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof EntryNod
 class EntryBasicBlock extends BasicBlock {
   EntryBasicBlock() { entryBB(this) }
 
-  override CfgScope getScope() { this.getFirstNode() = TEntryNode(result) }
+  override CfgScope getScope() {
+    this.getFirstNode() = any(EntryNode node | node.getScope() = result)
+  }
 }
 
 /**
@@ -178,7 +182,7 @@ class AnnotatedExitBasicBlock extends BasicBlock {
   private boolean normal;
 
   AnnotatedExitBasicBlock() {
-    exists(AnnotatedExitNode n |
+    exists(Impl::AnnotatedExitNode n |
       n = this.getANode() and
       if n.isNormal() then normal = true else normal = false
     )

swift/ql/lib/codeql/swift/controlflow/CfgNodes.qll

@@ -3,108 +3,39 @@
 private import swift
 private import BasicBlocks
 private import ControlFlowGraph
-private import internal.ControlFlowGraphImpl
+private import internal.ControlFlowGraphImpl as Impl
 private import internal.ControlFlowElements
 private import internal.Splitting
 
-/** An entry node for a given scope. */
-class EntryNode extends ControlFlowNode, TEntryNode {
-  private CfgScope scope;
-
-  EntryNode() { this = TEntryNode(scope) }
-
-  final override EntryBasicBlock getBasicBlock() { result = ControlFlowNode.super.getBasicBlock() }
-
-  final override Location getLocation() { result = scope.getLocation() }
-
-  final override string toString() { result = "enter " + scope }
-}
-
-/** An exit node for a given scope, annotated with the type of exit. */
-class AnnotatedExitNode extends ControlFlowNode, TAnnotatedExitNode {
-  private CfgScope scope;
-  private boolean normal;
-
-  AnnotatedExitNode() { this = TAnnotatedExitNode(scope, normal) }
-
-  /** Holds if this node represent a normal exit. */
-  final predicate isNormal() { normal = true }
-
-  final override AnnotatedExitBasicBlock getBasicBlock() {
-    result = ControlFlowNode.super.getBasicBlock()
-  }
-
-  final override Location getLocation() { result = scope.getLocation() }
-
-  final override string toString() {
-    exists(string s |
-      normal = true and s = "normal"
-      or
-      normal = false and s = "abnormal"
-    |
-      result = "exit " + scope + " (" + s + ")"
-    )
-  }
-}
-
-/** An exit node for a given scope. */
-class ExitNode extends ControlFlowNode, TExitNode {
-  private CfgScope scope;
-
-  ExitNode() { this = TExitNode(scope) }
-
-  final override Location getLocation() { result = scope.getLocation() }
-
-  final override string toString() { result = "exit " + scope }
-}
-
 /**
  * A node for an AST node.
  *
  * Each AST node maps to zero or more `CfgNode`s: zero when the node is unreachable
  * (dead) code or not important for control flow, and multiple when there are different
  * splits for the AST node.
  */
-class CfgNode extends ControlFlowNode, TElementNode {
-  private Splits splits;
-  ControlFlowElement n;
-
-  CfgNode() { this = TElementNode(_, n, splits) }
-
-  final override ControlFlowElement getNode() { result = n }
+class CfgNode extends ControlFlowNode instanceof Impl::AstCfgNode {
+  final override ControlFlowElement getNode() { result = this.getAstNode() }
 
-  override Location getLocation() { result = n.getLocation() }
-
-  final override string toString() {
-    exists(string s | s = n.toString() |
-      result = "[" + this.getSplitsString() + "] " + s
-      or
-      not exists(this.getSplitsString()) and result = s
-    )
-  }
+  /** Gets a split for this control flow node, if any. */
+  final Split getASplit() { result = super.getASplit() }
 
   /** Gets a comma-separated list of strings for each split in this node, if any. */
-  final string getSplitsString() {
-    result = splits.toString() and
-    result != ""
-  }
-
-  /** Gets a split for this control flow node, if any. */
-  final Split getASplit() { result = splits.getASplit() }
+  final string getSplitsString() { result = super.getSplitsString() }
 
   /** Gets the AST representation of this control flow node, if any. */
   Expr getAst() {
-    result = n.asAstNode()
+    result = this.getNode().asAstNode()
     or
-    result = n.(PropertyGetterElement).getRef()
+    result = this.getNode().(PropertyGetterElement).getRef()
     or
-    result = n.(PropertySetterElement).getAssignExpr()
+    result = this.getNode().(PropertySetterElement).getAssignExpr()
     or
-    result = n.(PropertyObserverElement).getAssignExpr()
+    result = this.getNode().(PropertyObserverElement).getAssignExpr()
     or
-    result = n.(ClosureElement).getAst()
+    result = this.getNode().(ClosureElement).getAst()
     or
-    result = n.(KeyPathElement).getAst()
+    result = this.getNode().(KeyPathElement).getAst()
   }
 }
 
@@ -130,7 +61,9 @@ class PatternCfgNode extends CfgNode {
 
 /** A control-flow node that wraps a property getter. */
 class PropertyGetterCfgNode extends CfgNode {
-  override PropertyGetterElement n;
+  PropertyGetterElement n;
+
+  PropertyGetterCfgNode() { n = this.getAstNode() }
 
   Expr getRef() { result = n.getRef() }
 
@@ -141,7 +74,9 @@ class PropertyGetterCfgNode extends CfgNode {
 
 /** A control-flow node that wraps a property setter. */
 class PropertySetterCfgNode extends CfgNode {
-  override PropertySetterElement n;
+  PropertySetterElement n;
+
+  PropertySetterCfgNode() { n = this.getAstNode() }
 
   AssignExpr getAssignExpr() { result = n.getAssignExpr() }
 
@@ -153,7 +88,9 @@ class PropertySetterCfgNode extends CfgNode {
 }
 
 class PropertyObserverCfgNode extends CfgNode {
-  override PropertyObserverElement n;
+  PropertyObserverElement n;
+
+  PropertyObserverCfgNode() { n = this.getAstNode() }
 
   AssignExpr getAssignExpr() { result = n.getAssignExpr() }
 
@@ -201,3 +138,9 @@ class KeyPathApplicationExprCfgNode extends ExprCfgNode {
 class KeyPathExprCfgNode extends ExprCfgNode {
   override KeyPathExpr e;
 }
+
+class EntryNode = Impl::EntryNode;
+
+class ExitNode = Impl::ExitNode;
+
+class AnnotatedExitNode = Impl::AnnotatedExitNode;

swift/ql/lib/codeql/swift/controlflow/ControlFlowGraph.qll

@@ -27,16 +27,10 @@ class CfgScope extends Scope instanceof CfgScope::Range_ {
  *
  * Only nodes that can be reached from an entry point are included in the CFG.
  */
-class ControlFlowNode extends TCfgNode {
-  /** Gets a textual representation of this control flow node. */
-  string toString() { none() }
-
+class ControlFlowNode extends Node {
   /** Gets the AST node that this node corresponds to, if any. */
   ControlFlowElement getNode() { none() }
 
-  /** Gets the location of this control flow node. */
-  Location getLocation() { none() }
-
   /** Gets the file of this control flow node. */
   final File getFile() { result = this.getLocation().getFile() }
 
@@ -50,7 +44,7 @@ class ControlFlowNode extends TCfgNode {
   BasicBlock getBasicBlock() { result.getANode() = this }
 
   /** Gets a successor node of a given type, if any. */
-  final ControlFlowNode getASuccessor(SuccessorType t) { result = getASuccessor(this, t) }
+  final ControlFlowNode getASuccessor(SuccessorType t) { result = super.getASuccessor(t) }
 
   /** Gets an immediate successor, if any. */
   final ControlFlowNode getASuccessor() { result = this.getASuccessor(_) }

swift/ql/lib/codeql/swift/controlflow/internal/AstControlFlowTrees.qll

@@ -1,6 +1,6 @@
 private import swift
 private import Completion
-private import ControlFlowGraphImplShared
+private import ControlFlowGraphImplSpecific::CfgImpl
 private import ControlFlowElements
 
 abstract class AstControlFlowTree extends ControlFlowTree {