Skip to content

Commit 527a099

Browse files
hvitvedhvitved
committed
C#: Fix CFG for conditional method calls with out parameters
1 parent 090205d commit 527a099

6 files changed

Lines changed: 25 additions & 8 deletions

File tree

‎csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,6 @@ module ControlFlow {
517517
e =
518518
any(QualifiableExpr qe |
519519
not qe instanceof ExtensionMethodCall and
520-
not qe.isConditional() and
521520
result = qe.getChild(i)
522521
)
523522
or
@@ -656,7 +655,7 @@ module ControlFlow {
656655
cfe =
657656
any(AssignOperationWithExpandedAssignment a | result = first(a.getExpandedAssignment()))
658657
or
659-
cfe = any(ConditionallyQualifiedExpr cqe | result = first(cqe.getChildExpr(-1)))
658+
cfe = any(ConditionallyQualifiedExpr cqe | result = first(getExprChildElement(cqe, 0)))
660659
or
661660
cfe =
662661
any(ArrayCreation ac |
@@ -882,7 +881,7 @@ module ControlFlow {
882881
c = getValidSelfCompletion(result)
883882
or
884883
// Qualifier exits with a `null` completion
885-
result = cqe.getChildExpr(-1) and
884+
result = getExprChildElement(cqe, 0) and
886885
c = TRec(TLastRecSpecificCompletion(any(NullnessCompletion nc | nc.isNull())))
887886
)
888887
or
@@ -1454,16 +1453,16 @@ module ControlFlow {
14541453
)
14551454
or
14561455
exists(ConditionallyQualifiedExpr parent, int i |
1457-
cfe = last(parent.getChildExpr(i), c) and
1456+
cfe = last(getExprChildElement(parent, i), c) and
14581457
c instanceof NormalCompletion and
1459-
not c.(NullnessCompletion).isNull()
1458+
if i = 0 then c.(NullnessCompletion).isNonNull() else any()
14601459
|
14611460
// Post-order: flow from last element of last child to element itself
1462-
i = max(int j | exists(parent.getChildExpr(j))) and
1461+
i = max(int j | exists(getExprChildElement(parent, j))) and
14631462
result = parent
14641463
or
14651464
// Standard left-to-right evaluation
1466-
result = first(parent.getChildExpr(i + 1))
1465+
result = first(getExprChildElement(parent, i + 1))
14671466
)
14681467
or
14691468
// Post-order: flow from last element of thrown expression to expression itself

‎csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@
8888
| ConditionalAccess.cs:21:10:21:11 | enter M7 | ConditionalAccess.cs:21:10:21:11 | exit M7 | 17 |
8989
| ConditionalAccess.cs:30:10:30:12 | enter Out | ConditionalAccess.cs:30:28:30:32 | ... = ... | 3 |
9090
| ConditionalAccess.cs:30:10:30:12 | exit Out | ConditionalAccess.cs:30:10:30:12 | exit Out | 1 |
91-
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:32:10:32:11 | exit M8 | 9 |
91+
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:35:9:35:12 | access to property Prop | 8 |
92+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:32:10:32:11 | exit M8 | 1 |
93+
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:35:14:35:24 | call to method Out | 1 |
9294
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | exit CommaJoinWith | 7 |
9395
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:5:13:5:15 | access to parameter inc | 4 |
9496
| Conditions.cs:3:10:3:19 | exit IncrOrDecr | Conditions.cs:3:10:3:19 | exit IncrOrDecr | 1 |

‎csharp/ql/test/library-tests/controlflow/graph/Condition.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ conditionBlock
5353
| ConditionalAccess.cs:13:25:13:25 | 0 | ConditionalAccess.cs:14:20:14:20 | 0 | true |
5454
| ConditionalAccess.cs:13:25:13:25 | 0 | ConditionalAccess.cs:16:20:16:20 | 1 | false |
5555
| ConditionalAccess.cs:19:12:19:13 | enter M6 | ConditionalAccess.cs:19:58:19:59 | access to parameter s2 | false |
56+
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:35:14:35:24 | call to method Out | false |
5657
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:6:13:6:16 | [inc (line 3): true] ...; | true |
5758
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:7:9:8:16 | [inc (line 3): false] if (...) ... | false |
5859
| Conditions.cs:11:9:11:10 | enter M1 | Conditions.cs:15:13:15:16 | [b (line 11): true] ...; | true |

‎csharp/ql/test/library-tests/controlflow/graph/Dominance.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ dominance
536536
| ConditionalAccess.cs:34:9:34:14 | ...; | ConditionalAccess.cs:34:13:34:13 | 0 |
537537
| ConditionalAccess.cs:34:13:34:13 | 0 | ConditionalAccess.cs:34:9:34:13 | ... = ... |
538538
| ConditionalAccess.cs:35:9:35:12 | access to property Prop | ConditionalAccess.cs:32:10:32:11 | exit M8 |
539+
| ConditionalAccess.cs:35:9:35:12 | access to property Prop | ConditionalAccess.cs:35:14:35:24 | call to method Out |
539540
| ConditionalAccess.cs:35:9:35:12 | this access | ConditionalAccess.cs:35:9:35:12 | access to property Prop |
540541
| ConditionalAccess.cs:35:9:35:25 | ...; | ConditionalAccess.cs:35:9:35:12 | this access |
541542
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:70:41:71 | access to parameter s1 |
@@ -3601,6 +3602,7 @@ postDominance
36013602
| ConditionalAccess.cs:30:32:30:32 | 0 | ConditionalAccess.cs:1:7:1:23 | enter ConditionalAccess |
36023603
| ConditionalAccess.cs:30:32:30:32 | 0 | ConditionalAccess.cs:30:10:30:12 | enter Out |
36033604
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:35:9:35:12 | access to property Prop |
3605+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:35:14:35:24 | call to method Out |
36043606
| ConditionalAccess.cs:33:5:36:5 | {...} | ConditionalAccess.cs:32:10:32:11 | enter M8 |
36053607
| ConditionalAccess.cs:34:9:34:13 | ... = ... | ConditionalAccess.cs:34:13:34:13 | 0 |
36063608
| ConditionalAccess.cs:34:9:34:14 | ...; | ConditionalAccess.cs:33:5:36:5 | {...} |
@@ -6266,6 +6268,10 @@ blockDominance
62666268
| ConditionalAccess.cs:30:10:30:12 | enter Out | ConditionalAccess.cs:30:10:30:12 | exit Out |
62676269
| ConditionalAccess.cs:30:10:30:12 | exit Out | ConditionalAccess.cs:30:10:30:12 | exit Out |
62686270
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:32:10:32:11 | enter M8 |
6271+
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:32:10:32:11 | exit M8 |
6272+
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:35:14:35:24 | call to method Out |
6273+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:32:10:32:11 | exit M8 |
6274+
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:35:14:35:24 | call to method Out |
62696275
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith |
62706276
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:3:10:3:19 | enter IncrOrDecr |
62716277
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:3:10:3:19 | exit IncrOrDecr |
@@ -8438,6 +8444,10 @@ postBlockDominance
84388444
| ConditionalAccess.cs:30:10:30:12 | exit Out | ConditionalAccess.cs:30:10:30:12 | enter Out |
84398445
| ConditionalAccess.cs:30:10:30:12 | exit Out | ConditionalAccess.cs:30:10:30:12 | exit Out |
84408446
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:32:10:32:11 | enter M8 |
8447+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:32:10:32:11 | enter M8 |
8448+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:32:10:32:11 | exit M8 |
8449+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:35:14:35:24 | call to method Out |
8450+
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:35:14:35:24 | call to method Out |
84418451
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith |
84428452
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:3:10:3:19 | enter IncrOrDecr |
84438453
| Conditions.cs:3:10:3:19 | exit IncrOrDecr | Conditions.cs:3:10:3:19 | enter IncrOrDecr |

‎csharp/ql/test/library-tests/controlflow/graph/EnclosingCallable.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,7 @@ nodeEnclosing
583583
| ConditionalAccess.cs:35:9:35:12 | access to property Prop | ConditionalAccess.cs:32:10:32:11 | M8 |
584584
| ConditionalAccess.cs:35:9:35:12 | this access | ConditionalAccess.cs:32:10:32:11 | M8 |
585585
| ConditionalAccess.cs:35:9:35:25 | ...; | ConditionalAccess.cs:32:10:32:11 | M8 |
586+
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:32:10:32:11 | M8 |
586587
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | CommaJoinWith |
587588
| ConditionalAccess.cs:41:26:41:38 | exit CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | CommaJoinWith |
588589
| ConditionalAccess.cs:41:70:41:71 | access to parameter s1 | ConditionalAccess.cs:41:26:41:38 | CommaJoinWith |
@@ -3410,6 +3411,8 @@ blockEnclosing
34103411
| ConditionalAccess.cs:30:10:30:12 | enter Out | ConditionalAccess.cs:30:10:30:12 | Out |
34113412
| ConditionalAccess.cs:30:10:30:12 | exit Out | ConditionalAccess.cs:30:10:30:12 | Out |
34123413
| ConditionalAccess.cs:32:10:32:11 | enter M8 | ConditionalAccess.cs:32:10:32:11 | M8 |
3414+
| ConditionalAccess.cs:32:10:32:11 | exit M8 | ConditionalAccess.cs:32:10:32:11 | M8 |
3415+
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:32:10:32:11 | M8 |
34133416
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:26:41:38 | CommaJoinWith |
34143417
| Conditions.cs:3:10:3:19 | enter IncrOrDecr | Conditions.cs:3:10:3:19 | IncrOrDecr |
34153418
| Conditions.cs:3:10:3:19 | exit IncrOrDecr | Conditions.cs:3:10:3:19 | IncrOrDecr |

‎csharp/ql/test/library-tests/controlflow/graph/NodeGraph.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,10 @@
567567
| ConditionalAccess.cs:34:9:34:14 | ...; | ConditionalAccess.cs:34:13:34:13 | 0 | semmle.label | successor |
568568
| ConditionalAccess.cs:34:13:34:13 | 0 | ConditionalAccess.cs:34:9:34:13 | ... = ... | semmle.label | successor |
569569
| ConditionalAccess.cs:35:9:35:12 | access to property Prop | ConditionalAccess.cs:32:10:32:11 | exit M8 | semmle.label | null |
570+
| ConditionalAccess.cs:35:9:35:12 | access to property Prop | ConditionalAccess.cs:35:14:35:24 | call to method Out | semmle.label | non-null |
570571
| ConditionalAccess.cs:35:9:35:12 | this access | ConditionalAccess.cs:35:9:35:12 | access to property Prop | semmle.label | successor |
571572
| ConditionalAccess.cs:35:9:35:25 | ...; | ConditionalAccess.cs:35:9:35:12 | this access | semmle.label | successor |
573+
| ConditionalAccess.cs:35:14:35:24 | call to method Out | ConditionalAccess.cs:32:10:32:11 | exit M8 | semmle.label | successor |
572574
| ConditionalAccess.cs:41:26:41:38 | enter CommaJoinWith | ConditionalAccess.cs:41:70:41:71 | access to parameter s1 | semmle.label | successor |
573575
| ConditionalAccess.cs:41:70:41:71 | access to parameter s1 | ConditionalAccess.cs:41:75:41:78 | ", " | semmle.label | successor |
574576
| ConditionalAccess.cs:41:70:41:78 | ... + ... | ConditionalAccess.cs:41:82:41:83 | access to parameter s2 | semmle.label | successor |

0 commit comments

Comments
 (0)