[clang] c26d097 - [alpha.webkit.RetainPtrCtorAdoptChecker] Support adopt(cast(copy(~)) (#132316)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 10 15:26:13 PDT 2025
Author: Ryosuke Niwa
Date: 2025-04-10T15:26:10-07:00
New Revision: c26d097d0c3e0e3924e273c0ec2d1e57192e66c8
URL: https://github.com/llvm/llvm-project/commit/c26d097d0c3e0e3924e273c0ec2d1e57192e66c8
DIFF: https://github.com/llvm/llvm-project/commit/c26d097d0c3e0e3924e273c0ec2d1e57192e66c8.diff
LOG: [alpha.webkit.RetainPtrCtorAdoptChecker] Support adopt(cast(copy(~)) (#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.
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 430f1c6db50b4..781b0de5abd2f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -125,6 +125,16 @@ bool isCheckedPtr(const std::string &Name) {
return Name == "CheckedPtr" || Name == "CheckedRef";
}
+bool isSmartPtrClass(const std::string &Name) {
+ return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) ||
+ Name == "WeakPtr" || Name == "WeakPtr" || Name == "WeakPtrFactory" ||
+ Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
+ Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
+ Name == "ThreadSafeWeakOrStrongPtr" ||
+ Name == "ThreadSafeWeakPtrControlBlock" ||
+ Name == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr";
+}
+
bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
assert(F);
const std::string &FunctionName = safeGetName(F);
@@ -222,8 +232,13 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
auto PointeeQT = QT->getPointeeType();
const RecordType *RT = PointeeQT->getAs<RecordType>();
- if (!RT)
+ if (!RT) {
+ if (TD->hasAttr<ObjCBridgeAttr>() || TD->hasAttr<ObjCBridgeMutableAttr>()) {
+ if (auto *Type = TD->getTypeForDecl())
+ RecordlessTypes.insert(Type);
+ }
return;
+ }
for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
if (Redecl->getAttr<ObjCBridgeAttr>() ||
@@ -240,6 +255,17 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
auto CanonicalType = QT.getCanonicalType();
auto PointeeType = CanonicalType->getPointeeType();
auto *RT = dyn_cast_or_null<RecordType>(PointeeType.getTypePtrOrNull());
+ if (!RT) {
+ auto *Type = QT.getTypePtrOrNull();
+ while (Type) {
+ if (RecordlessTypes.contains(Type))
+ return true;
+ auto *ET = dyn_cast_or_null<ElaboratedType>(Type);
+ if (!ET)
+ break;
+ Type = ET->desugar().getTypePtrOrNull();
+ }
+ }
return RT && CFPointees.contains(RT);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 096675fb912f2..97c9d0510e67d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -70,6 +70,7 @@ std::optional<bool> isUnchecked(const clang::QualType T);
/// underlying pointer type.
class RetainTypeChecker {
llvm::DenseSet<const RecordType *> CFPointees;
+ llvm::DenseSet<const Type *> RecordlessTypes;
bool IsARCEnabled{false};
public:
@@ -135,6 +136,9 @@ bool isCheckedPtr(const std::string &Name);
/// \returns true if \p Name is RetainPtr or its variant, false if not.
bool isRetainPtr(const std::string &Name);
+/// \returns true if \p Name is a smart pointer type name, false if not.
+bool isSmartPtrClass(const std::string &Name);
+
/// \returns true if \p M is getter of a ref-counted class, false if not.
std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index 13088920cfa19..20f7e9703c67c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -69,7 +69,7 @@ class RawPtrRefCallArgsChecker
}
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) override {
- if (isRefType(safeGetName(Decl)))
+ if (isSmartPtrClass(safeGetName(Decl)))
return true;
return DynamicRecursiveASTVisitor::TraverseClassTemplateDecl(Decl);
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 9975d1a91b681..a23f3aa356cb0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -261,6 +261,12 @@ class RawPtrRefLocalVarsChecker
return DynamicRecursiveASTVisitor::TraverseCompoundStmt(CS);
return true;
}
+
+ bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) override {
+ if (isSmartPtrClass(safeGetName(Decl)))
+ return true;
+ return DynamicRecursiveASTVisitor::TraverseClassTemplateDecl(Decl);
+ }
};
LocalVisitor visitor(this);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index fc3f510d1d499..d372c5d1ba626 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 = nullptr;
mutable std::unique_ptr<RetainSummaryManager> Summaries;
mutable llvm::DenseSet<const ValueDecl *> CreateOrCopyOutArguments;
+ mutable llvm::DenseSet<const Expr *> CreateOrCopyFnCall;
mutable RetainTypeChecker RTC;
public:
@@ -120,7 +121,7 @@ class RetainPtrCtorAdoptChecker
return;
if (!isAdoptFn(F) || !CE->getNumArgs()) {
- rememberOutArguments(CE, F);
+ checkCreateOrCopyFunction(CE, F, DeclWithIssue);
return;
}
@@ -129,24 +130,29 @@ 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();
@@ -165,7 +171,12 @@ class RetainPtrCtorAdoptChecker
if (!Decl)
continue;
CreateOrCopyOutArguments.insert(Decl);
+ hasOutArgument = true;
}
+ if (!RTC.isUnretained(Callee->getReturnType()))
+ return;
+ if (!hasOutArgument && !CreateOrCopyFnCall.contains(CE))
+ reportLeak(CE, DeclWithIssue);
}
void visitConstructExpr(const CXXConstructExpr *CE,
@@ -192,6 +203,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)
@@ -317,11 +335,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.
@@ -387,6 +416,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 9eb63b4745b21..3f075ca0a6e5b 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -24,6 +24,9 @@ typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
extern const CFAllocatorRef kCFAllocatorDefault;
typedef struct _NSZone NSZone;
+
+CFTypeID CFGetTypeID(CFTypeRef cf);
+
CFTypeID CFGetTypeID(CFTypeRef cf);
CFTypeID CFArrayGetTypeID();
CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity);
@@ -281,12 +284,12 @@ 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;
}
inline id bridge_id_cast(CFTypeRef object)
@@ -386,35 +389,35 @@ template <typename> struct CFTypeTrait;
template<typename T> T dynamic_cf_cast(CFTypeRef object)
{
- if (!object)
- return nullptr;
+ if (!object)
+ return nullptr;
- if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
- return nullptr;
+ if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
+ return nullptr;
- return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
+ return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
}
template<typename T> T checked_cf_cast(CFTypeRef object)
{
- if (!object)
- return nullptr;
+ if (!object)
+ return nullptr;
- if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
- WTFCrash();
+ if (CFGetTypeID(object) != CFTypeTrait<T>::typeID())
+ WTFCrash();
- return static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object));
+ 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 (!object)
+ return nullptr;
- if (CFGetTypeID(object.get()) != CFTypeTrait<T>::typeID())
- 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())));
+ return adoptCF(static_cast<T>(const_cast<CF_BRIDGED_TYPE(id) void*>(object.leakRef())));
}
} // namespace WTF
@@ -448,4 +451,4 @@ using WTF::is_objc;
using WTF::checked_objc_cast;
using WTF::dynamic_objc_cast;
using WTF::checked_cf_cast;
-using WTF::dynamic_cf_cast;
\ No newline at end of file
+using WTF::dynamic_cf_cast;
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 63383d4f49642..45db0506ae5f2 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 at -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 at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ CFCopyArray(cf1);
+ // expected-warning at -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 e94b7511dee14..c07f79c9f9650 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,9 @@
#include "objc-mock-types.h"
+CFTypeRef CFCopyArray(CFArrayRef);
+void* CreateCopy();
+
void basic_correct() {
auto ns1 = adoptNS([SomeObj alloc]);
auto ns2 = adoptNS([[SomeObj alloc] init]);
@@ -12,6 +15,8 @@ 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)));
+ CreateCopy();
}
CFMutableArrayRef provide_cf();
@@ -27,6 +32,8 @@ void basic_wrong() {
// expected-warning at -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 at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ CFCopyArray(cf1);
+ // expected-warning at -1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
}
RetainPtr<CVPixelBufferRef> cf_out_argument() {
@@ -68,7 +75,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 +87,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() {
More information about the cfe-commits
mailing list