https://github.com/rniwa created
https://github.com/llvm/llvm-project/pull/132316
This PR adds the support for recognizing calling adoptCF/adoptNS on the result
of a cast operation on the return value of a function which creates NS or CF
types. It also fixes a bug that we weren't reporting memory leaks when CF types
are created without ever calling RetainPtr's constructor, adoptCF, or adoptNS.
To do this, this PR adds a new mechanism to report a memory leak whenever
create or copy CF functions are invoked unless this CallExpr has already been
visited while validating a call to adoptCF. Also added an early exit when
isOwned returns IsOwnedResult::Skip due to an unresolved template argument.
>From 642057c409b1c3b98ee4ecb16e95b5fb5be47a01 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Thu, 20 Mar 2025 17:54:22 -0700
Subject: [PATCH] [alpha.webkit.RetainPtrCtorAdoptChecker] Support
adopt(cast(copy(~))
This PR adds the support for recognizing calling adoptCF/adoptNS on the result
of a cast
operation on the return value of a function which creates NS or CF types. It
also fixes
a bug that we weren't reporting memory leaks when CF types are created without
ever
calling RetainPtr's constructor, adoptCF, or adoptNS.
To do this, this PR adds a new mechanism to report a memory leak whenever
create or copy
CF functions are invoked unless this CallExpr has already been visited while
validating
a call to adoptCF. Also added an early exit when isOwned returns
IsOwnedResult::Skip due
to an unresolved template argument.
---
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 70 ++++++++++++++----
.../Checkers/WebKit/objc-mock-types.h | 72 ++++++++++++++++++-
.../WebKit/retain-ptr-ctor-adopt-use-arc.mm | 9 ++-
.../WebKit/retain-ptr-ctor-adopt-use.mm | 9 ++-
4 files changed, 140 insertions(+), 20 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index 4ce3262e90e13..2d9620cc8ee5e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -34,6 +34,7 @@ class RetainPtrCtorAdoptChecker
mutable BugReporter *BR;
mutable std::unique_ptr<RetainSummaryManager> Summaries;
mutable llvm::DenseSet<const ValueDecl *> CreateOrCopyOutArguments;
+ mutable llvm::DenseSet<const Expr *> CreateOrCopyFnCall;
mutable RetainTypeChecker RTC;
public:
@@ -119,7 +120,7 @@ class RetainPtrCtorAdoptChecker
return;
if (!isAdoptFn(F) || !CE->getNumArgs()) {
- rememberOutArguments(CE, F);
+ checkCreateOrCopyFunction(CE, F, DeclWithIssue);
return;
}
@@ -128,24 +129,30 @@ class RetainPtrCtorAdoptChecker
auto Name = safeGetName(F);
if (Result == IsOwnedResult::Unknown)
Result = IsOwnedResult::NotOwned;
- if (Result == IsOwnedResult::NotOwned && !isAllocInit(Arg) &&
- !isCreateOrCopy(Arg)) {
- if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
- if (CreateOrCopyOutArguments.contains(DRE->getDecl()))
- return;
- }
- if (RTC.isARCEnabled() && isAdoptNS(F))
- reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
- else
- reportUseAfterFree(Name, CE, DeclWithIssue);
+ if (isAllocInit(Arg) || isCreateOrCopy(Arg)) {
+ CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
+ return;
+ }
+ if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip)
+ return;
+
+ if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+ if (CreateOrCopyOutArguments.contains(DRE->getDecl()))
+ return;
}
+ if (RTC.isARCEnabled() && isAdoptNS(F))
+ reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
+ else
+ reportUseAfterFree(Name, CE, DeclWithIssue);
}
- void rememberOutArguments(const CallExpr *CE,
- const FunctionDecl *Callee) const {
+ void checkCreateOrCopyFunction(const CallExpr *CE,
+ const FunctionDecl *Callee,
+ const Decl *DeclWithIssue) const {
if (!isCreateOrCopyFunction(Callee))
return;
+ bool hasOutArgument = false;
unsigned ArgCount = CE->getNumArgs();
for (unsigned ArgIndex = 0; ArgIndex < ArgCount; ++ArgIndex) {
auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
@@ -164,7 +171,10 @@ class RetainPtrCtorAdoptChecker
if (!Decl)
continue;
CreateOrCopyOutArguments.insert(Decl);
+ hasOutArgument = true;
}
+ if (!hasOutArgument && !CreateOrCopyFnCall.contains(CE))
+ reportLeak(CE, DeclWithIssue);
}
void visitConstructExpr(const CXXConstructExpr *CE,
@@ -190,6 +200,13 @@ class RetainPtrCtorAdoptChecker
std::string Name = "RetainPtr constructor";
auto *Arg = CE->getArg(0)->IgnoreParenCasts();
auto Result = isOwned(Arg);
+
+ if (isCreateOrCopy(Arg))
+ CreateOrCopyFnCall.insert(Arg); // Avoid double reporting.
+
+ if (Result == IsOwnedResult::Skip)
+ return;
+
if (Result == IsOwnedResult::Unknown)
Result = IsOwnedResult::NotOwned;
if (Result == IsOwnedResult::Owned)
@@ -303,11 +320,22 @@ class RetainPtrCtorAdoptChecker
if (auto *Callee = CE->getDirectCallee()) {
if (isAdoptFn(Callee))
return IsOwnedResult::NotOwned;
- if (safeGetName(Callee) == "__builtin___CFStringMakeConstantString")
+ auto Name = safeGetName(Callee);
+ if (Name == "__builtin___CFStringMakeConstantString")
return IsOwnedResult::NotOwned;
+ if ((Name == "checked_cf_cast" || Name == "dynamic_cf_cast" ||
+ Name == "checked_objc_cast" || Name == "dynamic_objc_cast") &&
+ CE->getNumArgs() == 1) {
+ E = CE->getArg(0)->IgnoreParenCasts();
+ continue;
+ }
auto RetType = Callee->getReturnType();
if (isRetainPtrType(RetType))
return IsOwnedResult::NotOwned;
+ if (isCreateOrCopyFunction(Callee)) {
+ CreateOrCopyFnCall.insert(CE);
+ return IsOwnedResult::Owned;
+ }
} else if (auto *CalleeExpr = CE->getCallee()) {
if (isa<CXXDependentScopeMemberExpr>(CalleeExpr))
return IsOwnedResult::Skip; // Wait for instantiation.
@@ -371,6 +399,20 @@ class RetainPtrCtorAdoptChecker
Report->setDeclWithIssue(DeclWithIssue);
BR->emitReport(std::move(Report));
}
+
+ void reportLeak(const CallExpr *CE, const Decl *DeclWithIssue) const {
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << "The return value is +1 and results in a memory leak.";
+
+ PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(),
+ BR->getSourceManager());
+ auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+ Report->addRange(CE->getSourceRange());
+ Report->setDeclWithIssue(DeclWithIssue);
+ BR->emitReport(std::move(Report));
+ }
};
} // namespace
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 5bd265596a0b4..ab5e820b2c636 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -21,6 +21,11 @@ typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef)
__CFRunLoop * CFRunLoopRef;
extern const CFAllocatorRef kCFAllocatorDefault;
typedef struct _NSZone NSZone;
+typedef unsigned long CFTypeID;
+
+CFTypeID CFGetTypeID(CFTypeRef cf);
+
+CFTypeID CFArrayGetTypeID(void);
CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex
capacity);
extern void CFArrayAppendValue(CFMutableArrayRef theArray, const void *value);
CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values,
CFIndex numValues);
@@ -29,6 +34,7 @@ CFIndex CFArrayGetCount(CFArrayRef theArray);
typedef const struct CF_BRIDGED_TYPE(NSDictionary) __CFDictionary *
CFDictionaryRef;
typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableDictionary) __CFDictionary *
CFMutableDictionaryRef;
+CFTypeID CFDictionaryGetTypeID(void);
CFDictionaryRef CFDictionaryCreate(CFAllocatorRef allocator, const void
**keys, const void **values, CFIndex numValues);
CFDictionaryRef CFDictionaryCreateCopy(CFAllocatorRef allocator,
CFDictionaryRef theDict);
CFDictionaryRef CFDictionaryCreateMutableCopy(CFAllocatorRef allocator,
CFIndex capacity, CFDictionaryRef theDict);
@@ -135,6 +141,8 @@ __attribute__((objc_root_class))
namespace WTF {
+void WTFCrash(void);
+
template<typename T> class RetainPtr;
template<typename T> RetainPtr<T> adoptNS(T*);
template<typename T> RetainPtr<T> adoptCF(T);
@@ -265,19 +273,79 @@ template<typename T> inline RetainPtr<T> retainPtr(T* ptr)
inline NSObject *bridge_cast(CFTypeRef object)
{
- return (__bridge NSObject *)object;
+ return (__bridge NSObject *)object;
}
inline CFTypeRef bridge_cast(NSObject *object)
{
- return (__bridge CFTypeRef)object;
+ return (__bridge CFTypeRef)object;
+}
+
+template <typename> struct CFTypeTrait;
+
+// Use dynamic_cf_cast<> instead of checked_cf_cast<> when actively checking
CF types,
+// similar to dynamic_cast<> in C++. Be sure to include a nullptr check.
+
+template<typename T> T dynamic_cf_cast(CFTypeRef object)
+{
+ if (!object)
+ return nullptr;
+
+ if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
+ return nullptr;
+
+ return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
+}
+
+template<typename T, typename U> RetainPtr<T> dynamic_cf_cast(RetainPtr<U>&&
object)
+{
+ if (!object)
+ return nullptr;
+
+ if (CFGetTypeID(object.get()) != CFTypeTrait<T>::typeID())
+ return nullptr;
+
+ return adoptCF(static_cast<T>(const_cast<CF_BRIDGED_TYPE(id)
void*>(object.leakRef())));
}
+// Use checked_cf_cast<> instead of dynamic_cf_cast<> when a specific CF type
is required.
+
+template<typename T> T checked_cf_cast(CFTypeRef object)
+{
+ if (!object)
+ return nullptr;
+
+ if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
+ WTFCrash();
+
+ return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
}
+} // namespace WTF
+
+#define WTF_DECLARE_CF_TYPE_TRAIT(ClassName) \
+template <> \
+struct WTF::CFTypeTrait<ClassName##Ref> { \
+ static inline CFTypeID typeID(void) { return ClassName##GetTypeID(); } \
+};
+
+WTF_DECLARE_CF_TYPE_TRAIT(CFArray);
+WTF_DECLARE_CF_TYPE_TRAIT(CFDictionary);
+
+#define WTF_DECLARE_CF_MUTABLE_TYPE_TRAIT(ClassName, MutableClassName) \
+template <> \
+struct WTF::CFTypeTrait<MutableClassName##Ref> { \
+ static inline CFTypeID typeID(void) { return ClassName##GetTypeID(); } \
+};
+
+WTF_DECLARE_CF_MUTABLE_TYPE_TRAIT(CFArray, CFMutableArray);
+WTF_DECLARE_CF_MUTABLE_TYPE_TRAIT(CFDictionary, CFMutableDictionary);
+
using WTF::RetainPtr;
using WTF::adoptNS;
using WTF::adoptCF;
using WTF::retainPtr;
using WTF::downcast;
using WTF::bridge_cast;
+using WTF::dynamic_cf_cast;
+using WTF::checked_cf_cast;
\ No newline at end of file
diff --git
a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
index dd7208a534ea1..c2a93358b4cdd 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
@@ -3,6 +3,8 @@
#include "objc-mock-types.h"
+CFTypeRef CFCopyArray(CFArrayRef);
+
void basic_correct() {
auto ns1 = adoptNS([SomeObj alloc]);
auto ns2 = adoptNS([[SomeObj alloc] init]);
@@ -12,6 +14,7 @@ void basic_correct() {
auto ns6 = retainPtr([ns3 next]);
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault,
10));
auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
+ auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
}
CFMutableArrayRef provide_cf();
@@ -27,6 +30,8 @@ void basic_wrong() {
// expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and
results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
// expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument
is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ CFCopyArray(cf1);
+ // expected-warning@-1{{The return value is +1 and results in a memory leak
[alpha.webkit.RetainPtrCtorAdoptChecker]}}
}
RetainPtr<CVPixelBufferRef> cf_out_argument() {
@@ -68,7 +73,7 @@ void adopt_retainptr() {
class MemberInit {
public:
- MemberInit(CFMutableArrayRef array, NSString *str, CFRunLoopRef runLoop)
+ MemberInit(RetainPtr<CFMutableArrayRef>&& array, NSString *str, CFRunLoopRef
runLoop)
: m_array(array)
, m_str(str)
, m_runLoop(runLoop)
@@ -80,7 +85,7 @@ void adopt_retainptr() {
RetainPtr<CFRunLoopRef> m_runLoop;
};
void create_member_init() {
- MemberInit init { CFArrayCreateMutable(kCFAllocatorDefault, 10), @"hello",
CFRunLoopGetCurrent() };
+ MemberInit init { adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)),
@"hello", CFRunLoopGetCurrent() };
}
RetainPtr<CFStringRef> cfstr() {
diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
index 79e0bdb7c577b..c6d2c1fddb9e8 100644
--- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
@@ -3,6 +3,8 @@
#include "objc-mock-types.h"
+CFTypeRef CFCopyArray(CFArrayRef);
+
void basic_correct() {
auto ns1 = adoptNS([SomeObj alloc]);
auto ns2 = adoptNS([[SomeObj alloc] init]);
@@ -12,6 +14,7 @@ void basic_correct() {
auto ns6 = retainPtr([ns3 next]);
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault,
10));
auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
+ auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
}
CFMutableArrayRef provide_cf();
@@ -27,6 +30,8 @@ void basic_wrong() {
// expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and
results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
RetainPtr<CFTypeRef> cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault);
// expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument
is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ CFCopyArray(cf1);
+ // expected-warning@-1{{The return value is +1 and results in a memory leak
[alpha.webkit.RetainPtrCtorAdoptChecker]}}
}
RetainPtr<CVPixelBufferRef> cf_out_argument() {
@@ -68,7 +73,7 @@ void adopt_retainptr() {
class MemberInit {
public:
- MemberInit(CFMutableArrayRef array, NSString *str, CFRunLoopRef runLoop)
+ MemberInit(RetainPtr<CFMutableArrayRef>&& array, NSString *str, CFRunLoopRef
runLoop)
: m_array(array)
, m_str(str)
, m_runLoop(runLoop)
@@ -80,7 +85,7 @@ void adopt_retainptr() {
RetainPtr<CFRunLoopRef> m_runLoop;
};
void create_member_init() {
- MemberInit init { CFArrayCreateMutable(kCFAllocatorDefault, 10), @"hello",
CFRunLoopGetCurrent() };
+ MemberInit init { adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)),
@"hello", CFRunLoopGetCurrent() };
}
RetainPtr<CFStringRef> cfstr() {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
