dcoughlin created this revision.
dcoughlin added reviewers: zaks.anna, jordan_rose, xazax.hun.
dcoughlin added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.
In Objective-C, method calls with nil receivers are essentially no-ops. They
do not fault (although the returned value may be garbage depending on the
declared return type and architecture). Programmers are aware of this
behavior and will complain about a false alarm when the analyzer
diagnoses API violations for method calls when the receiver is known to
be nil.
Rather than require each individual checker to be aware of this behavior
and suppress a warning when the receiver is nil, this patch
changes ExprEngineObjC so that VisitObjCMessage skips calling checker
pre/post handlers when the receiver is nil definitely nil. Instead, it adds a
new event, ObjCMessageNil, that is only called in that case.
The CallAndMessageChecker explicitly cares about this case, so I've changed it
to add a callback for ObjCMessageNil and moved the logic that handles nil
receivers
in its PreObjCMessage callback to the new callback.
rdar://problem/18092611
http://reviews.llvm.org/D12123
Files:
include/clang/StaticAnalyzer/Core/Checker.h
include/clang/StaticAnalyzer/Core/CheckerManager.h
lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
lib/StaticAnalyzer/Core/CheckerManager.cpp
lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
test/Analysis/NSContainers.m
Index: test/Analysis/NSContainers.m
===================================================================
--- test/Analysis/NSContainers.m
+++ test/Analysis/NSContainers.m
@@ -24,6 +24,8 @@
@interface NSObject <NSObject> {}
- (id)init;
+ (id)alloc;
+
+- (id)mutableCopy;
@end
typedef struct {
@@ -292,3 +294,16 @@
[arr addObject:0 safe:1]; // no-warning
}
+@interface MyView : NSObject
+-(NSArray *)subviews;
+@end
+
+void testNoReportWhenReceiverNil(NSMutableArray *array, int b) {
+ if (array == 0) {
+ [array addObject:0];
+ }
+
+ MyView *view = b ? [[MyView alloc] init] : 0;
+ NSMutableArray *subviews = [[view subviews] mutableCopy];
+ [subviews addObject:view];
+}
Index: lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -139,6 +139,31 @@
CallEventRef<ObjCMethodCall> Msg =
CEMgr.getObjCMethodCall(ME, Pred->getState(), Pred->getLocationContext());
+ // If the receiver is definitely nil, skip the pre/post callbacks and instead
+ // call the ObjCMessageNil callbacks and return.
+ if (Msg->isInstanceMessage()) {
+ SVal recVal = Msg->getReceiverSVal();
+ if (!recVal.isUndef()) {
+ // Bifurcate the state into nil and non-nil ones.
+ DefinedOrUnknownSVal receiverVal =
+ recVal.castAs<DefinedOrUnknownSVal>();
+ ProgramStateRef State = Pred->getState();
+
+ ProgramStateRef notNilState, nilState;
+ std::tie(notNilState, nilState) = State->assume(receiverVal);
+ if (nilState && !notNilState) {
+ ExplodedNodeSet dstNil;
+ StmtNodeBuilder Bldr(Pred, dstNil, *currBldrCtx);
+
+ Pred = Bldr.generateNode(ME, Pred, nilState);
+ assert(Pred && "Should have cached out already!");
+ getCheckerManager().runCheckersForObjCMessageNil(Dst, dstNil,
+ *Msg, *this);
+ return;
+ }
+ }
+ }
+
// Handle the previsits checks.
ExplodedNodeSet dstPrevisit;
getCheckerManager().runCheckersForPreObjCMessage(dstPrevisit, Pred,
@@ -160,35 +185,12 @@
if (UpdatedMsg->isInstanceMessage()) {
SVal recVal = UpdatedMsg->getReceiverSVal();
if (!recVal.isUndef()) {
- // Bifurcate the state into nil and non-nil ones.
- DefinedOrUnknownSVal receiverVal =
- recVal.castAs<DefinedOrUnknownSVal>();
-
- ProgramStateRef notNilState, nilState;
- std::tie(notNilState, nilState) = State->assume(receiverVal);
-
- // There are three cases: can be nil or non-nil, must be nil, must be
- // non-nil. We ignore must be nil, and merge the rest two into non-nil.
- // FIXME: This ignores many potential bugs (<rdar://problem/11733396>).
- // Revisit once we have lazier constraints.
- if (nilState && !notNilState) {
- continue;
- }
-
- // Check if the "raise" message was sent.
- assert(notNilState);
if (ObjCNoRet.isImplicitNoReturn(ME)) {
// If we raise an exception, for now treat it as a sink.
// Eventually we will want to handle exceptions properly.
Bldr.generateSink(ME, Pred, State);
continue;
}
-
- // Generate a transition to non-Nil state.
- if (notNilState != State) {
- Pred = Bldr.generateNode(ME, Pred, notNilState);
- assert(Pred && "Should have cached out already!");
- }
}
} else {
// Check for special class methods that are known to not return
Index: lib/StaticAnalyzer/Core/CheckerManager.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -177,22 +177,38 @@
namespace {
struct CheckObjCMessageContext {
typedef std::vector<CheckerManager::CheckObjCMessageFunc> CheckersTy;
- bool IsPreVisit, WasInlined;
+
+ ObjCCheckerKind Kind;
+ bool WasInlined;
const CheckersTy &Checkers;
const ObjCMethodCall &Msg;
ExprEngine &Eng;
CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
- CheckObjCMessageContext(bool isPreVisit, const CheckersTy &checkers,
+ CheckObjCMessageContext(ObjCCheckerKind checkerKind,
+ const CheckersTy &checkers,
const ObjCMethodCall &msg, ExprEngine &eng,
bool wasInlined)
- : IsPreVisit(isPreVisit), WasInlined(wasInlined), Checkers(checkers),
+ : Kind(checkerKind), WasInlined(wasInlined), Checkers(checkers),
Msg(msg), Eng(eng) { }
void runChecker(CheckerManager::CheckObjCMessageFunc checkFn,
NodeBuilder &Bldr, ExplodedNode *Pred) {
+
+ bool IsPreVisit;
+
+ switch (Kind) {
+ case ObjCCheckerKind::PreVisit:
+ case ObjCCheckerKind::MessageNil:
+ IsPreVisit = true;
+ break;
+ case ObjCCheckerKind::PostVisit:
+ IsPreVisit = false;
+ break;
+ }
+
const ProgramPoint &L = Msg.getProgramPoint(IsPreVisit,checkFn.Checker);
CheckerContext C(Bldr, Eng, Pred, L, WasInlined);
@@ -202,19 +218,29 @@
}
/// \brief Run checkers for visiting obj-c messages.
-void CheckerManager::runCheckersForObjCMessage(bool isPreVisit,
+void CheckerManager::runCheckersForObjCMessage(ObjCCheckerKind checkerKind,
ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
const ObjCMethodCall &msg,
ExprEngine &Eng,
bool WasInlined) {
- CheckObjCMessageContext C(isPreVisit,
- isPreVisit ? PreObjCMessageCheckers
- : PostObjCMessageCheckers,
- msg, Eng, WasInlined);
+ auto &checkers = getObjCMessageCheckers(checkerKind);
+ CheckObjCMessageContext C(checkerKind, checkers, msg, Eng, WasInlined);
expandGraphWithCheckers(C, Dst, Src);
}
+const std::vector<CheckerManager::CheckObjCMessageFunc> &
+CheckerManager::getObjCMessageCheckers(ObjCCheckerKind Kind) {
+ switch (Kind) {
+ case ObjCCheckerKind::PreVisit:
+ return PreObjCMessageCheckers;
+ break;
+ case ObjCCheckerKind::PostVisit:
+ return PostObjCMessageCheckers;
+ case ObjCCheckerKind::MessageNil:
+ return ObjCMessageNilCheckers;
+ }
+}
namespace {
// FIXME: This has all the same signatures as CheckObjCMessageContext.
// Is there a way we can merge the two?
@@ -616,6 +642,11 @@
void CheckerManager::_registerForPreObjCMessage(CheckObjCMessageFunc checkfn) {
PreObjCMessageCheckers.push_back(checkfn);
}
+
+void CheckerManager::_registerForObjCMessageNil(CheckObjCMessageFunc checkfn) {
+ ObjCMessageNilCheckers.push_back(checkfn);
+}
+
void CheckerManager::_registerForPostObjCMessage(CheckObjCMessageFunc checkfn) {
PostObjCMessageCheckers.push_back(checkfn);
}
Index: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -40,6 +40,7 @@
: public Checker< check::PreStmt<CallExpr>,
check::PreStmt<CXXDeleteExpr>,
check::PreObjCMessage,
+ check::ObjCMessageNil,
check::PreCall > {
mutable std::unique_ptr<BugType> BT_call_null;
mutable std::unique_ptr<BugType> BT_call_undef;
@@ -60,6 +61,7 @@
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
+ void checkObjCMessageNil(const ObjCMethodCall &msg, CheckerContext &C) const;
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
private:
@@ -471,22 +473,14 @@
C.emitReport(std::move(R));
}
return;
- } else {
- // Bifurcate the state into nil and non-nil ones.
- DefinedOrUnknownSVal receiverVal = recVal.castAs<DefinedOrUnknownSVal>();
-
- ProgramStateRef state = C.getState();
- ProgramStateRef notNilState, nilState;
- std::tie(notNilState, nilState) = state->assume(receiverVal);
-
- // Handle receiver must be nil.
- if (nilState && !notNilState) {
- HandleNilReceiver(C, state, msg);
- return;
- }
}
}
+void CallAndMessageChecker::checkObjCMessageNil(const ObjCMethodCall &msg,
+ CheckerContext &C) const {
+ HandleNilReceiver(C, C.getState(), msg);
+}
+
void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C,
const ObjCMethodCall &msg,
ExplodedNode *N) const {
Index: include/clang/StaticAnalyzer/Core/CheckerManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -93,6 +93,12 @@
StringRef getName() const { return Name; }
};
+enum class ObjCCheckerKind {
+ PreVisit,
+ PostVisit,
+ MessageNil
+};
+
class CheckerManager {
const LangOptions LangOpts;
AnalyzerOptionsRef AOptions;
@@ -211,21 +217,30 @@
const ExplodedNodeSet &Src,
const ObjCMethodCall &msg,
ExprEngine &Eng) {
- runCheckersForObjCMessage(/*isPreVisit=*/true, Dst, Src, msg, Eng);
+ runCheckersForObjCMessage(ObjCCheckerKind::PreVisit, Dst, Src, msg, Eng);
}
/// \brief Run checkers for post-visiting obj-c messages.
void runCheckersForPostObjCMessage(ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
const ObjCMethodCall &msg,
ExprEngine &Eng,
bool wasInlined = false) {
- runCheckersForObjCMessage(/*isPreVisit=*/false, Dst, Src, msg, Eng,
+ runCheckersForObjCMessage(ObjCCheckerKind::PostVisit, Dst, Src, msg, Eng,
wasInlined);
}
+ /// \brief Run checkers for visiting an an obj-c message to a nil.
+ void runCheckersForObjCMessageNil(ExplodedNodeSet &Dst,
+ const ExplodedNodeSet &Src,
+ const ObjCMethodCall &msg,
+ ExprEngine &Eng) {
+ runCheckersForObjCMessage(ObjCCheckerKind::MessageNil, Dst, Src, msg, Eng);
+ }
+
+
/// \brief Run checkers for visiting obj-c messages.
- void runCheckersForObjCMessage(bool isPreVisit,
+ void runCheckersForObjCMessage(ObjCCheckerKind checkerKind,
ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
const ObjCMethodCall &msg, ExprEngine &Eng,
@@ -457,6 +472,8 @@
void _registerForPreObjCMessage(CheckObjCMessageFunc checkfn);
void _registerForPostObjCMessage(CheckObjCMessageFunc checkfn);
+ void _registerForObjCMessageNil(CheckObjCMessageFunc checkfn);
+
void _registerForPreCall(CheckCallFunc checkfn);
void _registerForPostCall(CheckCallFunc checkfn);
@@ -557,8 +574,12 @@
const CachedStmtCheckers &getCachedStmtCheckersFor(const Stmt *S,
bool isPreVisit);
+ const std::vector<CheckObjCMessageFunc> &
+ getObjCMessageCheckers(ObjCCheckerKind Kind);
+
std::vector<CheckObjCMessageFunc> PreObjCMessageCheckers;
std::vector<CheckObjCMessageFunc> PostObjCMessageCheckers;
+ std::vector<CheckObjCMessageFunc> ObjCMessageNilCheckers;
std::vector<CheckCallFunc> PreCallCheckers;
std::vector<CheckCallFunc> PostCallCheckers;
Index: include/clang/StaticAnalyzer/Core/Checker.h
===================================================================
--- include/clang/StaticAnalyzer/Core/Checker.h
+++ include/clang/StaticAnalyzer/Core/Checker.h
@@ -131,6 +131,21 @@
}
};
+class ObjCMessageNil {
+ template <typename CHECKER>
+ static void _checkObjCMessage(void *checker, const ObjCMethodCall &msg,
+ CheckerContext &C) {
+ ((const CHECKER *)checker)->checkObjCMessageNil(msg, C);
+ }
+
+public:
+ template <typename CHECKER>
+ static void _register(CHECKER *checker, CheckerManager &mgr) {
+ mgr._registerForObjCMessageNil(
+ CheckerManager::CheckObjCMessageFunc(checker, _checkObjCMessage<CHECKER>));
+ }
+};
+
class PostObjCMessage {
template <typename CHECKER>
static void _checkObjCMessage(void *checker, const ObjCMethodCall &msg,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
