[cfe-dev] Patch review archives from 2012

[Artem Dergachev via cfe-dev]

Just looked at some of these. The canonical example here is the 
following leak:

  1 void foo() {
  2   void *x = malloc(1); // note: Memory is allocated
  3   void *y = realloc(x, 2); // note: Attempt to reallocate memory
  4   if (y) { // note: Assuming 'y' is null
  5 // note: Reallocation failed
  6     free(y);
  7   } else {
  8 // warning: Potential leak of memory pointed to by 'x'
  9   }
10 }

The extra state trait is necessary to track the connection between 'x' 
and 'y' that appears on line 3 and dissolves on line 4. This is, 
essentially, yet another "Schrodinger" state split: realloc has both 
succeeded and failed until we check the return value. An eager state 
split would make null dereference checker unusable in any sort of code 
that doesn't check for failed reallocs.

On the other hand, using 'x' after realloc should probably be disallowed 
unless a prior branch in the code suggested that realloc has failed. And 
using 'y' would mean that the realloc is assumed to have succeeded.

On 11/19/18 5:00 PM, Kristóf Umann wrote:
> Okay, I'll ask specifics.
>
> I've spent the better part of last week familiarizing myself with 
> MallocChecker's code in order to eventually split it up (which has 
> been a surprisingly pleasant experience!). In the process of learning 
> how it works, I'm adding doxygen comments to every non-trivial method 
> and non-trivial hackery, from what I can gather from old cfe-dev 
> threads and bug reports.
> To me it isn't clear what ReallocPair[1] (added by this[2] commit) 
> does. I can come up with some ideas, but I don't have a grasp on it at 
> all, sadly. The very cryptic boolean 
> out-parameter ReleasedAllocated[3], related to [1], doesn't help this 
> case either. It was added by this[4] commit, but I'm still struggleing 
> to understand what's the happening here.
>
> [1]https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/MallocChecker.cpp#L141
> [2]https://github.com/llvm-mirror/clang/commit/c8bb3befcad8cd8fc9556bc265289b07dc3c94c8
> [2]https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/MallocChecker.cpp#L341
> [3]https://github.com/llvm-mirror/clang/commit/55dd956d521d4d650dfd929d67f4b98ede61c0ea
>
>
> Artem Dergachev <noqnoqneo at gmail.com <mailto:noqnoqneo at gmail.com>> ezt 
> írta (időpont: 2018. nov. 20., K, 1:43):
>
>     Back then Static Analyzer was developed almost exclusively by the
>     first
>     generation of Analyzer developers which was a small team at Apple,
>     none
>     of which are still active Analyzer developers. I guess they didn't
>     really need that much open documentation when they could simply
>     look at
>     each other's monitor and discuss things on a whiteboard. These
>     days the
>     community is much bigger (and i'm much more of a masochist who
>     does not
>     feel happy to just sit down and write code, and actively prevents
>     others
>     from doing the same, so that everybody knew the taste of code review
>     pain), so Phabricator turned out to be a spot-on tool.
>
>     But you should ask around for specific things. It might be that
>     people
>     have already managed to re-discover the motivation behind certain
>     solutions.
>
>     On 11/19/18 3:57 PM, Kristóf Umann via cfe-dev wrote:
>     > Hi!
>     >
>     > Where can I find design discussions and patch reviews similar to
>     > reviews.llvm.org <http://reviews.llvm.org>
>     <http://reviews.llvm.org>, but from 2012? There are a
>     > variety of commits to some files I'm working with in the Static
>     > Analyzer that could use more explanation. I didn't have any luck
>     > reading through the cfe-dev and cfe-commits archives.
>     >
>     > Cheers,
>     > Kristóf Umann
>     >
>     >
>     > _______________________________________________
>     > cfe-dev mailing list
>     > cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>     > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>