From: Ted Kremenek Date: Fri, 23 Mar 2012 06:26:56 +0000 (+0000) Subject: Avoid applying retain/release effects twice in RetainCountChecker when a function... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=514f2c9dcb9e04b52929c5b141a6fe88bd68b33f;p=clang Avoid applying retain/release effects twice in RetainCountChecker when a function call was inlined (i.e., we do not need to apply summaries in such cases). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@153309 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h index b8a024b9cc..79c94bcd85 100644 --- a/include/clang/StaticAnalyzer/Core/CheckerManager.h +++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -193,14 +193,16 @@ public: void runCheckersForPostStmt(ExplodedNodeSet &Dst, const ExplodedNodeSet &Src, const Stmt *S, - ExprEngine &Eng) { - runCheckersForStmt(/*isPreVisit=*/false, Dst, Src, S, Eng); + ExprEngine &Eng, + bool wasInlined = false) { + runCheckersForStmt(/*isPreVisit=*/false, Dst, Src, S, Eng, wasInlined); } /// \brief Run checkers for visiting Stmts. void runCheckersForStmt(bool isPreVisit, ExplodedNodeSet &Dst, const ExplodedNodeSet &Src, - const Stmt *S, ExprEngine &Eng); + const Stmt *S, ExprEngine &Eng, + bool wasInlined = false); /// \brief Run checkers for pre-visiting obj-c messages. void runCheckersForPreObjCMessage(ExplodedNodeSet &Dst, diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h index 052916177f..b051d33cbd 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -33,15 +33,21 @@ class CheckerContext { NodeBuilder &NB; public: + /// If we are post visiting a call, this flag will be set if the + /// call was inlined. In all other cases it will be false. + const bool wasInlined; + CheckerContext(NodeBuilder &builder, ExprEngine &eng, ExplodedNode *pred, - const ProgramPoint &loc) + const ProgramPoint &loc, + bool wasInlined = false) : Eng(eng), Pred(pred), Changed(false), Location(loc), - NB(builder) { + NB(builder), + wasInlined(wasInlined) { assert(Pred->getState() && "We should not call the checkers on an empty state."); } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 940228e679..bf4b76c640 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -2599,6 +2599,9 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE, void RetainCountChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { + if (C.wasInlined) + return; + // Get the callee. ProgramStateRef state = C.getState(); const Expr *Callee = CE->getCallee(); diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp index 6f7a47fb8f..e8de329daf 100644 --- a/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -138,13 +138,15 @@ namespace { const CheckersTy &Checkers; const Stmt *S; ExprEngine &Eng; + bool wasInlined; CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); } CheckersTy::const_iterator checkers_end() { return Checkers.end(); } CheckStmtContext(bool isPreVisit, const CheckersTy &checkers, - const Stmt *s, ExprEngine &eng) - : IsPreVisit(isPreVisit), Checkers(checkers), S(s), Eng(eng) { } + const Stmt *s, ExprEngine &eng, bool wasInlined = false) + : IsPreVisit(isPreVisit), Checkers(checkers), S(s), Eng(eng), + wasInlined(wasInlined) {} void runChecker(CheckerManager::CheckStmtFunc checkFn, NodeBuilder &Bldr, ExplodedNode *Pred) { @@ -153,8 +155,7 @@ namespace { ProgramPoint::PostStmtKind; const ProgramPoint &L = ProgramPoint::getProgramPoint(S, K, Pred->getLocationContext(), checkFn.Checker); - CheckerContext C(Bldr, Eng, Pred, L); - + CheckerContext C(Bldr, Eng, Pred, L, wasInlined); checkFn(S, C); } }; @@ -165,9 +166,10 @@ void CheckerManager::runCheckersForStmt(bool isPreVisit, ExplodedNodeSet &Dst, const ExplodedNodeSet &Src, const Stmt *S, - ExprEngine &Eng) { + ExprEngine &Eng, + bool wasInlined) { CheckStmtContext C(isPreVisit, *getCachedStmtCheckersFor(S, isPreVisit), - S, Eng); + S, Eng, wasInlined); expandGraphWithCheckers(C, Dst, Src); } diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 8c9154cf4c..66842050af 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -107,7 +107,8 @@ void ExprEngine::processCallExit(ExplodedNode *Pred) { &Ctx); SaveAndRestore CBISave(currentStmtIdx, calleeCtx->getIndex()); - getCheckerManager().runCheckersForPostStmt(Dst, N, CE, *this); + getCheckerManager().runCheckersForPostStmt(Dst, N, CE, *this, + /* wasInlined */ true); // Enqueue the next element in the block. for (ExplodedNodeSet::iterator I = Dst.begin(), E = Dst.end(); I != E; ++I) { diff --git a/test/Analysis/retain-release-inline.m b/test/Analysis/retain-release-inline.m index cbef9995bc..610df7f7e9 100644 --- a/test/Analysis/retain-release-inline.m +++ b/test/Analysis/retain-release-inline.m @@ -314,3 +314,34 @@ void test_test_return_retained() { [x retain]; [x release]; } + +//===----------------------------------------------------------------------===// +// Test not applying "double effects" from inlining and RetainCountChecker summaries. +// If we inline a call, we should already see its retain/release semantics. +//===----------------------------------------------------------------------===// + +__attribute__((cf_returns_retained)) CFStringRef test_return_inline(CFStringRef x) { + CFRetain(x); + return x; +} + +void test_test_return_inline(char *bytes) { + CFStringRef str = CFStringCreateWithCStringNoCopy(0, bytes, NSNEXTSTEPStringEncoding, 0); + // After this call, 'str' really has +2 reference count. + CFStringRef str2 = test_return_inline(str); + // After this call, 'str' really has a +1 reference count. + CFRelease(str); + // After this call, 'str2' and 'str' has a +0 reference count. + CFRelease(str2); +} + +void test_test_return_inline_2(char *bytes) { + CFStringRef str = CFStringCreateWithCStringNoCopy(0, bytes, NSNEXTSTEPStringEncoding, 0); // expected-warning {{leak}} + // After this call, 'str' really has +2 reference count. + CFStringRef str2 = test_return_inline(str); + // After this call, 'str' really has a +1 reference count. + CFRelease(str); +} + + + diff --git a/test/Analysis/retain-release.mm b/test/Analysis/retain-release.mm index 0b550a8e0f..c463f8ada9 100644 --- a/test/Analysis/retain-release.mm +++ b/test/Analysis/retain-release.mm @@ -341,9 +341,9 @@ int rdar10553686(void) } int rdar10553686_positive(void) { - NSObject* bar = static_objc_cast([[NSObject alloc] init]); // expected-warning {{Potential leak}} + NSObject* bar = static_objc_cast([[NSObject alloc] init]); [bar release]; - [bar retain]; + [bar retain]; // expected-warning {{used after it is released}} return 0; }