Merge pull request #1133 from hvitved/csharp/more-performance-tweaks

C#: More performance tweaks

Author: calumgrant

csharp/ql/src/semmle/code/csharp/commons/Constants.qll

@@ -4,18 +4,20 @@ import csharp
 private import semmle.code.csharp.commons.ComparisonTest
 private import semmle.code.csharp.commons.StructuralComparison as StructuralComparison
 
+pragma[noinline]
+private predicate isConstantCondition0(ControlFlow::Node cfn, boolean b) {
+  exists(
+    cfn.getASuccessorByType(any(ControlFlow::SuccessorTypes::BooleanSuccessor t | t.getValue() = b))
+  ) and
+  strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
+}
+
 /**
  * Holds if `e` is a condition that always evaluates to Boolean value `b`.
  */
 predicate isConstantCondition(Expr e, boolean b) {
   forex(ControlFlow::Node cfn | cfn = e.getAControlFlowNode() |
-    exists(
-      cfn
-          .getASuccessorByType(any(ControlFlow::SuccessorTypes::BooleanSuccessor t |
-              t.getValue() = b
-            ))
-    ) and
-    strictcount(ControlFlow::SuccessorType t | exists(cfn.getASuccessorByType(t))) = 1
+    isConstantCondition0(cfn, b)
   )
 }
 

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

@@ -75,6 +75,40 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
     getAControlFlowNode().getBasicBlock().getASuccessor+().getANode() = 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 ControlFlow::Internal::SplitControlFlowElement and
+    cb.getLastNode() = this.getAControlFlowNode() and
+    succ = cb.getASuccessorByType(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().getElement() = 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
@@ -96,19 +130,11 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
    */
   pragma[nomagic]
   private predicate immediatelyControlsBlockSplit(BasicBlock succ, ConditionalSuccessor s) {
-    // Only calculate dominance by explicit recursion for split nodes;
-    // all other nodes can use regular CFG dominance
-    this instanceof ControlFlow::Internal::SplitControlFlowElement and
-    exists(ConditionBlock cb | cb.getLastNode() = this.getAControlFlowNode() |
-      succ = cb.getASuccessorByType(s) and
+    exists(ConditionBlock cb | this.immediatelyControlsBlockSplit0(cb, succ, s) |
       forall(BasicBlock pred, SuccessorType t |
-        pred = succ.getAPredecessorByType(t) and pred != cb
+        this.immediatelyControlsBlockSplit1(cb, succ, s, pred, t)
       |
-        succ.dominates(pred)
-        or
-        // `pred` might be another split of this element
-        pred.getLastNode().getElement() = this and
-        t = s
+        this.immediatelyControlsBlockSplit2(cb, succ, s, pred, t)
       )
     )
   }

csharp/ql/src/semmle/code/csharp/dataflow/Nullness.qll

@@ -198,6 +198,21 @@ private predicate defMaybeNull(Ssa::Definition def, string msg, Element reason)
   )
 }
 
+pragma[noinline]
+private predicate sourceVariableMaybeNull(Ssa::SourceVariable v) {
+  defMaybeNull(v.getAnSsaDefinition(), _, _)
+}
+
+pragma[noinline]
+private predicate defNullImpliesStep0(
+  Ssa::SourceVariable v, Ssa::Definition def1, BasicBlock bb1, BasicBlock bb2
+) {
+  sourceVariableMaybeNull(v) and
+  def1.getSourceVariable() = v and
+  def1.isLiveAtEndOfBlock(bb1) and
+  bb2 = bb1.getASuccessor()
+}
+
 /**
  * Holds if `def1` being `null` in basic block `bb1` implies that `def2` might
  * be `null` in basic block `bb2`. The SSA definitions share the same source
@@ -206,10 +221,7 @@ private predicate defMaybeNull(Ssa::Definition def, string msg, Element reason)
 private predicate defNullImpliesStep(
   Ssa::Definition def1, BasicBlock bb1, Ssa::Definition def2, BasicBlock bb2
 ) {
-  exists(Ssa::SourceVariable v |
-    defMaybeNull(v.getAnSsaDefinition(), _, _) and
-    def1.getSourceVariable() = v
-  |
+  exists(Ssa::SourceVariable v | defNullImpliesStep0(v, def1, bb1, bb2) |
     def2.(Ssa::PseudoDefinition).getAnInput() = def1 and
     bb2 = def2.getBasicBlock()
     or
@@ -219,9 +231,7 @@ private predicate defNullImpliesStep(
       bb2 = def.getBasicBlock()
     )
   ) and
-  def1.isLiveAtEndOfBlock(bb1) and
   not ensureNotNullAt(bb1, _, def1) and
-  bb2 = bb1.getASuccessor() and
   not exists(SuccessorTypes::ConditionalSuccessor s, NullValue nv |
     bb1.getLastNode() = getANullCheck(def1, s, nv).getAControlFlowNode()
   |