<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <br>
    <br>
    <br>
    <div class="moz-cite-prefix">On 9/20/21 5:42 AM, Deep Majumder via
      cfe-dev wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAKt7x9SY9Pw9CK_-SBcVXhtSauCgg7VCULMWng5zMB+p20FcQQ@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="auto">The second one seems about right to me. <br>
      </div>
    </blockquote>
    <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>
    <blockquote type="cite"
cite="mid:CAKt7x9SY9Pw9CK_-SBcVXhtSauCgg7VCULMWng5zMB+p20FcQQ@mail.gmail.com">
      <div dir="auto">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.</div>
    </blockquote>
    <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>
    <blockquote type="cite"
cite="mid:CAKt7x9SY9Pw9CK_-SBcVXhtSauCgg7VCULMWng5zMB+p20FcQQ@mail.gmail.com">
      <div dir="auto">
        <div dir="auto">Warm regards,</div>
        <div dir="auto">Deep</div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Mon, 20 Sep, 2021, 6:03 pm
          via cfe-dev, <<a href="mailto:cfe-dev@lists.llvm.org"
            moz-do-not-send="true">cfe-dev@lists.llvm.org</a>> wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex">
          <div link="#0563C1" vlink="#954F72"
            style="word-wrap:break-word" lang="HU">
            <div class="m_-38726125425477192WordSection1">
              <p class="MsoNormal"><span lang="EN-US">Hi,</span></p>
              <p class="MsoNormal"><span lang="EN-US">Let’s examine this
                  code snippet:</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">  void
                  simply_deref_null() {</span></p>
              <p class="MsoNormal"><span lang="EN-US">    int *p = 0;</span></p>
              <p class="MsoNormal"><span lang="EN-US">    *p ; // no
                  warning?</span></p>
              <p class="MsoNormal"><span lang="EN-US">    *p = 42; //
                  warns!</span></p>
              <p class="MsoNormal"><span lang="EN-US">  }</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">Turns out the
                  NullDereference checker treats the two pointer derefs
                  differently.</span></p>
              <p class="MsoNormal"><span lang="EN-US">For simply reading
                  through a null pointer is allowed but storing a value
                  is prohibited.</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">Why don't we
                  prohibit reading through null pointers?</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">----</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">By returning a
                  null pointer from a function it suddenly we no longer
                  report an error:</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">  int *get() {
                  return 0; }</span></p>
              <p class="MsoNormal"><span lang="EN-US">  void foo() {</span></p>
              <p class="MsoNormal"><span lang="EN-US">    int *p =
                  get();</span></p>
              <p class="MsoNormal"><span lang="EN-US">    *p = 42; // no
                  warning?</span></p>
              <p class="MsoNormal"><span lang="EN-US">  }</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><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></p>
              <p class="MsoNormal"><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></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><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></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">/CC NoQ</span></p>
              <p class="MsoNormal"><span lang="EN-US"> </span></p>
              <p class="MsoNormal"><span lang="EN-US">Regards,</span></p>
              <p class="MsoNormal"><span lang="EN-US">Balazs</span></p>
            </div>
          </div>
          _______________________________________________<br>
          cfe-dev mailing list<br>
          <a href="mailto:cfe-dev@lists.llvm.org" target="_blank"
            rel="noreferrer" moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
          <a
            href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
            rel="noreferrer noreferrer" target="_blank"
            moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
        </blockquote>
      </div>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
cfe-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a>
</pre>
    </blockquote>
    <br>
  </body>
</html>