[analyzer] Combine all ObjC message CallEvents into ObjCMethodCall.
As pointed out by Anna, we only differentiate between explicit message sends
This also adds support for ObjCSubscriptExprs, which are basically the same
as properties in many ways. We were already checking these, but not emitting
nice messages for them.
This depends on the llvm::PointerIntPair change in r160456.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160461 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 69b331c..17ed692 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -34,6 +34,7 @@
mutable OwningPtr<BugType> BT_call_arg;
mutable OwningPtr<BugType> BT_msg_undef;
mutable OwningPtr<BugType> BT_objc_prop_undef;
+ mutable OwningPtr<BugType> BT_objc_subscript_undef;
mutable OwningPtr<BugType> BT_msg_arg;
mutable OwningPtr<BugType> BT_msg_ret;
public:
@@ -45,8 +46,8 @@
private:
static bool PreVisitProcessArg(CheckerContext &C, SVal V,
SourceRange argRange, const Expr *argEx,
- const bool checkUninitFields,
- const char *BT_desc, OwningPtr<BugType> &BT);
+ bool IsFirstArgument, bool checkUninitFields,
+ const CallEvent &Call, OwningPtr<BugType> &BT);
static void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE);
void emitNilReceiverBug(CheckerContext &C, const ObjCMethodCall &msg,
@@ -75,18 +76,46 @@
C.EmitReport(R);
}
+StringRef describeUninitializedArgumentInCall(const CallEvent &Call,
+ bool IsFirstArgument) {
+ switch (Call.getKind()) {
+ case CE_ObjCMessage: {
+ const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call);
+ switch (Msg.getMessageKind()) {
+ case OCM_Message:
+ return "Argument in message expression is an uninitialized value";
+ case OCM_PropertyAccess:
+ assert(Msg.isSetter() && "Getters have no args");
+ return "Argument for property setter is an uninitialized value";
+ case OCM_Subscript:
+ if (Msg.isSetter() && IsFirstArgument)
+ return "Argument for subscript setter is an uninitialized value";
+ return "Subscript index is an uninitialized value";
+ }
+ llvm_unreachable("Unknown message kind.");
+ }
+ case CE_Block:
+ return "Block call argument is an uninitialized value";
+ default:
+ return "Function call argument is an uninitialized value";
+ }
+}
+
bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
SVal V, SourceRange argRange,
const Expr *argEx,
- const bool checkUninitFields,
- const char *BT_desc,
+ bool IsFirstArgument,
+ bool checkUninitFields,
+ const CallEvent &Call,
OwningPtr<BugType> &BT) {
if (V.isUndef()) {
if (ExplodedNode *N = C.generateSink()) {
LazyInit_BT("Uninitialized argument value", BT);
// Generate a report for this bug.
- BugReport *R = new BugReport(*BT, BT_desc, N);
+ StringRef Desc = describeUninitializedArgumentInCall(Call,
+ IsFirstArgument);
+ BugReport *R = new BugReport(*BT, Desc, N);
R->addRange(argRange);
if (argEx)
R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, argEx,
@@ -148,7 +177,7 @@
if (F.Find(D->getRegion())) {
if (ExplodedNode *N = C.generateSink()) {
- LazyInit_BT(BT_desc, BT);
+ LazyInit_BT("Uninitialized argument value", BT);
SmallString<512> Str;
llvm::raw_svector_ostream os(Str);
os << "Passed-by-value struct argument contains uninitialized data";
@@ -220,32 +249,15 @@
(D && D->getBody()));
OwningPtr<BugType> *BT;
- const char *Desc;
-
- switch (Call.getKind()) {
- case CE_ObjCPropertyAccess:
+ if (isa<ObjCMethodCall>(Call))
BT = &BT_msg_arg;
- // Getters do not have arguments, so we don't need to worry about this.
- Desc = "Argument for property setter is an uninitialized value";
- break;
- case CE_ObjCMessage:
- BT = &BT_msg_arg;
- Desc = "Argument in message expression is an uninitialized value";
- break;
- case CE_Block:
+ else
BT = &BT_call_arg;
- Desc = "Block call argument is an uninitialized value";
- break;
- default:
- BT = &BT_call_arg;
- Desc = "Function call argument is an uninitialized value";
- break;
- }
for (unsigned i = 0, e = Call.getNumArgs(); i != e; ++i)
- if (PreVisitProcessArg(C, Call.getArgSVal(i),
- Call.getArgSourceRange(i), Call.getArgExpr(i),
- checkUninitFields, Desc, *BT))
+ if (PreVisitProcessArg(C, Call.getArgSVal(i), Call.getArgSourceRange(i),
+ Call.getArgExpr(i), /*IsFirstArgument=*/i == 0,
+ checkUninitFields, Call, *BT))
return;
}
@@ -255,22 +267,36 @@
if (recVal.isUndef()) {
if (ExplodedNode *N = C.generateSink()) {
BugType *BT = 0;
- if (isa<ObjCPropertyAccess>(msg)) {
- 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();
- } else {
+ switch (msg.getMessageKind()) {
+ case OCM_Message:
if (!BT_msg_undef)
BT_msg_undef.reset(new BuiltinBug("Receiver in message expression "
"is an uninitialized value"));
BT = BT_msg_undef.get();
+ break;
+ case OCM_PropertyAccess:
+ 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();
+ break;
+ case OCM_Subscript:
+ if (!BT_objc_subscript_undef)
+ BT_objc_subscript_undef.reset(new BuiltinBug("Subscript access on an "
+ "uninitialized object "
+ "pointer"));
+ BT = BT_objc_subscript_undef.get();
+ break;
}
+ assert(BT && "Unknown message kind.");
+
BugReport *R = new BugReport(*BT, BT->getName(), N);
- R->addRange(msg.getReceiverSourceRange());
+ const ObjCMessageExpr *ME = msg.getOriginExpr();
+ R->addRange(ME->getReceiverRange());
// FIXME: getTrackNullOrUndefValueVisitor can't handle "super" yet.
- if (const Expr *ReceiverE = msg.getInstanceReceiverExpr())
+ if (const Expr *ReceiverE = ME->getInstanceReceiver())
R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
ReceiverE,
R));
@@ -302,17 +328,19 @@
new BuiltinBug("Receiver in message expression is "
"'nil' and returns a garbage value"));
+ const ObjCMessageExpr *ME = msg.getOriginExpr();
+
SmallString<200> buf;
llvm::raw_svector_ostream os(buf);
- os << "The receiver of message '" << msg.getSelector().getAsString()
+ os << "The receiver of message '" << ME->getSelector().getAsString()
<< "' is nil and returns a value of type '";
msg.getResultType().print(os, C.getLangOpts());
os << "' that will be garbage";
BugReport *report = new BugReport(*BT_msg_ret, os.str(), N);
- report->addRange(msg.getReceiverSourceRange());
+ report->addRange(ME->getReceiverRange());
// FIXME: This won't track "self" in messages to super.
- if (const Expr *receiver = msg.getInstanceReceiverExpr()) {
+ if (const Expr *receiver = ME->getInstanceReceiver()) {
report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
receiver,
report));
diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index 842cbc5..0e9efaa 100644
--- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
@@ -81,8 +81,7 @@
///
/// This will be called before the analyzer core processes the method call.
/// This is called for any action which produces an Objective-C message send,
- /// including explicit message syntax and property access. See the subclasses
- /// of ObjCMethodCall for more details.
+ /// including explicit message syntax and property access.
///
/// check::PreObjCMessage
void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const {}
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index c44fc67..aca5f79 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -952,8 +952,7 @@
case CE_CXXAllocator:
// FIXME: These calls are currently unsupported.
return getPersistentStopSummary();
- case CE_ObjCMessage:
- case CE_ObjCPropertyAccess: {
+ case CE_ObjCMessage: {
const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call);
if (Msg.isInstanceMessage())
Summ = getInstanceMethodSummary(Msg, State);
@@ -1911,20 +1910,6 @@
isa<CXXBoolLiteralExpr>(E);
}
-static bool isPropertyAccess(const Stmt *S, ParentMap &PM) {
- unsigned maxDepth = 4;
- while (S && maxDepth) {
- if (const PseudoObjectExpr *PO = dyn_cast<PseudoObjectExpr>(S)) {
- if (!isa<ObjCMessageExpr>(PO->getSyntacticForm()))
- return true;
- return false;
- }
- S = PM.getParent(S);
- --maxDepth;
- }
- return false;
-}
-
PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N,
const ExplodedNode *PrevN,
BugReporterContext &BRC,
@@ -1989,10 +1974,19 @@
os << "function call";
}
else {
- assert(isa<ObjCMessageExpr>(S));
- // The message expression may have between written directly or as
- // a property access. Lazily determine which case we are looking at.
- os << (isPropertyAccess(S, N->getParentMap()) ? "Property" : "Method");
+ assert(isa<ObjCMessageExpr>(S));
+ ObjCMethodCall Call(cast<ObjCMessageExpr>(S), CurrSt, LCtx);
+ switch (Call.getMessageKind()) {
+ case OCM_Message:
+ os << "Method";
+ break;
+ case OCM_PropertyAccess:
+ os << "Property";
+ break;
+ case OCM_Subscript:
+ os << "Subscript";
+ break;
+ }
}
if (CurrV.getObjKind() == RetEffect::CF) {
@@ -2824,7 +2818,7 @@
state = updateSymbol(state, Sym, *T, Summ.getReceiverEffect(),
hasErr, C);
if (hasErr) {
- ErrorRange = MsgInvocation->getReceiverSourceRange();
+ ErrorRange = MsgInvocation->getOriginExpr()->getReceiverRange();
ErrorSym = Sym;
}
}
diff --git a/lib/StaticAnalyzer/Core/Calls.cpp b/lib/StaticAnalyzer/Core/Calls.cpp
index ba56364..5c161ba 100644
--- a/lib/StaticAnalyzer/Core/Calls.cpp
+++ b/lib/StaticAnalyzer/Core/Calls.cpp
@@ -518,7 +518,7 @@
if (!isInstanceMessage())
return UnknownVal();
- if (const Expr *Base = getInstanceReceiverExpr())
+ if (const Expr *Base = getOriginExpr()->getInstanceReceiver())
return getSVal(Base);
// An instance message with no expression means we are sending to super.
@@ -529,10 +529,67 @@
return getState()->getSVal(getState()->getRegion(SelfDecl, LCtx));
}
-SourceRange ObjCPropertyAccess::getSourceRange() const {
- const ParentMap &PM = getLocationContext()->getParentMap();
- const ObjCMessageExpr *ME = getOriginExpr();
- const PseudoObjectExpr *PO = cast<PseudoObjectExpr>(PM.getParent(ME));
- return PO->getSourceRange();
+SourceRange ObjCMethodCall::getSourceRange() const {
+ switch (getMessageKind()) {
+ case OCM_Message:
+ return getOriginExpr()->getSourceRange();
+ case OCM_PropertyAccess:
+ case OCM_Subscript:
+ return getContainingPseudoObjectExpr()->getSourceRange();
+ }
+}
+
+typedef llvm::PointerIntPair<const PseudoObjectExpr *, 2> ObjCMessageDataTy;
+
+const PseudoObjectExpr *ObjCMethodCall::getContainingPseudoObjectExpr() const {
+ assert(Data != 0 && "Lazy lookup not yet performed.");
+ assert(getMessageKind() != OCM_Message && "Explicit message send.");
+ return ObjCMessageDataTy::getFromOpaqueValue(Data).getPointer();
+}
+
+ObjCMessageKind ObjCMethodCall::getMessageKind() const {
+ if (Data == 0) {
+ ParentMap &PM = getLocationContext()->getParentMap();
+ const Stmt *S = PM.getParent(getOriginExpr());
+ if (const PseudoObjectExpr *POE = dyn_cast_or_null<PseudoObjectExpr>(S)) {
+ const Expr *Syntactic = POE->getSyntacticForm();
+
+ // This handles the funny case of assigning to the result of a getter.
+ // This can happen if the getter returns a non-const reference.
+ if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Syntactic))
+ Syntactic = BO->getLHS();
+
+ ObjCMessageKind K;
+ switch (Syntactic->getStmtClass()) {
+ case Stmt::ObjCPropertyRefExprClass:
+ K = OCM_PropertyAccess;
+ break;
+ case Stmt::ObjCSubscriptRefExprClass:
+ K = OCM_Subscript;
+ break;
+ default:
+ // FIXME: Can this ever happen?
+ K = OCM_Message;
+ break;
+ }
+
+ if (K != OCM_Message) {
+ const_cast<ObjCMethodCall *>(this)->Data
+ = ObjCMessageDataTy(POE, K).getOpaqueValue();
+ assert(getMessageKind() == K);
+ return K;
+ }
+ }
+
+ const_cast<ObjCMethodCall *>(this)->Data
+ = ObjCMessageDataTy(0, 1).getOpaqueValue();
+ assert(getMessageKind() == OCM_Message);
+ return OCM_Message;
+ }
+
+ ObjCMessageDataTy Info = ObjCMessageDataTy::getFromOpaqueValue(Data);
+ if (!Info.getPointer())
+ return OCM_Message;
+ return static_cast<ObjCMessageKind>(Info.getInt());
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index fb99d91..895e20e 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -890,35 +890,10 @@
case Stmt::ObjCMessageExprClass: {
Bldr.takeNodes(Pred);
- // Is this a property access?
-
- const LocationContext *LCtx = Pred->getLocationContext();
- const ParentMap &PM = LCtx->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();
-
- // This handles the funny case of assigning to the result of a getter.
- // This can happen if the getter returns a non-const reference.
- if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(syntactic))
- syntactic = BO->getLHS();
-
- if (const ObjCPropertyRefExpr *PR =
- dyn_cast<ObjCPropertyRefExpr>(syntactic)) {
- VisitObjCMessage(ObjCPropertyAccess(PR, PO->getSourceRange(), ME,
- Pred->getState(), LCtx),
- Pred, Dst);
- evaluated = true;
- }
- }
-
- if (!evaluated)
- VisitObjCMessage(ObjCMessageSend(ME, Pred->getState(), LCtx),
- Pred, Dst);
-
+ VisitObjCMessage(ObjCMethodCall(cast<ObjCMessageExpr>(S),
+ Pred->getState(),
+ Pred->getLocationContext()),
+ Pred, Dst);
Bldr.addNodes(Dst);
break;
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 493ca91..7cedb7a 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -306,7 +306,6 @@
break;
}
case CE_ObjCMessage:
- case CE_ObjCPropertyAccess:
// These always use dynamic dispatch; enabling inlining means assuming
// that a particular method will be called at runtime.
llvm_unreachable("Dynamic dispatch should be handled above.");