r176744 - [analyzer] Be more consistent about Objective-C methods that free memory.

[Jordan Rose]

Hm, I was playing with which one ought to include it, and wasn't sure which was better. The idea of ...for-malloc.h is that it could eventually be a common prefix for all the malloc tests, and it has to pull in ...-objc.h to add new methods that still look like "system methods". OTOH, malloc.mm uses methods directly declared in ...-objc.h, so in the spirit of "include what you use", it should also include ...-objc.h.

I did switch to #import since I knew I was including the headers twice, but maybe I should just remove the explicit import in malloc.mm.


On Mar 10, 2013, at 10:12 , Anton Yartsev <anton.yartsev at gmail.com> wrote:

> system-header-simulator-objc.h is included twice - from system-header-simulator-for-malloc.h and malloc.mm test itself, may one of includes is unnecessary?
> 
>> Author: jrose
>> Date: Fri Mar  8 18:59:10 2013
>> New Revision: 176744
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=176744&view=rev
>> Log:
>> [analyzer] Be more consistent about Objective-C methods that free memory.
>> 
>> Previously, MallocChecker's pointer escape check and its post-call state
>> update for Objective-C method calls had a fair amount duplicated logic
>> and not-entirely-consistent checks. This commit restructures all this to
>> be more consistent and possibly allow us to be more aggressive in warning
>> about double-frees.
>> 
>> New policy (applies to system header methods only):
>> (1) If this is a method we know about, model it as taking/holding ownership
>>     of the passed-in buffer.
>>   (1a) ...unless there's a "freeWhenDone:" parameter with a zero (NO) value.
>> (2) If there's a "freeWhenDone:" parameter (but it's not a method we know
>>     about), treat the buffer as escaping if the value is non-zero (YES) and
>>     non-escaping if it's zero (NO).
>> (3) If the first selector piece ends with "NoCopy" (but it's not a method we
>>     know about and there's no "freeWhenDone:" parameter), treat the buffer
>>     as escaping.
>> 
>> The reason that (2) and (3) don't explicitly model the ownership transfer is
>> because we can't be sure that they will actually free the memory using free(),
>> and we wouldn't want to emit a spurious "mismatched allocator" warning
>> (coming in Anton's upcoming patch). In the future, we may have an idea of a
>> "generic deallocation", i.e. we assume that the deallocator is correct but
>> still continue tracking the region so that we can warn about double-frees.
>> 
>> Patch by Anton Yartsev, with modifications from me.