Adopt ExprEngine and checkers to ObjC property refactoring. Everything was working, but now diagnostics are aware of message expressions implied by uses of properties. Fixes <rdar://problem/9241180>.

llvm-svn: 150888

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h

@@ -28,70 +28,56 @@ namespace ento {
 /// \brief Represents both explicit ObjC message expressions and implicit
 /// messages that are sent for handling properties in dot syntax.
 class ObjCMessage {
-  const Expr *MsgOrPropE;
-  const Expr *OriginE;
-  bool IsPropSetter;
-  SVal SetterArgV;
-
-protected:
-  ObjCMessage(const Expr *E, const Expr *origE, bool isSetter, SVal setArgV)
-    : MsgOrPropE(E), OriginE(origE),
-      IsPropSetter(isSetter), SetterArgV(setArgV) { }
-
+  const ObjCMessageExpr *Msg;
+  const ObjCPropertyRefExpr *PE;
+  const bool IsPropSetter;
 public:
-  ObjCMessage() : MsgOrPropE(0), OriginE(0) { }
+  ObjCMessage() : Msg(0), PE(0), IsPropSetter(false) {}
 
-  ObjCMessage(const ObjCMessageExpr *E)
-    : MsgOrPropE(E), OriginE(E) {
+  ObjCMessage(const ObjCMessageExpr *E, const ObjCPropertyRefExpr *pe = 0,
+              bool isSetter = false)
+    : Msg(E), PE(pe), IsPropSetter(isSetter) {
     assert(E && "should not be initialized with null expression");
   }
 
-  bool isValid() const { return MsgOrPropE != 0; }
-  bool isInvalid() const { return !isValid(); }
+  bool isValid() const { return Msg; }
+  
+  bool isPureMessageExpr() const { return !PE; }
 
-  bool isMessageExpr() const {
-    return isValid() && isa<ObjCMessageExpr>(MsgOrPropE);
-  }
-
-  bool isPropertyGetter() const {
-    return isValid() &&
-           isa<ObjCPropertyRefExpr>(MsgOrPropE) && !IsPropSetter;
-  }
+  bool isPropertyGetter() const { return PE && !IsPropSetter; }
 
   bool isPropertySetter() const {
-    return isValid() &&
-           isa<ObjCPropertyRefExpr>(MsgOrPropE) && IsPropSetter;
+    return IsPropSetter;
   }
-  
-  const Expr *getOriginExpr() const { return OriginE; }
 
-  QualType getType(ASTContext &ctx) const;
+  const Expr *getMessageExpr() const { 
+    return Msg;
+  }
+
+  QualType getType(ASTContext &ctx) const {
+    return Msg->getType();
+  }
 
   QualType getResultType(ASTContext &ctx) const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      if (const ObjCMethodDecl *MD = msgE->getMethodDecl())
-        return MD->getResultType();
+    if (const ObjCMethodDecl *MD = Msg->getMethodDecl())
+      return MD->getResultType();
     return getType(ctx);
   }
 
-  ObjCMethodFamily getMethodFamily() const;
+  ObjCMethodFamily getMethodFamily() const {
+    return Msg->getMethodFamily();
+  }
 
-  Selector getSelector() const;
+  Selector getSelector() const {
+    return Msg->getSelector();
+  }
 
   const Expr *getInstanceReceiver() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return msgE->getInstanceReceiver();
-    const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-    if (propE->isObjectReceiver())
-      return propE->getBase();
-    return 0;
+    return Msg->getInstanceReceiver();
   }
 
   SVal getInstanceReceiverSVal(ProgramStateRef State,
                                const LocationContext *LC) const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
     if (!isInstanceMessage())
       return UndefinedVal();
     if (const Expr *Ex = getInstanceReceiver())
@@ -105,101 +91,66 @@ public:
   }
 
   bool isInstanceMessage() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return msgE->isInstanceMessage();
-    const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-    // FIXME: 'super' may be super class.
-    return propE->isObjectReceiver() || propE->isSuperReceiver();
+    return Msg->isInstanceMessage();
   }
 
-  const ObjCMethodDecl *getMethodDecl() const;
+  const ObjCMethodDecl *getMethodDecl() const {
+    return Msg->getMethodDecl();
+  }
 
-  const ObjCInterfaceDecl *getReceiverInterface() const;
+  const ObjCInterfaceDecl *getReceiverInterface() const {
+    return Msg->getReceiverInterface();
+  }
 
   SourceLocation getSuperLoc() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return msgE->getSuperLoc();
-    return cast<ObjCPropertyRefExpr>(MsgOrPropE)->getReceiverLocation();
-  }
-
-  const Expr *getMsgOrPropExpr() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    return MsgOrPropE;
-  }
+    if (PE)
+      return PE->getReceiverLocation();
+    return Msg->getSuperLoc();
+  }  
 
   SourceRange getSourceRange() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    return MsgOrPropE->getSourceRange();
+    if (PE)
+      return PE->getSourceRange();
+    return Msg->getSourceRange();
   }
 
   unsigned getNumArgs() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return msgE->getNumArgs();
-    return isPropertySetter() ? 1 : 0;
+    return Msg->getNumArgs();
   }
 
   SVal getArgSVal(unsigned i,
                   const LocationContext *LCtx,
                   ProgramStateRef state) const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
     assert(i < getNumArgs() && "Invalid index for argument");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return state->getSVal(msgE->getArg(i), LCtx);
-    assert(isPropertySetter());
-    return SetterArgV;
+    return state->getSVal(Msg->getArg(i), LCtx);
   }
 
   QualType getArgType(unsigned i) const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
     assert(i < getNumArgs() && "Invalid index for argument");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return msgE->getArg(i)->getType();
-    assert(isPropertySetter());
-    return cast<ObjCPropertyRefExpr>(MsgOrPropE)->getType();
+    return Msg->getArg(i)->getType();
   }
 
-  const Expr *getArgExpr(unsigned i) const;
+  const Expr *getArgExpr(unsigned i) const {
+    assert(i < getNumArgs() && "Invalid index for argument");
+    return Msg->getArg(i);
+  }
 
   SourceRange getArgSourceRange(unsigned i) const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    assert(i < getNumArgs() && "Invalid index for argument");
-    if (const Expr *argE = getArgExpr(i))
-      return argE->getSourceRange();
-    return OriginE->getSourceRange();
+    const Expr *argE = getArgExpr(i);
+    return argE->getSourceRange();
   }
 
   SourceRange getReceiverSourceRange() const {
-    assert(isValid() && "This ObjCMessage is uninitialized!");
-    if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-      return msgE->getReceiverRange();
-
-    const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-    if (propE->isObjectReceiver())
-      return propE->getBase()->getSourceRange();
+    if (PE) {
+      if (PE->isObjectReceiver())
+        return PE->getBase()->getSourceRange();
+    }
+    else {
+      return Msg->getReceiverRange();
+    }
 
     // FIXME: This isn't a range.
-    return propE->getReceiverLocation();
-  }
-};
-
-class ObjCPropertyGetter : public ObjCMessage {
-public:
-  ObjCPropertyGetter(const ObjCPropertyRefExpr *propE, const Expr *originE)
-    : ObjCMessage(propE, originE, false, SVal()) {
-    assert(propE && originE &&
-           "should not be initialized with null expressions");
-  }
-};
-
-class ObjCPropertySetter : public ObjCMessage {
-public:
-  ObjCPropertySetter(const ObjCPropertyRefExpr *propE, const Expr *storeE,
-                     SVal argV)
-    : ObjCMessage(propE, storeE, true, argV) {
-    assert(propE && storeE &&"should not be initialized with null expressions");
+    return PE->getReceiverLocation();
   }
 };
 
@@ -252,7 +203,7 @@ public:
 
   const Expr *getOriginExpr() const {
     if (!CallE)
-      return Msg.getOriginExpr();
+      return Msg.getMessageExpr();
     if (const CXXConstructExpr *Ctor =
           CallE.dyn_cast<const CXXConstructExpr *>())
       return Ctor;

clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

@@ -32,6 +32,7 @@ class CallAndMessageChecker
   mutable OwningPtr<BugType> BT_call_undef;
   mutable OwningPtr<BugType> BT_call_arg;
   mutable OwningPtr<BugType> BT_msg_undef;
+  mutable OwningPtr<BugType> BT_objc_prop_undef;
   mutable OwningPtr<BugType> BT_msg_arg;
   mutable OwningPtr<BugType> BT_msg_ret;
 public:
@@ -228,11 +229,21 @@ void CallAndMessageChecker::checkPreObjCMessage(ObjCMessage msg,
     SVal recVal = state->getSVal(receiver, LCtx);
     if (recVal.isUndef()) {
       if (ExplodedNode *N = C.generateSink()) {
-        if (!BT_msg_undef)
-          BT_msg_undef.reset(new BuiltinBug("Receiver in message expression is "
-                                            "an uninitialized value"));
+        BugType *BT = 0;
+        if (msg.isPureMessageExpr()) {
+          if (!BT_msg_undef)
+            BT_msg_undef.reset(new BuiltinBug("Receiver in message expression "
+                                              "is an uninitialized value"));
+          BT = BT_msg_undef.get();
+        }
+        else {
+          if (!BT_objc_prop_undef)
+            BT_objc_prop_undef.reset(new BuiltinBug("Property access on an "
+                                              "uninitialized object pointer"));
+          BT = BT_objc_prop_undef.get();
+        }
         BugReport *R =
-          new BugReport(*BT_msg_undef, BT_msg_undef->getName(), N);
+          new BugReport(*BT, BT->getName(), N);
         R->addRange(receiver->getSourceRange());
         R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
                                                                    receiver));
@@ -306,13 +317,13 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C,
   if (CanRetTy->isStructureOrClassType()) {
     // Structure returns are safe since the compiler zeroes them out.
     SVal V = C.getSValBuilder().makeZeroVal(msg.getType(Ctx));
-    C.addTransition(state->BindExpr(msg.getOriginExpr(), LCtx, V));
+    C.addTransition(state->BindExpr(msg.getMessageExpr(), LCtx, V));
     return;
   }
 
   // Other cases: check if sizeof(return type) > sizeof(void*)
   if (CanRetTy != Ctx.VoidTy && C.getLocationContext()->getParentMap()
-                                  .isConsumedExpr(msg.getOriginExpr())) {
+                                  .isConsumedExpr(msg.getMessageExpr())) {
     // Compute: sizeof(void *) and sizeof(return type)
     const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
     const uint64_t returnTypeSize = Ctx.getTypeSize(CanRetTy);
@@ -343,7 +354,7 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C,
     // of this case unless we have *a lot* more knowledge.
     //
     SVal V = C.getSValBuilder().makeZeroVal(msg.getType(Ctx));
-    C.addTransition(state->BindExpr(msg.getOriginExpr(), LCtx, V));
+    C.addTransition(state->BindExpr(msg.getMessageExpr(), LCtx, V));
     return;
   }
 

clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp

@@ -196,7 +196,7 @@ void ObjCSelfInitChecker::checkPostObjCMessage(ObjCMessage msg,
     // value out when we return from this method.
     state = state->set<CalledInit>(true);
     
-    SVal V = state->getSVal(msg.getOriginExpr(), C.getLocationContext());
+    SVal V = state->getSVal(msg.getMessageExpr(), C.getLocationContext());
     addSelfFlag(state, V, SelfFlag_InitRes, C);
     return;
   }

clang/lib/StaticAnalyzer/Core/CheckerManager.cpp

@@ -190,8 +190,10 @@ namespace {
                     NodeBuilder &Bldr, ExplodedNode *Pred) {
       ProgramPoint::Kind K =  IsPreVisit ? ProgramPoint::PreStmtKind :
                                            ProgramPoint::PostStmtKind;
-      const ProgramPoint &L = ProgramPoint::getProgramPoint(Msg.getOriginExpr(),
-                                K, Pred->getLocationContext(), checkFn.Checker);
+      const ProgramPoint &L =
+        ProgramPoint::getProgramPoint(Msg.getMessageExpr(),
+                                      K, Pred->getLocationContext(),
+                                      checkFn.Checker);
       CheckerContext C(Bldr, Eng, Pred, L);
 
       checkFn(Msg, C);

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -808,11 +808,33 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       Bldr.addNodes(Dst);
       break;
 
-    case Stmt::ObjCMessageExprClass:
+    case Stmt::ObjCMessageExprClass: {
       Bldr.takeNodes(Pred);
-      VisitObjCMessage(cast<ObjCMessageExpr>(S), Pred, Dst);
+      // Is this a property access?
+      const ParentMap &PM = Pred->getLocationContext()->getParentMap();
+      const ObjCMessageExpr *ME = cast<ObjCMessageExpr>(S);
+      bool evaluated = false;
+      
+      if (const PseudoObjectExpr *PO =
+          dyn_cast_or_null<PseudoObjectExpr>(PM.getParent(S))) {
+        const Expr *syntactic = PO->getSyntacticForm();
+        if (const ObjCPropertyRefExpr *PR =
+              dyn_cast<ObjCPropertyRefExpr>(syntactic)) {
+          bool isSetter = ME->getNumArgs() > 0;
+          VisitObjCMessage(ObjCMessage(ME, PR, isSetter), Pred, Dst);
+          evaluated = true;
+        }
+        else if (isa<BinaryOperator>(syntactic)) {
+          VisitObjCMessage(ObjCMessage(ME, 0, true), Pred, Dst);
+        }
+      }
+      
+      if (!evaluated)
+        VisitObjCMessage(ME, Pred, Dst);
+
       Bldr.addNodes(Dst);
       break;
+    }
 
     case Stmt::ObjCAtThrowStmtClass: {
       // FIXME: This is not complete.  We basically treat @throw as
@@ -1439,9 +1461,7 @@ void ExprEngine::evalStore(ExplodedNodeSet &Dst, const Expr *AssignE,
   const Expr *StoreE = AssignE ? AssignE : LocationE;
 
   if (isa<loc::ObjCPropRef>(location)) {
-    loc::ObjCPropRef prop = cast<loc::ObjCPropRef>(location);
-    return VisitObjCMessage(ObjCPropertySetter(prop.getPropRefExpr(),
-                                               StoreE, Val), Pred, Dst);
+    assert(false);
   }
 
   // Evaluate the location (checks for bad dereferences).
@@ -1466,9 +1486,7 @@ void ExprEngine::evalLoad(ExplodedNodeSet &Dst, const Expr *Ex,
   assert(!isa<NonLoc>(location) && "location cannot be a NonLoc.");
 
   if (isa<loc::ObjCPropRef>(location)) {
-    loc::ObjCPropRef prop = cast<loc::ObjCPropRef>(location);
-    return VisitObjCMessage(ObjCPropertyGetter(prop.getPropRefExpr(), Ex),
-                            Pred, Dst);
+    assert(false);
   }
 
   // Are we loading from a region?  This actually results in two loads; one

clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp

@@ -267,7 +267,7 @@ void ExprEngine::evalObjCMessage(StmtNodeBuilder &Bldr,
   state = invalidateArguments(state, CallOrObjCMessage(msg, state, LCtx), LCtx);
 
   // And create the new node.
-  Bldr.generateNode(msg.getOriginExpr(), Pred, state, GenSink);
+  Bldr.generateNode(msg.getMessageExpr(), Pred, state, GenSink);
   assert(Bldr.hasGeneratedNodes());
 }
 

clang/lib/StaticAnalyzer/Core/ObjCMessage.cpp

@@ -18,104 +18,11 @@
 using namespace clang;
 using namespace ento;
 
-QualType ObjCMessage::getType(ASTContext &ctx) const {
-  assert(isValid() && "This ObjCMessage is uninitialized!");
-  if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-    return msgE->getType();
-  const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-  if (isPropertySetter())
-    return ctx.VoidTy;
-  return propE->getType();
-}
-
-Selector ObjCMessage::getSelector() const {
-  assert(isValid() && "This ObjCMessage is uninitialized!");
-  if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-    return msgE->getSelector();
-  const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-  if (isPropertySetter())
-    return propE->getSetterSelector();
-  return propE->getGetterSelector();
-}
-
-ObjCMethodFamily ObjCMessage::getMethodFamily() const {
-  assert(isValid() && "This ObjCMessage is uninitialized!");
-  // Case 1.  Explicit message send.
-  if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-    return msgE->getMethodFamily();
-
-  const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-
-  // Case 2.  Reference to implicit property.
-  if (propE->isImplicitProperty()) {
-    if (isPropertySetter())
-      return propE->getImplicitPropertySetter()->getMethodFamily();
-    else
-      return propE->getImplicitPropertyGetter()->getMethodFamily();
-  }
-
-  // Case 3.  Reference to explicit property.
-  const ObjCPropertyDecl *prop = propE->getExplicitProperty();
-  if (isPropertySetter()) {
-    if (prop->getSetterMethodDecl())
-      return prop->getSetterMethodDecl()->getMethodFamily();
-    return prop->getSetterName().getMethodFamily();
-  } else {
-    if (prop->getGetterMethodDecl())
-      return prop->getGetterMethodDecl()->getMethodFamily();
-    return prop->getGetterName().getMethodFamily();
-  }
-}
-
-const ObjCMethodDecl *ObjCMessage::getMethodDecl() const {
-  assert(isValid() && "This ObjCMessage is uninitialized!");
-  if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-    return msgE->getMethodDecl();
-  const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-  if (propE->isImplicitProperty())
-    return isPropertySetter() ? propE->getImplicitPropertySetter()
-                              : propE->getImplicitPropertyGetter();
-  return 0;
-}
-
-const ObjCInterfaceDecl *ObjCMessage::getReceiverInterface() const {
-  assert(isValid() && "This ObjCMessage is uninitialized!");
-  if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-    return msgE->getReceiverInterface();
-  const ObjCPropertyRefExpr *propE = cast<ObjCPropertyRefExpr>(MsgOrPropE);
-  if (propE->isClassReceiver())
-    return propE->getClassReceiver();
-  QualType recT;
-  if (const Expr *recE = getInstanceReceiver())
-    recT = recE->getType();
-  else {
-    assert(propE->isSuperReceiver());
-    recT = propE->getSuperReceiverType();
-  }
-  if (const ObjCObjectPointerType *Ptr = recT->getAs<ObjCObjectPointerType>())
-    return Ptr->getInterfaceDecl();
-  return 0;
-}
-
-const Expr *ObjCMessage::getArgExpr(unsigned i) const {
-  assert(isValid() && "This ObjCMessage is uninitialized!");
-  assert(i < getNumArgs() && "Invalid index for argument");
-  if (const ObjCMessageExpr *msgE = dyn_cast<ObjCMessageExpr>(MsgOrPropE))
-    return msgE->getArg(i);
-  assert(isPropertySetter());
-  if (const BinaryOperator *bop = dyn_cast<BinaryOperator>(OriginE))
-    if (bop->isAssignmentOp())
-      return bop->getRHS();
-  return 0;
-}
-
 QualType CallOrObjCMessage::getResultType(ASTContext &ctx) const {
   QualType resultTy;
   bool isLVal = false;
 
   if (isObjCMessage()) {
-    isLVal = isa<ObjCMessageExpr>(Msg.getOriginExpr()) &&
-             Msg.getOriginExpr()->isLValue();
     resultTy = Msg.getResultType(ctx);
   } else if (const CXXConstructExpr *Ctor =
               CallE.dyn_cast<const CXXConstructExpr *>()) {

clang/test/Analysis/properties.m

@@ -143,3 +143,26 @@ void rdar6611873() {
   return super.name;
 }
 @end
+
+// <rdar://problem/9241180> Static analyzer doesn't detect uninitialized variable issues for property accesses
+@interface RDar9241180
+@property (readwrite,assign) id x;
+-(id)testAnalyzer1:(int) y;
+-(void)testAnalyzer2;
+@end
+
+@implementation RDar9241180
+@synthesize x;
+-(id)testAnalyzer1:(int)y {
+    RDar9241180 *o;
+    if (y && o.x) // expected-warning {{Property access on an uninitialized object pointer}}
+      return o;
+    return o; // expected-warning {{Undefined or garbage value returned to caller}}
+}
+-(void)testAnalyzer2 {
+  id y;
+  self.x = y;  // expected-warning {{Argument for property setter is an uninitialized value}}
+}
+@end
+
+