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

Jordan Rose jordan_rose at apple.com
Fri Mar 8 17:01:44 PST 2013


On Mar 8, 2013, at 15:06 , Anton Yartsev <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> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130308/46b0b235/attachment.html>


More information about the cfe-commits mailing list