r178890 - [analyzer] Split new/delete checker into use-after-free and leaks parts.

[Anton Yartsev]

On 06.04.2013 0:30, Jordan Rose wrote:
> On Apr 5, 2013, at 13:22 , Anton Yartsev <anton.yartsev at gmail.com> wrote:
>
>> On 05.04.2013 21:55, Jordan Rose wrote:
>>>     if (!Filter.CMallocOptimistic && !Filter.CMallocPessimistic &&
>>> -      !Filter.CNewDeleteChecker)
>>> +      !Filter.CNewDeleteLeaksChecker)
>>>       return;
>>>   -  if (!isTrackedFamily(C, Sym))
>>> +  const RefState *RS = C.getState()->get<RegionState>(Sym);
>>> +  assert(RS && "cannot leak an untracked symbol");
>>> +  AllocationFamily Family = RS->getAllocationFamily();
>>> +  if (!isTrackedFamily(Family))
>>>       return;
>>>   +  // Special case for new and new[]; these are controlled by a separate checker
>>> +  // flag so that they can be selectively disabled.
>>> +  if (Family == AF_CXXNew || Family == AF_CXXNewArray)
>>> +    if (!Filter.CNewDeleteLeaksChecker)
>>> +      return;
>> We already checked for Filter.CNewDeleteLeaksChecker at the beginning.
>>
>> Now this works properly just because isTrackedFamily() does not know about CNewDeleteLeaksChecker and returns false.
>> What about adding bug types at this point?
> The first check says "any of {MallocOptimistic,MallocPessimistic,NewDeleteLeaks}". This one checks exclusively for NewDeleteLeaks and AF_CXXNew[Array].
Oops, sorry.

> This is basically a quick-and-dirty "bug types" implementation that's used because today we only have one bug type that's different. I'm not sure the added generalization will actually make things clearer—here we implicitly know the "type" is a leak because we're in reportLeak. None of the rest of the code has to know that leaks are any different from anything else. But maybe I'm missing some benefit of the general bug types.
No benefits other then just looks clearer to me :) Passing a bug kind 
shows, on the one hand, that sometimes we depend on the bug type, and 
allows to freely add other bug-dependent code on the other.
I quickly returned the bug types while waited for your replay, just to 
see, if it simplify the appearance, attached the patch so you could also 
see how it looks like.
I am under no circumstances insist on adding bug types, sending just for 
information, perhaps you like it. :)

In any case, these may be slightly simplified as isTrackedFamily() now 
treats AF_None as unacceptable (after r178899 )
> I'll admit it's a bit kludgy that the family is effectively checked twice, but I actually like the implication that alpha.cplusplus.NewDeleteLeaks is a "sub-checker" of cplusplus.NewDelete.
>
> Jordan

-- 
Anton

-------------- next part --------------
A non-text attachment was scrubbed...
Name: MallocChecker.cpp.BugTypes.patch
Type: text/x-diff
Size: 4540 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130406/41e2cc90/attachment.patch>