[clang] [alpha.webkit.webkit.RetainPtrCtorAdoptChecker] Add a new WebKit checker for correct use of RetainPtr, adoptNS, and adoptCF (PR #128679)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 9 22:24:48 PDT 2025
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/128679
>From b4001f95cf6d35f59ef8af6df8f2bdbe043da380 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 25 Feb 2025 00:47:45 -0800
Subject: [PATCH 1/6] [alpha.webkit.webkit.RetainPtrCtorAdoptChecker] Add a new
WebKit checker for correct use of RetainPtr, adoptNS, and adoptCF
Add a new WebKit checker to validate the correct use of RetainPtr constructor as well as adoptNS and adoptCF functions.
adoptNS and adoptCf are used for +1 semantics and RetainPtr constructor is used for +0 semantics.
---
clang/docs/analyzer/checkers.rst | 20 +
.../clang/StaticAnalyzer/Checkers/Checkers.td | 4 +
.../StaticAnalyzer/Checkers/CMakeLists.txt | 1 +
.../Checkers/WebKit/PtrTypesSemantics.cpp | 7 +-
.../Checkers/WebKit/PtrTypesSemantics.h | 3 +-
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 347 ++++++++++++++++++
.../Checkers/WebKit/objc-mock-types.h | 146 +++++++-
.../WebKit/retain-ptr-ctor-adopt-use-arc.mm | 85 +++++
.../WebKit/retain-ptr-ctor-adopt-use.mm | 85 +++++
9 files changed, 680 insertions(+), 18 deletions(-)
create mode 100644 clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
create mode 100644 clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
create mode 100644 clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index c1eedb33e74d2..4cbd31f44d3f6 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3713,6 +3713,26 @@ Here are some examples of situations that we warn about as they *might* be poten
NSObject* unretained = retained.get(); // warn
}
+webkit.RetainPtrCtorAdoptChecker
+""""""""""""""""""""""""""""""""
+The goal of this rule is to make sure the constructor of RetinPtr as well as adoptNS and adoptCF are used correctly.
+When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used.
+Warn otherwise.
+
+These are examples of cases that we consider correct:
+
+ .. code-block:: cpp
+
+ RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
+ RetainPtr ptr = CGImageGetColorSpace(image); // ok
+
+Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF
+
+ .. code-block:: cpp
+
+ RetainPtr ptr = [[NSObject alloc] init]; // warn
+ auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn
+
Debug Checkers
---------------
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 410f841630660..9aa696d8803b1 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1786,4 +1786,8 @@ def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">,
HelpText<"Check unretained local variables.">,
Documentation<HasDocumentation>;
+def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
+ HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
+ Documentation<HasDocumentation>;
+
} // end alpha.webkit
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 5910043440987..0b6b169d7b447 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -133,6 +133,7 @@ add_clang_library(clangStaticAnalyzerCheckers
WebKit/MemoryUnsafeCastChecker.cpp
WebKit/PtrTypesSemantics.cpp
WebKit/RefCntblBaseVirtualDtorChecker.cpp
+ WebKit/RetainPtrCtorAdoptChecker.cpp
WebKit/RawPtrRefCallArgsChecker.cpp
WebKit/UncountedLambdaCapturesChecker.cpp
WebKit/RawPtrRefLocalVarsChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 7899b19854806..7e7bd49ca0bdb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -225,15 +225,16 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
return;
for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
- if (Redecl->getAttr<ObjCBridgeAttr>()) {
+ if (Redecl->getAttr<ObjCBridgeAttr>() ||
+ Redecl->getAttr<ObjCBridgeMutableAttr>()) {
CFPointees.insert(RT);
return;
}
}
}
-bool RetainTypeChecker::isUnretained(const QualType QT) {
- if (ento::cocoa::isCocoaObjectRef(QT) && !IsARCEnabled)
+bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
+ if (ento::cocoa::isCocoaObjectRef(QT) && (!IsARCEnabled || ignoreARC))
return true;
auto CanonicalType = QT.getCanonicalType();
auto PointeeType = CanonicalType->getPointeeType();
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index fcc1a41dba78b..f69d374cbbf90 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -75,7 +75,8 @@ class RetainTypeChecker {
public:
void visitTranslationUnitDecl(const TranslationUnitDecl *);
void visitTypedef(const TypedefDecl *);
- bool isUnretained(const QualType);
+ bool isUnretained(const QualType, bool ignoreARC = false);
+ bool isARCEnabled() const { return IsARCEnabled; }
};
/// \returns true if \p Class is NS or CF objects AND not retained, false if
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
new file mode 100644
index 0000000000000..8727f89826807
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -0,0 +1,347 @@
+//=======- RefCntblBaseVirtualDtor.cpp ---------------------------*- C++ -*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ASTUtils.h"
+#include "DiagOutputUtils.h"
+#include "PtrTypesSemantics.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/RetainSummaryManager.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SetVector.h"
+#include <optional>
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class RetainPtrCtorAdoptChecker
+ : public Checker<check::ASTDecl<TranslationUnitDecl>> {
+private:
+ BugType Bug;
+ mutable BugReporter *BR;
+ mutable std::unique_ptr<RetainSummaryManager> Summaries;
+ mutable RetainTypeChecker RTC;
+
+public:
+ RetainPtrCtorAdoptChecker()
+ : Bug(this,
+ "Correct use of RetainPtr, adoptNS, and adoptCF",
+ "WebKit coding guidelines") {}
+
+ void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
+ BugReporter &BRArg) const {
+ BR = &BRArg;
+
+ // The calls to checkAST* from AnalysisConsumer don't
+ // visit template instantiations or lambda classes. We
+ // want to visit those, so we make our own RecursiveASTVisitor.
+ struct LocalVisitor : public RecursiveASTVisitor<LocalVisitor> {
+ const RetainPtrCtorAdoptChecker *Checker;
+ Decl *DeclWithIssue{nullptr};
+
+ using Base = RecursiveASTVisitor<LocalVisitor>;
+
+ explicit LocalVisitor(const RetainPtrCtorAdoptChecker *Checker)
+ : Checker(Checker) {
+ assert(Checker);
+ }
+
+ bool shouldVisitTemplateInstantiations() const { return true; }
+ bool shouldVisitImplicitCode() const { return false; }
+
+ bool TraverseDecl(Decl *D) {
+ llvm::SaveAndRestore SavedDecl(DeclWithIssue);
+ if (D && (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)))
+ DeclWithIssue = D;
+ return Base::TraverseDecl(D);
+ }
+
+ bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) {
+ if (safeGetName(CTD) == "RetainPtr")
+ return true; // Skip the contents of RetainPtr.
+ return Base::TraverseClassTemplateDecl(CTD);
+ }
+
+ bool VisitTypedefDecl(TypedefDecl *TD) {
+ Checker->RTC.visitTypedef(TD);
+ return true;
+ }
+
+ bool VisitCallExpr(const CallExpr *CE) {
+ Checker->visitCallExpr(CE, DeclWithIssue);
+ return true;
+ }
+
+ bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
+ Checker->visitConstructExpr(CE, DeclWithIssue);
+ return true;
+ }
+
+ llvm::SetVector<const CXXRecordDecl *> Decls;
+ llvm::DenseSet<const CXXRecordDecl *> CRTPs;
+ };
+
+ LocalVisitor visitor(this);
+ Summaries = std::make_unique<RetainSummaryManager>(TUD->getASTContext(),
+ true /* trackObjCAndCFObjects */, false /* trackOSObjects */);
+ RTC.visitTranslationUnitDecl(TUD);
+ visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
+ }
+
+ bool isAdoptFn(const Decl *FnDecl) const {
+ auto Name = safeGetName(FnDecl);
+ return Name == "adoptNS" || Name == "adoptCF" || Name == "adoptNSArc" ||
+ Name == "adoptCFArc";
+ }
+
+ bool isAdoptNS(const Decl *FnDecl) const {
+ auto Name = safeGetName(FnDecl);
+ return Name == "adoptNS" || Name == "adoptNSArc";
+ }
+
+ void visitCallExpr(const CallExpr *CE, const Decl* DeclWithIssue) const {
+ if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+ return;
+
+ auto *F = CE->getDirectCallee();
+ if (!F)
+ return;
+
+ if (!isAdoptFn(F) || !CE->getNumArgs())
+ return;
+
+ auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+ auto Result = isOwned(Arg);
+ auto Name = safeGetName(F);
+ if (Result == IsOwnedResult::Unknown)
+ reportUnknown(Name, CE, DeclWithIssue);
+ else if (Result == IsOwnedResult::NotOwned) {
+ if (RTC.isARCEnabled() && isAdoptNS(F)) {
+ if (!isAllocInit(Arg))
+ reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
+ } else
+ reportUseAfterFree(Name, CE, DeclWithIssue);
+ }
+ }
+
+ void visitConstructExpr(const CXXConstructExpr *CE,
+ const Decl* DeclWithIssue) const {
+ if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
+ return;
+
+ auto *Ctor = CE->getConstructor();
+ if (!Ctor)
+ return;
+
+ auto *Cls = Ctor->getParent();
+ if (!Cls)
+ return;
+
+ if (safeGetName(Cls) != "RetainPtr" || !CE->getNumArgs())
+ return;
+
+ // Ignore RetainPtr construction inside adoptNS, adoptCF, and retainPtr.
+ if (isAdoptFn(DeclWithIssue) || safeGetName(DeclWithIssue) == "retainPtr")
+ return;
+
+ std::string Name = "RetainPtr constructor";
+ auto *Arg = CE->getArg(0)->IgnoreParenCasts();
+ auto Result = isOwned(Arg);
+ if (Result == IsOwnedResult::Unknown)
+ reportUnknown(Name, CE, DeclWithIssue);
+ else if (Result == IsOwnedResult::Owned)
+ reportLeak(Name, CE, DeclWithIssue);
+ else if (RTC.isARCEnabled() && isAllocInit(Arg))
+ reportLeak(Name, CE, DeclWithIssue, "when ARC is disabled");
+ }
+
+ bool isAllocInit(const Expr *E) const {
+ auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E);
+ if (!ObjCMsgExpr)
+ return false;
+ auto Selector = ObjCMsgExpr->getSelector();
+ auto NameForFirstSlot = Selector.getNameForSlot(0);
+ if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") ||
+ NameForFirstSlot.starts_with("mutableCopy"))
+ return true;
+ if (!NameForFirstSlot.starts_with("init"))
+ return false;
+ if (!ObjCMsgExpr->isInstanceMessage())
+ return false;
+ auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
+ if (!Receiver)
+ return false;
+ auto *InnerObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver);
+ if (!InnerObjCMsgExpr)
+ return false;
+ auto InnerSelector = InnerObjCMsgExpr->getSelector();
+ return InnerSelector.getNameForSlot(0) == "alloc";
+ }
+
+ enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned };
+ IsOwnedResult isOwned(const Expr *E) const {
+ while (1) {
+ if (isa<CXXNullPtrLiteralExpr>(E))
+ return IsOwnedResult::NotOwned;
+ if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+ auto QT = DRE->getType();
+ if (isRetainPtrType(QT))
+ return IsOwnedResult::NotOwned;
+ QT = QT.getCanonicalType();
+ if (RTC.isUnretained(QT, true /* ignoreARC */))
+ return IsOwnedResult::NotOwned;
+ auto *PointeeType = QT->getPointeeType().getTypePtrOrNull();
+ if (PointeeType && PointeeType->isVoidType())
+ return IsOwnedResult::NotOwned; // Assume reading void* as +0.
+ }
+ if (auto *TE = dyn_cast<CXXBindTemporaryExpr>(E)) {
+ E = TE->getSubExpr();
+ continue;
+ }
+ if (auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E)) {
+ auto Summary = Summaries->getSummary(AnyCall(ObjCMsgExpr));
+ auto RetEffect = Summary->getRetEffect();
+ switch (RetEffect.getKind()) {
+ case RetEffect::NoRet:
+ return IsOwnedResult::Unknown;
+ case RetEffect::OwnedSymbol:
+ return IsOwnedResult::Owned;
+ case RetEffect::NotOwnedSymbol:
+ return IsOwnedResult::NotOwned;
+ case RetEffect::OwnedWhenTrackedReceiver:
+ if (auto *Receiver = ObjCMsgExpr->getInstanceReceiver()) {
+ E = Receiver->IgnoreParenCasts();
+ continue;
+ }
+ return IsOwnedResult::Unknown;
+ case RetEffect::NoRetHard:
+ return IsOwnedResult::Unknown;
+ }
+ }
+ if (auto *CXXCE = dyn_cast<CXXMemberCallExpr>(E)) {
+ if (auto *MD = CXXCE->getMethodDecl()) {
+ auto *Cls = MD->getParent();
+ if (auto *CD = dyn_cast<CXXConversionDecl>(MD)) {
+ auto QT = CD->getConversionType().getCanonicalType();
+ auto *ResultType = QT.getTypePtrOrNull();
+ if (safeGetName(Cls) == "RetainPtr" && ResultType &&
+ (ResultType->isPointerType() || ResultType->isReferenceType() ||
+ ResultType->isObjCObjectPointerType()))
+ return IsOwnedResult::NotOwned;
+ }
+ if (safeGetName(MD) == "leakRef" && safeGetName(Cls) == "RetainPtr")
+ return IsOwnedResult::Owned;
+ }
+ }
+ if (auto *CE = dyn_cast<CallExpr>(E)) {
+ if (auto *Callee = CE->getDirectCallee()) {
+ if (isAdoptFn(Callee))
+ return IsOwnedResult::NotOwned;
+ if (safeGetName(Callee) == "__builtin___CFStringMakeConstantString")
+ return IsOwnedResult::NotOwned;
+ auto RetType = Callee->getReturnType();
+ if (isRetainPtrType(RetType))
+ return IsOwnedResult::NotOwned;
+ } else if (auto *CalleeExpr = CE->getCallee()) {
+ if (isa<CXXDependentScopeMemberExpr>(CalleeExpr))
+ return IsOwnedResult::Skip; // Wait for instantiation.
+ }
+ auto Summary = Summaries->getSummary(AnyCall(CE));
+ auto RetEffect = Summary->getRetEffect();
+ switch (RetEffect.getKind()) {
+ case RetEffect::NoRet:
+ return IsOwnedResult::Unknown;
+ case RetEffect::OwnedSymbol:
+ return IsOwnedResult::Owned;
+ case RetEffect::NotOwnedSymbol:
+ return IsOwnedResult::NotOwned;
+ case RetEffect::OwnedWhenTrackedReceiver:
+ return IsOwnedResult::Unknown;
+ case RetEffect::NoRetHard:
+ return IsOwnedResult::Unknown;
+ }
+ }
+ break;
+ }
+ return IsOwnedResult::Unknown;
+ }
+
+ template <typename ExprType>
+ void reportUnknown(std::string& Name, const ExprType *CE,
+ const Decl* DeclWithIssue) const {
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << Name << " is called on an object in unknown state.";
+
+ 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));
+ }
+
+ void reportUseAfterFree(std::string& Name, const CallExpr *CE,
+ const Decl* DeclWithIssue,
+ const char* condition = nullptr) const {
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << "Incorrect use of " << Name <<
+ ". The argument is +0 and results in an use-after-free";
+ if (condition)
+ Os << " " << condition;
+ Os << ".";
+
+ 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));
+ }
+
+ void reportLeak(std::string& Name, const CXXConstructExpr *CE,
+ const Decl* DeclWithIssue,
+ const char* condition = nullptr) const {
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << "Incorrect use of " << Name <<
+ ". The argument is +1 and results in a memory leak";
+ if (condition)
+ Os << " " << condition;
+ Os << ".";
+
+ 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
+
+void ento::registerRetainPtrCtorAdoptChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<RetainPtrCtorAdoptChecker>();
+}
+
+bool ento::shouldRegisterRetainPtrCtorAdoptChecker(
+ const CheckerManager &mgr) {
+ return true;
+}
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 7bb33bcb6cf44..9b13810d0c5c9 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -1,30 +1,94 @@
@class NSString;
@class NSArray;
@class NSMutableArray;
+#define nil ((id)0)
#define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T)))
+#define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T)))
typedef CF_BRIDGED_TYPE(id) void * CFTypeRef;
+typedef signed char BOOL;
typedef signed long CFIndex;
-typedef const struct __CFAllocator * CFAllocatorRef;
+typedef unsigned long NSUInteger;
+typedef const struct CF_BRIDGED_TYPE(id) __CFAllocator * CFAllocatorRef;
typedef const struct CF_BRIDGED_TYPE(NSString) __CFString * CFStringRef;
typedef const struct CF_BRIDGED_TYPE(NSArray) __CFArray * CFArrayRef;
-typedef struct CF_BRIDGED_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
extern const CFAllocatorRef kCFAllocatorDefault;
+typedef struct _NSZone NSZone;
CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity);
extern void CFArrayAppendValue(CFMutableArrayRef theArray, const void *value);
CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values, CFIndex numValues);
CFIndex CFArrayGetCount(CFArrayRef theArray);
+CFRunLoopRef CFRunLoopGetCurrent(void);
+CFRunLoopRef CFRunLoopGetMain(void);
extern CFTypeRef CFRetain(CFTypeRef cf);
extern void CFRelease(CFTypeRef cf);
+#define CFSTR(cStr) ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr ""))
+extern Class NSClassFromString(NSString *aClassName);
__attribute__((objc_root_class))
@interface NSObject
+ (instancetype) alloc;
++ (Class) class;
++ (Class) superclass;
- (instancetype) init;
- (instancetype)retain;
- (void)release;
+- (BOOL)isKindOfClass:(Class)aClass;
+ at end
+
+ at protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+ at end
+
+ at protocol NSFastEnumeration
+- (int)countByEnumeratingWithState:(void *)state objects:(id *)objects count:(unsigned)count;
+- (void)protocolMethod;
+ at end
+
+ at interface NSEnumerator <NSFastEnumeration>
+ at end
+
+ at interface NSDictionary : NSObject <NSCopying>
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (id)objectForKeyedSubscript:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
++ (id)dictionary;
++ (id)dictionaryWithObject:(id)object forKey:(id <NSCopying>)key;
++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id <NSCopying> [])keys count:(NSUInteger)cnt;
+ at end
+
+ at interface NSArray : NSObject <NSCopying, NSFastEnumeration>
+- (NSUInteger)count;
+- (NSEnumerator *)objectEnumerator;
++ (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count;
+ at end
+
+ at interface NSString : NSObject <NSCopying>
+- (NSUInteger)length;
+- (NSString *)stringByAppendingString:(NSString *)aString;
+- ( const char *)UTF8String;
+- (id)initWithUTF8String:(const char *)nullTerminatedCString;
++ (id)stringWithUTF8String:(const char *)nullTerminatedCString;
+ at end
+
+ at interface NSMutableString : NSString
+ at end
+
+ at interface NSValue : NSObject <NSCopying>
+- (void)getValue:(void *)value;
+ at end
+
+ at interface NSNumber : NSValue
+- (char)charValue;
+- (id)initWithInt:(int)value;
++ (NSNumber *)numberWithInt:(int)value;
@end
@interface SomeObj : NSObject
+- (SomeObj *)mutableCopy;
+- (SomeObj *)copyWithValue:(int)value;
- (void)doWork;
- (SomeObj *)other;
- (SomeObj *)next;
@@ -57,28 +121,34 @@ template <typename T> struct RetainPtr {
PtrType t;
RetainPtr() : t(nullptr) { }
-
RetainPtr(PtrType t)
: t(t) {
if (t)
- CFRetain(t);
+ CFRetain(toCFTypeRef(t));
}
- RetainPtr(RetainPtr&& o)
- : RetainPtr(o.t)
- {
- o.t = nullptr;
- }
- RetainPtr(const RetainPtr& o)
+ RetainPtr(RetainPtr&&);
+ RetainPtr(const RetainPtr&);
+ template <typename U>
+ RetainPtr(const RetainPtr<U>& o)
: RetainPtr(o.t)
+ {}
+ RetainPtr operator=(const RetainPtr& o)
{
+ if (t)
+ CFRelease(toCFTypeRef(t));
+ t = o.t;
+ if (t)
+ CFRetain(toCFTypeRef(t));
+ return *this;
}
- RetainPtr operator=(const RetainPtr& o)
+ template <typename U>
+ RetainPtr operator=(const RetainPtr<U>& o)
{
if (t)
- CFRelease(t);
+ CFRelease(toCFTypeRef(t));
t = o.t;
if (t)
- CFRetain(t);
+ CFRetain(toCFTypeRef(t));
return *this;
}
~RetainPtr() {
@@ -86,7 +156,7 @@ template <typename T> struct RetainPtr {
}
void clear() {
if (t)
- CFRelease(t);
+ CFRelease(toCFTypeRef(t));
t = nullptr;
}
void swap(RetainPtr& o) {
@@ -102,10 +172,19 @@ template <typename T> struct RetainPtr {
swap(o);
return *this;
}
+ PtrType leakRef()
+ {
+ PtrType s = t;
+ t = nullptr;
+ return s;
+ }
operator PtrType() const { return t; }
operator bool() const { return t; }
private:
+ CFTypeRef toCFTypeRef(id ptr) { return (__bridge CFTypeRef)ptr; }
+ CFTypeRef toCFTypeRef(const void* ptr) { return (CFTypeRef)ptr; }
+
template <typename U> friend RetainPtr<U> adoptNS(U*);
template <typename U> friend RetainPtr<U> adoptCF(U);
@@ -113,9 +192,26 @@ template <typename T> struct RetainPtr {
RetainPtr(PtrType t, AdoptTag) : t(t) { }
};
+template <typename T>
+RetainPtr<T>::RetainPtr(RetainPtr<T>&& o)
+ : RetainPtr(o.t)
+{
+ o.t = nullptr;
+}
+
+template <typename T>
+RetainPtr<T>::RetainPtr(const RetainPtr<T>& o)
+ : RetainPtr(o.t)
+{
+}
+
template <typename T>
RetainPtr<T> adoptNS(T* t) {
+#if __has_feature(objc_arc)
+ return t;
+#else
return RetainPtr<T>(t, RetainPtr<T>::Adopt);
+#endif
}
template <typename T>
@@ -123,9 +219,31 @@ RetainPtr<T> adoptCF(T t) {
return RetainPtr<T>(t, RetainPtr<T>::Adopt);
}
+template<typename T> inline RetainPtr<T> retainPtr(T ptr)
+{
+ return ptr;
+}
+
+template<typename T> inline RetainPtr<T> retainPtr(T* ptr)
+{
+ return ptr;
+}
+
+inline NSObject *bridge_cast(CFTypeRef object)
+{
+ return (__bridge NSObject *)object;
+}
+
+inline CFTypeRef bridge_cast(NSObject *object)
+{
+ return (__bridge CFTypeRef)object;
+}
+
}
using WTF::RetainPtr;
using WTF::adoptNS;
using WTF::adoptCF;
+using WTF::retainPtr;
using WTF::downcast;
+using WTF::bridge_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
new file mode 100644
index 0000000000000..5d0c64e0f44b6
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm
@@ -0,0 +1,85 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.RetainPtrCtorAdoptChecker -fobjc-arc -verify %s
+
+#include "objc-mock-types.h"
+
+void basic_correct() {
+ auto ns1 = adoptNS([SomeObj alloc]);
+ auto ns2 = adoptNS([[SomeObj alloc] init]);
+ RetainPtr<SomeObj> ns3 = [ns1.get() next];
+ auto ns4 = adoptNS([ns3 mutableCopy]);
+ auto ns5 = adoptNS([ns3 copyWithValue:3]);
+ auto ns6 = retainPtr([ns3 next]);
+ CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+}
+
+CFMutableArrayRef provide_cf();
+
+void basic_wrong() {
+ RetainPtr<SomeObj> ns1 = [[SomeObj alloc] init];
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ auto ns2 = adoptNS([ns1.get() next]);
+ // expected-warning at -1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10);
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
+ // expected-warning at -1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+RetainPtr<SomeObj> return_nullptr() {
+ return nullptr;
+}
+
+RetainPtr<SomeObj> return_retainptr() {
+ RetainPtr<SomeObj> foo;
+ return foo;
+}
+
+CFTypeRef CopyValueForSomething();
+
+void cast_retainptr() {
+ RetainPtr<NSObject> foo;
+ RetainPtr<SomeObj> bar = static_cast<SomeObj*>(foo);
+
+ auto baz = adoptCF(CopyValueForSomething()).get();
+ RetainPtr<CFArrayRef> v = static_cast<CFArrayRef>(baz);
+}
+
+void adopt_retainptr() {
+ RetainPtr<NSObject> foo = adoptNS([[SomeObj alloc] init]);
+}
+
+RetainPtr<CFArrayRef> return_arg(CFArrayRef arg) {
+ return arg;
+}
+
+class MemberInit {
+public:
+ MemberInit(CFMutableArrayRef array, NSString *str, CFRunLoopRef runLoop)
+ : m_array(array)
+ , m_str(str)
+ , m_runLoop(runLoop)
+ { }
+
+private:
+ RetainPtr<CFMutableArrayRef> m_array;
+ RetainPtr<NSString> m_str;
+ RetainPtr<CFRunLoopRef> m_runLoop;
+};
+void create_member_init() {
+ MemberInit init { CFArrayCreateMutable(kCFAllocatorDefault, 10), @"hello", CFRunLoopGetCurrent() };
+}
+
+RetainPtr<CFStringRef> cfstr() {
+ return CFSTR("");
+}
+
+template <typename CF, typename NS>
+static RetainPtr<NS> bridge_cast(RetainPtr<CF>&& ptr)
+{
+ return adoptNS((__bridge NSArray *)(ptr.leakRef()));
+}
+
+RetainPtr<CFArrayRef> create_cf_array();
+RetainPtr<id> return_bridge_cast() {
+ return bridge_cast<CFArrayRef, NSArray>(create_cf_array());
+}
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
new file mode 100644
index 0000000000000..20b9e2d485c95
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm
@@ -0,0 +1,85 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.RetainPtrCtorAdoptChecker -verify %s
+
+#include "objc-mock-types.h"
+
+void basic_correct() {
+ auto ns1 = adoptNS([SomeObj alloc]);
+ auto ns2 = adoptNS([[SomeObj alloc] init]);
+ RetainPtr<SomeObj> ns3 = [ns1.get() next];
+ auto ns4 = adoptNS([ns3 mutableCopy]);
+ auto ns5 = adoptNS([ns3 copyWithValue:3]);
+ auto ns6 = retainPtr([ns3 next]);
+ CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+}
+
+CFMutableArrayRef provide_cf();
+
+void basic_wrong() {
+ RetainPtr<SomeObj> ns1 = [[SomeObj alloc] init];
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ auto ns2 = adoptNS([ns1.get() next]);
+ // expected-warning at -1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10);
+ // expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+ RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
+ // expected-warning at -1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}}
+}
+
+RetainPtr<SomeObj> return_nullptr() {
+ return nullptr;
+}
+
+RetainPtr<SomeObj> return_retainptr() {
+ RetainPtr<SomeObj> foo;
+ return foo;
+}
+
+CFTypeRef CopyValueForSomething();
+
+void cast_retainptr() {
+ RetainPtr<NSObject> foo;
+ RetainPtr<SomeObj> bar = static_cast<SomeObj*>(foo);
+
+ auto baz = adoptCF(CopyValueForSomething()).get();
+ RetainPtr<CFArrayRef> v = static_cast<CFArrayRef>(baz);
+}
+
+void adopt_retainptr() {
+ RetainPtr<NSObject> foo = adoptNS([[SomeObj alloc] init]);
+}
+
+RetainPtr<CFArrayRef> return_arg(CFArrayRef arg) {
+ return arg;
+}
+
+class MemberInit {
+public:
+ MemberInit(CFMutableArrayRef array, NSString *str, CFRunLoopRef runLoop)
+ : m_array(array)
+ , m_str(str)
+ , m_runLoop(runLoop)
+ { }
+
+private:
+ RetainPtr<CFMutableArrayRef> m_array;
+ RetainPtr<NSString> m_str;
+ RetainPtr<CFRunLoopRef> m_runLoop;
+};
+void create_member_init() {
+ MemberInit init { CFArrayCreateMutable(kCFAllocatorDefault, 10), @"hello", CFRunLoopGetCurrent() };
+}
+
+RetainPtr<CFStringRef> cfstr() {
+ return CFSTR("");
+}
+
+template <typename CF, typename NS>
+static RetainPtr<NS> bridge_cast(RetainPtr<CF>&& ptr)
+{
+ return adoptNS((__bridge NSArray *)(ptr.leakRef()));
+}
+
+RetainPtr<CFArrayRef> create_cf_array();
+RetainPtr<id> return_bridge_cast() {
+ return bridge_cast<CFArrayRef, NSArray>(create_cf_array());
+}
>From 1d24671b5742059b484a4c8f755a1ee645446499 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 25 Feb 2025 01:11:53 -0800
Subject: [PATCH 2/6] Fix formatting.
---
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 89 +++++++++----------
1 file changed, 44 insertions(+), 45 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index 8727f89826807..fedf8ea30cf8d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -1,4 +1,4 @@
-//=======- RefCntblBaseVirtualDtor.cpp ---------------------------*- C++ -*-==//
+//=======- RetainPtrCtorAdoptChecker.cpp -------------------------*- C++ -*-==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -12,12 +12,12 @@
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/Analysis/RetainSummaryManager.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/Analysis/RetainSummaryManager.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SetVector.h"
#include <optional>
@@ -37,8 +37,7 @@ class RetainPtrCtorAdoptChecker
public:
RetainPtrCtorAdoptChecker()
- : Bug(this,
- "Correct use of RetainPtr, adoptNS, and adoptCF",
+ : Bug(this, "Correct use of RetainPtr, adoptNS, and adoptCF",
"WebKit coding guidelines") {}
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -68,7 +67,7 @@ class RetainPtrCtorAdoptChecker
DeclWithIssue = D;
return Base::TraverseDecl(D);
}
-
+
bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) {
if (safeGetName(CTD) == "RetainPtr")
return true; // Skip the contents of RetainPtr.
@@ -95,8 +94,9 @@ class RetainPtrCtorAdoptChecker
};
LocalVisitor visitor(this);
- Summaries = std::make_unique<RetainSummaryManager>(TUD->getASTContext(),
- true /* trackObjCAndCFObjects */, false /* trackOSObjects */);
+ Summaries = std::make_unique<RetainSummaryManager>(
+ TUD->getASTContext(), true /* trackObjCAndCFObjects */,
+ false /* trackOSObjects */);
RTC.visitTranslationUnitDecl(TUD);
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
@@ -112,7 +112,7 @@ class RetainPtrCtorAdoptChecker
return Name == "adoptNS" || Name == "adoptNSArc";
}
- void visitCallExpr(const CallExpr *CE, const Decl* DeclWithIssue) const {
+ void visitCallExpr(const CallExpr *CE, const Decl *DeclWithIssue) const {
if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
return;
@@ -138,7 +138,7 @@ class RetainPtrCtorAdoptChecker
}
void visitConstructExpr(const CXXConstructExpr *CE,
- const Decl* DeclWithIssue) const {
+ const Decl *DeclWithIssue) const {
if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc()))
return;
@@ -215,20 +215,20 @@ class RetainPtrCtorAdoptChecker
auto Summary = Summaries->getSummary(AnyCall(ObjCMsgExpr));
auto RetEffect = Summary->getRetEffect();
switch (RetEffect.getKind()) {
- case RetEffect::NoRet:
- return IsOwnedResult::Unknown;
- case RetEffect::OwnedSymbol:
- return IsOwnedResult::Owned;
- case RetEffect::NotOwnedSymbol:
- return IsOwnedResult::NotOwned;
- case RetEffect::OwnedWhenTrackedReceiver:
- if (auto *Receiver = ObjCMsgExpr->getInstanceReceiver()) {
- E = Receiver->IgnoreParenCasts();
- continue;
- }
- return IsOwnedResult::Unknown;
- case RetEffect::NoRetHard:
- return IsOwnedResult::Unknown;
+ case RetEffect::NoRet:
+ return IsOwnedResult::Unknown;
+ case RetEffect::OwnedSymbol:
+ return IsOwnedResult::Owned;
+ case RetEffect::NotOwnedSymbol:
+ return IsOwnedResult::NotOwned;
+ case RetEffect::OwnedWhenTrackedReceiver:
+ if (auto *Receiver = ObjCMsgExpr->getInstanceReceiver()) {
+ E = Receiver->IgnoreParenCasts();
+ continue;
+ }
+ return IsOwnedResult::Unknown;
+ case RetEffect::NoRetHard:
+ return IsOwnedResult::Unknown;
}
}
if (auto *CXXCE = dyn_cast<CXXMemberCallExpr>(E)) {
@@ -262,16 +262,16 @@ class RetainPtrCtorAdoptChecker
auto Summary = Summaries->getSummary(AnyCall(CE));
auto RetEffect = Summary->getRetEffect();
switch (RetEffect.getKind()) {
- case RetEffect::NoRet:
- return IsOwnedResult::Unknown;
- case RetEffect::OwnedSymbol:
- return IsOwnedResult::Owned;
- case RetEffect::NotOwnedSymbol:
- return IsOwnedResult::NotOwned;
- case RetEffect::OwnedWhenTrackedReceiver:
- return IsOwnedResult::Unknown;
- case RetEffect::NoRetHard:
- return IsOwnedResult::Unknown;
+ case RetEffect::NoRet:
+ return IsOwnedResult::Unknown;
+ case RetEffect::OwnedSymbol:
+ return IsOwnedResult::Owned;
+ case RetEffect::NotOwnedSymbol:
+ return IsOwnedResult::NotOwned;
+ case RetEffect::OwnedWhenTrackedReceiver:
+ return IsOwnedResult::Unknown;
+ case RetEffect::NoRetHard:
+ return IsOwnedResult::Unknown;
}
}
break;
@@ -280,8 +280,8 @@ class RetainPtrCtorAdoptChecker
}
template <typename ExprType>
- void reportUnknown(std::string& Name, const ExprType *CE,
- const Decl* DeclWithIssue) const {
+ void reportUnknown(std::string &Name, const ExprType *CE,
+ const Decl *DeclWithIssue) const {
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
@@ -295,9 +295,9 @@ class RetainPtrCtorAdoptChecker
BR->emitReport(std::move(Report));
}
- void reportUseAfterFree(std::string& Name, const CallExpr *CE,
- const Decl* DeclWithIssue,
- const char* condition = nullptr) const {
+ void reportUseAfterFree(std::string &Name, const CallExpr *CE,
+ const Decl *DeclWithIssue,
+ const char *condition = nullptr) const {
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
@@ -315,14 +315,14 @@ class RetainPtrCtorAdoptChecker
BR->emitReport(std::move(Report));
}
- void reportLeak(std::string& Name, const CXXConstructExpr *CE,
- const Decl* DeclWithIssue,
- const char* condition = nullptr) const {
+ void reportLeak(std::string &Name, const CXXConstructExpr *CE,
+ const Decl *DeclWithIssue,
+ const char *condition = nullptr) const {
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
- Os << "Incorrect use of " << Name <<
- ". The argument is +1 and results in a memory leak";
+ Os << "Incorrect use of " << Name
+ << ". The argument is +1 and results in a memory leak";
if (condition)
Os << " " << condition;
Os << ".";
@@ -341,7 +341,6 @@ void ento::registerRetainPtrCtorAdoptChecker(CheckerManager &Mgr) {
Mgr.registerChecker<RetainPtrCtorAdoptChecker>();
}
-bool ento::shouldRegisterRetainPtrCtorAdoptChecker(
- const CheckerManager &mgr) {
+bool ento::shouldRegisterRetainPtrCtorAdoptChecker(const CheckerManager &mgr) {
return true;
}
>From 8f8b0b3115a48cf063563fd467d98dc88a60d367 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 25 Feb 2025 09:59:01 -0800
Subject: [PATCH 3/6] Fix formatting 2.
---
.../Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index fedf8ea30cf8d..a97e497ad315d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -301,8 +301,8 @@ class RetainPtrCtorAdoptChecker
SmallString<100> Buf;
llvm::raw_svector_ostream Os(Buf);
- Os << "Incorrect use of " << Name <<
- ". The argument is +0 and results in an use-after-free";
+ Os << "Incorrect use of " << Name
+ << ". The argument is +0 and results in an use-after-free";
if (condition)
Os << " " << condition;
Os << ".";
>From 869e0cef1a7cc55fa83bd3041c9f4bae9f7e7616 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 7 Mar 2025 22:59:19 -0800
Subject: [PATCH 4/6] Add the support for CF out arguments.
Also treat unknown state as +0.
---
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 79 +++++++++++++++----
.../Checkers/WebKit/objc-mock-types.h | 32 ++++++++
.../WebKit/retain-ptr-ctor-adopt-use-arc.mm | 13 +++
.../WebKit/retain-ptr-ctor-adopt-use.mm | 13 +++
4 files changed, 121 insertions(+), 16 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index a97e497ad315d..21c8eb6e88155 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -33,6 +33,7 @@ class RetainPtrCtorAdoptChecker
BugType Bug;
mutable BugReporter *BR;
mutable std::unique_ptr<RetainSummaryManager> Summaries;
+ mutable llvm::DenseSet<const ValueDecl *> CreateOrCopyOutArguments;
mutable RetainTypeChecker RTC;
public:
@@ -88,9 +89,6 @@ class RetainPtrCtorAdoptChecker
Checker->visitConstructExpr(CE, DeclWithIssue);
return true;
}
-
- llvm::SetVector<const CXXRecordDecl *> Decls;
- llvm::DenseSet<const CXXRecordDecl *> CRTPs;
};
LocalVisitor visitor(this);
@@ -120,22 +118,53 @@ class RetainPtrCtorAdoptChecker
if (!F)
return;
- if (!isAdoptFn(F) || !CE->getNumArgs())
+ if (!isAdoptFn(F) || !CE->getNumArgs()) {
+ rememberOutArguments(CE, F);
return;
+ }
auto *Arg = CE->getArg(0)->IgnoreParenCasts();
auto Result = isOwned(Arg);
auto Name = safeGetName(F);
if (Result == IsOwnedResult::Unknown)
- reportUnknown(Name, CE, DeclWithIssue);
- else if (Result == IsOwnedResult::NotOwned) {
- if (RTC.isARCEnabled() && isAdoptNS(F)) {
- if (!isAllocInit(Arg))
- reportUseAfterFree(Name, CE, DeclWithIssue, "when ARC is disabled");
- } else
+ 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);
}
}
+
+ void rememberOutArguments(const CallExpr *CE, const FunctionDecl *Callee) const {
+ auto CalleeName = Callee->getName();
+ if (!CalleeName.contains("Create") && !CalleeName.contains("Copy"))
+ return;
+
+ unsigned ArgCount = CE->getNumArgs();
+ for (unsigned ArgIndex = 0; ArgIndex < ArgCount; ++ArgIndex) {
+ auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
+ auto *Unary = dyn_cast<UnaryOperator>(Arg);
+ if (!Unary)
+ continue;
+ if (Unary->getOpcode() != UO_AddrOf)
+ continue;
+ auto *SubExpr = Unary->getSubExpr();
+ if (!SubExpr)
+ continue;
+ auto *DRE = dyn_cast<DeclRefExpr>(SubExpr->IgnoreParenCasts());
+ if (!DRE)
+ continue;
+ auto *Decl = DRE->getDecl();
+ if (!Decl)
+ continue;
+ CreateOrCopyOutArguments.insert(Decl);
+ }
+ }
void visitConstructExpr(const CXXConstructExpr *CE,
const Decl *DeclWithIssue) const {
@@ -161,11 +190,13 @@ class RetainPtrCtorAdoptChecker
auto *Arg = CE->getArg(0)->IgnoreParenCasts();
auto Result = isOwned(Arg);
if (Result == IsOwnedResult::Unknown)
- reportUnknown(Name, CE, DeclWithIssue);
- else if (Result == IsOwnedResult::Owned)
+ Result = IsOwnedResult::NotOwned;
+ if (Result == IsOwnedResult::Owned)
reportLeak(Name, CE, DeclWithIssue);
else if (RTC.isARCEnabled() && isAllocInit(Arg))
reportLeak(Name, CE, DeclWithIssue, "when ARC is disabled");
+ else if (isCreateOrCopy(Arg))
+ reportLeak(Name, CE, DeclWithIssue);
}
bool isAllocInit(const Expr *E) const {
@@ -184,11 +215,27 @@ class RetainPtrCtorAdoptChecker
auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts();
if (!Receiver)
return false;
- auto *InnerObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver);
- if (!InnerObjCMsgExpr)
+ if (auto *InnerObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Receiver)) {
+ auto InnerSelector = InnerObjCMsgExpr->getSelector();
+ return InnerSelector.getNameForSlot(0) == "alloc";
+ } else if (auto *CE = dyn_cast<CallExpr>(Receiver)) {
+ if (auto *Callee = CE->getDirectCallee()) {
+ auto CalleeName = Callee->getName();
+ return CalleeName.starts_with("alloc");
+ }
+ }
+ return false;
+ }
+
+ bool isCreateOrCopy(const Expr *E) const {
+ auto *CE = dyn_cast<CallExpr>(E);
+ if (!CE)
+ return false;
+ auto *Callee = CE->getDirectCallee();
+ if (!Callee)
return false;
- auto InnerSelector = InnerObjCMsgExpr->getSelector();
- return InnerSelector.getNameForSlot(0) == "alloc";
+ auto CalleeName = Callee->getName();
+ return CalleeName.contains("Create") || CalleeName.contains("Copy");
}
enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned };
diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
index 9b13810d0c5c9..c3925218c6c99 100644
--- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h
@@ -6,6 +6,7 @@
#define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T)))
typedef CF_BRIDGED_TYPE(id) void * CFTypeRef;
typedef signed char BOOL;
+typedef unsigned char Boolean;
typedef signed long CFIndex;
typedef unsigned long NSUInteger;
typedef const struct CF_BRIDGED_TYPE(id) __CFAllocator * CFAllocatorRef;
@@ -13,12 +14,43 @@ typedef const struct CF_BRIDGED_TYPE(NSString) __CFString * CFStringRef;
typedef const struct CF_BRIDGED_TYPE(NSArray) __CFArray * CFArrayRef;
typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef;
typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef;
+#define NS_RETURNS_RETAINED __attribute__((ns_returns_retained))
+#define CF_CONSUMED __attribute__((cf_consumed))
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
extern const CFAllocatorRef kCFAllocatorDefault;
typedef struct _NSZone NSZone;
CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity);
extern void CFArrayAppendValue(CFMutableArrayRef theArray, const void *value);
CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values, CFIndex numValues);
CFIndex CFArrayGetCount(CFArrayRef theArray);
+
+typedef const struct CF_BRIDGED_TYPE(NSDictionary) __CFDictionary * CFDictionaryRef;
+typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableDictionary) __CFDictionary * CFMutableDictionaryRef;
+
+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);
+CFIndex CFDictionaryGetCount(CFDictionaryRef theDict);
+Boolean CFDictionaryContainsKey(CFDictionaryRef theDict, const void *key);
+Boolean CFDictionaryContainsValue(CFDictionaryRef theDict, const void *value);
+const void *CFDictionaryGetValue(CFDictionaryRef theDict, const void *key);
+void CFDictionaryAddValue(CFMutableDictionaryRef theDict, const void *key, const void *value);
+void CFDictionarySetValue(CFMutableDictionaryRef theDict, const void *key, const void *value);
+void CFDictionaryReplaceValue(CFMutableDictionaryRef theDict, const void *key, const void *value);
+void CFDictionaryRemoveValue(CFMutableDictionaryRef theDict, const void *key);
+
+typedef struct CF_BRIDGED_TYPE(id) __SecTask * SecTaskRef;
+SecTaskRef SecTaskCreateFromSelf(CFAllocatorRef allocator);
+
+typedef struct CF_BRIDGED_TYPE(id) CF_BRIDGED_MUTABLE_TYPE(IOSurface) __IOSurface * IOSurfaceRef;
+IOSurfaceRef IOSurfaceCreate(CFDictionaryRef properties);
+
+typedef struct CF_BRIDGED_TYPE(id) __CVBuffer *CVBufferRef;
+typedef CVBufferRef CVImageBufferRef;
+typedef CVImageBufferRef CVPixelBufferRef;
+typedef signed int CVReturn;
+CVReturn CVPixelBufferCreateWithIOSurface(CFAllocatorRef allocator, IOSurfaceRef surface, CFDictionaryRef pixelBufferAttributes, CVPixelBufferRef * pixelBufferOut);
+
CFRunLoopRef CFRunLoopGetCurrent(void);
CFRunLoopRef CFRunLoopGetMain(void);
extern CFTypeRef CFRetain(CFTypeRef cf);
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 5d0c64e0f44b6..99f488b578910 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
@@ -10,6 +10,7 @@ void basic_correct() {
auto ns5 = adoptNS([ns3 copyWithValue:3]);
auto ns6 = retainPtr([ns3 next]);
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+ auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
}
CFMutableArrayRef provide_cf();
@@ -23,6 +24,15 @@ void basic_wrong() {
// expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
// 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]}}
+}
+
+RetainPtr<CVPixelBufferRef> cf_out_argument() {
+ auto surface = adoptCF(IOSurfaceCreate(nullptr));
+ CVPixelBufferRef rawBuffer = nullptr;
+ auto status = CVPixelBufferCreateWithIOSurface(kCFAllocatorDefault, surface.get(), nullptr, &rawBuffer);
+ return adoptCF(rawBuffer);
}
RetainPtr<SomeObj> return_nullptr() {
@@ -44,8 +54,11 @@ void cast_retainptr() {
RetainPtr<CFArrayRef> v = static_cast<CFArrayRef>(baz);
}
+SomeObj* allocSomeObj();
+
void adopt_retainptr() {
RetainPtr<NSObject> foo = adoptNS([[SomeObj alloc] init]);
+ auto bar = adoptNS([allocSomeObj() init]);
}
RetainPtr<CFArrayRef> return_arg(CFArrayRef arg) {
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 20b9e2d485c95..111342bc0e388 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
@@ -10,6 +10,7 @@ void basic_correct() {
auto ns5 = adoptNS([ns3 copyWithValue:3]);
auto ns6 = retainPtr([ns3 next]);
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
+ auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
}
CFMutableArrayRef provide_cf();
@@ -23,6 +24,15 @@ void basic_wrong() {
// expected-warning at -1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}}
RetainPtr<CFMutableArrayRef> cf2 = adoptCF(provide_cf());
// 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]}}
+}
+
+RetainPtr<CVPixelBufferRef> cf_out_argument() {
+ auto surface = adoptCF(IOSurfaceCreate(nullptr));
+ CVPixelBufferRef rawBuffer = nullptr;
+ auto status = CVPixelBufferCreateWithIOSurface(kCFAllocatorDefault, surface.get(), nullptr, &rawBuffer);
+ return adoptCF(rawBuffer);
}
RetainPtr<SomeObj> return_nullptr() {
@@ -44,8 +54,11 @@ void cast_retainptr() {
RetainPtr<CFArrayRef> v = static_cast<CFArrayRef>(baz);
}
+SomeObj* allocSomeObj();
+
void adopt_retainptr() {
RetainPtr<NSObject> foo = adoptNS([[SomeObj alloc] init]);
+ auto bar = adoptNS([allocSomeObj() init]);
}
RetainPtr<CFArrayRef> return_arg(CFArrayRef arg) {
>From 08d8cb1c6a1f5ad68491be49219ea056654d7735 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 7 Mar 2025 23:08:47 -0800
Subject: [PATCH 5/6] Fix formatting.
---
.../Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index 21c8eb6e88155..b942ff35b4fd1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -128,7 +128,8 @@ class RetainPtrCtorAdoptChecker
auto Name = safeGetName(F);
if (Result == IsOwnedResult::Unknown)
Result = IsOwnedResult::NotOwned;
- if (Result == IsOwnedResult::NotOwned && !isAllocInit(Arg) && !isCreateOrCopy(Arg)) {
+ if (Result == IsOwnedResult::NotOwned && !isAllocInit(Arg) &&
+ !isCreateOrCopy(Arg)) {
if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
if (CreateOrCopyOutArguments.contains(DRE->getDecl()))
return;
@@ -139,8 +140,9 @@ class RetainPtrCtorAdoptChecker
reportUseAfterFree(Name, CE, DeclWithIssue);
}
}
-
- void rememberOutArguments(const CallExpr *CE, const FunctionDecl *Callee) const {
+
+ void rememberOutArguments(const CallExpr *CE,
+ const FunctionDecl *Callee) const {
auto CalleeName = Callee->getName();
if (!CalleeName.contains("Create") && !CalleeName.contains("Copy"))
return;
>From 0e64fffbb82aa1e4c2b07cedd0c6fbe18d9b2926 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Fri, 7 Mar 2025 23:12:53 -0800
Subject: [PATCH 6/6] Remove unused reportUnknown
---
.../WebKit/RetainPtrCtorAdoptChecker.cpp | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
index b942ff35b4fd1..e55cd920c23ab 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp
@@ -328,22 +328,6 @@ class RetainPtrCtorAdoptChecker
return IsOwnedResult::Unknown;
}
- template <typename ExprType>
- void reportUnknown(std::string &Name, const ExprType *CE,
- const Decl *DeclWithIssue) const {
- SmallString<100> Buf;
- llvm::raw_svector_ostream Os(Buf);
-
- Os << Name << " is called on an object in unknown state.";
-
- 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));
- }
-
void reportUseAfterFree(std::string &Name, const CallExpr *CE,
const Decl *DeclWithIssue,
const char *condition = nullptr) const {
More information about the cfe-commits
mailing list