Skip to content

Commit ca86bb8

Browse files
hvitvedhvitved
committed
Address review comments
1 parent 9e7ca25 commit ca86bb8

File tree

4 files changed

+95
-61
lines changed

4 files changed

+95
-61
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ module Unification {
685685
private import Cached
686686

687687
/**
688-
* Holds if types `t1` and `t2` are unifiable. That is, is it possible to replace
688+
* Holds if types `t1` and `t2` are unifiable. That is, it is possible to replace
689689
* all type parameters in `t1` and `t2` with some (other) types to make the two
690690
* substituted terms equal.
691691
*
@@ -722,7 +722,7 @@ module Unification {
722722
}
723723

724724
/**
725-
* Holds if type `t1` subsumes type `t2`. That is, is it possible to replace all
725+
* Holds if type `t1` subsumes type `t2`. That is, it is possible to replace all
726726
* type parameters in `t1` with some (other) types to make the two types equal.
727727
*
728728
* The same limitations that apply to the predicate `unifiable()` apply to this

‎csharp/ql/src/semmle/code/csharp/dispatch/Dispatch.qll

Lines changed: 67 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -265,12 +265,13 @@ private module Internal {
265265
private predicate hasQualifierTypeOverridden0(ValueOrRefType t, DispatchMethodOrAccessorCall call) {
266266
hasOverrider(_, t) and
267267
(
268-
exists(Type t0 | t0 = getAPossibleType(call.getQualifier(), false) |
269-
t = t0
270-
or
271-
Unification::subsumes(t0, t)
268+
exists(Type t0, Type t1 |
269+
t0 = getAPossibleType(call.getQualifier(), false) and
270+
t1 = [t0, t0.(Unification::UnconstrainedTypeParameter).getAnUltimatelySuppliedType()]
271+
|
272+
t = t1
272273
or
273-
t = t0.(Unification::UnconstrainedTypeParameter).getAnUltimatelySuppliedType()
274+
Unification::subsumes(t1, t)
274275
)
275276
or
276277
constrainedTypeParameterQualifierTypeSubsumes(t,
@@ -325,15 +326,17 @@ private module Internal {
325326

326327
/**
327328
* Holds if the call `ctx` might act as a context that improves the set of
328-
* dispatch targets of this call, which occurs in a viable target of `ctx`.
329+
* dispatch targets of this call, depending on the type of the `i`th argument
330+
* of `ctx`.
329331
*/
330332
pragma[nomagic]
331333
private predicate relevantContext(DispatchCall ctx, int i) {
332334
this.mayBenefitFromCallContext(ctx.getADynamicTarget().getSourceDeclaration(), i)
333335
}
334336

335337
/**
336-
* Holds if the `i`th argument of `ctx` has type `t` and `ctx` is a relevant
338+
* Holds if the argument of `ctx`, which is passed for the parameter that is
339+
* accessed in the qualifier of this call, has type `t` and `ctx` is a relevant
337340
* call context.
338341
*/
339342
private predicate contextArgHasType(DispatchCall ctx, Type t, boolean isExact) {
@@ -377,25 +380,29 @@ private module Internal {
377380
* Example:
378381
*
379382
* ```csharp
380-
* class A {
381-
* public virtual void M() { }
383+
* class A
384+
* {
385+
* public virtual void M() { }
382386
* }
383387
*
384-
* class B : A {
385-
* public override void M() { }
388+
* class B : A
389+
* {
390+
* public override void M() { }
386391
* }
387392
*
388393
* class C : B { }
389394
*
390-
* class D {
391-
* void CallM() {
392-
* A x = new A();
393-
* x.M();
394-
* x = new B();
395-
* x.M();
396-
* x = new C();
397-
* x.M();
398-
* }
395+
* class D
396+
* {
397+
* void CallM()
398+
* {
399+
* A x = new A();
400+
* x.M();
401+
* x = new B();
402+
* x.M();
403+
* x = new C();
404+
* x.M();
405+
* }
399406
* }
400407
* ```
401408
*
@@ -421,27 +428,32 @@ private module Internal {
421428
* Example:
422429
*
423430
* ```csharp
424-
* class A {
425-
* public virtual void M() { }
431+
* class A
432+
* {
433+
* public virtual void M() { }
426434
* }
427435
*
428-
* class B : A {
429-
* public override void M() { }
436+
* class B : A
437+
* {
438+
* public override void M() { }
430439
* }
431440
*
432-
* class C : B {
433-
* public override void M() { }
441+
* class C : B
442+
* {
443+
* public override void M() { }
434444
* }
435445
*
436-
* class D {
437-
* void CallM() {
438-
* A x = new A();
439-
* x.M();
440-
* x = new B();
441-
* x.M();
442-
* x = new C();
443-
* x.M();
444-
* }
446+
* class D
447+
* {
448+
* void CallM()
449+
* {
450+
* A x = new A();
451+
* x.M();
452+
* x = new B();
453+
* x.M();
454+
* x = new C();
455+
* x.M();
456+
* }
445457
* }
446458
* ```
447459
*
@@ -507,12 +519,13 @@ private module Internal {
507519
) {
508520
exists(ValueOrRefType t |
509521
result = this.getAViableOverriderInCallContext0(c, t) and
510-
exists(Type t0 | this.contextArgHasType(ctx, t0, false) |
511-
t = t0
512-
or
513-
Unification::subsumes(t0, t)
522+
exists(Type t0, Type t1 |
523+
this.contextArgHasType(ctx, t0, false) and
524+
t1 = [t0, t0.(Unification::UnconstrainedTypeParameter).getAnUltimatelySuppliedType()]
525+
|
526+
t = t1
514527
or
515-
t = t0.(Unification::UnconstrainedTypeParameter).getAnUltimatelySuppliedType()
528+
Unification::subsumes(t1, t)
516529
)
517530
)
518531
}
@@ -764,22 +777,25 @@ private module Internal {
764777
* For reflection/dynamic calls, unless the type of the qualifier is exact,
765778
* all subtypes of the qualifier type must be considered relevant. Example:
766779
*
767-
* ```
768-
* class A {
769-
* public void M() { Console.WriteLine("A"); }
780+
* ```csharp
781+
* class A
782+
* {
783+
* public void M() { Console.WriteLine("A"); }
770784
* }
771785
*
772-
* class B : A {
773-
* new public void M() { Console.WriteLine("B"); }
786+
* class B : A
787+
* {
788+
* new public void M() { Console.WriteLine("B"); }
774789
* }
775790
*
776-
* class C {
777-
* void InvokeMDyn(A x) { ((dynamic) x).M(); }
791+
* class C
792+
* {
793+
* void InvokeMDyn(A x) { ((dynamic) x).M(); }
778794
*
779-
* void CallM() {
780-
* InvokeMDyn(new A()); // prints "A"
781-
* InvokeMDyn(new B()); // prints "B"
782-
* }
795+
* void CallM() {
796+
* InvokeMDyn(new A()); // prints "A"
797+
* InvokeMDyn(new B()); // prints "B"
798+
* }
783799
* }
784800
* ```
785801
*

‎csharp/ql/test/library-tests/dataflow/call-sensitivity/CallSensitivityFlow.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,12 @@ public static void Sink(object o)
163163

164164
public virtual void M(object o)
165165
{
166-
Sink(o); // no flow here
166+
Sink(o);
167+
}
168+
169+
public static void CallM(A2 a2, object o)
170+
{
171+
a2.M(o);
167172
}
168173

169174
public void Callsite(InterfaceB intF)
@@ -172,7 +177,10 @@ public void Callsite(InterfaceB intF)
172177
// in both possible implementations of foo, this callsite is relevant
173178
// in IntA, it improves virtual dispatch,
174179
// and in IntB, it improves the dataflow analysis.
175-
intF.Foo(b, new object(), false);
180+
intF.Foo(b, new object(), false); // no flow to `Sink()` via `A2.M()`, but flow via `IntA.Foo()`
181+
182+
CallM(b, new object()); // no flow to `Sink()`
183+
CallM(this, new object()); // flow to `Sink()`
176184
}
177185

178186
public class B : A2

‎csharp/ql/test/library-tests/dataflow/call-sensitivity/CallSensitivityFlow.expected

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,12 @@ edges
2626
| CallSensitivityFlow.cs:124:43:124:43 | o : Object | CallSensitivityFlow.cs:128:22:128:22 | access to parameter o |
2727
| CallSensitivityFlow.cs:133:44:133:44 | o : Object | CallSensitivityFlow.cs:137:22:137:22 | access to parameter o |
2828
| CallSensitivityFlow.cs:142:49:142:49 | o : Object | CallSensitivityFlow.cs:152:18:152:19 | access to local variable o3 |
29-
| CallSensitivityFlow.cs:175:21:175:32 | object creation of type Object : Object | CallSensitivityFlow.cs:189:40:189:40 | o : Object |
30-
| CallSensitivityFlow.cs:189:40:189:40 | o : Object | CallSensitivityFlow.cs:192:18:192:18 | access to parameter o |
29+
| CallSensitivityFlow.cs:164:34:164:34 | o : Object | CallSensitivityFlow.cs:166:14:166:14 | access to parameter o |
30+
| CallSensitivityFlow.cs:169:44:169:44 | o : Object | CallSensitivityFlow.cs:171:14:171:14 | access to parameter o : Object |
31+
| CallSensitivityFlow.cs:171:14:171:14 | access to parameter o : Object | CallSensitivityFlow.cs:164:34:164:34 | o : Object |
32+
| CallSensitivityFlow.cs:180:21:180:32 | object creation of type Object : Object | CallSensitivityFlow.cs:197:40:197:40 | o : Object |
33+
| CallSensitivityFlow.cs:183:21:183:32 | object creation of type Object : Object | CallSensitivityFlow.cs:169:44:169:44 | o : Object |
34+
| CallSensitivityFlow.cs:197:40:197:40 | o : Object | CallSensitivityFlow.cs:200:18:200:18 | access to parameter o |
3135
nodes
3236
| CallSensitivityFlow.cs:19:39:19:39 | o : Object | semmle.label | o : Object |
3337
| CallSensitivityFlow.cs:23:18:23:18 | access to parameter o | semmle.label | access to parameter o |
@@ -66,9 +70,14 @@ nodes
6670
| CallSensitivityFlow.cs:137:22:137:22 | access to parameter o | semmle.label | access to parameter o |
6771
| CallSensitivityFlow.cs:142:49:142:49 | o : Object | semmle.label | o : Object |
6872
| CallSensitivityFlow.cs:152:18:152:19 | access to local variable o3 | semmle.label | access to local variable o3 |
69-
| CallSensitivityFlow.cs:175:21:175:32 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
70-
| CallSensitivityFlow.cs:189:40:189:40 | o : Object | semmle.label | o : Object |
71-
| CallSensitivityFlow.cs:192:18:192:18 | access to parameter o | semmle.label | access to parameter o |
73+
| CallSensitivityFlow.cs:164:34:164:34 | o : Object | semmle.label | o : Object |
74+
| CallSensitivityFlow.cs:166:14:166:14 | access to parameter o | semmle.label | access to parameter o |
75+
| CallSensitivityFlow.cs:169:44:169:44 | o : Object | semmle.label | o : Object |
76+
| CallSensitivityFlow.cs:171:14:171:14 | access to parameter o : Object | semmle.label | access to parameter o : Object |
77+
| CallSensitivityFlow.cs:180:21:180:32 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
78+
| CallSensitivityFlow.cs:183:21:183:32 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
79+
| CallSensitivityFlow.cs:197:40:197:40 | o : Object | semmle.label | o : Object |
80+
| CallSensitivityFlow.cs:200:18:200:18 | access to parameter o | semmle.label | access to parameter o |
7281
#select
7382
| CallSensitivityFlow.cs:78:24:78:35 | object creation of type Object : Object | CallSensitivityFlow.cs:78:24:78:35 | object creation of type Object : Object | CallSensitivityFlow.cs:23:18:23:18 | access to parameter o | $@ | CallSensitivityFlow.cs:23:18:23:18 | access to parameter o | access to parameter o |
7483
| CallSensitivityFlow.cs:79:25:79:36 | object creation of type Object : Object | CallSensitivityFlow.cs:79:25:79:36 | object creation of type Object : Object | CallSensitivityFlow.cs:31:18:31:18 | access to parameter o | $@ | CallSensitivityFlow.cs:31:18:31:18 | access to parameter o | access to parameter o |
@@ -87,4 +96,5 @@ nodes
8796
| CallSensitivityFlow.cs:117:26:117:37 | object creation of type Object : Object | CallSensitivityFlow.cs:117:26:117:37 | object creation of type Object : Object | CallSensitivityFlow.cs:128:22:128:22 | access to parameter o | $@ | CallSensitivityFlow.cs:128:22:128:22 | access to parameter o | access to parameter o |
8897
| CallSensitivityFlow.cs:118:27:118:38 | object creation of type Object : Object | CallSensitivityFlow.cs:118:27:118:38 | object creation of type Object : Object | CallSensitivityFlow.cs:137:22:137:22 | access to parameter o | $@ | CallSensitivityFlow.cs:137:22:137:22 | access to parameter o | access to parameter o |
8998
| CallSensitivityFlow.cs:119:32:119:43 | object creation of type Object : Object | CallSensitivityFlow.cs:119:32:119:43 | object creation of type Object : Object | CallSensitivityFlow.cs:152:18:152:19 | access to local variable o3 | $@ | CallSensitivityFlow.cs:152:18:152:19 | access to local variable o3 | access to local variable o3 |
90-
| CallSensitivityFlow.cs:175:21:175:32 | object creation of type Object : Object | CallSensitivityFlow.cs:175:21:175:32 | object creation of type Object : Object | CallSensitivityFlow.cs:192:18:192:18 | access to parameter o | $@ | CallSensitivityFlow.cs:192:18:192:18 | access to parameter o | access to parameter o |
99+
| CallSensitivityFlow.cs:180:21:180:32 | object creation of type Object : Object | CallSensitivityFlow.cs:180:21:180:32 | object creation of type Object : Object | CallSensitivityFlow.cs:200:18:200:18 | access to parameter o | $@ | CallSensitivityFlow.cs:200:18:200:18 | access to parameter o | access to parameter o |
100+
| CallSensitivityFlow.cs:183:21:183:32 | object creation of type Object : Object | CallSensitivityFlow.cs:183:21:183:32 | object creation of type Object : Object | CallSensitivityFlow.cs:166:14:166:14 | access to parameter o | $@ | CallSensitivityFlow.cs:166:14:166:14 | access to parameter o | access to parameter o |

0 commit comments

Comments
 (0)