C#: Misc Guards-related cleanup.

csharp/ql/lib/semmle/code/csharp/Caching.qll

@@ -33,8 +33,6 @@ module Stages {
 
     cached
     private predicate forceCachingInSameStageRev() {
-      any(ControlFlowElement cfe).controlsBlock(_, _, _)
-      or
       exists(GuardedExpr ge)
       or
       forceCachingInSameStageRev()

csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowElement.qll

@@ -87,148 +87,20 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
       result.getAControlFlowNode()
   }
 
-  pragma[noinline]
-  private predicate immediatelyControlsBlockSplit0(
-    ConditionBlock cb, BasicBlock succ, ConditionalSuccessor s
-  ) {
-    // Only calculate dominance by explicit recursion for split nodes;
-    // all other nodes can use regular CFG dominance
-    this instanceof Impl::SplitAstNode and
-    cb.getLastNode() = this.getAControlFlowNode() and
-    succ = cb.getASuccessor(s)
-  }
-
-  pragma[noinline]
-  private predicate immediatelyControlsBlockSplit1(
-    ConditionBlock cb, BasicBlock succ, ConditionalSuccessor s, BasicBlock pred, SuccessorType t
-  ) {
-    this.immediatelyControlsBlockSplit0(cb, succ, s) and
-    pred = succ.getAPredecessorByType(t) and
-    pred != cb
-  }
-
-  pragma[noinline]
-  private predicate immediatelyControlsBlockSplit2(
-    ConditionBlock cb, BasicBlock succ, ConditionalSuccessor s, BasicBlock pred, SuccessorType t
-  ) {
-    this.immediatelyControlsBlockSplit1(cb, succ, s, pred, t) and
-    (
-      succ.dominates(pred)
-      or
-      // `pred` might be another split of this element
-      pred.getLastNode().getAstNode() = this and
-      t = s
-    )
-  }
-
   /**
-   * Holds if basic block `succ` is immediately controlled by this control flow
-   * element with conditional value `s`. That is, `succ` can only be reached from
-   * the callable entry point by going via the `s` edge out of *some* basic block
-   * `pred` ending with this element, and `pred` is an immediate predecessor
-   * of `succ`.
-   *
-   * Moreover, this control flow element corresponds to multiple control flow nodes,
-   * which is why
-   *
-   * ```ql
-   * exists(ConditionBlock cb |
-   *   cb.getLastNode() = this.getAControlFlowNode() |
-   *   cb.immediatelyControls(succ, s)
-   * )
-   * ```
+   * DEPRECATED: Use `Guard` class instead.
    *
-   * does not work.
-   *
-   * `cb` records all of the possible condition blocks for this control flow element
-   * that a path from the callable entry point to `succ` may go through.
-   */
-  pragma[nomagic]
-  private predicate immediatelyControlsBlockSplit(
-    BasicBlock succ, ConditionalSuccessor s, ConditionBlock cb
-  ) {
-    this.immediatelyControlsBlockSplit0(cb, succ, s) and
-    forall(BasicBlock pred, SuccessorType t |
-      this.immediatelyControlsBlockSplit1(cb, succ, s, pred, t)
-    |
-      this.immediatelyControlsBlockSplit2(cb, succ, s, pred, t)
-    )
-  }
-
-  pragma[noinline]
-  private predicate controlsJoinBlockPredecessor(
-    JoinBlock controlled, ConditionalSuccessor s, int i, ConditionBlock cb
-  ) {
-    this.controlsBlockSplit(controlled.getJoinBlockPredecessor(i), s, cb)
-  }
-
-  private predicate controlsJoinBlockSplit(JoinBlock controlled, ConditionalSuccessor s, int i) {
-    i = -1 and
-    this.controlsJoinBlockPredecessor(controlled, s, _, _)
-    or
-    this.controlsJoinBlockSplit(controlled, s, i - 1) and
-    (
-      this.controlsJoinBlockPredecessor(controlled, s, i, _)
-      or
-      controlled.dominates(controlled.getJoinBlockPredecessor(i))
-    )
-  }
-
-  cached
-  private predicate controlsBlockSplit(
-    BasicBlock controlled, ConditionalSuccessor s, ConditionBlock cb
-  ) {
-    Stages::GuardsStage::forceCachingInSameStage() and
-    this.immediatelyControlsBlockSplit(controlled, s, cb)
-    or
-    // Equivalent with
-    //
-    // ```ql
-    // exists(JoinBlockPredecessor pred | pred = controlled.getAPredecessor() |
-    //   this.controlsBlockSplit(pred, s)
-    // ) and
-    // forall(JoinBlockPredecessor pred | pred = controlled.getAPredecessor() |
-    //   this.controlsBlockSplit(pred, s)
-    //   or
-    //   controlled.dominates(pred)
-    // )
-    // ```
-    //
-    // but uses no universal recursion for better performance.
-    exists(int last |
-      last = max(int i | exists(controlled.(JoinBlock).getJoinBlockPredecessor(i)))
-    |
-      this.controlsJoinBlockSplit(controlled, s, last)
-    ) and
-    this.controlsJoinBlockPredecessor(controlled, s, _, cb)
-    or
-    not controlled instanceof JoinBlock and
-    this.controlsBlockSplit(controlled.getAPredecessor(), s, cb)
-  }
-
-  /**
    * Holds if basic block `controlled` is controlled by this control flow element
    * with conditional value `s`. That is, `controlled` can only be reached from
    * the callable entry point by going via the `s` edge out of *some* basic block
    * ending with this element.
    *
-   * This predicate is different from
-   *
-   * ```ql
-   * exists(ConditionBlock cb |
-   *   cb.getLastNode() = this.getAControlFlowNode() |
-   *   cb.controls(controlled, s)
-   * )
-   * ```
-   *
-   * as control flow splitting is taken into account.
-   *
    * `cb` records all of the possible condition blocks for this control flow element
    * that a path from the callable entry point to `controlled` may go through.
    */
-  predicate controlsBlock(BasicBlock controlled, ConditionalSuccessor s, ConditionBlock cb) {
-    this.controlsBlockSplit(controlled, s, cb)
-    or
+  deprecated predicate controlsBlock(
+    BasicBlock controlled, ConditionalSuccessor s, ConditionBlock cb
+  ) {
     cb.getLastNode() = this.getAControlFlowNode() and
     cb.edgeDominates(controlled, s)
   }

csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll

@@ -49,8 +49,15 @@ private module GuardsInput implements
     override predicate isNull() { any() }
   }
 
-  private class BooleanConstant extends ConstantExpr instanceof BoolLiteral {
-    override boolean asBooleanValue() { result = super.getBoolValue() }
+  private predicate boolConst(Expr e, boolean b) {
+    e.getType() instanceof BoolType and
+    e.getValue() = b.toString()
+  }
+
+  private class BooleanConstant extends ConstantExpr {
+    BooleanConstant() { boolConst(this, _) }
+
+    override boolean asBooleanValue() { boolConst(this, result) }
   }
 
   private predicate intConst(Expr e, int i) {

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -2583,10 +2583,10 @@ class NodeRegion instanceof ControlFlow::BasicBlock {
  * Holds if the nodes in `nr` are unreachable when the call context is `call`.
  */
 predicate isUnreachableInCall(NodeRegion nr, DataFlowCall call) {
-  exists(ExplicitParameterNode paramNode, Guard guard, ControlFlow::BooleanSuccessor bs |
-    viableConstantBooleanParamArg(paramNode, bs.getValue().booleanNot(), call) and
+  exists(ExplicitParameterNode paramNode, Guard guard, GuardValue val |
+    viableConstantParamArg(paramNode, val.getDualValue(), call) and
     paramNode.getSsaDefinition().getARead() = guard and
-    guard.controlsBlock(nr, bs, _)
+    guard.valueControls(nr, val)
   )
 }
 
@@ -2904,33 +2904,19 @@ class CastNode extends Node {
 
 class DataFlowExpr = Expr;
 
-/** Holds if `e` is an expression that always has the same Boolean value `val`. */
-private predicate constantBooleanExpr(Expr e, boolean val) {
-  e.getType() instanceof BoolType and
-  e.getValue() = val.toString()
-  or
-  exists(Ssa::ExplicitDefinition def, Expr src |
-    e = def.getARead() and
-    src = def.getADefinition().getSource() and
-    constantBooleanExpr(src, val)
-  )
-}
+/** An argument that always has the same value. */
+private class ConstantArgumentNode extends ExprNode {
+  ConstantArgumentNode() { Guards::InternalUtil::exprHasValue(this.(ArgumentNode).asExpr(), _) }
 
-/** An argument that always has the same Boolean value. */
-private class ConstantBooleanArgumentNode extends ExprNode {
-  ConstantBooleanArgumentNode() { constantBooleanExpr(this.(ArgumentNode).asExpr(), _) }
-
-  /** Gets the Boolean value of this expression. */
-  boolean getBooleanValue() { constantBooleanExpr(this.getExpr(), result) }
+  /** Gets the value of this expression. */
+  GuardValue getValue() { Guards::InternalUtil::exprHasValue(this.getExpr(), result) }
 }
 
 pragma[noinline]
-private predicate viableConstantBooleanParamArg(
-  ParameterNode paramNode, boolean b, DataFlowCall call
-) {
-  exists(ConstantBooleanArgumentNode arg |
+private predicate viableConstantParamArg(ParameterNode paramNode, GuardValue val, DataFlowCall call) {
+  exists(ConstantArgumentNode arg |
     viableParamArg(call, paramNode, arg) and
-    b = arg.getBooleanValue()
+    val = arg.getValue()
   )
 }
 

csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll

@@ -72,17 +72,10 @@ class ReverseDnsSource extends Source {
   }
 }
 
-pragma[noinline]
-private predicate conditionControlsCall0(
-  SensitiveExecutionMethodCall call, Expr e, ControlFlow::BooleanSuccessor s
-) {
-  forex(BasicBlock bb | bb = call.getAControlFlowNode().getBasicBlock() | e.controlsBlock(bb, s, _))
-}
-
 private predicate conditionControlsCall(
   SensitiveExecutionMethodCall call, SensitiveExecutionMethod def, Expr e, boolean cond
 ) {
-  exists(ControlFlow::BooleanSuccessor s | cond = s.getValue() | conditionControlsCall0(call, e, s)) and
+  e.(Guard).directlyControls(call.getBasicBlock(), cond) and
   def = call.getTarget().getUnboundDeclaration()
 }