Skip to content

Commit 3eb163f

Browse files
committed
C#: Account for split SSA definitions in guards library
On 03e69e9, I updated the guards library to account for control flow graph splitting. However, the logic that relates SSA qualifiers for the guard and the guarded expression was not updated accordingly.
1 parent 1a25f0a commit 3eb163f

File tree

4 files changed

+10
-17
lines changed

4 files changed

+10
-17
lines changed

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

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ class AccessOrCallExpr extends Expr {
285285
Declaration getTarget() { result = target }
286286

287287
/**
288-
* Gets the (non-trivial) SSA definition corresponding to the longest
288+
* Gets a (non-trivial) SSA definition corresponding to the longest
289289
* qualifier chain of this expression, if any.
290290
*
291291
* This includes the case where this expression is itself an access to an
@@ -299,25 +299,23 @@ class AccessOrCallExpr extends Expr {
299299
* x.Foo().Bar(); // SSA qualifier: SSA definition for `x`
300300
* x; // SSA qualifier: SSA definition for `x`
301301
* ```
302+
*
303+
* An expression can have more than one SSA qualifier in the presence
304+
* of control flow splitting.
302305
*/
303-
Ssa::Definition getSsaQualifier() { result = getSsaQualifier(this) }
304-
305-
/**
306-
* Holds if this expression has an SSA qualifier.
307-
*/
308-
predicate hasSsaQualifier() { exists(this.getSsaQualifier()) }
306+
Ssa::Definition getAnSsaQualifier() { result = getAnSsaQualifier(this) }
309307
}
310308

311309
private Declaration getDeclarationTarget(Expr e) {
312310
e = any(AssignableRead ar | result = ar.getTarget()) or
313311
result = e.(Call).getTarget()
314312
}
315313

316-
private Ssa::Definition getSsaQualifier(Expr e) {
314+
private Ssa::Definition getAnSsaQualifier(Expr e) {
317315
e = getATrackedRead(result)
318316
or
319317
not e = getATrackedRead(_) and
320-
result = getSsaQualifier(e.(QualifiableExpr).getQualifier())
318+
result = getAnSsaQualifier(e.(QualifiableExpr).getQualifier())
321319
}
322320

323321
private AssignableRead getATrackedRead(Ssa::Definition def) {
@@ -688,10 +686,9 @@ module Internal {
688686
predicate isGuardedBy(AccessOrCallExpr guarded, Guard g, AccessOrCallExpr sub, AbstractValue v) {
689687
isGuardedBy1(guarded, g, sub, v) and
690688
sub = g.getAChildExpr*() and
691-
(
692-
not guarded.hasSsaQualifier() and not sub.hasSsaQualifier()
693-
or
694-
guarded.getSsaQualifier() = sub.getSsaQualifier()
689+
forall(Ssa::Definition def |
690+
def = sub.getAnSsaQualifier() |
691+
def = guarded.getAnSsaQualifier()
695692
)
696693
}
697694
}

csharp/ql/test/library-tests/controlflow/guards/BooleanGuardedExpr.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,3 @@
8181
| Splitting.cs:119:13:119:13 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
8282
| Splitting.cs:120:16:120:16 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
8383
| Splitting.cs:132:25:132:25 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | false |
84-
| Splitting.cs:133:17:133:17 | access to local variable o | Splitting.cs:128:17:128:25 | ... != ... | Splitting.cs:128:17:128:17 | access to local variable o | true |

csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,5 +192,3 @@
192192
| Splitting.cs:120:16:120:16 | access to parameter o | Splitting.cs:116:22:116:22 | access to parameter o | Splitting.cs:116:22:116:22 | access to parameter o | non-null |
193193
| Splitting.cs:120:16:120:16 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
194194
| Splitting.cs:132:25:132:25 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | false |
195-
| Splitting.cs:133:17:133:17 | access to local variable o | Splitting.cs:128:17:128:17 | access to local variable o | Splitting.cs:128:17:128:17 | access to local variable o | non-null |
196-
| Splitting.cs:133:17:133:17 | access to local variable o | Splitting.cs:128:17:128:25 | ... != ... | Splitting.cs:128:17:128:17 | access to local variable o | true |

csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,3 @@
5353
| Splitting.cs:117:9:117:9 | access to parameter o |
5454
| Splitting.cs:119:13:119:13 | access to parameter o |
5555
| Splitting.cs:120:16:120:16 | access to parameter o |
56-
| Splitting.cs:133:17:133:17 | access to local variable o |

0 commit comments

Comments
 (0)