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)
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)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
