[PATCH] Fix crash in CheckObjCCircularContainer

[AlexDenisov]

> To clarify, are you saying that the warning may lead to false positives when used in subclasses ?

Seems I was wrong.
Just checked the behaviour with backing storage - it also leads to a circular container problem.

Also, you can find attachment with a ‘proper’ implementation, which also covers subclassing.

P.S. I didn’t measure performance, but I think this implementation might have negative impact on the speed.
--
AlexDenisov
Software Engineer, http://lowlevelbits.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: proper_fix_for_circular_containers.patch
Type: application/octet-stream
Size: 10075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150731/d42d45f0/attachment.obj>
-------------- next part --------------

> On 30 Jul 2015, at 18:18, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
> 
> 
>> On Jul 30, 2015, at 1:05 AM, AlexDenisov <1101.debian at gmail.com> wrote:
>> 
>> The patch is a simplest fix for crash when CheckObjCCircularContainer
>> applies to a message to a ’super’, e.g.:
>> 
>> @implementation Foo : NSMutableArray
>> - foo {
>> [super addObject:nil];
>> }
>> @end
>> 
>> 
>> This is, probably, not a proper fix for the problem,
>> but initial patch wasn’t intended to apply checks to any kind
>> of subclassing, because it, imho, over-complicates implementation:
> 
> To clarify, are you saying that the warning may lead to false positives when used in subclasses ?
> If that’s the case could we just disable it inside collection subclasses, at least until the false positives can be addressed ?
> 
>> 
>> This particular problem touches subclassing from a class-cluster,
>> which means that the concrete subclass will have some backing storage, e.g.:
>> 
>> @implementation FootableArray : NSMutableArray
>> {
>> NSMutableArray *_backingStorage;
>> }
>> 
>> - addObject:(id)object {
>> [_backingStorage addObject:object];
>> }
>> 
>> @end
>> 
>> In this case even adding `self` to `self` would not lead to a circular container:
>> 
>> - foo {
>> [self addObject:self]; // puts `self` into the `_backingStorage`
>> }
>> 
>> I would apply this patch as is and postpone a ‘proper and bullet-proof implementation’
>> when I, or somebody who is also interested, will have more time.
>> 
>> If there are any questions/suggestions/objections - let’s discuss them.
>> --
>> AlexDenisov
>> Software Engineer, http://lowlevelbits.org
>> 
>> 
>> <fix_circular_container_crash.patch>
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150731/d42d45f0/attachment.sig>