SSA: Add locations to ease debugging

github · Nov 21, 2023 · 1d6010c · 1d6010c
1 parent 2c5ce32
commit 1d6010c
Show file tree

Hide file tree

Showing 15 changed files with 61 additions and 40 deletions.
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
@@ -59,6 +59,9 @@ private module SourceVariables {
       then result = base.getType()
       else result = getTypeImpl(base.getType(), ind - 1)
     }
+
+    /** Gets the location of this variable. */
+    Location getLocation() { result = this.getBaseVariable().getLocation() }
   }
 }
 
@@ -869,7 +872,7 @@ private predicate sourceVariableIsGlobal(
   )
 }
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<Location> {
   import InputSigCommon
   import SourceVariables
 
@@ -1092,7 +1095,7 @@ class Def extends DefOrUse {
   predicate isCertain() { defOrUse.isCertain() }
 }
 
-private module SsaImpl = SsaImplCommon::Make<SsaInput>;
+private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
 class PhiNode extends SsaImpl::DefinitionExt {
   PhiNode() {

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
@@ -377,6 +377,9 @@ abstract private class AbstractBaseSourceVariable extends TBaseSourceVariable {
   /** Gets a textual representation of this element. */
   abstract string toString();
 
+  /** Gets the location of this variable. */
+  abstract Location getLocation();
+
   /** Gets the type of this base source variable. */
   final DataFlowType getType() { this.getLanguageType().hasUnspecifiedType(result, _) }
 
@@ -395,6 +398,8 @@ class BaseIRVariable extends AbstractBaseSourceVariable, TBaseIRVariable {
 
   override string toString() { result = var.toString() }
 
+  override Location getLocation() { result = var.getLocation() }
+
   override CppType getLanguageType() { result = var.getLanguageType() }
 }
 
@@ -407,6 +412,8 @@ class BaseCallVariable extends AbstractBaseSourceVariable, TBaseCallVariable {
 
   override string toString() { result = call.toString() }
 
+  override Location getLocation() { result = call.getLocation() }
+
   override CppType getLanguageType() { result = getResultLanguageType(call) }
 }
 

diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll
@@ -229,7 +229,7 @@ private class FinalParameterUse extends UseImpl, TFinalParameterUse {
   override predicate isCertain() { any() }
 }
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<Location> {
   import InputSigCommon
   import SourceVariables
 
@@ -335,7 +335,7 @@ class Def extends DefOrUse {
   predicate isIteratorDef() { defOrUse instanceof IteratorDef }
 }
 
-private module SsaImpl = SsaImplCommon::Make<SsaInput>;
+private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
 class PhiNode extends SsaImpl::DefinitionExt {
   PhiNode() {

diff --git a/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll
@@ -1,7 +1,7 @@
 private import cil
 private import codeql.ssa.Ssa as SsaImplCommon
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<CIL::Location> {
   class BasicBlock = CIL::BasicBlock;
 
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
@@ -29,7 +29,7 @@ private module SsaInput implements SsaImplCommon::InputSig {
   }
 }
 
-import SsaImplCommon::Make<SsaInput>
+import SsaImplCommon::Make<CIL::Location, SsaInput>
 
 cached
 private module Cached {

diff --git a/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll b/csharp/ql/lib/semmle/code/csharp/controlflow/internal/PreSsa.qll
@@ -36,7 +36,7 @@ module PreSsa {
     scopeFirst(c, bb)
   }
 
-  module SsaInput implements SsaImplCommon::InputSig {
+  module SsaInput implements SsaImplCommon::InputSig<Location> {
     class BasicBlock = PreBasicBlocks::PreBasicBlock;
 
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result.immediatelyDominates(bb) }
@@ -137,7 +137,7 @@ module PreSsa {
     }
   }
 
-  private module SsaImpl = SsaImplCommon::Make<SsaInput>;
+  private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
   class Definition extends SsaImpl::Definition {
     final AssignableRead getARead() {

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/BaseSSA.qll
@@ -24,7 +24,7 @@ module BaseSsa {
     )
   }
 
-  private module SsaInput implements SsaImplCommon::InputSig {
+  private module SsaInput implements SsaImplCommon::InputSig<Location> {
     class BasicBlock = ControlFlow::BasicBlock;
 
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
@@ -60,7 +60,7 @@ module BaseSsa {
     }
   }
 
-  private module SsaImpl = SsaImplCommon::Make<SsaInput>;
+  private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
   class Definition extends SsaImpl::Definition {
     final AssignableRead getARead() {

diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll
@@ -6,7 +6,7 @@ import csharp
 private import codeql.ssa.Ssa as SsaImplCommon
 private import AssignableDefinitions
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<Location> {
   class BasicBlock = ControlFlow::BasicBlock;
 
   BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) { result = bb.getImmediateDominator() }
@@ -49,7 +49,7 @@ private module SsaInput implements SsaImplCommon::InputSig {
   }
 }
 
-private import SsaImplCommon::Make<SsaInput> as Impl
+private import SsaImplCommon::Make<Location, SsaInput> as Impl
 
 class Definition = Impl::Definition;
 

diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll
@@ -61,11 +61,9 @@ private predicate closureFlowStep(Expr e1, Expr e2) {
   )
 }
 
-private module CaptureInput implements VariableCapture::InputSig {
+private module CaptureInput implements VariableCapture::InputSig<Location> {
   private import java as J
 
-  class Location = J::Location;
-
   class BasicBlock instanceof J::BasicBlock {
     string toString() { result = super.toString() }
 
@@ -146,7 +144,7 @@ class CapturedVariable = CaptureInput::CapturedVariable;
 
 class CapturedParameter = CaptureInput::CapturedParameter;
 
-module CaptureFlow = VariableCapture::Flow<CaptureInput>;
+module CaptureFlow = VariableCapture::Flow<Location, CaptureInput>;
 
 private CaptureFlow::ClosureNode asClosureNode(Node n) {
   result = n.(CaptureNode).getSynthesizedCaptureNode()

diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -278,14 +278,12 @@ module VariableCapture {
     )
   }
 
-  private module CaptureInput implements Shared::InputSig {
+  private module CaptureInput implements Shared::InputSig<Location> {
     private import ruby as R
     private import codeql.ruby.controlflow.ControlFlowGraph
     private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks
     private import TaintTrackingPrivate as TaintTrackingPrivate
 
-    class Location = R::Location;
-
     class BasicBlock extends BasicBlocks::BasicBlock {
       Callable getEnclosingCallable() { result = this.getScope() }
     }
@@ -366,7 +364,7 @@ module VariableCapture {
 
   class ClosureExpr = CaptureInput::ClosureExpr;
 
-  module Flow = Shared::Flow<CaptureInput>;
+  module Flow = Shared::Flow<Location, CaptureInput>;
 
   private Flow::ClosureNode asClosureNode(Node n) {
     result = n.(CaptureNode).getSynthesizedCaptureNode()

diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/SsaImpl.qll
@@ -6,7 +6,7 @@ private import codeql.ruby.dataflow.SSA
 private import codeql.ruby.ast.Variable
 private import Cfg::CfgNodes::ExprNodes
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<Location> {
   private import codeql.ruby.controlflow.BasicBlocks as BasicBlocks
 
   class BasicBlock = BasicBlocks::BasicBlock;
@@ -62,7 +62,7 @@ private module SsaInput implements SsaImplCommon::InputSig {
   }
 }
 
-private import SsaImplCommon::Make<SsaInput> as Impl
+private import SsaImplCommon::Make<Location, SsaInput> as Impl
 
 class Definition = Impl::Definition;
 

diff --git a/shared/dataflow/codeql/dataflow/VariableCapture.qll b/shared/dataflow/codeql/dataflow/VariableCapture.qll
@@ -5,15 +5,10 @@
 
 private import codeql.util.Boolean
 private import codeql.util.Unit
+private import codeql.util.Location
 private import codeql.ssa.Ssa as Ssa
 
-signature module InputSig {
-  class Location {
-    predicate hasLocationInfo(
-      string filepath, int startline, int startcolumn, int endline, int endcolumn
-    );
-  }
-
+signature module InputSig<LocationSig Location> {
   /**
    * A basic block, that is, a maximal straight-line sequence of control flow nodes
    * without branches or joins.
@@ -144,7 +139,7 @@ signature module InputSig {
   }
 }
 
-signature module OutputSig<InputSig I> {
+signature module OutputSig<LocationSig Location, InputSig<Location> I> {
   /**
    * A data flow node that we need to reference in the step relations for
    * captured variables.
@@ -165,7 +160,7 @@ signature module OutputSig<InputSig I> {
     string toString();
 
     /** Gets the location of this node. */
-    I::Location getLocation();
+    Location getLocation();
 
     /** Gets the enclosing callable. */
     I::Callable getEnclosingCallable();
@@ -243,7 +238,7 @@ signature module OutputSig<InputSig I> {
  * Constructs the type `ClosureNode` and associated step relations, which are
  * intended to be included in the data-flow node and step relations.
  */
-module Flow<InputSig Input> implements OutputSig<Input> {
+module Flow<LocationSig Location, InputSig<Location> Input> implements OutputSig<Location, Input> {
   private import Input
 
   additional module ConsistencyChecks {
@@ -638,6 +633,10 @@ module Flow<InputSig Input> implements OutputSig<Input> {
       or
       result = "this" and this = TThis(_)
     }
+
+    Location getLocation() {
+      exists(CapturedVariable v | this = TVariable(v) and result = v.getLocation())
+    }
   }
 
   /** Holds if `cc` needs a definition at the entry of its callable scope. */
@@ -659,7 +658,7 @@ module Flow<InputSig Input> implements OutputSig<Input> {
     )
   }
 
-  private module CaptureSsaInput implements Ssa::InputSig {
+  private module CaptureSsaInput implements Ssa::InputSig<Location> {
     final class BasicBlock = Input::BasicBlock;
 
     BasicBlock getImmediateBasicBlockDominator(BasicBlock bb) {
@@ -697,7 +696,7 @@ module Flow<InputSig Input> implements OutputSig<Input> {
     }
   }
 
-  private module CaptureSsa = Ssa::Make<CaptureSsaInput>;
+  private module CaptureSsa = Ssa::Make<Location, CaptureSsaInput>;
 
   private newtype TClosureNode =
     TSynthRead(CapturedVariable v, BasicBlock bb, int i, Boolean isPost) {

diff --git a/shared/ssa/codeql/ssa/Ssa.qll b/shared/ssa/codeql/ssa/Ssa.qll
@@ -3,15 +3,20 @@
  * (SSA) form.
  */
 
+private import codeql.util.Location
+
 /** Provides the input specification of the SSA implementation. */
-signature module InputSig {
+signature module InputSig<LocationSig Location> {
   /**
    * A basic block, that is, a maximal straight-line sequence of control flow nodes
    * without branches or joins.
    */
   class BasicBlock {
     /** Gets a textual representation of this basic block. */
     string toString();
+
+    /** Gets the location of this basic block. */
+    Location getLocation();
   }
 
   /**
@@ -49,6 +54,9 @@ signature module InputSig {
   class SourceVariable {
     /** Gets a textual representation of this variable. */
     string toString();
+
+    /** Gets the location of this variable. */
+    Location getLocation();
   }
 
   /**
@@ -88,7 +96,7 @@ signature module InputSig {
  * NB: If this predicate is exposed, it should be cached.
  * ```
  */
-module Make<InputSig Input> {
+module Make<LocationSig Location, InputSig<Location> Input> {
   private import Input
 
   private BasicBlock getABasicBlockPredecessor(BasicBlock bb) {

diff --git a/shared/ssa/qlpack.yml b/shared/ssa/qlpack.yml
@@ -2,4 +2,6 @@ name: codeql/ssa
 version: 0.2.4-dev
 groups: shared
 library: true
+dependencies:
+  codeql/util: ${workspace}
 warnOnImplicitThis: true
diff --git a/swift/ql/lib/codeql/swift/dataflow/Ssa.qll b/swift/ql/lib/codeql/swift/dataflow/Ssa.qll
@@ -6,7 +6,7 @@ module Ssa {
   private import codeql.swift.controlflow.ControlFlowGraph
   private import codeql.swift.controlflow.BasicBlocks as BasicBlocks
 
-  private module SsaInput implements SsaImplCommon::InputSig {
+  private module SsaInput implements SsaImplCommon::InputSig<Location> {
     private import internal.DataFlowPrivate
     private import codeql.swift.controlflow.ControlFlowGraph
     private import codeql.swift.controlflow.CfgNodes
@@ -33,6 +33,12 @@ module Ssa {
       EntryNode asKeyPath() { none() }
 
       DeclRefExpr getAnAccess() { result.getDecl() = this.asVarDecl() }
+
+      Location getLocation() {
+        result = this.asVarDecl().getLocation()
+        or
+        result = this.asKeyPath().getLocation()
+      }
     }
 
     private class NormalSourceVariable extends SourceVariable, TNormalSourceVariable {
@@ -127,7 +133,7 @@ module Ssa {
   /**
    * INTERNAL: Do not use.
    */
-  module SsaImpl = SsaImplCommon::Make<SsaInput>;
+  module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
   cached
   class Definition extends SsaImpl::Definition {

diff --git a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll
@@ -862,7 +862,7 @@ private predicate closureFlowStep(CaptureInput::Expr e1, CaptureInput::Expr e2)
   e2.(Pattern).getImmediateMatchingExpr() = e1
 }
 
-private module CaptureInput implements VariableCapture::InputSig {
+private module CaptureInput implements VariableCapture::InputSig<Location> {
   private import swift as S
   private import codeql.swift.controlflow.BasicBlocks as B
 
@@ -976,7 +976,7 @@ class CapturedVariable = CaptureInput::CapturedVariable;
 
 class CapturedParameter = CaptureInput::CapturedParameter;
 
-module CaptureFlow = VariableCapture::Flow<CaptureInput>;
+module CaptureFlow = VariableCapture::Flow<Location, CaptureInput>;
 
 private CaptureFlow::ClosureNode asClosureNode(Node n) {
   result = n.(CaptureNode).getSynthesizedCaptureNode()