Skip to content

Commit 8d09885

Browse files
authored
Merge pull request #4378 from tamasvajk/feature/flow-summary-nullable
Flow summary nullable
2 parents eece3ad + 576085a commit 8d09885

File tree

4 files changed

+112
-5
lines changed

4 files changed

+112
-5
lines changed

csharp/ql/src/semmle/code/csharp/dataflow/LibraryTypeDataFlow.qll

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ abstract class LibraryTypeDataFlow extends Type {
347347
* Holds if data may flow from `source` to `sink` when calling callable `c`.
348348
*
349349
* `sourceAp` describes the contents of `source` that flows to `sink`
350-
* (if any), and `sinkContent` describes the contents of `sink` that it
350+
* (if any), and `sinkAp` describes the contents of `sink` that it
351351
* flows to (if any).
352352
*/
353353
pragma[nomagic]
@@ -736,6 +736,49 @@ class SystemLazyFlow extends LibraryTypeDataFlow, SystemLazyClass {
736736
}
737737
}
738738

739+
/** Data flow for `System.Nullable<>`. */
740+
class SystemNullableFlow extends LibraryTypeDataFlow, SystemNullableStruct {
741+
override predicate callableFlow(
742+
CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink, AccessPath sinkAp,
743+
SourceDeclarationCallable c, boolean preservesValue
744+
) {
745+
preservesValue = true and
746+
c.(Constructor).getDeclaringType() = this and
747+
source = getFlowSourceArg(c, 0, sourceAp) and
748+
sourceAp = AccessPath::empty() and
749+
sink = TCallableFlowSinkReturn() and
750+
sinkAp = AccessPath::property(this.getValueProperty())
751+
or
752+
preservesValue = true and
753+
c = this.getAGetValueOrDefaultMethod() and
754+
source = TCallableFlowSourceQualifier() and
755+
sourceAp = AccessPath::property(this.getValueProperty()) and
756+
sink = TCallableFlowSinkReturn() and
757+
sinkAp = AccessPath::empty()
758+
or
759+
preservesValue = false and
760+
c = this.getHasValueProperty().getGetter() and
761+
source = TCallableFlowSourceQualifier() and
762+
sourceAp = AccessPath::property(this.getValueProperty()) and
763+
sink = TCallableFlowSinkReturn() and
764+
sinkAp = AccessPath::empty()
765+
or
766+
preservesValue = true and
767+
c = this.getAGetValueOrDefaultMethod() and
768+
source = getFlowSourceArg(c, 0, _) and
769+
sourceAp = AccessPath::empty() and
770+
sink = TCallableFlowSinkReturn() and
771+
sinkAp = AccessPath::empty()
772+
or
773+
preservesValue = false and
774+
c = this.getValueProperty().getGetter() and
775+
source = TCallableFlowSourceQualifier() and
776+
sourceAp = AccessPath::empty() and
777+
sink = TCallableFlowSinkReturn() and
778+
sinkAp = AccessPath::empty()
779+
}
780+
}
781+
739782
/** Data flow for `System.Collections.IEnumerable` (and sub types). */
740783
class IEnumerableFlow extends LibraryTypeDataFlow, RefType {
741784
IEnumerableFlow() { this.getABaseType*() instanceof SystemCollectionsIEnumerableInterface }

csharp/ql/src/semmle/code/csharp/frameworks/System.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ class SystemUnboundGenericClass extends UnboundGenericClass {
2121
SystemUnboundGenericClass() { this.getNamespace() instanceof SystemNamespace }
2222
}
2323

24+
/** An unbound generic struct in the `System` namespace. */
25+
class SystemUnboundGenericStruct extends UnboundGenericStruct {
26+
SystemUnboundGenericStruct() { this.getNamespace() instanceof SystemNamespace }
27+
}
28+
2429
/** An interface in the `System` namespace. */
2530
class SystemInterface extends Interface {
2631
SystemInterface() { this.getNamespace() instanceof SystemNamespace }
@@ -215,6 +220,35 @@ class SystemLazyClass extends SystemUnboundGenericClass {
215220
}
216221
}
217222

223+
/** The `System.Nullable<T>` struct. */
224+
class SystemNullableStruct extends SystemUnboundGenericStruct {
225+
SystemNullableStruct() {
226+
this.hasName("Nullable<>") and
227+
this.getNumberOfTypeParameters() = 1
228+
}
229+
230+
/** Gets the `Value` property. */
231+
Property getValueProperty() {
232+
result.getDeclaringType() = this and
233+
result.hasName("Value") and
234+
result.getType() = getTypeParameter(0)
235+
}
236+
237+
/** Gets the `HasValue` property. */
238+
Property getHasValueProperty() {
239+
result.getDeclaringType() = this and
240+
result.hasName("HasValue") and
241+
result.getType() instanceof BoolType
242+
}
243+
244+
/** Gets a `GetValueOrDefault()` method. */
245+
Method getAGetValueOrDefaultMethod() {
246+
result.getDeclaringType() = this and
247+
result.hasName("GetValueOrDefault") and
248+
result.getReturnType() = getTypeParameter(0)
249+
}
250+
}
251+
218252
/** The `System.NullReferenceException` class. */
219253
class SystemNullReferenceExceptionClass extends SystemClass {
220254
SystemNullReferenceExceptionClass() { this.hasName("NullReferenceException") }
Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,29 @@
11
using System;
2+
using System.Runtime.Serialization;
23

34
class C
45
{
5-
sbyte x1;
6-
sbyte? x2;
6+
int x1;
7+
int? x2;
78
Nullable<int> x3;
89
Nullable<char> x4;
910

1011
// Verify conversions
1112
void M()
1213
{
13-
x2 = x1;
14-
x3 = x4;
14+
x2 = x1; // T -> T? conversion: implicit, nullable -> implicit cast
15+
x3 = x4; // T1? -> T2? conversion: implicit, nullable -> implicit cast
16+
17+
x12 = x1; // T1 -> T2? conversion: implicit, nullable -> implicit cast
18+
x12 = null; // null -> T? conversion: implicit, null literal -> no cast
19+
20+
x3 = x2; // T? -> T? no conversion
21+
22+
x14 = x15; // T1? -> T2? conversion: implicit, user defined -> implicit cast
1523
}
1624

1725
// Cause the following types to exist in the database:
26+
sbyte? x0;
1827
byte? x5;
1928
short? x6;
2029
ushort? x7;
@@ -24,4 +33,19 @@ void M()
2433
double? x11;
2534
long? x12;
2635
float? x13;
36+
37+
A? x14;
38+
B? x15;
39+
40+
struct A
41+
{
42+
}
43+
44+
struct B
45+
{
46+
public static implicit operator A(B value)
47+
{
48+
return new A();
49+
}
50+
}
2751
}

csharp/ql/test/library-tests/dataflow/library/LibraryTypeDataFlow.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,8 @@ callableFlow
494494
| System.Net.WebUtility.HtmlEncode(string) | argument 0 -> return | false |
495495
| System.Net.WebUtility.HtmlEncode(string, TextWriter) | argument 0 -> return | false |
496496
| System.Net.WebUtility.UrlEncode(string) | argument 0 -> return | false |
497+
| System.Nullable<>.GetValueOrDefault(T) | argument 0 -> return | true |
498+
| System.Nullable<>.get_Value() | qualifier -> return | false |
497499
| System.Security.Cryptography.CryptoStream.BeginRead(Byte[], int, int, AsyncCallback, object) | qualifier -> argument 0 | false |
498500
| System.Security.Cryptography.CryptoStream.BeginWrite(Byte[], int, int, AsyncCallback, object) | argument 0 -> qualifier | false |
499501
| System.Security.Cryptography.CryptoStream.Read(Byte[], int, int) | qualifier -> argument 0 | false |
@@ -1767,6 +1769,10 @@ callableFlowAccessPath
17671769
| System.Net.NetworkInformation.IPAddressCollection.Add(IPAddress) | argument 0 [<empty>] -> qualifier [[]] | true |
17681770
| System.Net.NetworkInformation.IPAddressCollection.CopyTo(IPAddress[], int) | qualifier [[]] -> argument 0 [[]] | true |
17691771
| System.Net.NetworkInformation.IPAddressCollection.GetEnumerator() | qualifier [[]] -> return [Current] | true |
1772+
| System.Nullable<>.GetValueOrDefault() | qualifier [Value] -> return [<empty>] | true |
1773+
| System.Nullable<>.GetValueOrDefault(T) | qualifier [Value] -> return [<empty>] | true |
1774+
| System.Nullable<>.Nullable(T) | argument 0 [<empty>] -> return [Value] | true |
1775+
| System.Nullable<>.get_HasValue() | qualifier [Value] -> return [<empty>] | false |
17701776
| System.Resources.ResourceReader.GetEnumerator() | qualifier [[]] -> return [Current] | true |
17711777
| System.Resources.ResourceSet.GetEnumerator() | qualifier [[]] -> return [Current] | true |
17721778
| System.Runtime.CompilerServices.ConditionalWeakTable<,>.GetEnumerator() | qualifier [[]] -> return [Current] | true |

0 commit comments

Comments
 (0)