Skip to content

Commit f85f05d

Browse files
calumgrantcalumgrant
authored
Merge pull request #776 from hvitved/csharp/delegate-ref-assignment
C#: Recognize `ref` assignments through delegate calls
2 parents bca941d + 9031e19 commit f85f05d

File tree

7 files changed

+46
-10
lines changed

7 files changed

+46
-10
lines changed

‎csharp/ql/src/semmle/code/csharp/Assignable.qll‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,8 @@ module AssignableInternal {
162162
* Holds if the `ref` assignment to `aa` via call `c` is relevant.
163163
*/
164164
private predicate isRelevantRefCall(Call c, AssignableAccess aa) {
165-
c.getAnArgument() = aa and
166-
aa.isRefArgument() and
167-
(isNonAnalyzableRefCall(c, aa, _) or exists(getAnAnalyzableRefDef(c, aa, _)))
165+
isNonAnalyzableRefCall(c, aa) or
166+
exists(getAnAnalyzableRefDef(c, aa, _))
168167
}
169168

170169
private Callable getRefCallTarget(Call c, AssignableAccess aa, Parameter p) {
@@ -220,10 +219,16 @@ module AssignableInternal {
220219
* Equivalent with `not isAnalyzableRefCall(mc, aa, p)`, but avoids negative
221220
* recursion.
222221
*/
223-
private predicate isNonAnalyzableRefCall(Call c, AssignableAccess aa, Parameter p) {
224-
exists(Callable callable | callable = getRefCallTarget(c, aa, p) |
225-
callable.(Virtualizable).isOverridableOrImplementable() or
226-
not callable.hasBody()
222+
private predicate isNonAnalyzableRefCall(Call c, AssignableAccess aa) {
223+
aa = c.getAnArgument() and
224+
aa.isRefArgument() and
225+
(
226+
not exists(getRefCallTarget(c, aa, _))
227+
or
228+
exists(Callable callable | callable = getRefCallTarget(c, aa, _) |
229+
callable.(Virtualizable).isOverridableOrImplementable() or
230+
not callable.hasBody()
231+
)
227232
)
228233
}
229234

@@ -275,7 +280,8 @@ module AssignableInternal {
275280
not lvde.hasInitializer() and
276281
not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
277282
not lvde = any(IsPatternExpr ipe).getVariableDeclExpr() and
278-
not lvde = any(TypeCase tc).getVariableDeclExpr()
283+
not lvde = any(TypeCase tc).getVariableDeclExpr() and
284+
not lvde.isOutArgument()
279285
} or
280286
TImplicitParameterDefinition(Parameter p) {
281287
exists(Callable c | p = c.getAParameter() |

‎csharp/ql/test/library-tests/assignables/AssignableAccess.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@
8989
| Assignables.cs:110:36:110:36 | access to local variable s | Assignables.cs:109:16:109:16 | s | write |
9090
| Assignables.cs:116:13:116:13 | access to local variable s | Assignables.cs:116:13:116:13 | s | write |
9191
| Assignables.cs:117:9:117:9 | access to local variable s | Assignables.cs:116:13:116:13 | s | write |
92+
| Assignables.cs:123:13:123:13 | access to local variable x | Assignables.cs:123:13:123:13 | x | write |
93+
| Assignables.cs:124:9:124:9 | access to parameter d | Assignables.cs:121:31:121:31 | d | read |
94+
| Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:123:13:123:13 | x | read |
95+
| Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:123:13:123:13 | x | write |
96+
| Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | s | write |
9297
| Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:5:30:5:30 | x | write |
9398
| Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | z | write |
9499
| Discards.cs:25:27:25:30 | access to parameter args | Discards.cs:23:27:23:30 | args | read |

‎csharp/ql/test/library-tests/assignables/AssignableDefinition.expected‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,16 @@
7878
| Assignables.cs:115:13:115:13 | i | Assignables.cs:115:13:115:13 | Int32 i | Assignables.cs:115:13:115:13 | <none> | Assignables.cs:115:13:115:13 | <none> | certain |
7979
| Assignables.cs:116:13:116:13 | s | Assignables.cs:116:13:116:25 | String s = ... | Assignables.cs:116:13:116:13 | access to local variable s | Assignables.cs:116:17:116:25 | nameof(...) | certain |
8080
| Assignables.cs:116:13:116:13 | s | Assignables.cs:117:9:117:30 | ... = ... | Assignables.cs:117:9:117:9 | access to local variable s | Assignables.cs:117:13:117:30 | nameof(...) | certain |
81+
| Assignables.cs:121:31:121:31 | d | Assignables.cs:121:31:121:31 | d | Assignables.cs:121:31:121:31 | <none> | Assignables.cs:121:31:121:31 | <none> | certain |
82+
| Assignables.cs:123:13:123:13 | x | Assignables.cs:123:13:123:17 | Int32 x = ... | Assignables.cs:123:13:123:13 | access to local variable x | Assignables.cs:123:17:123:17 | 0 | certain |
83+
| Assignables.cs:123:13:123:13 | x | Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:124:15:124:15 | <none> | certain |
84+
| Assignables.cs:124:29:124:29 | s | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:29:124:29 | <none> | certain |
8185
| Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | x | Discards.cs:5:30:5:30 | <none> | Discards.cs:5:30:5:30 | <none> | certain |
8286
| Discards.cs:5:30:5:30 | x | Discards.cs:7:9:7:17 | ... = ... | Discards.cs:7:9:7:9 | access to parameter x | Discards.cs:7:13:7:17 | false | certain |
8387
| Discards.cs:19:14:19:14 | x | Discards.cs:19:9:19:29 | ... = ... | Discards.cs:19:9:19:29 | <none> | Discards.cs:19:9:19:29 | <none> | certain |
8488
| Discards.cs:19:14:19:14 | x | Discards.cs:19:14:19:14 | Int32 x | Discards.cs:19:14:19:14 | <none> | Discards.cs:19:14:19:14 | <none> | certain |
8589
| Discards.cs:20:17:20:17 | y | Discards.cs:20:9:20:33 | ... = ... | Discards.cs:20:9:20:33 | <none> | Discards.cs:20:9:20:33 | <none> | certain |
8690
| Discards.cs:20:17:20:17 | y | Discards.cs:20:17:20:17 | Double y | Discards.cs:20:17:20:17 | <none> | Discards.cs:20:17:20:17 | <none> | certain |
87-
| Discards.cs:20:32:20:32 | z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | <none> | Discards.cs:20:32:20:32 | <none> | certain |
8891
| Discards.cs:20:32:20:32 | z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | <none> | certain |
8992
| Discards.cs:23:27:23:30 | args | Discards.cs:23:27:23:30 | args | Discards.cs:23:27:23:30 | <none> | Discards.cs:23:27:23:30 | <none> | certain |
9093
| Discards.cs:30:24:30:24 | o | Discards.cs:30:24:30:24 | o | Discards.cs:30:24:30:24 | <none> | Discards.cs:30:24:30:24 | <none> | certain |

‎csharp/ql/test/library-tests/assignables/AssignableDefinitionNode.expected‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@
7777
| Assignables.cs:115:13:115:13 | Int32 i | Assignables.cs:115:13:115:13 | Int32 i |
7878
| Assignables.cs:116:13:116:25 | String s = ... | Assignables.cs:116:13:116:25 | String s = ... |
7979
| Assignables.cs:117:9:117:30 | ... = ... | Assignables.cs:117:9:117:30 | ... = ... |
80+
| Assignables.cs:121:31:121:31 | d | Assignables.cs:121:10:121:20 | enter DelegateRef |
81+
| Assignables.cs:123:13:123:17 | Int32 x = ... | Assignables.cs:123:13:123:17 | Int32 x = ... |
82+
| Assignables.cs:124:15:124:15 | access to local variable x | Assignables.cs:124:9:124:30 | delegate call |
83+
| Assignables.cs:124:29:124:29 | String s | Assignables.cs:124:9:124:30 | delegate call |
8084
| Discards.cs:5:30:5:30 | x | Discards.cs:5:19:5:19 | enter f |
8185
| Discards.cs:7:9:7:17 | ... = ... | Discards.cs:7:9:7:17 | ... = ... |
8286
| Discards.cs:13:9:13:20 | ... = ... | Discards.cs:13:9:13:20 | ... = ... |
@@ -89,7 +93,6 @@
8993
| Discards.cs:20:9:20:33 | ... = ... | Discards.cs:20:9:20:33 | ... = ... |
9094
| Discards.cs:20:17:20:17 | Double y | Discards.cs:20:17:20:17 | Double y |
9195
| Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:22:20:33 | call to method f |
92-
| Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | Boolean z |
9396
| Discards.cs:23:27:23:30 | args | Discards.cs:23:10:23:16 | enter Foreach |
9497
| Discards.cs:30:24:30:24 | o | Discards.cs:30:10:30:15 | enter Switch |
9598
| Finally.cs:5:16:5:16 | b | Finally.cs:5:9:5:9 | enter M |

‎csharp/ql/test/library-tests/assignables/Assignables.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,11 @@ void Nameof()
116116
var s = nameof(i); // not a read of `i`
117117
s = nameof(this.Field); // not a read of `this.Field`
118118
}
119+
120+
delegate void Delegate(ref int i, out string s);
121+
void DelegateRef(Delegate d)
122+
{
123+
var x = 0;
124+
d(ref x, out string s);
125+
}
119126
}

‎csharp/ql/test/library-tests/assignables/Assignables.expected‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,17 @@
8282
| Assignables.cs:109:16:109:16 | s |
8383
| Assignables.cs:115:13:115:13 | i |
8484
| Assignables.cs:116:13:116:13 | s |
85+
| Assignables.cs:120:36:120:36 | i |
86+
| Assignables.cs:120:36:120:36 | i |
87+
| Assignables.cs:120:36:120:36 | i |
88+
| Assignables.cs:120:36:120:36 | i |
89+
| Assignables.cs:120:50:120:50 | s |
90+
| Assignables.cs:120:50:120:50 | s |
91+
| Assignables.cs:120:50:120:50 | s |
92+
| Assignables.cs:120:50:120:50 | s |
93+
| Assignables.cs:121:31:121:31 | d |
94+
| Assignables.cs:123:13:123:13 | x |
95+
| Assignables.cs:124:29:124:29 | s |
8596
| Discards.cs:5:6:5:8 | Item1 |
8697
| Discards.cs:5:11:5:16 | Item2 |
8798
| Discards.cs:5:30:5:30 | x |

‎csharp/ql/test/library-tests/assignables/GetAnAssignedValue.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
| Assignables.cs:109:16:109:16 | s | Assignables.cs:109:20:109:21 | "" |
3636
| Assignables.cs:116:13:116:13 | s | Assignables.cs:116:17:116:25 | nameof(...) |
3737
| Assignables.cs:116:13:116:13 | s | Assignables.cs:117:13:117:30 | nameof(...) |
38+
| Assignables.cs:123:13:123:13 | x | Assignables.cs:123:17:123:17 | 0 |
3839
| Discards.cs:5:30:5:30 | x | Discards.cs:7:13:7:17 | false |
3940
| Finally.cs:7:13:7:13 | i | Finally.cs:7:17:7:17 | 0 |
4041
| Finally.cs:7:13:7:13 | i | Finally.cs:15:17:15:17 | 1 |

0 commit comments

Comments
 (0)