Skip to content

[alpha.webkit.RetainPtrCtorAdoptChecker] Support adopt(cast(copy(~)) #132316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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>() ||
Expand All @@ -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);
}

Expand Down
4 changes: 4 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -119,7 +120,7 @@ class RetainPtrCtorAdoptChecker
return;

if (!isAdoptFn(F) || !CE->getNumArgs()) {
rememberOutArguments(CE, F);
checkCreateOrCopyFunction(CE, F, DeclWithIssue);
return;
}

Expand All @@ -128,24 +129,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();
Expand All @@ -164,7 +170,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,
Expand All @@ -190,6 +201,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)
Expand Down Expand Up @@ -315,11 +333,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.
Expand Down Expand Up @@ -383,6 +412,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

Expand Down
39 changes: 21 additions & 18 deletions clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
using WTF::dynamic_cf_cast;
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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();
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
11 changes: 9 additions & 2 deletions clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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();
Expand All @@ -27,6 +32,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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand Down