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 + static void _checkObjCMessage(void *checker, const ObjCMethodCall &msg, + CheckerContext &C) { + ((const CHECKER *)checker)->checkObjCMessageNil(msg, C); + } + +public: + template + static void _register(CHECKER *checker, CheckerManager &mgr) { + mgr._registerForObjCMessageNil( + CheckerManager::CheckObjCMessageFunc(checker, _checkObjCMessage)); + } +}; + class PostObjCMessage { template static void _checkObjCMessage(void *checker, const ObjCMethodCall &msg, 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,7 +217,7 @@ 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. @@ -220,12 +226,21 @@ 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 & + getObjCMessageCheckers(ObjCCheckerKind Kind); + std::vector PreObjCMessageCheckers; std::vector PostObjCMessageCheckers; + std::vector ObjCMessageNilCheckers; std::vector PreCallCheckers; std::vector PostCallCheckers; Index: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -40,6 +40,7 @@ : public Checker< check::PreStmt, check::PreStmt, check::PreObjCMessage, + check::ObjCMessageNil, check::PreCall > { mutable std::unique_ptr BT_call_null; mutable std::unique_ptr 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(); - - 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: lib/StaticAnalyzer/Core/CheckerManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/CheckerManager.cpp +++ lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -177,7 +177,9 @@ namespace { struct CheckObjCMessageContext { typedef std::vector CheckersTy; - bool IsPreVisit, WasInlined; + + ObjCCheckerKind Kind; + bool WasInlined; const CheckersTy &Checkers; const ObjCMethodCall &Msg; ExprEngine &Eng; @@ -185,14 +187,28 @@ 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::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/Core/ExprEngineObjC.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -139,6 +139,31 @@ CallEventRef 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(); + 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(); - - 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 (). - // 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: test/Analysis/NSContainers.m =================================================================== --- test/Analysis/NSContainers.m +++ test/Analysis/NSContainers.m @@ -24,6 +24,8 @@ @interface 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]; +}