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

[Jordan Rose]

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].

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.

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