r178388 - [analyzer] Enabled unix.Malloc checker.

[Anton Yartsev]

On 01.04.2013 21:30, Anna Zaks wrote:
>
> On Mar 29, 2013, at 6:35 PM, Anton Yartsev <anton.yartsev at gmail.com 
> <mailto:anton.yartsev at gmail.com>> wrote:
>
>> On 30.03.2013 4:59, Jordan Rose wrote:
>>>
>>> On Mar 29, 2013, at 17:50 , Anton Yartsev <anton.yartsev at gmail.com 
>>> <mailto:anton.yartsev at gmail.com>> wrote:
>>>
>>>> +//----- Test free standard new
>>>> +void testFreeOpNew() {
>>>> +  void *p = operator new(0);
>>>> +  free(p);
>>>> +} // expected-warning{{Memory is never released; potential leak}}
>>>> +// FIXME: Pointer should escape
>>>> +
>>>> +void testFreeNewExpr() {
>>>> +  int *p = new int;
>>>> +  free(p);
>>>> +} // expected-warning{{Memory is never released; potential leak}}
>>>> +// FIXME: Pointer should escape
>>>> +
>>>> +void testObjcFreeNewed() {
>>>> +  int *p = new int;
>>>> +  NSData *nsdata = [NSData dataWithBytesNoCopy:p 
>>>> length:sizeof(int) freeWhenDone:1]; // expected-warning{{Memory is 
>>>> never released; potential leak}}
>>>> +}
>>>> +// FIXME: Pointer should escape
>>>
>>> These don't escape because we assume arbitrary system functions 
>>> don't free memory. I think these are fine when unix.Malloc is disabled.
>> Now, when allocation families got in, I would like to propose to 
>> split doesNotFreeMemOrInteresting() into two: doesNotFreeMem() and 
>> isInteresting() or something similar.
>>
>> doesNotFreeMem() should return "true" for a given call if it is known 
>> not to free memory, and "false" otherwise.
>>
>> For a known deallocation function isInteresting() should return 
>> "true", if a deallocation function is of the same family, as 
>> allocation one, and "false" otherwise. It also may contain any 
>> additional logic for non-deallocation functions if required.
>>
>> With this logic the cases will be considered as pointer escape that 
>> is symmetrical to the reverse case, when malloced memory is deleted 
>> (assume NewDelete id ON and UnixMalloc is OFF):
>> void testDeleteMalloced() {
>>   int *p = (int *)malloc(sizeof(int)); // do not add RefState to p
>>   delete p; // keep silent
>> }
>>
>> One more argument: that cases should be the subject of 
>> unix.MismatchedDeallocator - the memory/is/deallocated, but the 
>> deallocator do not match the allocator.
>>
>> What do you think?
>>
>
> Anton,
>
> I think that the current behavior is fine. First, if a malloced 
> pointer is freed with a delete (or wise versa), it is an error. When 
> the MismatchedDeallocator checker is turned off, we do not do a good 
> job diagnosing the issue, but it is still better to warn than not, in 
> my opinion. Second, my hope is that after the initial evaluation, all 
> these checkers will be turned on by default, so it it fine if the 
> diagnostic is suboptimal.
>
> I don't mind refactoring the doesNotFreeMemOrInteresting so that it 
> better reflects families; the name might be outdated now. However, I 
> don't think we should change the current behavior not to report a leak 
> on mismatched deallocator.
>
> Does this sound convincing?
Yes, I agree. Just maybe refactor doesNotFreeMemOrInteresting.

>
> Anna.
>
>> -- 
>> Anton
>


-- 
Anton

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130402/31f5e833/attachment.html>