C#: Add more guard implication steps

2026-04-29 18:55:14 +02:00 · 2018-12-05 15:14:36 +01:00
parent 078dc7b6c0
commit e2f271bddb
3 changed files with 65 additions and 3 deletions
--- a/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll
+++ b/csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll
@@ -694,7 +694,7 @@ module Internal {
    predicate preControls(PreBasicBlocks::PreBasicBlock bb, AbstractValue v) {
      exists(AbstractValue v0, Guard g |
        g.preControlsDirect(bb, v0) |
-        impliesSteps(g, v0, this, v)
+        preImpliesSteps(g, v0, this, v)
      )
    }

@@ -781,7 +781,7 @@ module Internal {
      def.nullGuardedReturn(ret, isNull)
      or
      exists(NullValue nv |
-        impliesSteps(ret, retVal, def.getARead(), nv) |
+        preImpliesSteps(ret, retVal, def.getARead(), nv) |
        if nv.isNull() then isNull = true else isNull = false
      )
    )
@@ -1123,6 +1123,28 @@ module Internal {
        def = guarded.getAnSsaQualifier()
      )
    }
+
+    /**
+     * Holds if the assumption that `g1` has abstract value `v1` implies that
+     * `g2` has abstract value `v2`, using one step of reasoning. That is, the
+     * evaluation of `g2` to `v2` dominates the evaluation of `g1` to `v1`.
+     *
+     * This predicate relies on the control flow graph.
+     */
+    cached
+    predicate impliesStep(Guard g1, AbstractValue v1, Guard g2, AbstractValue v2) {
+      preImpliesStep(g1, v1, g2, v2)
+      or
+      forex(ControlFlow::Node cfn |
+        cfn = g1.getAControlFlowNode() |
+        exists(Ssa::ExplicitDefinition def |
+          def.getADefinition().getSource() = g2 |
+          g1 = def.getAReadAtNode(cfn) and
+          v1 = g1.getAValue() and
+          v2 = v1
+        )
+      )
+    }
  }
  import Cached

@@ -1143,9 +1165,11 @@ module Internal {
     * Holds if the assumption that `g1` has abstract value `v1` implies that
     * `g2` has abstract value `v2`, using one step of reasoning. That is, the
     * evaluation of `g2` to `v2` dominates the evaluation of `g1` to `v1`.
+     *
+     * This predicate does not rely on the control flow graph.
     */
    cached
-    predicate impliesStep(Guard g1, AbstractValue v1, Guard g2, AbstractValue v2) {
+    predicate preImpliesStep(Guard g1, AbstractValue v1, Guard g2, AbstractValue v2) {
      g1 = any(BinaryOperation bo |
        (
          bo instanceof BitwiseAndExpr or
@@ -1279,6 +1303,26 @@ module Internal {
   * Holds if the assumption that `g1` has abstract value `v1` implies that
   * `g2` has abstract value `v2`, using zero or more steps of reasoning. That is,
   * the evaluation of `g2` to `v2` dominates the evaluation of `g1` to `v1`.
+   *
+   * This predicate does not rely on the control flow graph.
+   */
+  predicate preImpliesSteps(Guard g1, AbstractValue v1, Guard g2, AbstractValue v2) {
+    g1 = g2 and
+    v1 = v2 and
+    v1 = g1.getAValue()
+    or
+    exists(Expr mid, AbstractValue vMid |
+      preImpliesSteps(g1, v1, mid, vMid) |
+      preImpliesStep(mid, vMid, g2, v2)
+    )
+  }
+
+  /**
+   * Holds if the assumption that `g1` has abstract value `v1` implies that
+   * `g2` has abstract value `v2`, using zero or more steps of reasoning. That is,
+   * the evaluation of `g2` to `v2` dominates the evaluation of `g1` to `v1`.
+   *
+   * This predicate relies on the control flow graph.
   */
  predicate impliesSteps(Guard g1, AbstractValue v1, Guard g2, AbstractValue v2) {
    g1 = g2 and
--- a/csharp/ql/test/library-tests/controlflow/guards/Implications.expected
+++ b/csharp/ql/test/library-tests/controlflow/guards/Implications.expected
@@ -251,6 +251,10 @@
 | Guards.cs:106:9:106:25 | ... = ... | non-null | Guards.cs:106:22:106:25 | null | non-null |
 | Guards.cs:106:9:106:25 | ... = ... | null | Guards.cs:106:9:106:18 | access to property Property | null |
 | Guards.cs:106:9:106:25 | ... = ... | null | Guards.cs:106:22:106:25 | null | null |
+| Guards.cs:107:27:107:36 | access to property Property | non-null | Guards.cs:106:22:106:25 | null | non-null |
+| Guards.cs:107:27:107:36 | access to property Property | null | Guards.cs:106:22:106:25 | null | null |
+| Guards.cs:108:27:108:36 | access to property Property | non-null | Guards.cs:106:22:106:25 | null | non-null |
+| Guards.cs:108:27:108:36 | access to property Property | null | Guards.cs:106:22:106:25 | null | null |
 | Guards.cs:113:13:114:38 | String dummy = ... | non-null | Guards.cs:113:13:113:17 | access to local variable dummy | non-null |
 | Guards.cs:113:13:114:38 | String dummy = ... | null | Guards.cs:113:13:113:17 | access to local variable dummy | null |
 | Guards.cs:115:9:115:55 | ... = ... | non-null | Guards.cs:115:9:115:13 | access to local variable dummy | non-null |
@@ -261,6 +265,10 @@
 | Guards.cs:117:9:117:25 | ... = ... | non-null | Guards.cs:117:22:117:25 | null | non-null |
 | Guards.cs:117:9:117:25 | ... = ... | null | Guards.cs:117:9:117:18 | access to property Property | null |
 | Guards.cs:117:9:117:25 | ... = ... | null | Guards.cs:117:22:117:25 | null | null |
+| Guards.cs:118:27:118:36 | access to property Property | non-null | Guards.cs:117:22:117:25 | null | non-null |
+| Guards.cs:118:27:118:36 | access to property Property | null | Guards.cs:117:22:117:25 | null | null |
+| Guards.cs:119:27:119:36 | access to property Property | non-null | Guards.cs:117:22:117:25 | null | non-null |
+| Guards.cs:119:27:119:36 | access to property Property | null | Guards.cs:117:22:117:25 | null | null |
 | Guards.cs:124:13:124:30 | Boolean b1 = ... | false | Guards.cs:124:13:124:14 | access to local variable b1 | false |
 | Guards.cs:124:13:124:30 | Boolean b1 = ... | true | Guards.cs:124:13:124:14 | access to local variable b1 | true |
 | Guards.cs:125:13:125:31 | Nullable<Boolean> b2 = ... | non-null | Guards.cs:125:13:125:14 | access to local variable b2 | non-null |
--- a/csharp/ql/test/query-tests/Nullness/Implications.expected
+++ b/csharp/ql/test/query-tests/Nullness/Implications.expected
@@ -930,6 +930,8 @@
 | D.cs:187:13:187:28 | ... = ... | non-null | D.cs:187:20:187:28 | call to method MkMaybe | non-null |
 | D.cs:187:13:187:28 | ... = ... | null | D.cs:187:13:187:16 | access to local variable obj3 | null |
 | D.cs:187:13:187:28 | ... = ... | null | D.cs:187:20:187:28 | call to method MkMaybe | null |
+| D.cs:190:9:190:12 | access to local variable obj3 | non-null | D.cs:187:20:187:28 | call to method MkMaybe | non-null |
+| D.cs:190:9:190:12 | access to local variable obj3 | null | D.cs:187:20:187:28 | call to method MkMaybe | null |
 | D.cs:195:13:195:28 | Object o = ... | non-null | D.cs:195:13:195:13 | access to local variable o | non-null |
 | D.cs:195:13:195:28 | Object o = ... | null | D.cs:195:13:195:13 | access to local variable o | null |
 | D.cs:196:13:196:13 | access to local variable o | non-null | D.cs:195:17:195:28 | object creation of type Object | non-null |
@@ -985,6 +987,8 @@
 | D.cs:230:13:230:28 | ... = ... | non-null | D.cs:230:17:230:28 | object creation of type Object | non-null |
 | D.cs:230:13:230:28 | ... = ... | null | D.cs:230:13:230:13 | access to local variable o | null |
 | D.cs:230:13:230:28 | ... = ... | null | D.cs:230:17:230:28 | object creation of type Object | null |
+| D.cs:232:13:232:13 | access to local variable o | non-null | D.cs:230:17:230:28 | object creation of type Object | non-null |
+| D.cs:232:13:232:13 | access to local variable o | null | D.cs:230:17:230:28 | object creation of type Object | null |
 | D.cs:234:9:234:16 | ... = ... | non-null | D.cs:234:9:234:9 | access to local variable o | non-null |
 | D.cs:234:9:234:16 | ... = ... | non-null | D.cs:234:13:234:16 | null | non-null |
 | D.cs:234:9:234:16 | ... = ... | null | D.cs:234:9:234:9 | access to local variable o | null |
@@ -993,6 +997,8 @@
 | D.cs:236:13:236:18 | ... = ... | non-null | D.cs:236:17:236:18 | "" | non-null |
 | D.cs:236:13:236:18 | ... = ... | null | D.cs:236:13:236:13 | access to local variable o | null |
 | D.cs:236:13:236:18 | ... = ... | null | D.cs:236:17:236:18 | "" | null |
+| D.cs:238:13:238:13 | access to local variable o | non-null | D.cs:236:17:236:18 | "" | non-null |
+| D.cs:238:13:238:13 | access to local variable o | null | D.cs:236:17:236:18 | "" | null |
 | D.cs:240:9:240:16 | ... = ... | non-null | D.cs:240:9:240:9 | access to local variable o | non-null |
 | D.cs:240:9:240:16 | ... = ... | non-null | D.cs:240:13:240:16 | null | non-null |
 | D.cs:240:9:240:16 | ... = ... | null | D.cs:240:9:240:9 | access to local variable o | null |
@@ -1410,12 +1416,16 @@
 | E.cs:217:13:217:20 | ... = ... | non-null | E.cs:217:17:217:20 | null | non-null |
 | E.cs:217:13:217:20 | ... = ... | null | E.cs:217:13:217:13 | access to local variable x | null |
 | E.cs:217:13:217:20 | ... = ... | null | E.cs:217:17:217:20 | null | null |
+| E.cs:220:13:220:13 | access to local variable x | non-null | E.cs:217:17:217:20 | null | non-null |
+| E.cs:220:13:220:13 | access to local variable x | null | E.cs:217:17:217:20 | null | null |
 | E.cs:225:13:225:18 | String x = ... | non-null | E.cs:225:13:225:13 | access to local variable x | non-null |
 | E.cs:225:13:225:18 | String x = ... | null | E.cs:225:13:225:13 | access to local variable x | null |
 | E.cs:227:13:227:20 | ... = ... | non-null | E.cs:227:13:227:13 | access to local variable x | non-null |
 | E.cs:227:13:227:20 | ... = ... | non-null | E.cs:227:17:227:20 | null | non-null |
 | E.cs:227:13:227:20 | ... = ... | null | E.cs:227:13:227:13 | access to local variable x | null |
 | E.cs:227:13:227:20 | ... = ... | null | E.cs:227:17:227:20 | null | null |
+| E.cs:229:13:229:13 | access to local variable x | non-null | E.cs:227:17:227:20 | null | non-null |
+| E.cs:229:13:229:13 | access to local variable x | null | E.cs:227:17:227:20 | null | null |
 | E.cs:245:13:245:22 | access to property HasValue | false | E.cs:245:13:245:13 | access to parameter i | null |
 | E.cs:245:13:245:22 | access to property HasValue | true | E.cs:245:13:245:13 | access to parameter i | non-null |
 | E.cs:252:13:252:21 | ... != ... | false | E.cs:252:13:252:13 | access to parameter i | null |