Merge pull request #16896 from hvitved/ssa/dataflow-integration-prep

hvitved · web-flow · commit da0909c0803f · 2024-07-03T19:56:35.000+02:00
SSA: Add `BasicBlock.{getNode/1,length/0}` to the input signature
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
@@ -1809,7 +1809,7 @@ module IteratorFlow {
      * Holds if `(bb, i)` contains a write to an iterator that may have been obtained
      * by calling `begin` (or related functions) on the variable `v`.
      */
-    predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
+    predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
       certain = false and
       exists(GetsIteratorCall beginCall, Instruction writeToDeref, IRBlock bbQual, int iQual |
         isIteratorStoreInstruction(beginCall, writeToDeref) and
@@ -1820,7 +1820,7 @@ module IteratorFlow {
     }
 
     /** Holds if `(bb, i)` reads the container variable `v`. */
-    predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {
+    predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
       Ssa::variableRead(bb, i, v, certain)
     }
   }
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
@@ -981,7 +981,7 @@ private module SsaInput implements SsaImplCommon::InputSig<Location> {
    * Holds if the `i`'th write in block `bb` writes to the variable `v`.
    * `certain` is `true` if the write is guaranteed to overwrite the entire variable.
    */
-  predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {
+  predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {
     DataFlowImplCommon::forceCachingInSameStage() and
     (
       exists(DefImpl def | def.hasIndexInBlock(bb, i, v) |
@@ -999,7 +999,7 @@ private module SsaInput implements SsaImplCommon::InputSig<Location> {
    * Holds if the `i`'th read in block `bb` reads to the variable `v`.
    * `certain` is `true` if the read is guaranteed. For C++, this is always the case.
    */
-  predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {
+  predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {
     exists(UseImpl use | use.hasIndexInBlock(bb, i, v) |
       if use.isCertain() then certain = true else certain = false
     )
diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll
@@ -757,13 +757,19 @@ import Cached
  * between the SSA pruning stage, and the final SSA stage.
  */
 module InputSigCommon {
-  class BasicBlock = IRBlock;
+  class BasicBlock extends IRBlock {
+    ControlFlowNode getNode(int i) { result = this.getInstruction(i) }
+
+    int length() { result = this.getInstructionCount() }
+  }
+
+  class ControlFlowNode = Instruction;
 
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
-  class ExitBasicBlock extends IRBlock {
+  class ExitBasicBlock extends BasicBlock {
     ExitBasicBlock() { this.getLastInstruction() instanceof ExitFunctionInstruction }
   }
 }
diff --git a/csharp/ql/lib/semmle/code/cil/Ssa.qll b/csharp/ql/lib/semmle/code/cil/Ssa.qll
@@ -35,7 +35,7 @@ deprecated module Ssa {
     }
 
     /** Gets the location of this SSA definition. */
-    Location getLocation() { result = this.getVariableUpdate().getLocation() }
+    override Location getLocation() { result = this.getVariableUpdate().getLocation() }
   }
 
   /** A phi node. */
diff --git a/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll
@@ -1,14 +1,17 @@
 private import cil
+private import CIL
 private import codeql.ssa.Ssa as SsaImplCommon
 
 deprecated private module SsaInput implements SsaImplCommon::InputSig<CIL::Location> {
   class BasicBlock = CIL::BasicBlock;
 
+  class ControlFlowNode = CIL::ControlFlowNode;
+
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
-  class ExitBasicBlock = CIL::ExitBasicBlock;
+  class ExitBasicBlock extends BasicBlock, CIL::ExitBasicBlock { }
 
   class SourceVariable = CIL::StackVariable;
 
diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll
@@ -80,7 +80,11 @@ module PreSsa {
   }
 
   module SsaInput implements SsaImplCommon::InputSig<Location> {
-    class BasicBlock = PreBasicBlocks::PreBasicBlock;
+    class BasicBlock extends PreBasicBlocks::PreBasicBlock {
+      ControlFlowNode getNode(int i) { result = this.getElement(i) }
+    }
+
+    class ControlFlowNode = ControlFlowElement;
 
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) }
 
@@ -192,7 +196,7 @@ module PreSsa {
       SsaImpl::ssaDefReachesEndOfBlock(bb, this, _)
     }
 
-    Location getLocation() {
+    override Location getLocation() {
       result = this.getDefinition().getLocation()
       or
       exists(Callable c, SsaInput::BasicBlock bb, SsaInput::SourceVariable v |
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/SSA.qll
@@ -427,7 +427,7 @@ module Ssa {
     }
 
     /** Gets the location of this SSA definition. */
-    Location getLocation() { none() }
+    override Location getLocation() { none() }
   }
 
   /**
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll
@@ -36,13 +36,15 @@ module BaseSsa {
 
     class BasicBlock = ControlFlow::BasicBlock;
 
+    class ControlFlowNode = ControlFlow::Node;
+
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
       result = bb.getImmediateDominator()
     }
 
     BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
-    class ExitBasicBlock = ControlFlow::BasicBlocks::ExitBlock;
+    class ExitBasicBlock extends BasicBlock, ControlFlow::BasicBlocks::ExitBlock { }
 
     class SourceVariable = PreSsa::SimpleLocalScopeVariable;
 
@@ -93,7 +95,7 @@ module BaseSsa {
       not result instanceof PhiNode
     }
 
-    Location getLocation() {
+    override Location getLocation() {
       result = this.getDefinition().getLocation()
       or
       exists(Callable c, SsaInput::BasicBlock bb, SsaInput::SourceVariable v |
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
@@ -263,14 +263,16 @@ module VariableCapture {
 
   private module CaptureInput implements Shared::InputSig<Location> {
     private import csharp as Cs
-    private import semmle.code.csharp.controlflow.ControlFlowGraph
+    private import semmle.code.csharp.controlflow.ControlFlowGraph as Cfg
     private import semmle.code.csharp.controlflow.BasicBlocks as BasicBlocks
     private import TaintTrackingPrivate as TaintTrackingPrivate
 
     class BasicBlock extends BasicBlocks::BasicBlock {
       Callable getEnclosingCallable() { result = super.getCallable() }
     }
 
+    class ControlFlowNode = Cfg::ControlFlow::Node;
+
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
       result = bb.getImmediateDominator()
     }
diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll
@@ -10,11 +10,13 @@ private import semmle.code.csharp.controlflow.internal.PreSsa
 private module SsaInput implements SsaImplCommon::InputSig<Location> {
   class BasicBlock = ControlFlow::BasicBlock;
 
+  class ControlFlowNode = ControlFlow::Node;
+
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
-  class ExitBasicBlock = ControlFlow::BasicBlocks::ExitBlock;
+  class ExitBasicBlock extends BasicBlock, ControlFlow::BasicBlocks::ExitBlock { }
 
   class SourceVariable = Ssa::SourceVariable;
 
@@ -24,7 +26,7 @@ private module SsaInput implements SsaImplCommon::InputSig<Location> {
    *
    * This includes implicit writes via calls.
    */
-  predicate variableWrite(ControlFlow::BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) {
+  predicate variableWrite(BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) {
     variableWriteDirect(bb, i, v, certain)
     or
     variableWriteQualifier(bb, i, v, certain)
@@ -38,7 +40,7 @@ private module SsaInput implements SsaImplCommon::InputSig<Location> {
    *
    * This includes implicit reads via calls.
    */
-  predicate variableRead(ControlFlow::BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) {
+  predicate variableRead(BasicBlock bb, int i, Ssa::SourceVariable v, boolean certain) {
     variableReadActual(bb, i, v) and
     certain = true
     or
@@ -1089,7 +1091,7 @@ class DefinitionExt extends Impl::DefinitionExt {
   override string toString() { result = this.(Ssa::Definition).toString() }
 
   /** Gets the location of this definition. */
-  Location getLocation() { result = this.(Ssa::Definition).getLocation() }
+  override Location getLocation() { result = this.(Ssa::Definition).getLocation() }
 
   /** Gets the enclosing callable of this definition. */
   Callable getEnclosingCallable() { result = this.(Ssa::Definition).getEnclosingCallable() }
diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll
@@ -72,11 +72,17 @@ private module CaptureInput implements VariableCapture::InputSig<Location> {
   class BasicBlock instanceof J::BasicBlock {
     string toString() { result = super.toString() }
 
+    ControlFlowNode getNode(int i) { result = super.getNode(i) }
+
+    int length() { result = super.length() }
+
     Callable getEnclosingCallable() { result = super.getEnclosingCallable() }
 
     Location getLocation() { result = super.getLocation() }
   }
 
+  class ControlFlowNode = J::ControlFlowNode;
+
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { bbIDominates(result, bb) }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) {
diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll b/python/ql/lib/semmle/python/dataflow/new/internal/VariableCapture.qll
@@ -24,6 +24,8 @@ private module CaptureInput implements Shared::InputSig<Location> {
   }
 
   class BasicBlock extends PY::BasicBlock {
+    int length() { result = count(int i | exists(this.getNode(i))) }
+
     Callable getEnclosingCallable() { result = this.getScope() }
 
     // Note `PY:BasicBlock` does not have a `getLocation`.
@@ -34,6 +36,8 @@ private module CaptureInput implements Shared::InputSig<Location> {
     Location getLocation() { result = super.getNode(0).getLocation() }
   }
 
+  class ControlFlowNode = PY::ControlFlowNode;
+
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/SSA.qll b/ruby/ql/lib/codeql/ruby/dataflow/SSA.qll
@@ -176,9 +176,6 @@ module Ssa {
 
     override string toString() { result = this.getControlFlowNode().toString() }
 
-    /** Gets the location of this SSA definition. */
-    Location getLocation() { result = this.getControlFlowNode().getLocation() }
-
     /** Gets the scope of this SSA definition. */
     CfgScope getScope() { result = this.getBasicBlock().getScope() }
   }
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -352,15 +352,16 @@ module VariableCapture {
   }
 
   private module CaptureInput implements Shared::InputSig<Location> {
-    private import ruby as R
-    private import codeql.ruby.controlflow.ControlFlowGraph
+    private import codeql.ruby.controlflow.ControlFlowGraph as Cfg
     private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks
     private import TaintTrackingPrivate as TaintTrackingPrivate
 
     class BasicBlock extends BasicBlocks::BasicBlock {
       Callable getEnclosingCallable() { result = this.getScope() }
     }
 
+    class ControlFlowNode = Cfg::CfgNode;
+
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
       result = bb.getImmediateDominator()
     }
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll
@@ -7,15 +7,18 @@ private import codeql.ruby.ast.Variable
 private import Cfg::CfgNodes::ExprNodes
 
 private module SsaInput implements SsaImplCommon::InputSig<Location> {
+  private import codeql.ruby.controlflow.ControlFlowGraph as Cfg
   private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks
 
   class BasicBlock = BasicBlocks::BasicBlock;
 
+  class ControlFlowNode = Cfg::CfgNode;
+
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
 
   BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
-  class ExitBasicBlock = BasicBlocks::ExitBasicBlock;
+  class ExitBasicBlock extends BasicBlock, BasicBlocks::ExitBasicBlock { }
 
   class SourceVariable = LocalVariable;
 
@@ -494,8 +497,7 @@ class DefinitionExt extends Impl::DefinitionExt {
 
   override string toString() { result = this.(Ssa::Definition).toString() }
 
-  /** Gets the location of this definition. */
-  Location getLocation() { result = this.(Ssa::Definition).getLocation() }
+  override Location getLocation() { result = this.(Ssa::Definition).getLocation() }
 }
 
 /**
@@ -506,5 +508,5 @@ class DefinitionExt extends Impl::DefinitionExt {
 class PhiReadNode extends DefinitionExt, Impl::PhiReadNode {
   override string toString() { result = "SSA phi read(" + this.getSourceVariable() + ")" }
 
-  override Location getLocation() { result = this.getBasicBlock().getLocation() }
+  override Location getLocation() { result = Impl::PhiReadNode.super.getLocation() }
 }
diff --git a/shared/dataflow/codeql/dataflow/VariableCapture.qll b/shared/dataflow/codeql/dataflow/VariableCapture.qll
@@ -17,13 +17,28 @@ signature module InputSig<LocationSig Location> {
     /** Gets a textual representation of this basic block. */
     string toString();
 
+    /** Gets the `i`th node in this basic block. */
+    ControlFlowNode getNode(int i);
+
+    /** Gets the length of this basic block. */
+    int length();
+
     /** Gets the enclosing callable. */
     Callable getEnclosingCallable();
 
     /** Gets the location of this basic block. */
     Location getLocation();
   }
 
+  /** A control flow node. */
+  class ControlFlowNode {
+    /** Gets a textual representation of this control flow node. */
+    string toString();
+
+    /** Gets the location of this control flow node. */
+    Location getLocation();
+  }
+
   /**
    * Gets the basic block that immediately dominates basic block `bb`, if any.
    *
@@ -672,6 +687,8 @@ module Flow<LocationSig Location, InputSig<Location> Input> implements OutputSig
   private module CaptureSsaInput implements Ssa::InputSig<Location> {
     final class BasicBlock = Input::BasicBlock;
 
+    final class ControlFlowNode = Input::ControlFlowNode;
+
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
       result = Input::getImmediateBasicBlockDominator(bb)
     }
@@ -717,10 +734,10 @@ module Flow<LocationSig Location, InputSig<Location> Input> implements OutputSig
     TSynthPhi(CaptureSsa::DefinitionExt phi) {
       phi instanceof CaptureSsa::PhiNode or phi instanceof CaptureSsa::PhiReadNode
     } or
-    TExprNode(Expr expr, boolean isPost) {
-      expr instanceof VariableRead and isPost = [false, true]
+    TExprNode(Expr expr, Boolean isPost) {
+      expr instanceof VariableRead
       or
-      synthRead(_, _, _, _, expr) and isPost = [false, true]
+      synthRead(_, _, _, _, expr)
     } or
     TParamNode(CapturedParameter p) or
     TThisParamNode(Callable c) { captureAccess(_, c) } or
diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll
diff --git a/swift/ql/lib/codeql/swift/dataflow/Ssa.qll b/swift/ql/lib/codeql/swift/dataflow/Ssa.qll
diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

Original file line number	Diff line number	Diff line change
`@@ -1809,7 +1809,7 @@ module IteratorFlow {`
`1809`	`1809`	* Holds if `(bb, i)` contains a write to an iterator that may have been obtained
`1810`	`1810`	* by calling `begin` (or related functions) on the variable `v`.
`1811`	`1811`	`*/`
`1812`		`- predicate variableWrite(IRBlock bb, int i, SourceVariable v, boolean certain) {`
	`1812`	`+ predicate variableWrite(BasicBlock bb, int i, SourceVariable v, boolean certain) {`
`1813`	`1813`	`certain = false and`
`1814`	`1814`	`exists(GetsIteratorCall beginCall, Instruction writeToDeref, IRBlock bbQual, int iQual \|`
`1815`	`1815`	`isIteratorStoreInstruction(beginCall, writeToDeref) and`
`@@ -1820,7 +1820,7 @@ module IteratorFlow {`
`1820`	`1820`	`}`
`1821`	`1821`
`1822`	`1822`	/** Holds if `(bb, i)` reads the container variable `v`. */
`1823`		`- predicate variableRead(IRBlock bb, int i, SourceVariable v, boolean certain) {`
	`1823`	`+ predicate variableRead(BasicBlock bb, int i, SourceVariable v, boolean certain) {`
`1824`	`1824`	`Ssa::variableRead(bb, i, v, certain)`
`1825`	`1825`	`}`
`1826`	`1826`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ deprecated module Ssa {`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`/** Gets the location of this SSA definition. */`
`38`		`- Location getLocation() { result = this.getVariableUpdate().getLocation() }`
	`38`	`+ override Location getLocation() { result = this.getVariableUpdate().getLocation() }`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`/** A phi node. */`
Original file line number	Diff line number	Diff line change
`@@ -427,7 +427,7 @@ module Ssa {`
`427`	`427`	`}`
`428`	`428`
`429`	`429`	`/** Gets the location of this SSA definition. */`
`430`		`- Location getLocation() { none() }`
	`430`	`+ override Location getLocation() { none() }`
`431`	`431`	`}`
`432`	`432`
`433`	`433`	`/**`