NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, JDevlieghere, szepet,
xazax.hun.
https://reviews.llvm.org/D38877 added support for the misplaced
`CF_RETURNS_RETAINED` annotation on `CFRetain()` wrappers. It works by trusting
the function's name (seeing if it confirms to the CoreFoundation naming
convention) rather than the annotation.
There are more false positives caused by users using a different naming
convention, namely starting the function name with "retain" or "release" rather
than suffixing it with "retain" or "release" respectively.
Because this isn't according to the naming convention, these functions are
usually inlined and the annotation is therefore ignored, which is correct. But
sometimes we run out of inlining stack depth and the function is evaluated
conservatively and then the annotation is trusted.
Add support for the "alternative" naming convention and test the situation when
we're running out of inlining stack depth.
Repository:
rC Clang
https://reviews.llvm.org/D45335
Files:
lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
test/Analysis/retain-release-safe.c
Index: test/Analysis/retain-release-safe.c
===================================================================
--- test/Analysis/retain-release-safe.c
+++ test/Analysis/retain-release-safe.c
@@ -1,4 +1,5 @@
// RUN: %clang_analyze_cc1
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount
-verify %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount
-analyzer-inline-max-stack-depth=0 -verify %s
#pragma clang arc_cf_code_audited begin
typedef const void * CFTypeRef;
@@ -35,6 +36,19 @@
CFRelease(cf); // no-warning (when inlined)
}
+// The same thing, just with a different naming style.
+CFTypeRef retainCFType(CFTypeRef cf) CF_RETURNS_RETAINED {
+ if (cf) {
+ return CFRetain(cf);
+ }
+ return cf;
+}
+
+void releaseCFType(CFTypeRef CF_CONSUMED cf) {
+ if (cf)
+ CFRelease(cf); // no-warning (when inlined)
+}
+
void escape(CFTypeRef cf);
void makeSureTestsWork() {
@@ -70,3 +84,10 @@
SafeCFRelease(cf);
SafeCFRelease(cf); // no-warning after inlining this.
}
+
+void testTheOtherNamingConvention(CFTypeRef cf) {
+ retainCFType(cf);
+ retainCFType(cf);
+ releaseCFType(cf);
+ releaseCFType(cf); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -883,21 +883,22 @@
//===----------------------------------------------------------------------===//
static bool isRetain(const FunctionDecl *FD, StringRef FName) {
- return FName.endswith("Retain");
+ return FName.startswith_lower("retain") || FName.endswith_lower("retain");
}
static bool isRelease(const FunctionDecl *FD, StringRef FName) {
- return FName.endswith("Release");
+ return FName.startswith_lower("release") || FName.endswith_lower("release");
}
static bool isAutorelease(const FunctionDecl *FD, StringRef FName) {
- return FName.endswith("Autorelease");
+ return FName.startswith_lower("autorelease") ||
+ FName.endswith_lower("autorelease");
}
static bool isMakeCollectable(const FunctionDecl *FD, StringRef FName) {
// FIXME: Remove FunctionDecl parameter.
// FIXME: Is it really okay if MakeCollectable isn't a suffix?
- return FName.find("MakeCollectable") != StringRef::npos;
+ return FName.find_lower("MakeCollectable") != StringRef::npos;
}
static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) {
Index: test/Analysis/retain-release-safe.c
===================================================================
--- test/Analysis/retain-release-safe.c
+++ test/Analysis/retain-release-safe.c
@@ -1,4 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.RetainCount -analyzer-inline-max-stack-depth=0 -verify %s
#pragma clang arc_cf_code_audited begin
typedef const void * CFTypeRef;
@@ -35,6 +36,19 @@
CFRelease(cf); // no-warning (when inlined)
}
+// The same thing, just with a different naming style.
+CFTypeRef retainCFType(CFTypeRef cf) CF_RETURNS_RETAINED {
+ if (cf) {
+ return CFRetain(cf);
+ }
+ return cf;
+}
+
+void releaseCFType(CFTypeRef CF_CONSUMED cf) {
+ if (cf)
+ CFRelease(cf); // no-warning (when inlined)
+}
+
void escape(CFTypeRef cf);
void makeSureTestsWork() {
@@ -70,3 +84,10 @@
SafeCFRelease(cf);
SafeCFRelease(cf); // no-warning after inlining this.
}
+
+void testTheOtherNamingConvention(CFTypeRef cf) {
+ retainCFType(cf);
+ retainCFType(cf);
+ releaseCFType(cf);
+ releaseCFType(cf); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -883,21 +883,22 @@
//===----------------------------------------------------------------------===//
static bool isRetain(const FunctionDecl *FD, StringRef FName) {
- return FName.endswith("Retain");
+ return FName.startswith_lower("retain") || FName.endswith_lower("retain");
}
static bool isRelease(const FunctionDecl *FD, StringRef FName) {
- return FName.endswith("Release");
+ return FName.startswith_lower("release") || FName.endswith_lower("release");
}
static bool isAutorelease(const FunctionDecl *FD, StringRef FName) {
- return FName.endswith("Autorelease");
+ return FName.startswith_lower("autorelease") ||
+ FName.endswith_lower("autorelease");
}
static bool isMakeCollectable(const FunctionDecl *FD, StringRef FName) {
// FIXME: Remove FunctionDecl parameter.
// FIXME: Is it really okay if MakeCollectable isn't a suffix?
- return FName.find("MakeCollectable") != StringRef::npos;
+ return FName.find_lower("MakeCollectable") != StringRef::npos;
}
static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
