diff --git a/csharp/ql/src/semmle/code/cil/CallableReturns.qll b/csharp/ql/src/semmle/code/cil/CallableReturns.qll index 044635f1d147..b1a666954b9a 100644 --- a/csharp/ql/src/semmle/code/cil/CallableReturns.qll +++ b/csharp/ql/src/semmle/code/cil/CallableReturns.qll @@ -30,13 +30,25 @@ private module Cached { } import Cached +pragma[noinline] +private predicate alwaysNullVariableUpdate(VariableUpdate vu) { + forex(Expr src | src = vu.getSource() | alwaysNullExpr(src)) +} + /** Holds if expression `expr` always evaluates to `null`. */ private predicate alwaysNullExpr(Expr expr) { expr instanceof NullLiteral or alwaysNullMethod(expr.(StaticCall).getTarget()) or - forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | alwaysNullExpr(vu.getSource())) + forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | + alwaysNullVariableUpdate(vu) + ) +} + +pragma[noinline] +private predicate alwaysNotNullVariableUpdate(VariableUpdate vu) { + forex(Expr src | src = vu.getSource() | alwaysNotNullExpr(src)) } /** Holds if expression `expr` always evaluates to non-null. */ @@ -48,6 +60,6 @@ private predicate alwaysNotNullExpr(Expr expr) { alwaysNotNullMethod(expr.(StaticCall).getTarget()) or forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | - alwaysNotNullExpr(vu.getSource()) + alwaysNotNullVariableUpdate(vu) ) } diff --git a/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.ql b/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.ql index cff9621199dd..6aaca97ddde2 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.ql +++ b/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.ql @@ -8,7 +8,7 @@ predicate relevantMethod(CIL::Method m) { or m.getDeclaringType().getName() = "ThrowHelper" or - m.getLocation().(CIL::Assembly).getName() = "DataFlow" + m.getLocation().(CIL::Assembly).getName().matches("DataFlow%") } // Check that the assembly hasn't been marked as a stub. diff --git a/csharp/ql/test/library-tests/cil/dataflow/DataFlow.cs_ b/csharp/ql/test/library-tests/cil/dataflow/DataFlow.cs_ index 075cd1e28bb7..1f6128f353a2 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/DataFlow.cs_ +++ b/csharp/ql/test/library-tests/cil/dataflow/DataFlow.cs_ @@ -1,3 +1,5 @@ +// Generate DataFlow.dll: `csc /o /target:library DataFlow.cs_ /out:DataFlow.dll` + using System; namespace Dataflow diff --git a/csharp/ql/test/library-tests/cil/dataflow/DataFlow.dll b/csharp/ql/test/library-tests/cil/dataflow/DataFlow.dll old mode 100644 new mode 100755 index 6ae443fa8fef..2f35b3a3ff2c Binary files a/csharp/ql/test/library-tests/cil/dataflow/DataFlow.dll and b/csharp/ql/test/library-tests/cil/dataflow/DataFlow.dll differ diff --git a/csharp/ql/test/library-tests/cil/dataflow/DataFlow.expected b/csharp/ql/test/library-tests/cil/dataflow/DataFlow.expected index d5f1336a21b8..438debaf6413 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/DataFlow.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/DataFlow.expected @@ -12,6 +12,6 @@ | dataflow.cs:46:35:46:39 | "t1b" | dataflow.cs:46:18:46:40 | call to method Taint1 | | dataflow.cs:49:35:49:38 | "t6" | dataflow.cs:49:18:49:45 | call to method TaintIndirect | | dataflow.cs:49:41:49:44 | "t6" | dataflow.cs:49:18:49:45 | call to method TaintIndirect | -| dataflow.cs:95:30:95:33 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | -| dataflow.cs:95:30:95:33 | null | dataflow.cs:101:20:101:33 | call to method IndirectNull | -| dataflow.cs:102:23:102:26 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | +| dataflow.cs:102:30:102:33 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | +| dataflow.cs:102:30:102:33 | null | dataflow.cs:108:20:108:33 | call to method IndirectNull | +| dataflow.cs:109:23:109:26 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | diff --git a/csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.cs_ b/csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.cs_ new file mode 100644 index 000000000000..7ef1e6fee187 --- /dev/null +++ b/csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.cs_ @@ -0,0 +1,24 @@ +// Generate DataFlowUnoptimized.dll: `csc /target:library DataFlowUnoptimized.cs_ /out:DataFlowUnoptimized.dll` + +using System; + +namespace DataflowUnoptimized +{ + public class MaybeNullMethods + { + public bool cond = false; + + public string MaybeNull() + { + if (cond) + return null; + else + return "not null"; + } + + public string MaybeNull2() + { + return cond ? null : "not null"; + } + } +} diff --git a/csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.dll b/csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.dll new file mode 100644 index 000000000000..b290ece70652 Binary files /dev/null and b/csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.dll differ diff --git a/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected b/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected index 8d840cd1e487..2daeb3fce6cf 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected @@ -30,3 +30,8 @@ alwaysNotNull | dataflow.cs:90:24:90:34 | access to local variable nullMethods | | dataflow.cs:91:24:91:34 | access to local variable nullMethods | | dataflow.cs:92:26:92:32 | access to local variable nonNull | +| dataflow.cs:95:25:95:31 | access to local variable nonNull | +| 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:22:99:37 | access to local variable maybeNullMethods | diff --git a/csharp/ql/test/library-tests/cil/dataflow/TaintTracking.expected b/csharp/ql/test/library-tests/cil/dataflow/TaintTracking.expected index 2fdc021527d0..6124b5560055 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/TaintTracking.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/TaintTracking.expected @@ -21,8 +21,8 @@ | dataflow.cs:48:28:48:28 | 1 | dataflow.cs:48:18:48:29 | call to method Taint3 | | dataflow.cs:49:35:49:38 | "t6" | dataflow.cs:49:18:49:45 | call to method TaintIndirect | | dataflow.cs:49:41:49:44 | "t6" | dataflow.cs:49:18:49:45 | call to method TaintIndirect | -| dataflow.cs:95:30:95:33 | null | dataflow.cs:74:21:74:52 | ... ?? ... | -| dataflow.cs:95:30:95:33 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | -| dataflow.cs:95:30:95:33 | null | dataflow.cs:101:20:101:33 | call to method IndirectNull | -| dataflow.cs:102:23:102:26 | null | dataflow.cs:74:21:74:52 | ... ?? ... | -| dataflow.cs:102:23:102:26 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | +| dataflow.cs:102:30:102:33 | null | dataflow.cs:74:21:74:52 | ... ?? ... | +| dataflow.cs:102:30:102:33 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | +| dataflow.cs:102:30:102:33 | null | dataflow.cs:108:20:108:33 | call to method IndirectNull | +| dataflow.cs:109:23:109:26 | null | dataflow.cs:74:21:74:52 | ... ?? ... | +| dataflow.cs:109:23:109:26 | null | dataflow.cs:89:24:89:51 | ... ? ... : ... | diff --git a/csharp/ql/test/library-tests/cil/dataflow/dataflow.cs b/csharp/ql/test/library-tests/cil/dataflow/dataflow.cs index 82bf2aff9f7e..a8a763321578 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/dataflow.cs +++ b/csharp/ql/test/library-tests/cil/dataflow/dataflow.cs @@ -90,6 +90,13 @@ void Nullness() var notNull2 = nullMethods.VirtualReturnsNull(); var notNull3 = nullMethods.VirtualNullProperty; var notNonNull = nonNull.VirtualNonNull; + + // The following are maybe null + var maybeNull = nonNull.MaybeNull(); + var maybeNull2 = nonNull.MaybeNull2(); + var maybeNullMethods = new DataflowUnoptimized.MaybeNullMethods(); + maybeNull = maybeNullMethods.MaybeNull(); + maybeNull2 = maybeNullMethods.MaybeNull2(); } object IndirectNull() => null;