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

[Jordan Rose]

On Feb 28, 2013, at 19:18 , Anna Zaks <ganna at apple.com> wrote:

> 
> On Feb 28, 2013, at 6:48 PM, Anton Yartsev <anton.yartsev at gmail.com> wrote:
> 
>> On 01.03.2013 5:55, Jordan Rose wrote:
>>>>>> Memory allocated by malloc() should be deallocated by free(), not operator delete[].
>>>>> I'm really not sure whether it's better to mention the deallocator's expected allocator, or the allocated region's expected deallocator. Which mistake do you think people are more likely to make?
>>>>> 
>>>>> (Also, I came up with that phrasing in ten seconds; it could totally change.)
>>>> The second one of course. I definitely prefer to print the allocated region's expected deallocator.
>>>> The single downside I see is that this breaks the uniformity of reports and complicate ReportBadFree(). (other reports about, for example, freeing addresses of labels, could not be printed in the new fashion)
>>> Hm. I think we should still go with the second one.
>>> 
>>> One last major comment: in talking offline to Ted (Kremenek) about this, he pointed out that it has the potential to be a very noisy new warning. In the same way that we have unix.Malloc and alpha.unix.MallocWithAnnotations, it might be good to separate out cplusplus.NewDelete and...we don't have a good category for MismatchedFree, but maybe unix.MismatchedFree. That way we can control these checks individually; look for "MallocOptimistic" to see how it's currently being used.
>> Have not got the idea yet. Please clarify what should cplusplus.NewDelete and unix.MismatchedFree check for individually, and what about other allocation families.
>> Can we just separate out the whole new check to something like .MismatchedDeallocator?
> 
> NewDelete checker would check for use-after-free and leaks involving the new and delete "family".
> MismatchedDeallocator would check for mismatched deallocators (better name than MismatchedFree). This checker could consult with the state that the other checkers populate, for example, to check which function was used to allocate a symbol.

To be clear, these would still all live inside MallocChecker, as you've designed it. They would just be different checks the user could turn on and off. If they're turned off, those bug reports just won't be emitted. (This probably means making sure mismatched deallocators don't produce spurious leak warnings when the check is off.)