r198953 - [analyzer] Model getters of known- at synthesized Objective-C properties.

Jordan Rose jordan_rose at apple.com
Fri Jan 10 12:06:07 PST 2014


Author: jrose
Date: Fri Jan 10 14:06:06 2014
New Revision: 198953

URL: http://llvm.org/viewvc/llvm-project?rev=198953&view=rev
Log:
[analyzer] Model getters of known- at synthesized Objective-C properties.

...by synthesizing their body to be "return self->_prop;", with an extra
nudge to RetainCountChecker to still treat the value as +0 if we have no
other information.

This doesn't handle weak properties, but that's mostly correct anyway,
since they can go to nil at any time. This also doesn't apply to properties
whose implementations we can't see, since they may not be backed by an
ivar at all. And finally, this doesn't handle properties of C++ class type,
because we can't invoke the copy constructor. (Sema has actually done this
work already, but the AST it synthesizes is one the analyzer doesn't quite
handle -- it has an rvalue DeclRefExpr.)

Modeling setters is likely to be more difficult (since it requires
handling strong/copy), but not impossible.

<rdar://problem/11956898>

Added:
    cfe/trunk/test/Analysis/properties.mm
Modified:
    cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
    cfe/trunk/lib/Analysis/BodyFarm.cpp
    cfe/trunk/lib/Analysis/BodyFarm.h
    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
    cfe/trunk/test/Analysis/properties.m

Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=198953&r1=198952&r2=198953&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
+++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Fri Jan 10 14:06:06 2014
@@ -94,14 +94,21 @@ Stmt *AnalysisDeclContext::getBody(bool
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     Stmt *Body = FD->getBody();
     if (!Body && Manager && Manager->synthesizeBodies()) {
-      IsAutosynthesized = true;
-      return getBodyFarm(getASTContext()).getBody(FD);
+      Body = getBodyFarm(getASTContext()).getBody(FD);
+      if (Body)
+        IsAutosynthesized = true;
     }
     return Body;
   }
-  else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))
-    return MD->getBody();
-  else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D))
+  else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
+    Stmt *Body = MD->getBody();
+    if (!Body && Manager && Manager->synthesizeBodies()) {
+      Body = getBodyFarm(getASTContext()).getBody(MD);
+      if (Body)
+        IsAutosynthesized = true;
+    }
+    return Body;
+  } else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D))
     return BD->getBody();
   else if (const FunctionTemplateDecl *FunTmpl
            = dyn_cast_or_null<FunctionTemplateDecl>(D))

Modified: cfe/trunk/lib/Analysis/BodyFarm.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.cpp?rev=198953&r1=198952&r2=198953&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BodyFarm.cpp (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp Fri Jan 10 14:06:06 2014
@@ -74,6 +74,9 @@ public:
   
   /// Create an Objective-C bool literal.
   ObjCBoolLiteralExpr *makeObjCBool(bool Val);
+
+  /// Create an Objective-C ivar reference.
+  ObjCIvarRefExpr *makeObjCIvarRef(const Expr *Base, const ObjCIvarDecl *IVar);
   
   /// Create a Return statement.
   ReturnStmt *makeReturn(const Expr *RetVal);
@@ -147,6 +150,15 @@ ObjCBoolLiteralExpr *ASTMaker::makeObjCB
   return new (C) ObjCBoolLiteralExpr(Val, Ty, SourceLocation());
 }
 
+ObjCIvarRefExpr *ASTMaker::makeObjCIvarRef(const Expr *Base,
+                                           const ObjCIvarDecl *IVar) {
+  return new (C) ObjCIvarRefExpr(const_cast<ObjCIvarDecl*>(IVar),
+                                 IVar->getType(), SourceLocation(),
+                                 SourceLocation(), const_cast<Expr*>(Base),
+                                 /*arrow=*/true, /*free=*/false);
+}
+
+
 ReturnStmt *ASTMaker::makeReturn(const Expr *RetVal) {
   return new (C) ReturnStmt(SourceLocation(), const_cast<Expr*>(RetVal), 0);
 }
@@ -374,3 +386,61 @@ Stmt *BodyFarm::getBody(const FunctionDe
   return Val.getValue();
 }
 
+static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
+                                      const ObjCPropertyDecl *Prop) {
+  const ObjCIvarDecl *IVar = Prop->getPropertyIvarDecl();
+  if (!IVar)
+    return 0;
+  if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
+    return 0;
+  if (IVar->getType().getCanonicalType() !=
+      Prop->getType().getNonReferenceType().getCanonicalType())
+    return 0;
+
+  // C++ records require copy constructors, so we can't just synthesize an AST.
+  // FIXME: Use ObjCPropertyImplDecl's already-synthesized AST. Currently it's
+  // not in a form the analyzer can use.
+  if (Prop->getType()->getAsCXXRecordDecl())
+    return 0;
+
+  ASTMaker M(Ctx);
+
+  const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl();
+
+  Expr *loadedIVar =
+    M.makeObjCIvarRef(
+      M.makeLvalueToRvalue(
+        M.makeDeclRefExpr(selfVar),
+        selfVar->getType()),
+      IVar);
+
+  if (!Prop->getType()->isReferenceType())
+    loadedIVar = M.makeLvalueToRvalue(loadedIVar, IVar->getType());
+
+  return M.makeReturn(loadedIVar);
+}
+
+Stmt *BodyFarm::getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop) {
+  if (!D->isPropertyAccessor())
+    return 0;
+
+  D = D->getCanonicalDecl();
+
+  Optional<Stmt *> &Val = Bodies[D];
+  if (Val.hasValue())
+    return Val.getValue();
+  Val = 0;
+
+  if (!Prop)
+    Prop = D->findPropertyDecl();
+  if (!Prop)
+    return 0;
+
+  if (D->param_size() != 0)
+    return 0;
+
+  Val = createObjCPropertyGetter(C, Prop);
+
+  return Val.getValue();
+}
+

Modified: cfe/trunk/lib/Analysis/BodyFarm.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.h?rev=198953&r1=198952&r2=198953&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BodyFarm.h (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.h Fri Jan 10 14:06:06 2014
@@ -24,6 +24,8 @@ namespace clang {
 class ASTContext;
 class Decl;
 class FunctionDecl;
+class ObjCMethodDecl;
+class ObjCPropertyDecl;
 class Stmt;
   
 class BodyFarm {
@@ -32,7 +34,10 @@ public:
   
   /// Factory method for creating bodies for ordinary functions.
   Stmt *getBody(const FunctionDecl *D);
-  
+
+  /// Factory method for creating bodies for Objective-C properties.
+  Stmt *getBody(const ObjCMethodDecl *D, const ObjCPropertyDecl *Prop = 0);
+
 private:
   typedef llvm::DenseMap<const Decl *, Optional<Stmt *> > BodyMap;
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=198953&r1=198952&r2=198953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Jan 10 14:06:06 2014
@@ -2735,6 +2735,16 @@ static QualType GetReturnType(const Expr
   return RetTy;
 }
 
+static bool wasSynthesizedProperty(const ObjCMethodCall *Call,
+                                   ExplodedNode *N) {
+  if (!Call || !Call->getDecl()->isPropertyAccessor())
+    return false;
+
+  CallExitEnd PP = N->getLocation().castAs<CallExitEnd>();
+  const StackFrameContext *Frame = PP.getCalleeContext();
+  return Frame->getAnalysisDeclContext()->isBodyAutosynthesized();
+}
+
 // We don't always get the exact modeling of the function with regards to the
 // retain count checker even when the function is inlined. For example, we need
 // to stop tracking the symbols which were marked with StopTrackingHard.
@@ -2769,6 +2779,15 @@ void RetainCountChecker::processSummaryO
     SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
     if (Sym)
       state = removeRefBinding(state, Sym);
+  } else if (RE.getKind() == RetEffect::NotOwnedSymbol) {
+    if (wasSynthesizedProperty(MsgInvocation, C.getPredecessor())) {
+      // Believe the summary if we synthesized the body and the return value is
+      // untracked. This handles property getters.
+      SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
+      if (Sym && !getRefBinding(state, Sym))
+        state = setRefBinding(state, Sym, RefVal::makeNotOwned(RE.getObjKind(),
+                                                               Sym->getType()));
+    }
   }
   
   C.addTransition(state);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=198953&r1=198952&r2=198953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Fri Jan 10 14:06:06 2014
@@ -863,9 +863,17 @@ RuntimeDefinition ObjCMethodCall::getRun
         Optional<const ObjCMethodDecl *> &Val = PMC[std::make_pair(IDecl, Sel)];
 
         // Query lookupPrivateMethod() if the cache does not hit.
-        if (!Val.hasValue())
+        if (!Val.hasValue()) {
           Val = IDecl->lookupPrivateMethod(Sel);
 
+          // If the method is a property accessor, we should try to "inline" it
+          // even if we don't actually have an implementation.
+          if (!*Val)
+            if (const ObjCMethodDecl *CompileTimeMD = E->getMethodDecl())
+              if (CompileTimeMD->isPropertyAccessor())
+                Val = IDecl->lookupInstanceMethod(Sel);
+        }
+
         const ObjCMethodDecl *MD = Val.getValue();
         if (CanBeSubClassed)
           return RuntimeDefinition(MD, Receiver);

Modified: cfe/trunk/test/Analysis/properties.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/properties.m?rev=198953&r1=198952&r2=198953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/properties.m (original)
+++ cfe/trunk/test/Analysis/properties.m Fri Jan 10 14:06:06 2014
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount -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 %s
+
+void clang_analyzer_eval(int);
 
 typedef signed char BOOL;
 typedef unsigned int NSUInteger;
@@ -14,6 +16,7 @@ typedef struct _NSZone NSZone;
 -(id)autorelease;
 -(id)copy;
 -(id)retain;
+-(oneway void)release;
 @end
 @interface NSString : NSObject <NSCopying, NSMutableCopying, NSCoding>
 - (NSUInteger)length;
@@ -166,3 +169,79 @@ void rdar6611873() {
 @end
 
 
+//------
+// Property accessor synthesis
+//------
+
+void testConsistency(Person *p) {
+  clang_analyzer_eval(p.name == p.name); // expected-warning{{TRUE}}
+
+  extern void doSomethingWithPerson(Person *p);
+  id origName = p.name;
+  clang_analyzer_eval(p.name == origName); // expected-warning{{TRUE}}
+  doSomethingWithPerson(p);
+  clang_analyzer_eval(p.name == origName); // expected-warning{{UNKNOWN}}
+}
+
+void testOverrelease(Person *p) {
+  [p.name release]; // expected-warning{{not owned}}
+}
+
+ at interface IntWrapper
+ at property int value;
+ at end
+
+ at implementation IntWrapper
+ at synthesize value;
+ at end
+
+void testConsistencyInt(IntWrapper *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 testConsistencyInt2(IntWrapper *w) {
+  if (w.value != 42)
+    return;
+
+  clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+typedef struct {
+  int value;
+} IntWrapperStruct;
+
+ at interface StructWrapper
+ at property IntWrapperStruct inner;
+ at end
+
+ at implementation StructWrapper
+ at synthesize inner;
+ at end
+
+void testConsistencyStruct(StructWrapper *w) {
+  clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}}
+
+  int origValue = w.inner.value;
+  if (origValue != 42)
+    return;
+
+  clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
+}
+
+
+ at interface OpaqueIntWrapper
+ at property int value;
+ at end
+
+// For now, don't assume a property is implemented using an ivar unless we can
+// actually see that it is.
+void testOpaqueConsistency(OpaqueIntWrapper *w) {
+  clang_analyzer_eval(w.value == w.value); // expected-warning{{UNKNOWN}}
+}
+

Added: cfe/trunk/test/Analysis/properties.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/properties.mm?rev=198953&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/properties.mm (added)
+++ cfe/trunk/test/Analysis/properties.mm Fri Jan 10 14:06:06 2014
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_checkInlined(bool);
+
+ at interface IntWrapper
+ at property (readonly) int &value;
+ at end
+
+ at implementation IntWrapper
+ at synthesize value;
+ at end
+
+void testReferenceConsistency(IntWrapper *w) {
+  clang_analyzer_eval(w.value == w.value); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&w.value == &w.value); // expected-warning{{TRUE}}
+
+  if (w.value != 42)
+    return;
+
+  clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+void testReferenceAssignment(IntWrapper *w) {
+  w.value = 42;
+  clang_analyzer_eval(w.value == 42); // expected-warning{{TRUE}}
+}
+
+
+// FIXME: Handle C++ structs, which need to go through the copy constructor.
+
+struct IntWrapperStruct {
+  int value;
+};
+
+ at interface StructWrapper
+ at property IntWrapperStruct inner;
+ at end
+
+ at implementation StructWrapper
+ at synthesize inner;
+ at end
+
+void testConsistencyStruct(StructWrapper *w) {
+  clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{UNKNOWN}}
+
+  int origValue = w.inner.value;
+  if (origValue != 42)
+    return;
+
+  clang_analyzer_eval(w.inner.value == 42); // expected-warning{{UNKNOWN}}
+}
+
+
+class CustomCopy {
+public:
+  CustomCopy() : value(0) {}
+  CustomCopy(const CustomCopy &other) {
+    clang_analyzer_checkInlined(false);
+  }
+  int value;
+};
+
+ at interface CustomCopyWrapper
+ at property CustomCopy inner;
+ at end
+
+ at implementation CustomCopyWrapper
+ at synthesize inner;
+ at end
+
+void testConsistencyCustomCopy(CustomCopyWrapper *w) {
+  clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{UNKNOWN}}
+
+  int origValue = w.inner.value;
+  if (origValue != 42)
+    return;
+
+  clang_analyzer_eval(w.inner.value == 42); // expected-warning{{UNKNOWN}}
+}





More information about the cfe-commits mailing list