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>.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150888 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
index c74d07c..e87e08f 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
@@ -28,70 +28,56 @@
/// \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 @@
}
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 @@
const Expr *getOriginExpr() const {
if (!CallE)
- return Msg.getOriginExpr();
+ return Msg.getMessageExpr();
if (const CXXConstructExpr *Ctor =
CallE.dyn_cast<const CXXConstructExpr *>())
return Ctor;
diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 88a3be4..3a4c15f 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -32,6 +32,7 @@
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 @@
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 @@
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 @@
// 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;
}
diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
index dbbab48..165dff5 100644
--- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
@@ -196,7 +196,7 @@
// 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;
}
diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index cb4da7c..6f7a47f 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -190,8 +190,10 @@
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);
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 6f588c5..34ad23a 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -808,11 +808,33 @@
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 @@
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 @@
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
diff --git a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index b5ceb64..72d03a1 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -267,7 +267,7 @@
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());
}
diff --git a/lib/StaticAnalyzer/Core/ObjCMessage.cpp b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
index ac571a5..65cdcd9 100644
--- a/lib/StaticAnalyzer/Core/ObjCMessage.cpp
+++ b/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 *>()) {
diff --git a/test/Analysis/properties.m b/test/Analysis/properties.m
index 6d04a4a..231923b 100644
--- a/test/Analysis/properties.m
+++ b/test/Analysis/properties.m
@@ -143,3 +143,26 @@
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
+
+