r347942 - [analyzer] Reference leaked object by name, even if it was created in an inlined function.

George Karpenkov via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 29 18:17:31 PST 2018


Author: george.karpenkov
Date: Thu Nov 29 18:17:31 2018
New Revision: 347942

URL: http://llvm.org/viewvc/llvm-project?rev=347942&view=rev
Log:
[analyzer] Reference leaked object by name, even if it was created in an inlined function.

rdar://45532181

Differential Revision: https://reviews.llvm.org/D54973

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
    cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist
    cfe/trunk/test/Analysis/osobject-retain-release.cpp
    cfe/trunk/test/Analysis/retain-release-path-notes.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp?rev=347942&r1=347941&r2=347942&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp Thu Nov 29 18:17:31 2018
@@ -306,9 +306,8 @@ struct AllocationInfo {
 };
 } // end anonymous namespace
 
-static AllocationInfo
-GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N,
-                  SymbolRef Sym) {
+static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
+                                        const ExplodedNode *N, SymbolRef Sym) {
   const ExplodedNode *AllocationNode = N;
   const ExplodedNode *AllocationNodeInCurrentOrParentContext = N;
   const MemRegion *FirstBinding = nullptr;
@@ -353,9 +352,9 @@ GetAllocationSite(ProgramStateManager& S
     // Find the last init that was called on the given symbol and store the
     // init method's location context.
     if (!InitMethodContext)
-      if (Optional<CallEnter> CEP = N->getLocation().getAs<CallEnter>()) {
+      if (auto CEP = N->getLocation().getAs<CallEnter>()) {
         const Stmt *CE = CEP->getCallExpr();
-        if (const ObjCMessageExpr *ME = dyn_cast_or_null<ObjCMessageExpr>(CE)) {
+        if (const auto *ME = dyn_cast_or_null<ObjCMessageExpr>(CE)) {
           const Stmt *RecExpr = ME->getInstanceReceiver();
           if (RecExpr) {
             SVal RecV = St->getSVal(RecExpr, NContext);
@@ -365,7 +364,7 @@ GetAllocationSite(ProgramStateManager& S
         }
       }
 
-    N = N->pred_empty() ? nullptr : *(N->pred_begin());
+    N = N->getFirstPred();
   }
 
   // If we are reporting a leak of the object that was allocated with alloc,
@@ -382,9 +381,11 @@ GetAllocationSite(ProgramStateManager& S
   // If allocation happened in a function different from the leak node context,
   // do not report the binding.
   assert(N && "Could not find allocation node");
-  if (N->getLocationContext() != LeakContext) {
+
+  if (AllocationNodeInCurrentOrParentContext &&
+      AllocationNodeInCurrentOrParentContext->getLocationContext() !=
+          LeakContext)
     FirstBinding = nullptr;
-  }
 
   return AllocationInfo(AllocationNodeInCurrentOrParentContext,
                         FirstBinding,
@@ -409,8 +410,7 @@ CFRefLeakReportVisitor::getEndPath(BugRe
   // We are reporting a leak.  Walk up the graph to get to the first node where
   // the symbol appeared, and also get the first VarDecl that tracked object
   // is stored to.
-  AllocationInfo AllocI =
-    GetAllocationSite(BRC.getStateManager(), EndN, Sym);
+  AllocationInfo AllocI = GetAllocationSite(BRC.getStateManager(), EndN, Sym);
 
   const MemRegion* FirstBinding = AllocI.R;
   BR.markInteresting(AllocI.InterestingMethodContext);
@@ -448,11 +448,12 @@ CFRefLeakReportVisitor::getEndPath(BugRe
     os << (isa<ObjCMethodDecl>(D) ? " is returned from a method "
                                   : " is returned from a function ");
 
-    if (D->hasAttr<CFReturnsNotRetainedAttr>())
+    if (D->hasAttr<CFReturnsNotRetainedAttr>()) {
       os << "that is annotated as CF_RETURNS_NOT_RETAINED";
-    else if (D->hasAttr<NSReturnsNotRetainedAttr>())
+    } else if (D->hasAttr<NSReturnsNotRetainedAttr>()) {
       os << "that is annotated as NS_RETURNS_NOT_RETAINED";
-    else {
+      // TODO: once the patch is ready, insert a case for OS_RETURNS_NOT_RETAINED
+    } else {
       if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
         if (BRC.getASTContext().getLangOpts().ObjCAutoRefCount) {
           os << "managed by Automatic Reference Counting";
@@ -497,7 +498,8 @@ void CFRefLeakReport::deriveParamLocatio
   }
 }
 
-void CFRefLeakReport::deriveAllocLocation(CheckerContext &Ctx,SymbolRef sym) {
+void CFRefLeakReport::deriveAllocLocation(CheckerContext &Ctx,
+                                          SymbolRef sym) {
   // Most bug reports are cached at the location where they occurred.
   // With leaks, we want to unique them by the location where they were
   // allocated, and only report a single path.  To do this, we need to find
@@ -511,7 +513,7 @@ void CFRefLeakReport::deriveAllocLocatio
   const SourceManager& SMgr = Ctx.getSourceManager();
 
   AllocationInfo AllocI =
-    GetAllocationSite(Ctx.getStateManager(), getErrorNode(), sym);
+      GetAllocationSite(Ctx.getStateManager(), getErrorNode(), sym);
 
   AllocNode = AllocI.N;
   AllocBinding = AllocI.R;

Modified: cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist?rev=347942&r1=347941&r2=347942&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist (original)
+++ cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist Thu Nov 29 18:17:31 2018
@@ -4233,12 +4233,12 @@
      </array>
      <key>depth</key><integer>0</integer>
      <key>extended_message</key>
-     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+     <string>Object leaked: object allocated and stored into 'y' is not referenced later in this execution path and has a retain count of +1</string>
      <key>message</key>
-     <string>Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1</string>
+     <string>Object leaked: object allocated and stored into 'y' is not referenced later in this execution path and has a retain count of +1</string>
     </dict>
    </array>
-   <key>description</key><string>Potential leak of an object</string>
+   <key>description</key><string>Potential leak of an object stored into 'y'</string>
    <key>category</key><string>Memory (Core Foundation/Objective-C)</string>
    <key>type</key><string>Leak</string>
    <key>check_name</key><string>osx.cocoa.RetainCount</string>

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=347942&r1=347941&r2=347942&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Thu Nov 29 18:17:31 2018
@@ -24,6 +24,8 @@ struct OSObject {
 };
 
 struct OSIterator : public OSObject {
+
+  static const OSMetaClass * const metaClass;
 };
 
 struct OSArray : public OSObject {
@@ -41,7 +43,6 @@ struct OSArray : public OSObject {
   static OS_RETURNS_NOT_RETAINED OSArray *MaskedGetter();
   static OS_RETURNS_RETAINED OSArray *getOoopsActuallyCreate();
 
-
   static const OSMetaClass * const metaClass;
 };
 
@@ -92,6 +93,25 @@ struct ArrayOwner : public OSObject {
   OSArray *getArraySourceUnknown();
 };
 
+OSArray *generateArray() {
+  return OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+}
+
+unsigned int check_leak_good_error_message() {
+  unsigned int out;
+  {
+    OSArray *leaked = generateArray(); // expected-note{{Calling 'generateArray'}}
+                                       // expected-note at -1{{Returning from 'generateArray'}}
+    out = leaked->getCount(); // expected-warning{{Potential leak of an object stored into 'leaked'}}
+                              // expected-note at -1{{Object leaked: object allocated and stored into 'leaked' is not referenced later in this execution path and has a retain count of +1}}
+  }
+  return out;
+}
+
+unsigned int check_leak_msg_temporary() {
+  return generateArray()->getCount();
+}
+
 void check_confusing_getters() {
   OSArray *arr = OSArray::withCapacity(10);
 
@@ -143,7 +163,7 @@ void check_dynamic_cast_null_branch(OSOb
   OSArray *arr1 = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject}}
   OSArray *arr = OSDynamicCast(OSArray, obj);
   if (!arr) // expected-note{{Taking true branch}}
-    return; // expected-warning{{Potential leak}}
+    return; // expected-warning{{Potential leak of an object stored into 'arr1'}}
             // expected-note at -1{{Object leaked}}
   arr1->release();
 }

Modified: cfe/trunk/test/Analysis/retain-release-path-notes.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-path-notes.m?rev=347942&r1=347941&r2=347942&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/retain-release-path-notes.m (original)
+++ cfe/trunk/test/Analysis/retain-release-path-notes.m Thu Nov 29 18:17:31 2018
@@ -235,7 +235,7 @@ static int Cond;
 
   // initZ is not inlined
   id z = [[MyObj alloc] initZ]; // expected-warning {{Potential leak of an object}}
-                                // expected-note at -1 {{Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1}}
+                                // expected-note at -1 {{Object leaked: object allocated and stored into 'y' is not referenced later in this execution path and has a retain count of +1}}
 
   [x release];
   [z release];




More information about the cfe-commits mailing list