<div dir="ltr"><div>Just to be clear I am not against having more heuristics to suppress false positives, I just have some concerns around some scenarios:<br></div><div>1.) Debugging: it will get harder and harder to actually write examples that trigger the desired parts of the analyzer unless we turn some of the heuristics off, but this of course only a minor concern.</div><div>2.) Probing: When someone evaluates the analyzer she is likely to do it using little code examples. While I do agree that the analyzer should be optimized for real world code I think it is good if it works for silly little examples as well to give a good impression for the users. When we writes such examples we often do not notice subtle properties of the code like the one you written before. And the user might have the impression that the analyzer is unable to catch those simple examples.<br></div><div>3.) Fixing reports: how can a user be user if she really fixed a problem or only triggered yet another suppression heuristics?</div><div>4.) Introducing reports: changing seemingly unrelated parts of the code can "untrigger" suppressions introducing new reports and the user might wonder what change triggered the new warnings and why.<br></div><div><br></div><div>Of course 3) and 4) are already kind of inherent properties of symbolic execution as the analysis coverage can change pretty easily by slightly changing the code but I am still a bit uncomfortable with making this worse without being able to give a good explanation what is going on. <br></div><div><br></div><div>All in all I think a heuristic like this is a good idea if we can give the users a good and intuitive description why the bug was suppressed in a code snippet. And that description should not require the knowledge of reaching definitions or other similar concepts. So I think if we can easily extend the FAQ with such information I am on board with this change. For example, inline defensive check suppression is easy to explain, we can tell the users the analyzer ignore some checks in the callees since they might only be applicable for other callers. If we can have a similar clear description for this one let's move ahead. What do you think?</div><div><br></div>Taking another look at the example:<div>It is only false positive if bar never returns null. Letting the analyzer know that this is the case either with an assertion or an annotation makes the issue disappear. I guess this is a question how static analysis friendly one is. But my preferred solution in this case would be to let the analyzer know the invariants of the code rather than suppressing the report. But of course, to truly know the usefulness of the heuristics we need to try it on real world code :) <br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 17 Jul 2019 at 02:03, Kristóf Umann via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I talked with Gábor, and am shamelessly stealing his comment, but I'm doing this for the greater good so it's not forgotten :^)<div><br></div><div>I don't immediately see anything wrong with this heuristic -- I mean, we have to see how it behaves, but let's presume it works. It's obvious though we would sometimes suppress true positives. How many similar suppressing tricks do we have? Does this mean that a small change in my code in order to solve a bug found by static analysis could trigger some suppression and "untrigger" others? Should I expect seemingly random results after minimal changes? I guess there is an argument to be made with also being conservative with how many suppression techniques do we have, and how would they interact.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 28 Jun 2019 at 02:21, Artem Dergachev via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF">
    We do occasionally make things configurable but this particular
    situation strikes me as something that i'd find hard to explain to
    the users if i add such option. I guess we can eventually add a
    global "optimism level" option that would tweak a lot of such
    behaviors.<br>
    <br>
    But even when we do that, we'll be pretty far away from a
    verification machine. Even in the most optimistic mode we won't give
    any guarantees that we'll prevent all the bugs of the given kind, so
    for a seriously critical piece of code you'll have to use other
    tools.<br>
    <br>
    <div class="gmail-m_1924152829067058867gmail-m_4258237887395020367moz-cite-prefix">On 6/27/19 3:13 PM, Phil King wrote:<br>
    </div>
    <blockquote type="cite">
      
      Would it make sense to allow this sort of behaviour to be
      configurable?
      <div><br>
      </div>
      <div>For example, much of the time I might not want to be nagged
        with “this may be a problem” and would like a pragmatic
        approach, but if I’m writing some critical code I would like to
        know “this cannot be proven to be correct” and would like the
        check to be pessimistic.</div>
      <div><br>
      </div>
      <div>These different use-cases can also be adopted when checking
        legacy code (pragmatic) or new code (pessimistic).</div>
      <div><br>
      </div>
      <div>For the pessimistic case, there is still the chance to use
        information about bar() to drop the warning if it can be shown
        never to yield a null pointer.<br>
        <br>
        <div id="gmail-m_1924152829067058867gmail-m_4258237887395020367AppleMailSignature" dir="ltr">Sent from my iPhone</div>
        <div dir="ltr"><br>
          On 27 Jun 2019, at 22:55, Artem Dergachev via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>>
          wrote:<br>
          <br>
        </div>
        <blockquote type="cite">
          <div dir="ltr">
            
            Yeah, i mean, we cannot be sure, therefore we want to be
            conservative and not bother the user with a warning. It
            might be a true positive, but it's very wonky and there's
            nothing in the code that indicates that bar() may return
            null; the code makes perfect sense even if bar() doesn't
            ever return null.<br>
            <br>
            <div class="gmail-m_1924152829067058867gmail-m_4258237887395020367moz-cite-prefix">On 6/27/19 2:49 PM, Gábor
              Horváth wrote:<br>
            </div>
            <blockquote type="cite">
              
              <div dir="ltr">I am not sure I follow why do we think that
                the second example is a false positive. <br>
                <div>I think it depends on the user intent. If the user
                  wanted to check if b was reassigned (i.e. checking for
                  the source of the value), and bar never returns a null
                  pointer than it is definitely a false positive. But we
                  cannot be sure what the intent of the check was. What
                  if the user just wanted to check the value regardless
                  of its source. <br>
                </div>
                <br>
                <div class="gmail_quote">
                  <div dir="ltr" class="gmail_attr">On Thu, 27 Jun 2019
                    at 13:56, Artem Dergachev <<a href="mailto:noqnoqneo@gmail.com" target="_blank">noqnoqneo@gmail.com</a>>
                    wrote:<br>
                  </div>
                  <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This is very
                    loosely related to Kristof's GSoC and this is my
                    favorite <br>
                    subject: weird assumption chains.<br>
                    <br>
                    Consider:<br>
                    <br>
                       void foo1() {<br>
                         int *a = bar();<br>
                         int *b = a;<br>
                         if (b) { /* ... */ }<br>
                         *a = 1;<br>
                       }<br>
                    <br>
                    This is a valid null dereference bug. Like, 'b' is
                    probably null <br>
                    (otherwise why check?), therefore 'a', which is
                    equal to 'b', may also <br>
                    be null.<br>
                    <br>
                    Now consider:<br>
                    <br>
                       void foo2() {<br>
                         int *a = bar();<br>
                         int *b = nullptr;<br>
                         if (coin()) {<br>
                           b = a;<br>
                         }<br>
                         if (b) { /* ... */ }<br>
                         *a = 1;<br>
                       }<br>
                    <br>
                    In foo2 we will report a null dereference as well,
                    however the null <br>
                    check for 'b' is well-justified even if bar() never
                    returns null, <br>
                    therefore it's a false positive.<br>
                    <br>
                    How 'bout we suppress the null dereference warning
                    when the <br>
                    reaching-definition analysis for 'b' that starts at
                    'if (b)' - i.e. at <br>
                    the collapse point - yields multiple definitions and
                    some of them is a <br>
                    plain null?<br>
                    <br>
                    Note that the plain-null definition would never be a
                    part of the bug <br>
                    path because it would not have a corresponding
                    collapse point (it's <br>
                    already a concrete null).<br>
                  </blockquote>
                </div>
              </div>
            </blockquote>
            <br>
          </div>
        </blockquote>
        <blockquote type="cite">
          <div dir="ltr"><span>_______________________________________________</span><br>
            <span>cfe-dev mailing list</span><br>
            <span><a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a></span><br>
            <span><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></span><br>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </div>

_______________________________________________<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" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>
_______________________________________________<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" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>