[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