r199869 - [analyzer] Tighten up sanity checks on Objective-C property getter synthesis.
Jordan Rose
jordan_rose at apple.com
Wed Jan 22 19:59:11 PST 2014
Author: jrose
Date: Wed Jan 22 21:59:10 2014
New Revision: 199869
URL: http://llvm.org/viewvc/llvm-project?rev=199869&view=rev
Log:
[analyzer] Tighten up sanity checks on Objective-C property getter synthesis.
If there are non-trivially-copyable types /other/ than C++ records, we
won't have a synthesized copy expression, but we can't just use a simple
load/return.
Also, add comments and shore up tests, making sure to test in both ARC
and non-ARC.
Modified:
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/lib/Analysis/BodyFarm.h
cfe/trunk/test/Analysis/properties.m
cfe/trunk/test/Analysis/properties.mm
Modified: cfe/trunk/lib/Analysis/BodyFarm.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.cpp?rev=199869&r1=199868&r2=199869&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BodyFarm.cpp (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp Wed Jan 22 21:59:10 2014
@@ -387,12 +387,20 @@ Stmt *BodyFarm::getBody(const FunctionDe
static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
const ObjCPropertyDecl *Prop) {
+ // First, find the backing ivar.
const ObjCIvarDecl *IVar = Prop->getPropertyIvarDecl();
if (!IVar)
return 0;
+
+ // Ignore weak variables, which have special behavior.
if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
return 0;
+ // Look to see if Sema has synthesized a body for us. This happens in
+ // Objective-C++ because the return value may be a C++ class type with a
+ // non-trivial copy constructor. We can only do this if we can find the
+ // @synthesize for this property, though (or if we know it's been auto-
+ // synthesized).
const ObjCImplementationDecl *ImplDecl =
IVar->getContainingInterface()->getImplementation();
if (ImplDecl) {
@@ -410,10 +418,18 @@ static Stmt *createObjCPropertyGetter(AS
}
}
- if (IVar->getType().getCanonicalType() !=
- Prop->getType().getNonReferenceType().getCanonicalType())
+ // Sanity check that the property is the same type as the ivar, or a
+ // reference to it, and that it is either an object pointer or trivially
+ // copyable.
+ if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
+ Prop->getType().getNonReferenceType()))
+ return 0;
+ if (!IVar->getType()->isObjCLifetimeType() &&
+ !IVar->getType().isTriviallyCopyableType(Ctx))
return 0;
+ // Generate our body:
+ // return self->_ivar;
ASTMaker M(Ctx);
const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl();
@@ -431,7 +447,8 @@ static Stmt *createObjCPropertyGetter(AS
return M.makeReturn(loadedIVar);
}
-Stmt *BodyFarm::getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop) {
+Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
+ // We currently only know how to synthesize property accessors.
if (!D->isPropertyAccessor())
return 0;
@@ -442,11 +459,11 @@ Stmt *BodyFarm::getBody(const ObjCMethod
return Val.getValue();
Val = 0;
- if (!Prop)
- Prop = D->findPropertyDecl();
+ const ObjCPropertyDecl *Prop = D->findPropertyDecl();
if (!Prop)
return 0;
+ // For now, we only synthesize getters.
if (D->param_size() != 0)
return 0;
Modified: cfe/trunk/lib/Analysis/BodyFarm.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.h?rev=199869&r1=199868&r2=199869&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BodyFarm.h (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.h Wed Jan 22 21:59:10 2014
@@ -36,7 +36,7 @@ public:
Stmt *getBody(const FunctionDecl *D);
/// Factory method for creating bodies for Objective-C properties.
- Stmt *getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop = 0);
+ Stmt *getBody(const ObjCMethodDecl *D);
private:
typedef llvm::DenseMap<const Decl *, Optional<Stmt *> > BodyMap;
Modified: cfe/trunk/test/Analysis/properties.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/properties.m?rev=199869&r1=199868&r2=199869&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/properties.m (original)
+++ cfe/trunk/test/Analysis/properties.m Wed Jan 22 21:59:10 2014
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class -fobjc-arc %s
void clang_analyzer_eval(int);
@@ -39,6 +40,8 @@ typedef struct _NSZone NSZone;
@end
+#if !__has_feature(objc_arc)
+
@implementation Test1
@synthesize text;
@@ -117,6 +120,8 @@ NSNumber* numberFromMyNumberProperty(MyN
return [result autorelease]; // expected-warning {{Object autoreleased too many times}}
}
+#endif
+
// rdar://6611873
@@ -124,18 +129,23 @@ NSNumber* numberFromMyNumberProperty(MyN
NSString *_name;
}
@property (retain) NSString * name;
+ at property (assign) id friend;
@end
@implementation Person
@synthesize name = _name;
@end
+#if !__has_feature(objc_arc)
void rdar6611873() {
Person *p = [[[Person alloc] init] autorelease];
p.name = [[NSString string] retain]; // expected-warning {{leak}}
p.name = [[NSString alloc] init]; // expected-warning {{leak}}
+
+ p.friend = [[Person alloc] init]; // expected-warning {{leak}}
}
+#endif
@interface SubPerson : Person
-(NSString *)foo;
@@ -147,6 +157,8 @@ void rdar6611873() {
}
@end
+
+#if !__has_feature(objc_arc)
// <rdar://problem/9241180> Static analyzer doesn't detect uninitialized variable issues for property accesses
@interface RDar9241180
@property (readwrite,assign) id x;
@@ -167,13 +179,14 @@ void rdar6611873() {
self.x = y; // expected-warning {{Argument for property setter is an uninitialized value}}
}
@end
+#endif
//------
// Property accessor synthesis
//------
-void testConsistency(Person *p) {
+void testConsistencyRetain(Person *p) {
clang_analyzer_eval(p.name == p.name); // expected-warning{{TRUE}}
extern void doSomethingWithPerson(Person *p);
@@ -183,9 +196,24 @@ void testConsistency(Person *p) {
clang_analyzer_eval(p.name == origName); // expected-warning{{UNKNOWN}}
}
-void testOverrelease(Person *p) {
- [p.name release]; // expected-warning{{not owned}}
+void testConsistencyAssign(Person *p) {
+ clang_analyzer_eval(p.friend == p.friend); // expected-warning{{TRUE}}
+
+ extern void doSomethingWithPerson(Person *p);
+ id origFriend = p.friend;
+ clang_analyzer_eval(p.friend == origFriend); // expected-warning{{TRUE}}
+ doSomethingWithPerson(p);
+ clang_analyzer_eval(p.friend == origFriend); // expected-warning{{UNKNOWN}}
+}
+
+#if !__has_feature(objc_arc)
+void testOverrelease(Person *p, int coin) {
+ if (coin)
+ [p.name release]; // expected-warning{{not owned}}
+ else
+ [p.friend release]; // expected-warning{{not owned}}
}
+#endif
@interface IntWrapper
@property int value;
@@ -212,6 +240,32 @@ void testConsistencyInt2(IntWrapper *w)
clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
}
+
+ at interface IntWrapperAuto
+ at property int value;
+ at end
+
+ at implementation IntWrapperAuto
+ at end
+
+void testConsistencyIntAuto(IntWrapperAuto *w) {
+ clang_analyzer_eval(w.value == w.value); // expected-warning{{TRUE}}
+
+ int origValue = w.value;
+ if (origValue != 42)
+ return;
+
+ clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+void testConsistencyIntAuto2(IntWrapperAuto *w) {
+ if (w.value != 42)
+ return;
+
+ clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+
typedef struct {
int value;
} IntWrapperStruct;
Modified: cfe/trunk/test/Analysis/properties.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/properties.mm?rev=199869&r1=199868&r2=199869&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/properties.mm (original)
+++ cfe/trunk/test/Analysis/properties.mm Wed Jan 22 21:59:10 2014
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class -fobjc-arc %s
void clang_analyzer_eval(bool);
void clang_analyzer_checkInlined(bool);
@@ -27,8 +28,6 @@ void testReferenceAssignment(IntWrapper
}
-// FIXME: Handle C++ structs, which need to go through the copy constructor.
-
struct IntWrapperStruct {
int value;
};
@@ -66,7 +65,7 @@ public:
@end
@implementation CustomCopyWrapper
- at synthesize inner;
+//@synthesize inner;
@end
void testConsistencyCustomCopy(CustomCopyWrapper *w) {
More information about the cfe-commits
mailing list