<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=HU link=blue vlink=purple style='word-wrap:break-word'><div class=WordSection1><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>Thanks for the clarification Artem!<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>All clear I think :)<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>PS: The defensive checks explanation was really useful. I appreciate that.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'>Regards, Balazs<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='mso-fareast-language:EN-US'><o:p> </o:p></span></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span lang=EN-US>From:</span></b><span lang=EN-US> Artem Dergachev <noqnoqneo@gmail.com> <br><b>Sent:</b> 2021. szeptember 20., hétfő 22:16<br><b>To:</b> Deep Majumder <deep.majumder2019@gmail.com>; Benics Balázs <benicsbalazs@gmail.com><br><b>Cc:</b> cfe-dev <cfe-dev@lists.llvm.org><br><b>Subject:</b> Re: [cfe-dev] [analyzer] Questions about the null dereference checker<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal style='margin-bottom:12.0pt'><br><br><o:p></o:p></p><div><p class=MsoNormal>On 9/20/21 5:42 AM, Deep Majumder via cfe-dev wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>The second one seems about right to me. <o:p></o:p></p></div></blockquote><p class=MsoNormal><br>The second example is an example of so-called inlined-defensive-check suppression (2nd type, to be exact). The specific warning in your example would have been a true positive but in most practical examples it wouldn't be.<br><br>Inlined-defensive-check suppressions are a partial solution that tries to eliminate infeasible paths that arise from the huge difference between state splits in nested ("inlined") stack frames and state splits in the top-level stack frame. Namely, a state split in the top-level stack frame is justified by saying that "if both states aren't possible then one of the branches is dead code". However, in a nested stack frame that would only mean that the code is dead *for the current call site* but there may be other call sites on which that code is executed. Fundamentally speaking, this is a very real architectural problem with inlining. We need summaries or call widening to deal with it properly but as of today we only have this partial solution.<br><br>As an example,<br><br> void foo(int *p) {<br> if (p) {}<br> *p = 1; // null deref?<br> }<br><br>is a true positive because if it was impossible for p to be null then the check is dead. However,<br><br> void bar(int *p) {<br> if (p) {} // defensive check<br> }<br><br> void foo(int *p) {<br> bar(p); // inline the defensive check<br> *p = 1; // null deref?<br> }<br><br>is a false positive because there's no indication in the code that the false-branch can in fact be hit from the call site within foo().<br><br>Inlined-defensive-check suppressions of the first type suppress null dereference warnings where the null pointer is tracked back (with the help of trackExpressionValue()) to a state split in a nested stack frame.<br><br>Inlined-defensive-check suppressions of the second type deal with the fact that it is common for an unrelated defensive check to return null. For example:<br><br> int *bar(int x) {<br> if (x == 0) // defensive check<br> return nullptr; // the defensive behavior<br><br> return new int;<br> }<br><br> void foo(int x) {<br> int *p = bar(x); // inline the defensive check<br> *p = 1; // null deref?<br> }<br><br>In this case the null dereference would, again, be a false positive because there's no indication that x can be zero at the current call site. <br><br>Inlined-defensive-check suppressions of the second type suppress null dereference warnings where the null pointer is tracked back (with the help of trackExpressionValue()) to a 'return nullptr' in a nested stack frame. This takes care of this other example.<br><br>Unfortunately this specific implementation also suppresses your example where there's no defensive check to begin with. However, in practice such functions don't exist: there's no reason for a function to have a return value if it's always null. So it's an extremely minor false negative. You can fix it if you want but I honestly think it'll do more harm than good to mess with this already-flimsy facility.<br><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>The first one is a bit perplexing. In fact, if p were a unique_ptr, the first one would have emitted a warning on *p.<o:p></o:p></p></div></blockquote><p class=MsoNormal><br>Yes, to add to my other email, the rules for smart pointers are different. Invoking overloaded operator * or operator -> on a null smart pointer is an immediate undefined behavior according to the standard even if no actual dereference is being performed after the fact. In particular, this clearly makes a lot of sense for operator * because it would otherwise have to produce a null lvalue reference as its return value.<br><br>So there's no consistency in the language rules in the first place.<br><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><p class=MsoNormal>Warm regards,<o:p></o:p></p></div><div><p class=MsoNormal>Deep<o:p></o:p></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>On Mon, 20 Sep, 2021, 6:03 pm via cfe-dev, <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Hi,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Let’s examine this code snippet:</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> void simply_deref_null() {</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> int *p = 0;</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> *p ; // no warning?</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> *p = 42; // warns!</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> }</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Turns out the NullDereference checker treats the two pointer derefs differently.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>For simply reading through a null pointer is allowed but storing a value is prohibited.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Why don't we prohibit reading through null pointers?</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>----</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>By returning a null pointer from a function it suddenly we no longer report an error:</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> int *get() { return 0; }</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> void foo() {</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> int *p = get();</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> *p = 42; // no warning?</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> }</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>According to my investigation the bug actually found and a sink node will be generated in the Exploded graph, but the bug report will be marked as invalid by the ReturnVisitor.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>This behavior could be altered to prevent such suppression from happening by setting the `suppress-null-return-paths` analyzer option to `true`.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Am I right that this is the intentional behavior and if we want to catch bugs like this, then we should enable the aforementioned option?</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>/CC NoQ</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Regards,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=EN-US>Balazs</span><o:p></o:p></p></div></div><p class=MsoNormal>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><o:p></o:p></p></blockquote></div><p class=MsoNormal><br><br><o:p></o:p></p><pre>_______________________________________________<o:p></o:p></pre><pre>cfe-dev mailing list<o:p></o:p></pre><pre><a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><o:p></o:p></pre><pre><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><o:p></o:p></pre></blockquote><p class=MsoNormal><o:p> </o:p></p></div></body></html>