[PATCH][Review request] unix.Malloc checker improvement: +handling new/delete, +memory.MismatchedFree, +memory.MismatchedDelete, +improved display names for allocators/deallocators
Anton Yartsev
anton.yartsev at gmail.com
Thu Feb 28 17:56:41 PST 2013
On 28.02.2013 21:11, Jordan Rose wrote:
>
> On Feb 28, 2013, at 4:26 , Anton Yartsev <anton.yartsev at gmail.com
> <mailto: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))
> returntrue;
>
> 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.
Right. Fixed it.
I guess that all methods with the first selector ending with NoCopy and
'freeWhenDone' param set to 1 should be handled by
checkPostObjCMessage(). Am I right?
Also propose to fix confusing name of doesNotFreeMemory() (that also
return true for calls that does free memory, but interesting to us) and
incorrect logic of checking Objective-C messages in
checkPostObjCMessage() that conflicts with the logic in
checkPostObjCMessage().
All this solutions are included in the attached patch.
--
Anton
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130301/3a4a0ed9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MallocChecker_v7_1.patch
Type: text/x-diff
Size: 38167 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130301/3a4a0ed9/attachment.patch>
More information about the cfe-commits
mailing list