[cfe-commits] r99951 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaObjCProperty.cpp test/SemaObjC/nsobject-attribute.m

[Fariborz Jahanian]

On Apr 5, 2010, at 3:32 PM, Ted Kremenek wrote:

>
> On Mar 30, 2010, at 3:40 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Tue Mar 30 17:40:11 2010
>> New Revision: 99951
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=99951&view=rev
>> Log:
>> Recognize __attribute__((NSObject)) directly applied
>> on retain properties. (radar 7809468).
>>
>> Modified:
>>    cfe/trunk/lib/Sema/Sema.h
>>    cfe/trunk/lib/Sema/SemaObjCProperty.cpp
>>    cfe/trunk/test/SemaObjC/nsobject-attribute.m
>>
>
> <SNIP>
>
>>
>> -void Sema::CheckObjCPropertyAttributes(QualType PropertyTy,
>> +void Sema::CheckObjCPropertyAttributes(DeclPtrTy PropertyPtrTy,
>>                                        SourceLocation Loc,
>>                                        unsigned &Attributes) {
>>   // FIXME: Improve the reported location.
>> +  Decl *PDecl = PropertyPtrTy.getAs<Decl>();
>> +  ObjCPropertyDecl *PropertyDecl =  
>> dyn_cast_or_null<ObjCPropertyDecl>(PDecl);
>> +  QualType PropertyTy = PropertyDecl->getType();
>
> Hi Fariborz,
>
> This doesn't quite look right.  On the second line there is  
> 'dyn_cast_or_null' and on the third line PropetyDecl gets  
> dereferenced without checking if it is null.  Is it always the case  
> that 'PDecl' will be both non-null and a ObjCProperyDecl?  If that  
> is the case, we should be using 'cast', not 'dyn_cast'.  If 'PDecl'  
> could be null we should probably check for that up front and then  
> bail out of the method before even doing the cast to  
> ObjCPropetyDecl*.  Otherwise, this looks like a potential crash.

No it is not right. We should bail out if is it null, or if we do not  
see an ObjCPropertyDecl   passed down.
(which is caused by detection of error in property declaration earlier).

- fariborz



>
> Ted

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100406/88af06df/attachment.html>