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

[Anton Yartsev]

On 09.03.2013 2:51, Anton Yartsev wrote:
> On 08.03.2013 23:25, Jordan Rose wrote:
>>
>> On Mar 8, 2013, at 11:22 , Anna Zaks <ganna at apple.com 
>> <mailto:ganna at apple.com>> wrote:
>>
>>> Anton,
>>>
>>> We've briefly discussed this with Jordan. Below are the cases we 
>>> should handle:
>>>
>>> Case 1: The selector starts with "dataWithBytesNoCopy" or 
>>> "initWithBytesNoCopy" or "initWithCharactersNoCopy" and 
>>> "freeWhenDone" is not set to "0".
>>> We should assume that the call preforms hold action from malloc 
>>> family. (So the pointer should not escape and we should model this 
>>> during ObjCMsgCall processing)
>>
>> To explain the rationalization here, Anna pointed out that while it's 
>> /likely/ that methods with a "freeWhenDone:" or "...NoCopy:" selector 
>> piece all behave as ownership-holders, we can't actually prove it. In 
>> particular, if/when MallocChecker gains the ability to reason about 
>> custom allocators, someone could very well use "...NoCopy" to mean "I 
>> will free this using my custom deallocator", and we don't want to 
>> produce a bogus allocator mismatch bug in that case.
>>
>> By the way, by "the selector starts with __", we mean the existing 
>> logic of "the first selector piece is __", not "the first selector 
>> piece starts with __".
>>
>> Thanks for bearing with us.
>> Jordan
> Aaaa, got it!
> Updated the patch.
>
> > Case 1: The selector starts with "dataWithBytesNoCopy" or 
> "initWithBytesNoCopy" or "initWithCharactersNoCopy" and "freeWhenDone" 
> is not set to "0".
> > We should assume that the call preforms hold action from malloc 
> family. (So the pointer should not escape and we should model this 
> during ObjCMsgCall processing)
> >
> > Case 2: The selector contains "freeWhenDone", which is set to '0'.
> > The call results in no escape of the pointer.
> >
> > Case 3: Otherwise (this is not Case 1 nor Case 3) and the selector 
> ends with "NoCopy"
> > The pointer should escape.
> >
> > Please, work off your previous patch, but change the logic to handle 
> these cases and include the tests. Also, I think it's
> > important to include a commit message with your patch so that there 
> would be no ambiguity in what the patch is trying to accomplish.
> > Usually, the commit message and the tests would resolve that.
> I'll put this cases in a commit message.
> -- 
> Anton
Or even with a better name for doesNotFreeMemory() and comment.

-- 
Anton

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130309/c47481c5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MallocCheckerRefactoring_v3_1.patch
Type: text/x-diff
Size: 7629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130309/c47481c5/attachment.patch>