[PATCH][Review request] unix.Malloc checker improvement: +handling new/delete, +memory.MismatchedFree, +memory.MismatchedDelete, +improved display names for allocators/deallocators

[Anton Yartsev]

> One last question for you before jumping into patch review... Right now, the new diagnostic looks like this:
>
>> Argument to operator delete[] was allocated by malloc(), not operator new[]
> I wonder if this is more helpful, or some variation of this:
>
>> Memory allocated by malloc() should be deallocated by free(), not operator delete[].
> I'm really not sure whether it's better to mention the deallocator's expected allocator, or the allocated region's expected deallocator. Which mistake do you think people are more likely to make?
>
> (Also, I came up with that phrasing in ten seconds; it could totally change.)
The second one of course. I definitely prefer to print the allocated 
region's expected deallocator.
The single downside I see is that this breaks the uniformity of reports 
and complicate ReportBadFree(). (other reports about, for example, 
freeing addresses of labels, could not be printed in the new fashion)

>> -     << " from the start of memory allocated by malloc()";
>> +     << " from the start of memory allocated by ";
>> +  if (AllocExpr)
>> +    if(!printAllocDeallocName(os, C, AllocExpr))
>> +      os << "an allocator";
>> +  else
>> +    printExpectedAllocName(os, C, DeallocExpr);
> This is also not quite as nice as it could be (same reason). (Also, the "else" case doesn't match the right "if"!)
Oops!

>> +void testDeleteOp1() {
>> +  int *p = (int *)malloc(sizeof(int));
>> +  operator delete(p); // FIXME: should complain "Argument to operator delete() was allocated by malloc(), not operator new"
>> +}
> Hm. Any idea why this is not working? Is it not showing up as a standard operator delete?
Nasty error.
There appears to be no RefState attached to a symbolic region for 'p' 
when operator delete(p) is processed by FreeMemAux(). Everything works 
fine if the call to operator delete is replaced by a delete expression 
or free().
Debugging of the following example
void test() {
   int *p = (int *)malloc(sizeof(int));
   operator delete(p); // case 1
   free(p); // case2
}
showed that in both cases all the addresses of SVals and regions remain 
the same from one call to FreeMemAux() to another but in the first case 
"const RefState *RsBase = State->get<RegionState>(SymBase);" returns 0.
The same thing happens for Objective-C methods.
The problem seem to lie somewhere in the guts of the analyzer.

The new patch attached.

-- 
Anton

-------------- next part --------------
A non-text attachment was scrubbed...
Name: MallocChecker_v7.patch
Type: text/x-diff
Size: 32643 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130228/c3eb8844/attachment.patch>