[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
Sun Mar 10 10:17:49 PDT 2013


On 09.03.2013 5:01, Jordan Rose wrote:
>
> On Mar 8, 2013, at 15:06 , Anton Yartsev <anton.yartsev at gmail.com 
> <mailto:anton.yartsev at gmail.com>> wrote:
>
>> 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.
>
> Thanks, Anton; committed in r176744! We thought of another test case 
> and so I had to restructure it a bit, but I think after rebasing we 
> can move forward with your main patch.
>
> Jordan

Attached is an updated patch.

-- 
Anton

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


More information about the cfe-commits mailing list