r176744 - [analyzer] Be more consistent about Objective-C methods that free memory.
Jordan Rose
jordan_rose at apple.com
Mon Mar 11 09:18:27 PDT 2013
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.
More information about the cfe-commits
mailing list