hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 03:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

CIL: Account for multiple VariableUpdate::getSource()s in nullness analysis

Browse Source 
For methods compiled without optimization (and possibly also with optimization),
it is possible for a variable update to have multiple possible assigned values.
For example, the non-optimized CIL for

```
return cond ? null : "not null"
```

is

```
0: nop
1: ldarg.0
2: ldfld cond
3: brtrue.s 6:
4: ldstr "not null"
5: br.s 7:
6: ldnull
7: stloc.0 L0 // stores either `null` or "not null"
8: br.s 9:
9: ldloc.0
10: ret
```

Consequently, an existential in `CallableReturns.qll` must be a `forex`.
This commit is contained in:
Tom Hvitved
2019-05-27 16:33:00 +02:00
parent 4fc61ebbf3
commit 0ee5fe88d9
3 changed files with 4 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
csharp/ql/src/semmle/code/cil/CallableReturns.qll
View File

@@ -36,7 +36,9 @@ private predicate alwaysNullExpr(Expr expr) {
or
alwaysNullMethod(expr.(StaticCall).getTarget())
or
forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | alwaysNullExpr(vu.getSource()))
forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) |
forex(Expr src | src = vu.getSource() | alwaysNullExpr(src))
)
}
/** Holds if expression `expr` always evaluates to non-null. */
@@ -48,6 +50,6 @@ private predicate alwaysNotNullExpr(Expr expr) {
alwaysNotNullMethod(expr.(StaticCall).getTarget())
or
forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) |
alwaysNotNullExpr(vu.getSource())
forex(Expr src | src = vu.getSource() | alwaysNotNullExpr(src))
)
}

2
csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected
View File

@@ -7,7 +7,6 @@ alwaysNull
| System.Object Dataflow.NullMethods.get_NullProperty() | 0: ldnull, 1: ret |
| System.Object Dataflow.NullMethods.get_VirtualNullProperty() | 0: ldnull, 1: ret |
| System.Object System.Collections.EmptyReadOnlyDictionaryInternal.get_Item(System.Object) | 0: ldarg.1, 1: brtrue.s 6:, 2: ldstr "key", 3: call System.SR.get_ArgumentNull_Key, 4: newobj System.ArgumentNullException..ctor, 5: throw, 6: ldnull, 7: ret |
| System.String DataflowUnoptimized.MaybeNullMethods.MaybeNull2() | 0: nop, 1: ldarg.0, 2: ldfld cond, 3: brtrue.s 6:, 4: ldstr "not null", 5: br.s 7:, 6: ldnull, 7: stloc.0 L0, 8: br.s 9:, 9: ldloc.0, 10: ret |
alwaysNonNull
| System.ArgumentException System.ThrowHelper.GetAddingDuplicateWithKeyArgumentException(System.Object) |
| System.ArgumentException System.ThrowHelper.GetArgumentException(System.ExceptionResource) |
@@ -25,7 +24,6 @@ alwaysNonNull
| System.Object Dataflow.NonNullMethods.get_VirtualNonNull() |
| System.Object Dataflow.NonNullMethods.get_VirtualNonNullProperty() |
| System.String Dataflow.NonNullMethods.get_NonNullProperty2() |
| System.String DataflowUnoptimized.MaybeNullMethods.MaybeNull2() |
| System.Text.Encoder System.Text.ASCIIEncoding.GetEncoder() |
| System.Text.Encoder System.Text.Encoding.GetEncoder() |
| System.Text.Encoder System.Text.EncodingNLS.GetEncoder() |

4
csharp/ql/test/library-tests/cil/dataflow/Nullness.expected
View File

@@ -6,8 +6,6 @@ alwaysNull
| dataflow.cs:79:21:79:46 | call to method ReturnsNull2 |
| dataflow.cs:80:21:80:44 | access to property NullProperty |
| dataflow.cs:89:31:89:44 | call to method NullFunction |
| dataflow.cs:99:9:99:50 | ... = ... |
| dataflow.cs:99:22:99:50 | call to method MaybeNull2 |
alwaysNotNull
| dataflow.cs:71:24:71:35 | default(...) |
| dataflow.cs:72:27:72:30 | this access |
@@ -36,6 +34,4 @@ alwaysNotNull
| dataflow.cs:96:26:96:32 | access to local variable nonNull |
| dataflow.cs:97:32:97:73 | object creation of type MaybeNullMethods |
| dataflow.cs:98:21:98:36 | access to local variable maybeNullMethods |
| dataflow.cs:99:9:99:50 | ... = ... |
| dataflow.cs:99:22:99:37 | access to local variable maybeNullMethods |
| dataflow.cs:99:22:99:50 | call to method MaybeNull2 |