From 4fc61ebbf3504ad52be109c7d54d4b194acee907 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 27 May 2019 16:32:39 +0200 Subject: [PATCH 1/3] C#: Add tests for maybe-`null` CIL methods --- .../cil/dataflow/CallableReturns.expected | 2 ++ .../cil/dataflow/CallableReturns.ql | 2 +- .../library-tests/cil/dataflow/DataFlow.cs_ | 2 ++ .../library-tests/cil/dataflow/DataFlow.dll | Bin 6656 -> 5632 bytes .../cil/dataflow/DataFlow.expected | 6 ++--- .../cil/dataflow/DataFlowUnoptimized.cs_ | 24 ++++++++++++++++++ .../cil/dataflow/DataFlowUnoptimized.dll | Bin 0 -> 3584 bytes .../cil/dataflow/Nullness.expected | 9 +++++++ .../cil/dataflow/TaintTracking.expected | 10 ++++---- .../library-tests/cil/dataflow/dataflow.cs | 7 +++++ 10 files changed, 53 insertions(+), 9 deletions(-) mode change 100644 => 100755 csharp/ql/test/library-tests/cil/dataflow/DataFlow.dll create mode 100644 csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.cs_ create mode 100644 csharp/ql/test/library-tests/cil/dataflow/DataFlowUnoptimized.dll diff --git a/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected b/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected index 8e9a36971f1b..b1f7c463d0c0 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected @@ -7,6 +7,7 @@ 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) | @@ -24,6 +25,7 @@ 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() | 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 6ae443fa8fef574889264b226e834b17ca1e460e..2f35b3a3ff2c692378b6c1091c6524f6c123b0b7 GIT binary patch literal 5632 zcmeHLYit}>6+U-%y;*PYCaImYapPnXJIQv{t{mG9DU`By95+UGoMs&-l!86GJK0P! zp0Q?Toj57TR5T(I;!zb;Dj}6WsN$u9DoA+(H8>S^ku=;3RV;lgRHg61iP0+LuV=kot^B`eI3v`^@C;NF>kZvk5zYz(^!>dn63w zDpHkX;|~k7+qjAi5etN&?mDGD5~7P7^wSdj~;`%jnXIMyO>;r@qDPmZHL z3;Yb@G1o7n=7OcD&TM~9K}GZrHB39=J-HbWuH?+-xcG=<8$KraXvpB<_;|U-OZJN@ zu2VFpP8%s&7CNT-jV)A1H!1pt@lTbagCZXy<`W|67P?)rYX zdlOA))G4~df(+^wluEFrK|2L)(P)pLk7>&RLBAljOwv7wY^!3~t8@@FqYlzZlY75L zuc;zJl;5xoK9=7^1P1*9RMC3A zUGlVIo2Ri``wT_fKvP)heMW{334f2!QK6GUAE7?AmEKQZHnwBtHS%;_NVy)_G;WvcwVG9S?KN3!Zbgpoow8l)vT)&={#4zIoJE_>pej4A z6RoUimo2w$Q~Z*maVxSWT>ny`>bh%LmxRtaK~%R~ovn5AKw8_F9n1zH8?CUk5=QnS zO?v3ism@^1CC+1)EA|qHPK8S32NatX#vQBX`C;T#LL@oxFClZ&c68pahHxx%-+;P> z9%Z4CypkO(I~6jpdNRh^(! zjL|k_tzMc!Sy4^YNOHWiINg%>bIe?pT=UXytjD(Gua~b|yKS~{mfGCN;MyduDQGH) zwt3`JL?(Gu$5#!$YeeF;O5Qn3=gFZEvY0m7SdfHhU8gcQ3w%g6Edsf;A}7JORzCED z*I$T=&zyMaw+DKj=%J*kR61dj>HxECTauK@;#kUNv*~YNd2DXy$Sc>4lwqXUW*43; zj+k9&w2MhL+hMXPXgZySAkAj!L=L7onC2**R{9>cn?G4*jU8tKYYE9~uD*EM73>U& zFsZmI>U$CMk1}sE_r}t=KgF8)qkb^%y3;u8;*-L*Wk0d@)_(X{cXNBAzyGd;zv*x% z=GR19iNAaYH;}X0aXe>>xW~r{WD)og&^ap6Jm@hx1#S{I?GwP0pcC<}o4%j?%fI;D zxxTzt3;1o);#Q>NpbB0s3gE}V+Opx(qjOlfTn6MFGz+Z-X$ZZAHR(X>iJs_%?BUs1C0o{v;RMUPOy@ym0KTB=>n6Mqh>RYp&gKYrowO9u6 z#$S7g-i3FslF>wB>mha=sGy6$MB_3R3%$D-&n2|3g5%o&ZFl(8kuDD=6#LGxK6@{j|l1*e}oxnAKluxc#K3xq|GSC-+C&T cZ`uw`Z_*bl>xKb#UA7|8fTY4fHkl^8f$< literal 6656 zcmeHLZ)_Y#6@R;XcFsLiX+pOci-G}*gbI~%We zkKMh*PEv4$QWYhFR)wH|1XZDpkSL01Kv5A=B_Ki~K>`6P6hsh$<`XR+!iNe7zj-@* zzPp$p_?ph$zM0>9Z{ECl^JaGL1`j?@8WF|t+`dh81+^$$Oz%!+AWp1&BSDwzuCKbH z?7qHg(-v|G_=({FzM zUNqVbTCLAl+KKK1r#9q%2at3;U3iEVaa#@j77XD(ml;s-MbW71j8GNd*;+!uLOBe( zL!!||v_b@;x*R520N&KYL?hL*yJ4LuUM7zNKVF8DfjJcbUlG1R#&eZ^198#eUefcu z91#1~g@%~uVmxJ~3*IL^({`aKe5J#9gso+G%1ReePZ{Lq7chTOgl{U<&~q(B@69Jt znEStz`o(Pzs;%fkvZ}exU$2>)V=rt^n(#URd9ME&&JHnmw2)L{*P^5-_%2 zUECJ(CF7&DAzTc_CZRuTy#a%qXl@0!7GuFCP(>A~)@yC6RjqY}P+YBPtqX;Bg7bjj zKpH8o)?%$#u==+9wAhnv3C(Or4G(q=+rnPrqX(w=j1g%I8z|@0FokQ8%fnhN?N9|BQgm%0c`#E; zCzJ`bmXhjUL4U0X`XbXyOqVb}$L()x??AeN?W9Pwi8+fv>#0c*oHOb>Jn;8{8T~VqNt-pR z2<2JWdjwBhqQgKcHA?g5nE}X;Sj~+w4WO2phOSA%Ld9xG!9vUCL}rqS%QKPv6Y2% zDn#@nc(Rb5Y2n#Qnb>(Nv7aOQ3U*D@4*6H1uF~&86{>Dma!l+Eg-o@Mnl*(QL2Hre z&00OOxLI379Za_}?PfZ_^a-XPXKI1glEHME>1n2)r8Z>=y+9YWW=bMXW1vmsf-a}e zfws_>KtDp)K|ASf&`0PdXfNFY-NpP2Xg56tCud@UzQk08&l)~4num`#lkC_*&`nG& zreCC|!MUgkewI1snSP$>D$0UC1F9mz8t6RagNn0i9%ut>s#ufxpp8H(j=P1R_XDZ4 zhEImIpen7SCGh3Lps^Vd$?~FMjbhM=xH>K8W(=HJ-h!x!3!`Br`SEvia0p=2LT-4>hH!$dOd& zWM^1y469AFebCJpZSzs;M;?enV^`Z?bUO7bmGvV9wRq_cT&G) z+Ibpdnsc2zWv2bWoJ{t*j_=xL1c!@GU`?8Iz_S8#x8<1B>rNId+w^vrj_DbJnePtp zlFJqYlhS6kI6iJhz;beu8a01L6Hf>{yTmGGcU;*;QTyUpA7%-gaS%C}^^8+hu*;Y^{ zJ!aU&>Ubul(^v=!?+BDO0;At{k0pj*jH;9)DUA{liATtQau_ac zX-Um7+suiW(0~Kyto#r%o}eZHP1Z`w+GGOh@1B@ z6eoTJWfsYZAe+Ku$(Y_TD`)yLMMQu6%=AenxrL^j2HW?k+dghg`x0Owb4EZz*`x5g zYWf3C-tx-Xn16{^Zl%QJO<6)w*u#A#7Jye`Of;rA`eI*4o3Ua!s`*tLw?a3JQiU0w zTm`2*1SJ7AIyk`t(#fGBZkf1eGRGnS8 zIh+-fqrO}8a^|RK7F>TcV>tP&J2e_jBQ@$~kB-V|j5=nJLqv>10dXklBn$a0WghQN zZQMk1ytBR>Pt0k@nWmq$ZOPUB#X36j9Q=;T%c8AEsIrd!hX}ccunuY49>n{TCVk<8 z$p@E^-d+nE)l;uD_5u1oBOrb|@!v)>l?KB54VLc?zR(l@9X;7a^k%r11PkpAp!+F9 zhd}#i7_9*sqCLO^p#5lz@`KpB{|fI}B2z^P>&HTX`&RTjQ!-Gc@hGrrkOw;!&KeUo z9o$D>P%q79wn%-hC&B5%{s^eM+<7b<+4t8Y6FGo|F;e)RRw zdv-n@xLvoG9-?iy;VK-FfOQ@CgSU3c!|E}Tl!sjW!$8!^_uJj*H_pApEDJcxMU=+z z77+hg>Ues1j3LPv5fo!Y^AjkH#@)sW6ZaIzdS$S zq|y@+Z*d5JC0lIi7fB-jfk5SdvyV-2QGUzwA+9mur6GqhiA zv5nAL4`~kyamLg-56n;Zvf&4QnPJ5(Pet&piD}ZEH+;+S2vl5`>6Bath!ZzT?>)ZJ!v*@ z&$cPVkk;Kkg!IL$iJo_5ctT3l4h|YEzBa1G%w%bIMvR;>oEyIU0u6F^*q-E&?0djF z><2|JhQ94q13`Sw$-3ER7mL`YF2^GK%Hm{!{dctG0A@!Wuf$%2F70}E0>{PpQyUh- zOv#y|@`}zqh!t09p?&On)WJn$GVDIQ1*PsE}>wC=7i&|ExO)nXxHpb ztD=0j5BSQS%Yap?SF2{pv95*OoKiisFy_@(ZO064&z-j%b6sgcGd^E;nb#5+LnYg> z!;NP2km-$z<6w_7iW@;_)nJrE&yNkGV4GFf3qrdbG?CL*xZ+iU1}2`@i1=B4(em%u zWh;mm!IQ;W(3~y3WtHVVqsxVd5IS7cc_QUZ{_^zCh2)FR6+Y|zK6`4Z`T!|i(=tgN z+A%6!U8%N2qBGg1b!NWayt&jj^6;ZXI>B&fMvJfdSveXDiJt4exjN&y+i@cggz^B!5++8dEuqNQZwcd=r#8u7+8NrDY$u{$ zU3`=J?XT$7Y>bThmt#}jqn5PFTVSg(s|lZZY~C@8IbGc5%@Y|=uh;^kCcOdUCU2z8 zsH@_@-PBLKFB&<8MA`IL56N^_VRC5v8$pRWaEXkxJ$!+)2PmsNtNZ5BiOul_H?d2^ z`b0Om}i@VZBV$Xafbc7L&6(T+bR7$=yRVZ;g33iK;MXg>iX% z1{K#SdaEkam_lzTwpFZZ=L8yYW;J}>ycBPbqcw2`>W7g qztjSFUX8iNIt4`5Sak;*WchF7IP^#!wE}wdXX1cd*CYAgRp4*OKt!DY literal 0 HcmV?d00001 diff --git a/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected b/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected index 8d840cd1e487..b357d55ea350 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected @@ -6,6 +6,8 @@ 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 | @@ -30,3 +32,10 @@ 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:9:99:50 | ... = ... | +| dataflow.cs:99:22:99:37 | access to local variable maybeNullMethods | +| dataflow.cs:99:22:99:50 | call to method MaybeNull2 | 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; From 0ee5fe88d9012238fbacb8d1c670c4d2fe9b85a2 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Mon, 27 May 2019 16:33:00 +0200 Subject: [PATCH 2/3] CIL: Account for multiple `VariableUpdate::getSource()`s in nullness analysis 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`. --- csharp/ql/src/semmle/code/cil/CallableReturns.qll | 6 ++++-- .../library-tests/cil/dataflow/CallableReturns.expected | 2 -- csharp/ql/test/library-tests/cil/dataflow/Nullness.expected | 4 ---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/csharp/ql/src/semmle/code/cil/CallableReturns.qll b/csharp/ql/src/semmle/code/cil/CallableReturns.qll index 044635f1d147..00fe8fe49a5f 100644 --- a/csharp/ql/src/semmle/code/cil/CallableReturns.qll +++ b/csharp/ql/src/semmle/code/cil/CallableReturns.qll @@ -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)) ) } diff --git a/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected b/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected index b1f7c463d0c0..8e9a36971f1b 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/CallableReturns.expected @@ -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() | diff --git a/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected b/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected index b357d55ea350..2daeb3fce6cf 100644 --- a/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected +++ b/csharp/ql/test/library-tests/cil/dataflow/Nullness.expected @@ -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 | From 428ad72694fee2b6c699f3e95efec84f89ebeb14 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 29 May 2019 09:34:54 +0200 Subject: [PATCH 3/3] C#: Improve performance of `always[Not]NullMethod()` --- csharp/ql/src/semmle/code/cil/CallableReturns.qll | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/csharp/ql/src/semmle/code/cil/CallableReturns.qll b/csharp/ql/src/semmle/code/cil/CallableReturns.qll index 00fe8fe49a5f..b1a666954b9a 100644 --- a/csharp/ql/src/semmle/code/cil/CallableReturns.qll +++ b/csharp/ql/src/semmle/code/cil/CallableReturns.qll @@ -30,6 +30,11 @@ 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 @@ -37,10 +42,15 @@ private predicate alwaysNullExpr(Expr expr) { alwaysNullMethod(expr.(StaticCall).getTarget()) or forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | - forex(Expr src | src = vu.getSource() | alwaysNullExpr(src)) + 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. */ private predicate alwaysNotNullExpr(Expr expr) { expr instanceof Opcodes::Newobj @@ -50,6 +60,6 @@ private predicate alwaysNotNullExpr(Expr expr) { alwaysNotNullMethod(expr.(StaticCall).getTarget()) or forex(VariableUpdate vu | DefUse::variableUpdateUse(_, vu, expr) | - forex(Expr src | src = vu.getSource() | alwaysNotNullExpr(src)) + alwaysNotNullVariableUpdate(vu) ) }