Merge pull request github#5440 from hvitved/csharp/cil/ssa

C#: Add CIL SSA library

Author: hvitved

config/identical-files.json

@@ -429,10 +429,11 @@
   "SSA C#": [
     "csharp/ql/src/semmle/code/csharp/dataflow/internal/SsaImplCommon.qll",
     "csharp/ql/src/semmle/code/csharp/controlflow/internal/pressa/SsaImplCommon.qll",
-    "csharp/ql/src/semmle/code/csharp/dataflow/internal/basessa/SsaImplCommon.qll"
+    "csharp/ql/src/semmle/code/csharp/dataflow/internal/basessa/SsaImplCommon.qll",
+    "csharp/ql/src/semmle/code/cil/internal/SsaImplCommon.qll"
   ],
   "CryptoAlgorithms Python/JS": [
     "javascript/ql/src/semmle/javascript/security/CryptoAlgorithms.qll",
     "python/ql/src/semmle/crypto/Crypto.qll"
   ]
-}
+}

csharp/change-notes/2021-03-24-cil-ssa.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A static single assignment (SSA) library has been added to the CIL analysis library. The SSA library replaces the existing `DefUse` module, which has been deprecated.

csharp/ql/src/semmle/code/cil/BasicBlock.qll

@@ -240,6 +240,9 @@ class BasicBlock extends Cached::TBasicBlockStart {
 
   /** Gets a textual representation of this basic block. */
   string toString() { result = getFirstNode().toString() }
+
+  /** Gets the location of this basic block. */
+  Location getLocation() { result = this.getFirstNode().getLocation() }
 }
 
 /**
@@ -305,7 +308,7 @@ class EntryBasicBlock extends BasicBlock {
 }
 
 /** Holds if `bb` is an entry basic block. */
-private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof EntryPoint }
+private predicate entryBB(BasicBlock bb) { bb.getFirstNode() instanceof MethodImplementation }
 
 /**
  * An exit basic block, that is, a basic block whose last node is

csharp/ql/src/semmle/code/cil/CIL.qll

@@ -20,3 +20,4 @@ import Attribute
 import Stubs
 import CustomModifierReceiver
 import Parameterizable
+import semmle.code.cil.Ssa

csharp/ql/src/semmle/code/cil/CallableReturns.qll

@@ -42,7 +42,11 @@ private predicate alwaysNullExpr(Expr expr) {
   or
   alwaysNullMethod(expr.(StaticCall).getTarget())
   or
-  forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | alwaysNullVariableUpdate(vu))
+  forex(Ssa::Definition def |
+    expr = any(Ssa::Definition def0 | def = def0.getAnUltimateDefinition()).getARead()
+  |
+    alwaysNullVariableUpdate(def.getVariableUpdate())
+  )
 }
 
 pragma[noinline]
@@ -58,7 +62,9 @@ private predicate alwaysNotNullExpr(Expr expr) {
   or
   alwaysNotNullMethod(expr.(StaticCall).getTarget())
   or
-  forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) |
-    alwaysNotNullVariableUpdate(vu)
+  forex(Ssa::Definition def |
+    expr = any(Ssa::Definition def0 | def = def0.getAnUltimateDefinition()).getARead()
+  |
+    alwaysNotNullVariableUpdate(def.getVariableUpdate())
   )
 }

csharp/ql/src/semmle/code/cil/ControlFlow.qll

@@ -9,6 +9,9 @@ class ControlFlowNode extends @cil_controlflow_node {
   /** Gets a textual representation of this control flow node. */
   string toString() { none() }
 
+  /** Gets the location of this control flow node. */
+  Location getLocation() { none() }
+
   /**
    * Gets the number of items this node pushes onto the stack.
    * This value is either 0 or 1, except for the instruction `dup`

csharp/ql/src/semmle/code/cil/DataFlow.qll

@@ -57,12 +57,40 @@ class Untainted extends TaintType, TExactValue { }
 /** A taint type where the data is tainted. */
 class Tainted extends TaintType, TTaintedValue { }
 
+private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
+  exists(Ssa::Definition def, BasicBlock bb, int i | phi.hasLastInputRef(def, bb, i) |
+    def.definesAt(_, bb, i) and
+    input = def.getVariableUpdate().getSource()
+    or
+    input =
+      any(ReadAccess ra |
+        bb.getNode(i) = ra and
+        ra.getTarget() = def.getSourceVariable()
+      )
+  )
+  or
+  exists(Ssa::PhiNode mid, BasicBlock bb, int i |
+    localFlowPhiInput(input, mid) and
+    phi.hasLastInputRef(mid, bb, i) and
+    mid.definesAt(_, bb, i)
+  )
+}
+
 private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
   src = sink.(Opcodes::Dup).getAnOperand()
   or
-  defUse(_, src, sink)
+  exists(Ssa::Definition def, VariableUpdate vu |
+    vu = def.getVariableUpdate() and
+    src = vu.getSource() and
+    sink = def.getAFirstRead()
+  )
+  or
+  any(Ssa::Definition def).hasAdjacentReads(src, sink)
   or
-  src = sink.(ParameterReadAccess).getTarget()
+  exists(Ssa::PhiNode phi |
+    localFlowPhiInput(src, phi) and
+    sink = phi.getAFirstRead()
+  )
   or
   src = sink.(Conversion).getExpr()
   or
@@ -73,12 +101,6 @@ private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
   src = sink.(Return).getExpr()
   or
   src = sink.(ConditionalBranch).getAnOperand()
-  or
-  src = sink.(MethodParameter).getAWrite()
-  or
-  exists(VariableUpdate update |
-    update.getVariable().(Parameter) = sink and src = update.getSource()
-  )
 }
 
 private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
@@ -87,8 +109,7 @@ private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
   src = sink.(UnaryBitwiseOperation).getOperand()
 }
 
-cached
-module DefUse {
+deprecated module DefUse {
   /**
    * A classification of variable references into reads and writes.
    */
@@ -189,7 +210,7 @@ module DefUse {
 
   /** Holds if the variable update `vu` can be used at the read `use`. */
   cached
-  predicate variableUpdateUse(StackVariable target, VariableUpdate vu, ReadAccess use) {
+  deprecated predicate variableUpdateUse(StackVariable target, VariableUpdate vu, ReadAccess use) {
     defReachesReadWithinBlock(target, vu, use)
     or
     exists(BasicBlock bb, int i |
@@ -202,23 +223,40 @@ module DefUse {
 
   /** Holds if the update `def` can be used at the read `use`. */
   cached
-  predicate defUse(StackVariable target, Expr def, ReadAccess use) {
+  deprecated predicate defUse(StackVariable target, Expr def, ReadAccess use) {
     exists(VariableUpdate vu | def = vu.getSource() | variableUpdateUse(target, vu, use))
   }
 }
 
-private import DefUse
-
-abstract library class VariableUpdate extends Instruction {
-  abstract Expr getSource();
+/** A node that updates a variable. */
+abstract class VariableUpdate extends DataFlowNode {
+  /** Gets the value assigned, if any. */
+  abstract DataFlowNode getSource();
 
+  /** Gets the variable that is updated. */
   abstract Variable getVariable();
+
+  /** Holds if this variable update happens at index `i` in basic block `bb`. */
+  abstract predicate updatesAt(BasicBlock bb, int i);
+}
+
+private class MethodParameterDef extends VariableUpdate, MethodParameter {
+  override MethodParameter getSource() { result = this }
+
+  override MethodParameter getVariable() { result = this }
+
+  override predicate updatesAt(BasicBlock bb, int i) {
+    bb.(EntryBasicBlock).getANode().getImplementation().getMethod() = this.getMethod() and
+    i = -1
+  }
 }
 
 private class VariableWrite extends VariableUpdate, WriteAccess {
-  override Expr getSource() { result = getExpr() }
+  override Expr getSource() { result = this.getExpr() }
 
-  override Variable getVariable() { result = getTarget() }
+  override Variable getVariable() { result = this.getTarget() }
+
+  override predicate updatesAt(BasicBlock bb, int i) { this = bb.getNode(i) }
 }
 
 private class MethodOutOrRefTarget extends VariableUpdate, Call {
@@ -230,5 +268,7 @@ private class MethodOutOrRefTarget extends VariableUpdate, Call {
     result = this.getRawArgument(parameterIndex).(ReadAccess).getTarget()
   }
 
-  override Expr getSource() { result = this }
+  override Expr getSource() { none() }
+
+  override predicate updatesAt(BasicBlock bb, int i) { this = bb.getNode(i) }
 }

csharp/ql/src/semmle/code/cil/Method.qll

@@ -19,7 +19,7 @@ class MethodImplementation extends EntryPoint, @cil_method_implementation {
   override MethodImplementation getImplementation() { result = this }
 
   /** Gets the location of this implementation. */
-  Assembly getLocation() { cil_method_implementation(this, _, result) }
+  override Assembly getLocation() { cil_method_implementation(this, _, result) }
 
   /** Gets the instruction at index `index`. */
   Instruction getInstruction(int index) { cil_instruction(result, _, index, this) }

csharp/ql/src/semmle/code/cil/Ssa.qll

@@ -0,0 +1,66 @@
+/**
+ * Provides the module `Ssa` for working with static single assignment (SSA) form.
+ */
+
+private import CIL
+
+/**
+ * Provides classes for working with static single assignment (SSA) form.
+ */
+module Ssa {
+  private import internal.SsaImplCommon as SsaImpl
+  private import internal.SsaImpl
+
+  /** An SSA definition. */
+  class Definition extends SsaImpl::Definition {
+    /** Gets a read of this SSA definition. */
+    final ReadAccess getARead() { result = getARead(this) }
+
+    /** Gets the underlying variable update, if any. */
+    final VariableUpdate getVariableUpdate() {
+      exists(BasicBlock bb, int i |
+        result.updatesAt(bb, i) and
+        this.definesAt(result.getVariable(), bb, i)
+      )
+    }
+
+    /** Gets a first read of this SSA definition. */
+    final ReadAccess getAFirstRead() { result = getAFirstRead(this) }
+
+    /** Holds if `first` and `second` are adjacent reads of this SSA definition. */
+    final predicate hasAdjacentReads(ReadAccess first, ReadAccess second) {
+      hasAdjacentReads(this, first, second)
+    }
+
+    private Definition getAPhiInput() { result = this.(PhiNode).getAnInput() }
+
+    /**
+     * Gets a definition that ultimately defines this SSA definition and is
+     * not itself a phi node.
+     */
+    final Definition getAnUltimateDefinition() {
+      result = this.getAPhiInput*() and
+      not result instanceof PhiNode
+    }
+
+    /** Gets the location of this SSA definition. */
+    Location getLocation() { result = this.getVariableUpdate().getLocation() }
+  }
+
+  /** A phi node. */
+  class PhiNode extends SsaImpl::PhiNode, Definition {
+    final override Location getLocation() { result = this.getBasicBlock().getLocation() }
+
+    /** Gets an input to this phi node. */
+    final Definition getAnInput() { result = getAPhiInput(this) }
+
+    /**
+     * Holds if if `def` is an input to this phi node, and a reference to `def` at
+     * index `i` in basic block `bb` can reach this phi node without going through
+     * other references.
+     */
+    final predicate hasLastInputRef(Definition def, BasicBlock bb, int i) {
+      hasLastInputRef(this, def, bb, i)
+    }
+  }
+}

csharp/ql/src/semmle/code/cil/internal/SsaImpl.qll

@@ -0,0 +1,45 @@
+private import semmle.code.cil.CIL
+private import SsaImplCommon
+
+cached
+private module Cached {
+  cached
+  predicate forceCachingInSameStage() { any() }
+
+  cached
+  ReadAccess getARead(Definition def) {
+    exists(BasicBlock bb, int i |
+      ssaDefReachesRead(_, def, bb, i) and
+      result = bb.getNode(i)
+    )
+  }
+
+  cached
+  ReadAccess getAFirstRead(Definition def) {
+    exists(BasicBlock bb1, int i1, BasicBlock bb2, int i2 |
+      def.definesAt(_, bb1, i1) and
+      adjacentDefRead(def, bb1, i1, bb2, i2) and
+      result = bb2.getNode(i2)
+    )
+  }
+
+  cached
+  predicate hasAdjacentReads(Definition def, ReadAccess first, ReadAccess second) {
+    exists(BasicBlock bb1, int i1, BasicBlock bb2, int i2 |
+      first = bb1.getNode(i1) and
+      adjacentDefRead(def, bb1, i1, bb2, i2) and
+      second = bb2.getNode(i2)
+    )
+  }
+
+  cached
+  Definition getAPhiInput(PhiNode phi) { phiHasInputFromBlock(phi, result, _) }
+
+  cached
+  predicate hasLastInputRef(Definition phi, Definition def, BasicBlock bb, int i) {
+    lastRefRedef(def, bb, i, phi) and
+    def = getAPhiInput(phi)
+  }
+}
+
+import Cached