[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, martong, balazske, vsavchenko, steakhal, ASDenysPetrov.
Szelethus added a project: clang.
Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity.
Szelethus requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.

D105553 <https://reviews.llvm.org/D105553> added NoStateChangeFuncVisitor, an abstract class to aid in creating notes such as "Returning without writing to 'x'", or "Returning without changing the ownership status of allocated memory". Its clients need to define, among other things, what a change of state is.

For code like this:

  f() {
    g();
  }
  
  foo() {
    f();
    h();
  }

We'd have a path in the `ExplodedGraph` that looks like this:

               -- <g> -->
              /          \       
           ---     <f>    -------->        --- <h> --->
          /                        \      /            \
  --------        <foo>             ------    <foo>     -->

When we're interested in whether `f` neglected to change some property, `NoStateChangeFuncVisitor` asks these questions:

                         ÷×~     
                  -- <g> -->
             ß   /          \$    @&#*       
              ---     <f>    -------->        --- <h> --->
             /                        \      /            \
     --------        <foo>             ------    <foo>     -->
  
                             
  Has anything changed in between # and *?
  Has anything changed in between & and *?
  Has anything changed in between @ and *?
  ...
  Has anything changed in between $ and *?
  Has anything changed in between × and ~?
  Has anything changed in between ÷ and ~?
  ...
  Has anything changed in between ß and *?
  ...

This is a rather thorough line of questioning, which is why in D105819 <https://reviews.llvm.org/D105819>, I was only interested in whether state *right before* and *right after* a function call changed, and early returned to the `CallEnter` location:

  if (!CurrN->getLocationAs<CallEnter>())
    return;

Except that I made a typo, and forgot to negate the condition. So, in this patch, I'm fixing that, and under the same hood allow all clients to decide to do this whole-function check instead of the thorough one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108695

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108695.368609.patch
Type: text/x-patch
Size: 8459 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210825/0c4379af/attachment-0001.bin>