<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 27.03.2013 20:45, Jordan Rose wrote:<br>
    </div>
    <blockquote
      cite="mid:587549F8-11B9-463F-ACC0-E0D7E5D67455@apple.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html;
        charset=ISO-8859-1">
      <br>
      <div>
        <div>On Mar 27, 2013, at 5:32 , Anton Yartsev <<a
            moz-do-not-send="true" href="mailto:anton.yartsev@gmail.com">anton.yartsev@gmail.com</a>>
          wrote:</div>
        <br class="Apple-interchange-newline">
        <blockquote type="cite">
          <meta content="text/html; charset=ISO-8859-1"
            http-equiv="Content-Type">
          <div text="#000000" bgcolor="#FFFFFF">
            <div class="moz-cite-prefix">On 27.03.2013 5:23, Jordan Rose
              wrote:<br>
            </div>
            <blockquote
              cite="mid:9BEA794A-8D25-4F43-B742-A62F39837FFD@apple.com"
              type="cite">
              <meta http-equiv="Content-Type" content="text/html;
                charset=ISO-8859-1">
              <br>
              <div>
                <div>On Mar 26, 2013, at 17:10 , Anton Yartsev <<a
                    moz-do-not-send="true"
                    href="mailto:anton.yartsev@gmail.com">anton.yartsev@gmail.com</a>>

                  wrote:</div>
                <br class="Apple-interchange-newline">
                <blockquote type="cite">
                  <meta content="text/html; charset=ISO-8859-1"
                    http-equiv="Content-Type">
                  <div text="#000000" bgcolor="#FFFFFF">
                    <div class="moz-cite-prefix">On 25.03.2013 21:39,
                      Anna Zaks wrote:<br>
                    </div>
                    <blockquote
                      cite="mid:9084B92B-44BB-497B-9F7D-2A1AAA599822@apple.com"
                      type="cite">
                      <meta http-equiv="Content-Type"
                        content="text/html; charset=ISO-8859-1">
                      <div>
                        <div>
                          <div>On Mar 25, 2013, at 9:30 AM, Jordan Rose
                            <<a moz-do-not-send="true"
                              href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>>



                            wrote:</div>
                          <br class="Apple-interchange-newline">
                          <blockquote type="cite">
                            <div style="font-family: Helvetica;
                              font-size: 12px; font-style: normal;
                              font-variant: normal; font-weight: normal;
                              letter-spacing: normal; line-height:
                              normal; orphans: auto; text-align: start;
                              text-indent: 0px; text-transform: none;
                              white-space: normal; widows: auto;
                              word-spacing: 0px;
                              -webkit-text-size-adjust: auto;
                              -webkit-text-stroke-width: 0px;">
                              <div><br class="Apple-interchange-newline">
                                On Mar 25, 2013, at 8:01 , Anton Yartsev
                                <<a moz-do-not-send="true"
                                  href="mailto:anton.yartsev@gmail.com">anton.yartsev@gmail.com</a>>



                                wrote:</div>
                              <br class="Apple-interchange-newline">
                              <blockquote type="cite">
                                <div text="#000000" bgcolor="#FFFFFF">
                                  <div class="moz-cite-prefix">Committed
                                    as r177849</div>
                                  <br>
                                  Manage to find several random real
                                  bugs (report-843813.html,
                                  report-230257.html, recursive case in
                                  report-727931.html), but for now it is
                                  hard to detect real bugs among tons of
                                  false-positives.<br>
                                  <br>
                                  There are two types of false-positives
                                  that form the majority of reports:<br>
                                  1) Illustrated by the following test
                                  (added similar test to
                                  NewDelete-checker-test.mm):<br>
                                  int *global;<br>
                                  void testMemIsOnHeap() {<br>
                                    int *p = new int; // FIXME:
                                  currently not heap allocated!<span
                                    class="Apple-converted-space"> </span><br>
                                    if (global != p) // evaluates to
                                  UnknownVal() rather then 'true'<br>
                                      global = p;<br>
                                  } // report false-positive leak<br>
                                  <br>
                                  As I understand the problem is that
                                  currently a memory region for 'new' is
                                  not a heap region and this lead to
                                  false-positives like
                                  report-863595.html and others. (e.g.
                                  that causes 'global != p' evaluate to
                                  UnknownVal() rather then 'true' (logic
                                  of SimpleSValBuilder::evalBinOpLL))<br>
                                  <br>
                                  Attached is the proposed patch that
                                  fixes these issues.     </div>
                              </blockquote>
                              <div><br>
                              </div>
                              <div>There are two reasons I didn't use
                                getConjuredHeapSymbol when I originally
                                put in this code:</div>
                              <div>(1) It handles all CXXNewExprs, even
                                if the allocator is not one of the
                                global ones.</div>
                              <div>(2) Heap symbols weren't used yet
                                (Anna added them later for
                                MallocChecker).</div>
                              <div><br>
                              </div>
                              <div>Obviously #2 is bogus now. #1 worries
                                me a bit because it requires duplicating
                                some of the checks you just added to
                                MallocChecker.</div>
                              <div><br>
                              </div>
                              <div>In the long run, the design would be
                                to get the appropriate memory from the
                                allocator call, and we have PR12014's
                                restructuring of the CFG blocking that.
                                I'm not sure if we'd then move the heap
                                symbol logic into a checker, or if it
                                would still stay in Core.</div>
                              <div><br>
                              </div>
                              <div>In the short term, I guess the best
                                idea is to duplicate some of the checks
                                (or refactor them to a helper function
                                somewhere...though probably<span
                                  class="Apple-converted-space"> </span><i>not</i> into


                                AST) and then conjure a heap symbol if
                                we know we can.</div>
                            </div>
                          </blockquote>
                        </div>
                      </div>
                    </blockquote>
                    Failed to find any suitable place other then AST :)
                    Eventually noticed, that actually only a single
                    check should be duplicated. Decided to leave the
                    wide comment added when I tried to find the proper
                    place for isStandardNewDelete().<br>
                    New fix attached.<br>
                  </div>
                </blockquote>
                <div><br>
                </div>
                <div>I don't think this is safe -- what if the user has
                  custom libraries in their system include path? We can
                  really only assume heap allocation for the implicit
                  case and for ::operator new(std::size_t) and
                  ::operator new(std::size_t, std::nothrow_t).</div>
                <div><br>
                </div>
                <div>However, I think just checking "global namespace"
                  might be a good enough approximation. If the user
                  overrides one of the default allocators, it ought to
                  behave like the real one, so we'd only be messing up
                  if they had a new global "allocate from pool" or
                  something.</div>
                <div><br>
                </div>
                <div>Also, in theory we'd have to check placement new,
                  but that's handled specially below anyway.</div>
              </div>
            </blockquote>
            Updated the logic, improved test coverage.<br>
          </div>
        </blockquote>
      </div>
      <br>
      <div>
        <div>+  // Skip all operator new/delete methods (including
          static ones).</div>
      </div>
      <div><br>
      </div>
      <div>All operator new/delete methods are implicitly static, so you
        can just leave out the parenthetical. On the other hand, it
        might be worth noting that we realize this is an approximation
        that might not correctly model a custom global allocator.</div>
    </blockquote>
    Fixed at r178244. The note gone to ExprEngine::VisitCXXNewExpr().<br>
    <br>
    <pre class="moz-signature" cols="72">-- 
Anton</pre>
  </body>
</html>