Skip to content

Commit 9d933c7

Browse files
authored
[WebKit checkers] Add the support for OSObjectPtr (#159484)
Add the support for OSObjectPtr, which behaves like RetainPtr.
1 parent 446a490 commit 9d933c7

24 files changed

+630
-128
lines changed

clang/docs/analyzer/checkers.rst

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3633,26 +3633,28 @@ See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebK
36333633
36343634
alpha.webkit.NoUnretainedMemberChecker
36353635
""""""""""""""""""""""""""""""""""""""""
3636-
Raw pointers and references to a NS or CF object can't be used as class members or ivars. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled. Only RetainPtr is allowed for NS types when ARC is disabled.
3636+
Raw pointers and references to a NS or CF object can't be used as class members or ivars. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled. Only RetainPtr or OSObjectPtr is allowed for NS types when ARC is disabled.
36373637
36383638
.. code-block:: cpp
36393639
36403640
struct Foo {
36413641
NSObject *ptr; // warn
3642+
dispatch_queue_t queue; // warn
36423643
// ...
36433644
};
36443645
36453646
See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ for details.
36463647
36473648
alpha.webkit.UnretainedLambdaCapturesChecker
36483649
""""""""""""""""""""""""""""""""""""""""""""
3649-
Raw pointers and references to NS or CF types can't be captured in lambdas. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled, and only RetainPtr is allowed for NS types when ARC is disabled.
3650+
Raw pointers and references to NS or CF types can't be captured in lambdas. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled, and only RetainPtr or OSObjectPtr is allowed for NS types when ARC is disabled.
36503651
36513652
.. code-block:: cpp
36523653
3653-
void foo(NSObject *a, NSObject *b) {
3654+
void foo(NSObject *a, NSObject *b, dispatch_queue_t c) {
36543655
[&, a](){ // warn about 'a'
36553656
do_something(b); // warn about 'b'
3657+
dispatch_queue_get_specific(c, "some"); // warn about 'c'
36563658
};
36573659
};
36583660
@@ -3755,7 +3757,7 @@ alpha.webkit.UnretainedCallArgsChecker
37553757
""""""""""""""""""""""""""""""""""""""
37563758
The goal of this rule is to make sure that lifetime of any dynamically allocated NS or CF objects passed as a call argument keeps its memory region past the end of the call. This applies to call to any function, method, lambda, function pointer or functor. NS or CF objects aren't supposed to be allocated on stack so we check arguments for parameters of raw pointers and references to unretained types.
37573759
3758-
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3760+
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
37593761
37603762
alpha.webkit.UncountedLocalVarsChecker
37613763
""""""""""""""""""""""""""""""""""""""
@@ -3845,9 +3847,9 @@ Here are some examples of situations that we warn about as they *might* be poten
38453847
38463848
alpha.webkit.UnretainedLocalVarsChecker
38473849
"""""""""""""""""""""""""""""""""""""""
3848-
The goal of this rule is to make sure that any NS or CF local variable is backed by a RetainPtr with lifetime that is strictly larger than the scope of the unretained local variable. To be on the safe side we require the scope of an unretained variable to be embedded in the scope of Retainptr object that backs it.
3850+
The goal of this rule is to make sure that any NS or CF local variable is backed by a RetainPtr or OSObjectPtr with lifetime that is strictly larger than the scope of the unretained local variable. To be on the safe side we require the scope of an unretained variable to be embedded in the scope of RetainPtr or OSObjectPtr object that backs it.
38493851
3850-
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3852+
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
38513853
38523854
These are examples of cases that we consider safe:
38533855
@@ -3890,8 +3892,8 @@ Here are some examples of situations that we warn about as they *might* be poten
38903892
38913893
webkit.RetainPtrCtorAdoptChecker
38923894
""""""""""""""""""""""""""""""""
3893-
The goal of this rule is to make sure the constructor of RetainPtr as well as adoptNS and adoptCF are used correctly.
3894-
When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used.
3895+
The goal of this rule is to make sure the constructors of RetainPtr and OSObjectPtr as well as adoptNS, adoptCF, and adoptOSObject are used correctly.
3896+
When creating a RetainPtr or OSObjectPtr with +1 semantics, adoptNS, adoptCF, or adoptOSObject should be used, and in +0 semantics, RetainPtr or OSObjectPtr constructor should be used.
38953897
Warn otherwise.
38963898
38973899
These are examples of cases that we consider correct:
@@ -3900,13 +3902,15 @@ These are examples of cases that we consider correct:
39003902
39013903
RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
39023904
RetainPtr ptr = CGImageGetColorSpace(image); // ok
3905+
OSObjectPtr ptr = adoptOSObject(dispatch_queue_create("some queue", nullptr)); // ok
39033906
39043907
Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF
39053908
39063909
.. code-block:: cpp
39073910
39083911
RetainPtr ptr = [[NSObject alloc] init]; // warn
39093912
auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn
3913+
OSObjectPtr ptr = dispatch_queue_create("some queue", nullptr); // warn
39103914
39113915
Debug Checkers
39123916
---------------

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1726,7 +1726,7 @@ def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">,
17261726
Documentation<HasDocumentation>;
17271727

17281728
def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
1729-
HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
1729+
HelpText<"Check for correct use of RetainPtr/OSObjectPtr constructor, adoptNS, adoptCF, and adoptOSObject">,
17301730
Documentation<HasDocumentation>;
17311731

17321732
} // end alpha.webkit

clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ bool tryToFindPtrOrigin(
115115
if (auto *Callee = operatorCall->getDirectCallee()) {
116116
auto ClsName = safeGetName(Callee->getParent());
117117
if (isRefType(ClsName) || isCheckedPtr(ClsName) ||
118-
isRetainPtr(ClsName) || ClsName == "unique_ptr" ||
118+
isRetainPtrOrOSPtr(ClsName) || ClsName == "unique_ptr" ||
119119
ClsName == "UniqueRef" || ClsName == "WeakPtr" ||
120120
ClsName == "WeakRef") {
121121
if (operatorCall->getNumArgs() == 1) {

clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
162162
// Ref-counted smartpointers actually have raw-pointer to uncounted type as
163163
// a member but we trust them to handle it correctly.
164164
auto R = llvm::dyn_cast_or_null<CXXRecordDecl>(RD);
165-
if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtr(R))
165+
if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtrOrOSPtr(R))
166166
return;
167167

168168
for (auto *Member : RD->fields()) {

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,17 @@ bool isRefType(const std::string &Name) {
129129
Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
130130
}
131131

132-
bool isRetainPtr(const std::string &Name) {
133-
return Name == "RetainPtr" || Name == "RetainPtrArc";
132+
bool isRetainPtrOrOSPtr(const std::string &Name) {
133+
return Name == "RetainPtr" || Name == "RetainPtrArc" ||
134+
Name == "OSObjectPtr" || Name == "OSObjectPtrArc";
134135
}
135136

136137
bool isCheckedPtr(const std::string &Name) {
137138
return Name == "CheckedPtr" || Name == "CheckedRef";
138139
}
139140

140141
bool isSmartPtrClass(const std::string &Name) {
141-
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) ||
142+
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
142143
Name == "WeakPtr" || Name == "WeakPtrFactory" ||
143144
Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
144145
Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
@@ -166,15 +167,17 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
166167
return isCheckedPtr(safeGetName(F));
167168
}
168169

169-
bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
170+
bool isCtorOfRetainPtrOrOSPtr(const clang::FunctionDecl *F) {
170171
const std::string &FunctionName = safeGetName(F);
171172
return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
172173
FunctionName == "adoptCF" || FunctionName == "retainPtr" ||
173-
FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc";
174+
FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc" ||
175+
FunctionName == "adoptOSObject" || FunctionName == "adoptOSObjectArc";
174176
}
175177

176178
bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
177-
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || isCtorOfRetainPtr(F);
179+
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) ||
180+
isCtorOfRetainPtrOrOSPtr(F);
178181
}
179182

180183
template <typename Predicate>
@@ -198,8 +201,8 @@ bool isRefOrCheckedPtrType(const clang::QualType T) {
198201
T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); });
199202
}
200203

201-
bool isRetainPtrType(const clang::QualType T) {
202-
return isPtrOfType(T, [](auto Name) { return isRetainPtr(Name); });
204+
bool isRetainPtrOrOSPtrType(const clang::QualType T) {
205+
return isPtrOfType(T, [](auto Name) { return isRetainPtrOrOSPtr(Name); });
203206
}
204207

205208
bool isOwnerPtrType(const clang::QualType T) {
@@ -273,7 +276,7 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
273276
std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) {
274277
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
275278
if (auto *Decl = Subst->getAssociatedDecl()) {
276-
if (isRetainPtr(safeGetName(Decl)))
279+
if (isRetainPtrOrOSPtr(safeGetName(Decl)))
277280
return false;
278281
}
279282
}
@@ -373,7 +376,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
373376
method == "impl"))
374377
return true;
375378

376-
if (isRetainPtr(className) && method == "get")
379+
if (isRetainPtrOrOSPtr(className) && method == "get")
377380
return true;
378381

379382
// Ref<T> -> T conversion
@@ -394,7 +397,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
394397
}
395398
}
396399

397-
if (isRetainPtr(className)) {
400+
if (isRetainPtrOrOSPtr(className)) {
398401
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
399402
auto QT = maybeRefToRawOperator->getConversionType();
400403
auto *T = QT.getTypePtrOrNull();
@@ -425,10 +428,10 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
425428
return false;
426429
}
427430

428-
bool isRetainPtr(const CXXRecordDecl *R) {
431+
bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) {
429432
assert(R);
430433
if (auto *TmplR = R->getTemplateInstantiationPattern())
431-
return isRetainPtr(safeGetName(TmplR));
434+
return isRetainPtrOrOSPtr(safeGetName(TmplR));
432435
return false;
433436
}
434437

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ bool isRefCounted(const clang::CXXRecordDecl *Class);
5757
bool isCheckedPtr(const clang::CXXRecordDecl *Class);
5858

5959
/// \returns true if \p Class is a RetainPtr, false if not.
60-
bool isRetainPtr(const clang::CXXRecordDecl *Class);
60+
bool isRetainPtrOrOSPtr(const clang::CXXRecordDecl *Class);
6161

6262
/// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...),
6363
/// false if not.
@@ -116,7 +116,7 @@ std::optional<bool> isUnsafePtr(const QualType T, bool IsArcEnabled);
116116
bool isRefOrCheckedPtrType(const clang::QualType T);
117117

118118
/// \returns true if \p T is a RetainPtr, false if not.
119-
bool isRetainPtrType(const clang::QualType T);
119+
bool isRetainPtrOrOSPtrType(const clang::QualType T);
120120

121121
/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or
122122
/// unique_ptr, false if not.
@@ -141,7 +141,7 @@ bool isRefType(const std::string &Name);
141141
bool isCheckedPtr(const std::string &Name);
142142

143143
/// \returns true if \p Name is RetainPtr or its variant, false if not.
144-
bool isRetainPtr(const std::string &Name);
144+
bool isRetainPtrOrOSPtr(const std::string &Name);
145145

146146
/// \returns true if \p Name is a smart pointer type name, false if not.
147147
bool isSmartPtrClass(const std::string &Name);

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,11 +465,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker {
465465
}
466466

467467
bool isSafePtr(const CXXRecordDecl *Record) const final {
468-
return isRetainPtr(Record);
468+
return isRetainPtrOrOSPtr(Record);
469469
}
470470

471471
bool isSafePtrType(const QualType type) const final {
472-
return isRetainPtrType(type);
472+
return isRetainPtrOrOSPtrType(type);
473473
}
474474

475475
bool isSafeExpr(const Expr *E) const final {

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
500500
}
501501

502502
virtual bool isPtrType(const std::string &Name) const final {
503-
return isRetainPtr(Name);
503+
return isRetainPtrOrOSPtr(Name);
504504
}
505505

506506
const char *ptrKind(QualType QT) const final { return "unretained"; }

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,10 +434,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
434434
return RTC->isUnretained(T);
435435
}
436436
bool isSafePtr(const CXXRecordDecl *Record) const final {
437-
return isRetainPtr(Record);
437+
return isRetainPtrOrOSPtr(Record);
438438
}
439439
bool isSafePtrType(const QualType type) const final {
440-
return isRetainPtrType(type);
440+
return isRetainPtrOrOSPtrType(type);
441441
}
442442
bool isSafeExpr(const Expr *E) const final {
443443
return ento::cocoa::isCocoaObjectRef(E->getType()) &&

clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,11 @@ class RawPtrRefMemberChecker
119119
auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
120120
if (!Desugared)
121121
return nullptr;
122-
auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared);
123-
if (!ObjCType)
124-
return nullptr;
125-
return ObjCType->getDecl();
122+
if (auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared))
123+
return ObjCType->getDecl();
124+
if (auto *ObjCType = dyn_cast<ObjCObjectType>(Desugared))
125+
return ObjCType->getInterface();
126+
return nullptr;
126127
}
127128

128129
void visitObjCDecl(const ObjCContainerDecl *CD) const {
@@ -369,7 +370,7 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
369370
const char *typeName() const final { return "retainable type"; }
370371

371372
const char *invariant() const final {
372-
return "member variables must be a RetainPtr";
373+
return "member variables must be a RetainPtr or OSObjectPtr";
373374
}
374375

375376
PrintDeclKind printPointer(llvm::raw_svector_ostream &Os,

0 commit comments

Comments
 (0)