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).

llvm-svn: 153309

clang/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,

clang/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.");
   }

clang/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();

clang/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);
 }
 

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -107,7 +107,8 @@ void ExprEngine::processCallExit(ExplodedNode *Pred) {
                                                     &Ctx);
   SaveAndRestore<unsigned> 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) {

clang/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);
+}
+
+
+

clang/test/Analysis/retain-release.mm

@@ -341,9 +341,9 @@ int rdar10553686(void)
 }
 int rdar10553686_positive(void)
 {
-  NSObject* bar = static_objc_cast<NSObject*>([[NSObject alloc] init]); // expected-warning {{Potential leak}}
+  NSObject* bar = static_objc_cast<NSObject*>([[NSObject alloc] init]);
   [bar release];
-  [bar retain];
+  [bar retain]; // expected-warning {{used after it is released}}
   return 0;
 }