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

[Jordan Rose]

On Feb 28, 2013, at 4:26 , Anton Yartsev <anton.yartsev at gmail.com> wrote:
>>> +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.

Aha. Haven't read the new patch yet, but I think I figured this one out. We're doing the free checks in a post-CallExpr check, but by that point we've already evaluated the function, which causes the pointers to escape, which causes checkPointerEscape to clear out the RefState for these objects. Note this part in doesNotFreeMemory():

  // If it's one of the allocation functions we can reason about, we model
  // its behavior explicitly.
  if (isMemFunction(FD, ASTC))
    return true;

You should just be able to add isStandardNewDelete to isMemFunction. As for Objective-C methods, the same thing applies: if they're one of the methods we handle in checkPostObjCMessage(), we can actually return true from doesNotFreeMemory(), but we need to keep the existing escaping behavior for methods we don't handle.

Jordan


> The new patch attached.

Will get to it today!


> <MallocChecker_v7.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130228/7b72ecb3/attachment.html>