[PATCH] D36750: [analyzer] RetainCount: When diagnosing overrelease, mention if it's coming from a nested block.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 113102.
NoQ marked an inline comment as done.
NoQ added a comment.

Avoid creating a new `RefVal` kind.

In https://reviews.llvm.org/D36750#843427, @dcoughlin wrote:

> > By the way, plist-based tests in retain-release.m are disabled since r163536 (~2012), and need to be updated. It's trivial to re-enable them but annoying to maintain - would we prefer to re-enable or delete them or replace with -analyzer-output=text tests?
>
> My preference would be to factor out/re-target some specific tests into their own file and check that with -verify + -analyzer-output=text and with plist comparisons


Hmm, which specific tests do you have in mind? And is it worth it to try to recover the intended arrows in plists from the old plist tests?


https://reviews.llvm.org/D36750

Files:
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  test/Analysis/retain-release.m


Index: test/Analysis/retain-release.m
===================================================================
--- test/Analysis/retain-release.m
+++ test/Analysis/retain-release.m
@@ -2300,6 +2300,23 @@
   CFRelease(obj);
 }
 
+//===----------------------------------------------------------------------===//
+// When warning within blocks make it obvious that warnings refer to blocks.
+//===----------------------------------------------------------------------===//
+
+int useBlock(int (^block)());
+void rdar31699502_hardToNoticeBlocks() {
+  if (useBlock(^{
+    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+    NSArray *array = [NSArray array];
+    [array release]; // expected-warning{{Incorrect decrement of the reference count of an object that is not owned at this point by the current block}}
+    [pool drain];
+    return 0;
+  })) {
+    return;
+  }
+}
+
 // CHECK:  <key>diagnostics</key>
 // CHECK-NEXT:  <array>
 // CHECK-NEXT:   <dict>
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1722,6 +1722,17 @@
     }
   };
 
+  class BadReleaseByBlock : public CFRefBug {
+  public:
+    BadReleaseByBlock(const CheckerBase *checker)
+        : CFRefBug(checker, "Bad release") {}
+
+    const char *getDescription() const override {
+      return "Incorrect decrement of the reference count of an object that is "
+             "not owned at this point by the current block";
+    }
+  };
+
   class DeallocGC : public CFRefBug {
   public:
     DeallocGC(const CheckerBase *checker)
@@ -2560,7 +2571,8 @@
                     check::RegionChanges,
                     eval::Assume,
                     eval::Call > {
-  mutable std::unique_ptr<CFRefBug> useAfterRelease, releaseNotOwned;
+  mutable std::unique_ptr<CFRefBug> useAfterRelease;
+  mutable std::unique_ptr<CFRefBug> releaseNotOwned, releaseNotOwnedByBlock;
   mutable std::unique_ptr<CFRefBug> deallocGC, deallocNotOwned;
   mutable std::unique_ptr<CFRefBug> overAutorelease, returnNotOwnedForOwned;
   mutable std::unique_ptr<CFRefBug> leakWithinFunction, leakAtReturn;
@@ -3396,9 +3408,15 @@
       BT = useAfterRelease.get();
       break;
     case RefVal::ErrorReleaseNotOwned:
-      if (!releaseNotOwned)
-        releaseNotOwned.reset(new BadRelease(this));
-      BT = releaseNotOwned.get();
+      if (isa<BlockDecl>(C.getLocationContext()->getDecl())) {
+        if (!releaseNotOwnedByBlock)
+          releaseNotOwnedByBlock.reset(new BadReleaseByBlock(this));
+        BT = releaseNotOwnedByBlock.get();
+      } else {
+        if (!releaseNotOwned)
+          releaseNotOwned.reset(new BadRelease(this));
+        BT = releaseNotOwned.get();
+      }
       break;
     case RefVal::ErrorDeallocGC:
       if (!deallocGC)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36750.113102.patch
Type: text/x-patch
Size: 2922 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170829/54f1eb57/attachment.bin>