Skip to content

Commit 05b9519

Browse files
committed
C#: Handle multiple-field Boolean CFG splitting
The internal pre-SSA library was extended on 3e78c26 to include fields/properties that are local-scope-like. The CFG splitting logic uses ranking of SSA definitions to define an (arbitrary) order of splits, but for fields/properties the implicit entry definition all have the same line and column. In effect, such SSA definitions incorrectly get the same rank. Adding the name of the field/property to the lexicographic ordering resolves the issue.
1 parent 610be85 commit 05b9519

File tree

9 files changed

+145
-1
lines changed

9 files changed

+145
-1
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3638,7 +3638,7 @@ module ControlFlow {
36383638
kind = rank[r](BooleanSplitSubKind kind0 |
36393639
kind0.getEnclosingCallable() = c and
36403640
kind0.startsSplit(_) |
3641-
kind0 order by kind0.getLocation().getStartLine(), kind0.getLocation().getStartColumn()
3641+
kind0 order by kind0.getLocation().getStartLine(), kind0.getLocation().getStartColumn(), kind0.toString()
36423642
)
36433643
)
36443644
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,9 @@
173173
| Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... | Conditions.cs:122:17:122:24 | ... = ... | 6 |
174174
| Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:133:17:133:22 | access to field Field1 | 8 |
175175
| Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true | Conditions.cs:133:17:133:22 | [Field1 (line 129): false] access to field Field1 | 5 |
176+
| Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field2 | 9 |
176177
| Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} | Conditions.cs:135:21:135:26 | [Field1 (line 129): true] access to field Field2 | 4 |
178+
| Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field2 | 14 |
177179
| ExitMethods.cs:7:10:7:11 | enter M1 | ExitMethods.cs:7:10:7:11 | exit M1 | 7 |
178180
| ExitMethods.cs:13:10:13:11 | enter M2 | ExitMethods.cs:13:10:13:11 | exit M2 | 7 |
179181
| ExitMethods.cs:19:10:19:11 | enter M3 | ExitMethods.cs:19:10:19:11 | exit M3 | 6 |

csharp/ql/test/library-tests/controlflow/graph/BasicBlockDominance.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,9 @@
383383
| post | Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... | Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... |
384384
| post | Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:129:10:129:12 | enter M10 |
385385
| post | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true |
386+
| post | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
386387
| post | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} |
388+
| post | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
387389
| post | ExitMethods.cs:7:10:7:11 | enter M1 | ExitMethods.cs:7:10:7:11 | enter M1 |
388390
| post | ExitMethods.cs:13:10:13:11 | enter M2 | ExitMethods.cs:13:10:13:11 | enter M2 |
389391
| post | ExitMethods.cs:19:10:19:11 | enter M3 | ExitMethods.cs:19:10:19:11 | enter M3 |
@@ -1752,9 +1754,15 @@
17521754
| pre | Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... | Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... |
17531755
| pre | Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:129:10:129:12 | enter M10 |
17541756
| pre | Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true |
1757+
| pre | Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
17551758
| pre | Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} |
1759+
| pre | Conditions.cs:129:10:129:12 | enter M10 | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
17561760
| pre | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true |
1761+
| pre | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
1762+
| pre | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
17571763
| pre | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} |
1764+
| pre | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
1765+
| pre | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
17581766
| pre | ExitMethods.cs:7:10:7:11 | enter M1 | ExitMethods.cs:7:10:7:11 | enter M1 |
17591767
| pre | ExitMethods.cs:13:10:13:11 | enter M2 | ExitMethods.cs:13:10:13:11 | enter M2 |
17601768
| pre | ExitMethods.cs:19:10:19:11 | enter M3 | ExitMethods.cs:19:10:19:11 | enter M3 |

csharp/ql/test/library-tests/controlflow/graph/BooleanNode.expected

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,56 @@
33
| Field1 (line 129): false | Conditions.cs:133:13:139:13 | [Field1 (line 129): false] if (...) ... |
44
| Field1 (line 129): false | Conditions.cs:133:17:133:22 | [Field1 (line 129): false] access to field Field1 |
55
| Field1 (line 129): false | Conditions.cs:133:17:133:22 | [Field1 (line 129): false] this access |
6+
| Field1 (line 129): true | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
7+
| Field1 (line 129): true | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): true] true |
8+
| Field1 (line 129): true | Conditions.cs:132:9:140:9 | [Field1 (line 129): true, Field2 (line 129): false] {...} |
9+
| Field1 (line 129): true | Conditions.cs:132:9:140:9 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
10+
| Field1 (line 129): true | Conditions.cs:133:13:139:13 | [Field1 (line 129): true, Field2 (line 129): false] if (...) ... |
11+
| Field1 (line 129): true | Conditions.cs:133:13:139:13 | [Field1 (line 129): true, Field2 (line 129): true] if (...) ... |
12+
| Field1 (line 129): true | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field1 |
13+
| Field1 (line 129): true | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): false] this access |
14+
| Field1 (line 129): true | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field1 |
15+
| Field1 (line 129): true | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): true] this access |
16+
| Field1 (line 129): true | Conditions.cs:134:13:139:13 | [Field1 (line 129): true, Field2 (line 129): false] {...} |
17+
| Field1 (line 129): true | Conditions.cs:134:13:139:13 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
618
| Field1 (line 129): true | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} |
19+
| Field1 (line 129): true | Conditions.cs:135:17:138:17 | [Field1 (line 129): true, Field2 (line 129): false] if (...) ... |
20+
| Field1 (line 129): true | Conditions.cs:135:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] if (...) ... |
721
| Field1 (line 129): true | Conditions.cs:135:17:138:17 | [Field1 (line 129): true] if (...) ... |
22+
| Field1 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field2 |
23+
| Field1 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): false] this access |
24+
| Field1 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field2 |
25+
| Field1 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): true] this access |
826
| Field1 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true] access to field Field2 |
927
| Field1 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true] this access |
28+
| Field1 (line 129): true | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
29+
| Field1 (line 129): true | Conditions.cs:137:21:137:26 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field1 |
30+
| Field1 (line 129): true | Conditions.cs:137:21:137:26 | [Field1 (line 129): true, Field2 (line 129): true] this access |
31+
| Field1 (line 129): true | Conditions.cs:137:21:137:37 | [Field1 (line 129): true, Field2 (line 129): true] call to method ToString |
32+
| Field1 (line 129): true | Conditions.cs:137:21:137:38 | [Field1 (line 129): true, Field2 (line 129): true] ...; |
33+
| Field2 (line 129): false | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
34+
| Field2 (line 129): false | Conditions.cs:132:9:140:9 | [Field1 (line 129): true, Field2 (line 129): false] {...} |
35+
| Field2 (line 129): false | Conditions.cs:133:13:139:13 | [Field1 (line 129): true, Field2 (line 129): false] if (...) ... |
36+
| Field2 (line 129): false | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field1 |
37+
| Field2 (line 129): false | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): false] this access |
38+
| Field2 (line 129): false | Conditions.cs:134:13:139:13 | [Field1 (line 129): true, Field2 (line 129): false] {...} |
39+
| Field2 (line 129): false | Conditions.cs:135:17:138:17 | [Field1 (line 129): true, Field2 (line 129): false] if (...) ... |
40+
| Field2 (line 129): false | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field2 |
41+
| Field2 (line 129): false | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): false] this access |
42+
| Field2 (line 129): true | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): true] true |
43+
| Field2 (line 129): true | Conditions.cs:132:9:140:9 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
44+
| Field2 (line 129): true | Conditions.cs:133:13:139:13 | [Field1 (line 129): true, Field2 (line 129): true] if (...) ... |
45+
| Field2 (line 129): true | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field1 |
46+
| Field2 (line 129): true | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): true] this access |
47+
| Field2 (line 129): true | Conditions.cs:134:13:139:13 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
48+
| Field2 (line 129): true | Conditions.cs:135:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] if (...) ... |
49+
| Field2 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field2 |
50+
| Field2 (line 129): true | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): true] this access |
51+
| Field2 (line 129): true | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
52+
| Field2 (line 129): true | Conditions.cs:137:21:137:26 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field1 |
53+
| Field2 (line 129): true | Conditions.cs:137:21:137:26 | [Field1 (line 129): true, Field2 (line 129): true] this access |
54+
| Field2 (line 129): true | Conditions.cs:137:21:137:37 | [Field1 (line 129): true, Field2 (line 129): true] call to method ToString |
55+
| Field2 (line 129): true | Conditions.cs:137:21:137:38 | [Field1 (line 129): true, Field2 (line 129): true] ...; |
1056
| b2 (line 22): false | Conditions.cs:28:9:29:16 | [b2 (line 22): false] if (...) ... |
1157
| b2 (line 22): false | Conditions.cs:28:13:28:14 | [b2 (line 22): false] access to parameter b2 |
1258
| b2 (line 22): true | Conditions.cs:27:17:27:17 | [b2 (line 22): true] access to local variable x |

csharp/ql/test/library-tests/controlflow/graph/ConditionBlock.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,11 @@
163163
| Conditions.cs:119:18:119:21 | access to local variable last | Conditions.cs:120:17:120:23 | [last (line 118): false] ...; | false |
164164
| Conditions.cs:119:18:119:21 | access to local variable last | Conditions.cs:121:13:122:25 | [last (line 118): true] if (...) ... | true |
165165
| Conditions.cs:133:17:133:22 | access to field Field1 | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true | false |
166+
| Conditions.cs:133:17:133:22 | access to field Field1 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true | true |
166167
| Conditions.cs:133:17:133:22 | access to field Field1 | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} | true |
168+
| Conditions.cs:133:17:133:22 | access to field Field1 | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} | true |
169+
| Conditions.cs:135:21:135:26 | [Field1 (line 129): true] access to field Field2 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true | false |
170+
| Conditions.cs:135:21:135:26 | [Field1 (line 129): true] access to field Field2 | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} | true |
167171
| ExitMethods.cs:43:9:46:9 | [exception: Exception] catch (...) {...} | ExitMethods.cs:47:9:50:9 | [exception: Exception] catch (...) {...} | false |
168172
| ExitMethods.cs:55:13:55:13 | access to parameter b | ExitMethods.cs:56:19:56:33 | object creation of type Exception | true |
169173
| ExitMethods.cs:61:13:61:13 | access to parameter b | ExitMethods.cs:62:19:62:33 | object creation of type Exception | true |

csharp/ql/test/library-tests/controlflow/graph/ConditionalFlow.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,18 @@
200200
| 127 | 32 | cflow.cs:127:32:127:44 | ... == ... | false | 127 | 53 | cflow.cs:127:53:127:57 | this access |
201201
| 127 | 32 | cflow.cs:127:32:127:44 | ... == ... | true | 127 | 48 | cflow.cs:127:48:127:49 | "" |
202202
| 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true | true | 132 | 9 | Conditions.cs:132:9:140:9 | [Field1 (line 129): false] {...} |
203+
| 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true | true | 132 | 9 | Conditions.cs:132:9:140:9 | [Field1 (line 129): true, Field2 (line 129): false] {...} |
204+
| 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): true] true | true | 132 | 9 | Conditions.cs:132:9:140:9 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
203205
| 131 | 16 | Conditions.cs:131:16:131:19 | true | true | 132 | 9 | Conditions.cs:132:9:140:9 | {...} |
204206
| 133 | 17 | Conditions.cs:133:17:133:22 | [Field1 (line 129): false] access to field Field1 | false | 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true |
207+
| 133 | 17 | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field1 | true | 134 | 13 | Conditions.cs:134:13:139:13 | [Field1 (line 129): true, Field2 (line 129): false] {...} |
208+
| 133 | 17 | Conditions.cs:133:17:133:22 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field1 | true | 134 | 13 | Conditions.cs:134:13:139:13 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
205209
| 133 | 17 | Conditions.cs:133:17:133:22 | access to field Field1 | false | 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): false] true |
206210
| 133 | 17 | Conditions.cs:133:17:133:22 | access to field Field1 | true | 134 | 13 | Conditions.cs:134:13:139:13 | [Field1 (line 129): true] {...} |
211+
| 135 | 21 | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): false] access to field Field2 | false | 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
212+
| 135 | 21 | Conditions.cs:135:21:135:26 | [Field1 (line 129): true, Field2 (line 129): true] access to field Field2 | true | 136 | 17 | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
213+
| 135 | 21 | Conditions.cs:135:21:135:26 | [Field1 (line 129): true] access to field Field2 | false | 131 | 16 | Conditions.cs:131:16:131:19 | [Field1 (line 129): true, Field2 (line 129): false] true |
214+
| 135 | 21 | Conditions.cs:135:21:135:26 | [Field1 (line 129): true] access to field Field2 | true | 136 | 17 | Conditions.cs:136:17:138:17 | [Field1 (line 129): true, Field2 (line 129): true] {...} |
207215
| 162 | 48 | cflow.cs:162:48:162:51 | [exception: Exception] true | true | 163 | 9 | cflow.cs:163:9:165:9 | {...} |
208216
| 170 | 21 | cflow.cs:170:21:170:24 | true | true | 170 | 27 | cflow.cs:170:27:170:32 | throw ...; |
209217
| 194 | 48 | cflow.cs:194:48:194:51 | [exception: Exception] true | true | 195 | 9 | cflow.cs:195:9:197:9 | {...} |

0 commit comments

Comments
 (0)