Skip to content

Commit 3449472

Browse files
rniwarniwa
committed
[WebKit checkers] Add the support for OSObjectPtr (llvm#159484)
Add the support for OSObjectPtr, which behaves like RetainPtr.
1 parent 4a420fb commit 3449472

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
@@ -3499,26 +3499,28 @@ See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebK
34993499
35003500
alpha.webkit.NoUnretainedMemberChecker
35013501
""""""""""""""""""""""""""""""""""""""""
3502-
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.
3502+
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.
35033503
35043504
.. code-block:: cpp
35053505
35063506
struct Foo {
35073507
NSObject *ptr; // warn
3508+
dispatch_queue_t queue; // warn
35083509
// ...
35093510
};
35103511
35113512
See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ for details.
35123513
35133514
alpha.webkit.UnretainedLambdaCapturesChecker
35143515
""""""""""""""""""""""""""""""""""""""""""""
3515-
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.
3516+
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.
35163517
35173518
.. code-block:: cpp
35183519
3519-
void foo(NSObject *a, NSObject *b) {
3520+
void foo(NSObject *a, NSObject *b, dispatch_queue_t c) {
35203521
[&, a](){ // warn about 'a'
35213522
do_something(b); // warn about 'b'
3523+
dispatch_queue_get_specific(c, "some"); // warn about 'c'
35223524
};
35233525
};
35243526
@@ -3621,7 +3623,7 @@ alpha.webkit.UnretainedCallArgsChecker
36213623
""""""""""""""""""""""""""""""""""""""
36223624
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.
36233625
3624-
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3626+
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
36253627
36263628
alpha.webkit.UncountedLocalVarsChecker
36273629
""""""""""""""""""""""""""""""""""""""
@@ -3711,9 +3713,9 @@ Here are some examples of situations that we warn about as they *might* be poten
37113713
37123714
alpha.webkit.UnretainedLocalVarsChecker
37133715
"""""""""""""""""""""""""""""""""""""""
3714-
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.
3716+
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.
37153717
3716-
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3718+
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
37173719
37183720
These are examples of cases that we consider safe:
37193721
@@ -3756,8 +3758,8 @@ Here are some examples of situations that we warn about as they *might* be poten
37563758
37573759
webkit.RetainPtrCtorAdoptChecker
37583760
""""""""""""""""""""""""""""""""
3759-
The goal of this rule is to make sure the constructor of RetainPtr as well as adoptNS and adoptCF are used correctly.
3760-
When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used.
3761+
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.
3762+
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.
37613763
Warn otherwise.
37623764
37633765
These are examples of cases that we consider correct:
@@ -3766,13 +3768,15 @@ These are examples of cases that we consider correct:
37663768
37673769
RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
37683770
RetainPtr ptr = CGImageGetColorSpace(image); // ok
3771+
OSObjectPtr ptr = adoptOSObject(dispatch_queue_create("some queue", nullptr)); // ok
37693772
37703773
Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF
37713774
37723775
.. code-block:: cpp
37733776
37743777
RetainPtr ptr = [[NSObject alloc] init]; // warn
37753778
auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn
3779+
OSObjectPtr ptr = dispatch_queue_create("some queue", nullptr); // warn
37763780
37773781
Debug Checkers
37783782
---------------

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

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

18341834
def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
1835-
HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
1835+
HelpText<"Check for correct use of RetainPtr/OSObjectPtr constructor, adoptNS, adoptCF, and adoptOSObject">,
18361836
Documentation<HasDocumentation>;
18371837

18381838
} // end alpha.webkit

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ bool tryToFindPtrOrigin(
116116
if (auto *Callee = operatorCall->getDirectCallee()) {
117117
auto ClsName = safeGetName(Callee->getParent());
118118
if (isRefType(ClsName) || isCheckedPtr(ClsName) ||
119-
isRetainPtr(ClsName) || ClsName == "unique_ptr" ||
119+
isRetainPtrOrOSPtr(ClsName) || ClsName == "unique_ptr" ||
120120
ClsName == "UniqueRef" || ClsName == "WeakPtr" ||
121121
ClsName == "WeakRef") {
122122
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
}
@@ -388,7 +391,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
388391
method == "impl"))
389392
return true;
390393

391-
if (isRetainPtr(className) && method == "get")
394+
if (isRetainPtrOrOSPtr(className) && method == "get")
392395
return true;
393396

394397
// Ref<T> -> T conversion
@@ -409,7 +412,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
409412
}
410413
}
411414

412-
if (isRetainPtr(className)) {
415+
if (isRetainPtrOrOSPtr(className)) {
413416
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
414417
auto QT = maybeRefToRawOperator->getConversionType();
415418
auto *T = QT.getTypePtrOrNull();
@@ -440,10 +443,10 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
440443
return false;
441444
}
442445

443-
bool isRetainPtr(const CXXRecordDecl *R) {
446+
bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) {
444447
assert(R);
445448
if (auto *TmplR = R->getTemplateInstantiationPattern())
446-
return isRetainPtr(safeGetName(TmplR));
449+
return isRetainPtrOrOSPtr(safeGetName(TmplR));
447450
return false;
448451
}
449452

‎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
@@ -470,11 +470,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker {
470470
}
471471

472472
bool isSafePtr(const CXXRecordDecl *Record) const final {
473-
return isRetainPtr(Record);
473+
return isRetainPtrOrOSPtr(Record);
474474
}
475475

476476
bool isSafePtrType(const QualType type) const final {
477-
return isRetainPtrType(type);
477+
return isRetainPtrOrOSPtrType(type);
478478
}
479479

480480
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
@@ -504,7 +504,7 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
504504
}
505505

506506
virtual bool isPtrType(const std::string &Name) const final {
507-
return isRetainPtr(Name);
507+
return isRetainPtrOrOSPtr(Name);
508508
}
509509

510510
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
@@ -440,10 +440,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
440440
return RTC->isUnretained(T);
441441
}
442442
bool isSafePtr(const CXXRecordDecl *Record) const final {
443-
return isRetainPtr(Record);
443+
return isRetainPtrOrOSPtr(Record);
444444
}
445445
bool isSafePtrType(const QualType type) const final {
446-
return isRetainPtrType(type);
446+
return isRetainPtrOrOSPtrType(type);
447447
}
448448
bool isSafeExpr(const Expr *E) const final {
449449
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
@@ -122,10 +122,11 @@ class RawPtrRefMemberChecker
122122
auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
123123
if (!Desugared)
124124
return nullptr;
125-
auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared);
126-
if (!ObjCType)
127-
return nullptr;
128-
return ObjCType->getDecl();
125+
if (auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared))
126+
return ObjCType->getDecl();
127+
if (auto *ObjCType = dyn_cast<ObjCObjectType>(Desugared))
128+
return ObjCType->getInterface();
129+
return nullptr;
129130
}
130131

131132
void visitObjCDecl(const ObjCContainerDecl *CD) const {
@@ -372,7 +373,7 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
372373
const char *typeName() const final { return "retainable type"; }
373374

374375
const char *invariant() const final {
375-
return "member variables must be a RetainPtr";
376+
return "member variables must be a RetainPtr or OSObjectPtr";
376377
}
377378

378379
PrintDeclKind printPointer(llvm::raw_svector_ostream &Os,

0 commit comments

Comments
 (0)