Skip to content

Commit 6fbd9c4

Browse files
rniwarniwa
committed
[WebKit checkers] Add the support for OSObjectPtr (llvm#159484)
Add the support for OSObjectPtr, which behaves like RetainPtr. (cherry picked from commit 9d933c7)
1 parent 3023c39 commit 6fbd9c4

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
@@ -3598,26 +3598,28 @@ See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebK
35983598
35993599
alpha.webkit.NoUnretainedMemberChecker
36003600
""""""""""""""""""""""""""""""""""""""""
3601-
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.
3601+
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.
36023602
36033603
.. code-block:: cpp
36043604
36053605
struct Foo {
36063606
NSObject *ptr; // warn
3607+
dispatch_queue_t queue; // warn
36073608
// ...
36083609
};
36093610
36103611
See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ for details.
36113612
36123613
alpha.webkit.UnretainedLambdaCapturesChecker
36133614
""""""""""""""""""""""""""""""""""""""""""""
3614-
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.
3615+
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.
36153616
36163617
.. code-block:: cpp
36173618
3618-
void foo(NSObject *a, NSObject *b) {
3619+
void foo(NSObject *a, NSObject *b, dispatch_queue_t c) {
36193620
[&, a](){ // warn about 'a'
36203621
do_something(b); // warn about 'b'
3622+
dispatch_queue_get_specific(c, "some"); // warn about 'c'
36213623
};
36223624
};
36233625
@@ -3720,7 +3722,7 @@ alpha.webkit.UnretainedCallArgsChecker
37203722
""""""""""""""""""""""""""""""""""""""
37213723
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.
37223724
3723-
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3725+
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
37243726
37253727
alpha.webkit.UncountedLocalVarsChecker
37263728
""""""""""""""""""""""""""""""""""""""
@@ -3810,9 +3812,9 @@ Here are some examples of situations that we warn about as they *might* be poten
38103812
38113813
alpha.webkit.UnretainedLocalVarsChecker
38123814
"""""""""""""""""""""""""""""""""""""""
3813-
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.
3815+
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.
38143816
3815-
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3817+
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
38163818
38173819
These are examples of cases that we consider safe:
38183820
@@ -3855,8 +3857,8 @@ Here are some examples of situations that we warn about as they *might* be poten
38553857
38563858
webkit.RetainPtrCtorAdoptChecker
38573859
""""""""""""""""""""""""""""""""
3858-
The goal of this rule is to make sure the constructor of RetainPtr as well as adoptNS and adoptCF are used correctly.
3859-
When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used.
3860+
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.
3861+
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.
38603862
Warn otherwise.
38613863
38623864
These are examples of cases that we consider correct:
@@ -3865,13 +3867,15 @@ These are examples of cases that we consider correct:
38653867
38663868
RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
38673869
RetainPtr ptr = CGImageGetColorSpace(image); // ok
3870+
OSObjectPtr ptr = adoptOSObject(dispatch_queue_create("some queue", nullptr)); // ok
38683871
38693872
Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF
38703873
38713874
.. code-block:: cpp
38723875
38733876
RetainPtr ptr = [[NSObject alloc] init]; // warn
38743877
auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn
3878+
OSObjectPtr ptr = dispatch_queue_create("some queue", nullptr); // warn
38753879
38763880
Debug Checkers
38773881
---------------

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

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

17901790
def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
1791-
HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
1791+
HelpText<"Check for correct use of RetainPtr/OSObjectPtr constructor, adoptNS, adoptCF, and adoptOSObject">,
17921792
Documentation<HasDocumentation>;
17931793

17941794
} // 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>
@@ -202,8 +205,8 @@ bool isRefOrCheckedPtrType(const clang::QualType T) {
202205
T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); });
203206
}
204207

205-
bool isRetainPtrType(const clang::QualType T) {
206-
return isPtrOfType(T, [](auto Name) { return isRetainPtr(Name); });
208+
bool isRetainPtrOrOSPtrType(const clang::QualType T) {
209+
return isPtrOfType(T, [](auto Name) { return isRetainPtrOrOSPtr(Name); });
207210
}
208211

209212
bool isOwnerPtrType(const clang::QualType T) {
@@ -286,7 +289,7 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
286289
std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) {
287290
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
288291
if (auto *Decl = Subst->getAssociatedDecl()) {
289-
if (isRetainPtr(safeGetName(Decl)))
292+
if (isRetainPtrOrOSPtr(safeGetName(Decl)))
290293
return false;
291294
}
292295
}
@@ -386,7 +389,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
386389
method == "impl"))
387390
return true;
388391

389-
if (isRetainPtr(className) && method == "get")
392+
if (isRetainPtrOrOSPtr(className) && method == "get")
390393
return true;
391394

392395
// Ref<T> -> T conversion
@@ -407,7 +410,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
407410
}
408411
}
409412

410-
if (isRetainPtr(className)) {
413+
if (isRetainPtrOrOSPtr(className)) {
411414
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
412415
auto QT = maybeRefToRawOperator->getConversionType();
413416
auto *T = QT.getTypePtrOrNull();
@@ -438,10 +441,10 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
438441
return false;
439442
}
440443

441-
bool isRetainPtr(const CXXRecordDecl *R) {
444+
bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) {
442445
assert(R);
443446
if (auto *TmplR = R->getTemplateInstantiationPattern())
444-
return isRetainPtr(safeGetName(TmplR));
447+
return isRetainPtrOrOSPtr(safeGetName(TmplR));
445448
return false;
446449
}
447450

‎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)