[PATCH] Fix crash in CheckObjCCircularContainer

[AlexDenisov]

Hi guys, any updates?
--
AlexDenisov
Software Engineer, http://lowlevelbits.org

> On 31 Jul 2015, at 22:55, AlexDenisov <1101.debian at gmail.com> wrote:
> 
>> 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
> 
> <proper_fix_for_circular_containers.patch>
>> 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/20150805/4d167f11/attachment.sig>