Skip to content

Commit bfc5e0a

Browse files
Merge pull request #11640 from rniwa/merge-webkit-fixes-2025-10-16
Merge Webkit Fixes as of 2025-10-16
2 parents 1e2085c + da925f3 commit bfc5e0a

29 files changed

+1137
-203
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: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,17 @@ bool tryToFindPtrOrigin(
2626
const Expr *E, bool StopAtFirstRefCountedObj,
2727
std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
2828
std::function<bool(const clang::QualType)> isSafePtrType,
29+
std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
2930
std::function<bool(const clang::Expr *, bool)> callback) {
3031
while (E) {
3132
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
3233
if (auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
3334
auto QT = VD->getType();
34-
if (VD->hasGlobalStorage() && QT.isConstQualified()) {
35+
auto IsImmortal = safeGetName(VD) == "NSApp";
36+
if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified()))
37+
return callback(E, true);
38+
if (VD->hasGlobalStorage() && isSafeGlobalDecl(VD))
3539
return callback(E, true);
36-
}
3740
}
3841
}
3942
if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
@@ -71,9 +74,11 @@ bool tryToFindPtrOrigin(
7174
}
7275
if (auto *Expr = dyn_cast<ConditionalOperator>(E)) {
7376
return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj,
74-
isSafePtr, isSafePtrType, callback) &&
77+
isSafePtr, isSafePtrType, isSafeGlobalDecl,
78+
callback) &&
7579
tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj,
76-
isSafePtr, isSafePtrType, callback);
80+
isSafePtr, isSafePtrType, isSafeGlobalDecl,
81+
callback);
7782
}
7883
if (auto *cast = dyn_cast<CastExpr>(E)) {
7984
if (StopAtFirstRefCountedObj) {
@@ -93,7 +98,8 @@ bool tryToFindPtrOrigin(
9398
if (auto *call = dyn_cast<CallExpr>(E)) {
9499
if (auto *Callee = call->getCalleeDecl()) {
95100
if (Callee->hasAttr<CFReturnsRetainedAttr>() ||
96-
Callee->hasAttr<NSReturnsRetainedAttr>()) {
101+
Callee->hasAttr<NSReturnsRetainedAttr>() ||
102+
Callee->hasAttr<NSReturnsAutoreleasedAttr>()) {
97103
return callback(E, true);
98104
}
99105
}
@@ -115,7 +121,7 @@ bool tryToFindPtrOrigin(
115121
if (auto *Callee = operatorCall->getDirectCallee()) {
116122
auto ClsName = safeGetName(Callee->getParent());
117123
if (isRefType(ClsName) || isCheckedPtr(ClsName) ||
118-
isRetainPtr(ClsName) || ClsName == "unique_ptr" ||
124+
isRetainPtrOrOSPtr(ClsName) || ClsName == "unique_ptr" ||
119125
ClsName == "UniqueRef" || ClsName == "WeakPtr" ||
120126
ClsName == "WeakRef") {
121127
if (operatorCall->getNumArgs() == 1) {
@@ -158,7 +164,9 @@ bool tryToFindPtrOrigin(
158164

159165
auto Name = safeGetName(callee);
160166
if (Name == "__builtin___CFStringMakeConstantString" ||
161-
Name == "NSClassFromString")
167+
Name == "NSStringFromSelector" || Name == "NSSelectorFromString" ||
168+
Name == "NSStringFromClass" || Name == "NSClassFromString" ||
169+
Name == "NSStringFromProtocol" || Name == "NSProtocolFromString")
162170
return callback(E, true);
163171
} else if (auto *CalleeE = call->getCallee()) {
164172
if (auto *E = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
@@ -196,6 +204,8 @@ bool tryToFindPtrOrigin(
196204
!Selector.getNumArgs())
197205
return callback(E, true);
198206
}
207+
if (auto *ObjCProtocol = dyn_cast<ObjCProtocolExpr>(E))
208+
return callback(ObjCProtocol, true);
199209
if (auto *ObjCDict = dyn_cast<ObjCDictionaryLiteral>(E))
200210
return callback(ObjCDict, true);
201211
if (auto *ObjCArray = dyn_cast<ObjCArrayLiteral>(E))
@@ -208,6 +218,8 @@ bool tryToFindPtrOrigin(
208218
continue;
209219
}
210220
if (auto *BoxedExpr = dyn_cast<ObjCBoxedExpr>(E)) {
221+
if (StopAtFirstRefCountedObj)
222+
return callback(BoxedExpr, true);
211223
E = BoxedExpr->getSubExpr();
212224
continue;
213225
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ bool tryToFindPtrOrigin(
5656
const clang::Expr *E, bool StopAtFirstRefCountedObj,
5757
std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
5858
std::function<bool(const clang::QualType)> isSafePtrType,
59+
std::function<bool(const clang::Decl *)> isSafeGlobalDecl,
5960
std::function<bool(const clang::Expr *, bool)> callback);
6061

6162
/// For \p E referring to a ref-countable/-counted pointer/reference we return

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

Lines changed: 33 additions & 8 deletions
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()) {
@@ -263,18 +263,43 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
263263
void visitCallArg(const Expr *Arg, const ParmVarDecl *Param,
264264
const Decl *DeclWithIssue) const {
265265
auto *ArgExpr = Arg->IgnoreParenCasts();
266-
if (auto *InnerCE = dyn_cast<CallExpr>(Arg)) {
267-
auto *InnerCallee = InnerCE->getDirectCallee();
268-
if (InnerCallee && InnerCallee->isInStdNamespace() &&
269-
safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
270-
ArgExpr = InnerCE->getArg(0);
271-
if (ArgExpr)
272-
ArgExpr = ArgExpr->IgnoreParenCasts();
266+
while (ArgExpr) {
267+
ArgExpr = ArgExpr->IgnoreParenCasts();
268+
if (auto *InnerCE = dyn_cast<CallExpr>(ArgExpr)) {
269+
auto *InnerCallee = InnerCE->getDirectCallee();
270+
if (InnerCallee && InnerCallee->isInStdNamespace() &&
271+
safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) {
272+
ArgExpr = InnerCE->getArg(0);
273+
continue;
274+
}
275+
}
276+
if (auto *UO = dyn_cast<UnaryOperator>(ArgExpr)) {
277+
auto OpCode = UO->getOpcode();
278+
if (OpCode == UO_Deref || OpCode == UO_AddrOf) {
279+
ArgExpr = UO->getSubExpr();
280+
continue;
281+
}
273282
}
283+
break;
274284
}
285+
286+
if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(ArgExpr)) {
287+
if (isOwnerPtrType(MemberCallExpr->getObjectType()))
288+
return;
289+
}
290+
291+
if (auto *OpCE = dyn_cast<CXXOperatorCallExpr>(ArgExpr)) {
292+
auto *Method = dyn_cast_or_null<CXXMethodDecl>(OpCE->getDirectCallee());
293+
if (Method && isOwnerPtr(safeGetName(Method->getParent()))) {
294+
if (OpCE->getOperator() == OO_Star && OpCE->getNumArgs() == 1)
295+
return;
296+
}
297+
}
298+
275299
if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) ||
276300
isa<CXXDefaultArgExpr>(ArgExpr))
277301
return;
302+
278303
if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) {
279304
if (auto *ValDecl = DRE->getDecl()) {
280305
if (isa<ParmVarDecl>(ValDecl))

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,22 @@ 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

141+
bool isOwnerPtr(const std::string &Name) {
142+
return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
143+
Name == "UniqueRef" || Name == "LazyUniqueRef";
144+
}
145+
140146
bool isSmartPtrClass(const std::string &Name) {
141-
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) ||
147+
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
142148
Name == "WeakPtr" || Name == "WeakPtrFactory" ||
143149
Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
144150
Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
@@ -166,15 +172,17 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
166172
return isCheckedPtr(safeGetName(F));
167173
}
168174

169-
bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
175+
bool isCtorOfRetainPtrOrOSPtr(const clang::FunctionDecl *F) {
170176
const std::string &FunctionName = safeGetName(F);
171177
return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
172178
FunctionName == "adoptCF" || FunctionName == "retainPtr" ||
173-
FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc";
179+
FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc" ||
180+
FunctionName == "adoptOSObject" || FunctionName == "adoptOSObjectArc";
174181
}
175182

176183
bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
177-
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || isCtorOfRetainPtr(F);
184+
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) ||
185+
isCtorOfRetainPtrOrOSPtr(F);
178186
}
179187

180188
template <typename Predicate>
@@ -202,15 +210,12 @@ bool isRefOrCheckedPtrType(const clang::QualType T) {
202210
T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); });
203211
}
204212

205-
bool isRetainPtrType(const clang::QualType T) {
206-
return isPtrOfType(T, [](auto Name) { return isRetainPtr(Name); });
213+
bool isRetainPtrOrOSPtrType(const clang::QualType T) {
214+
return isPtrOfType(T, [](auto Name) { return isRetainPtrOrOSPtr(Name); });
207215
}
208216

209217
bool isOwnerPtrType(const clang::QualType T) {
210-
return isPtrOfType(T, [](auto Name) {
211-
return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
212-
Name == "UniqueRef" || Name == "LazyUniqueRef";
213-
});
218+
return isPtrOfType(T, [](auto Name) { return isOwnerPtr(Name); });
214219
}
215220

216221
std::optional<bool> isUncounted(const QualType T) {
@@ -286,7 +291,7 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
286291
std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) {
287292
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
288293
if (auto *Decl = Subst->getAssociatedDecl()) {
289-
if (isRetainPtr(safeGetName(Decl)))
294+
if (isRetainPtrOrOSPtr(safeGetName(Decl)))
290295
return false;
291296
}
292297
}
@@ -386,7 +391,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
386391
method == "impl"))
387392
return true;
388393

389-
if (isRetainPtr(className) && method == "get")
394+
if (isRetainPtrOrOSPtr(className) && method == "get")
390395
return true;
391396

392397
// Ref<T> -> T conversion
@@ -407,7 +412,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
407412
}
408413
}
409414

410-
if (isRetainPtr(className)) {
415+
if (isRetainPtrOrOSPtr(className)) {
411416
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
412417
auto QT = maybeRefToRawOperator->getConversionType();
413418
auto *T = QT.getTypePtrOrNull();
@@ -438,10 +443,10 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
438443
return false;
439444
}
440445

441-
bool isRetainPtr(const CXXRecordDecl *R) {
446+
bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) {
442447
assert(R);
443448
if (auto *TmplR = R->getTemplateInstantiationPattern())
444-
return isRetainPtr(safeGetName(TmplR));
449+
return isRetainPtrOrOSPtr(safeGetName(TmplR));
445450
return false;
446451
}
447452

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

Lines changed: 7 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,11 @@ 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);
145+
146+
/// \returns true if \p Name is an owning smar pointer such as Ref, CheckedPtr,
147+
/// and unique_ptr.
148+
bool isOwnerPtr(const std::string &Name);
145149

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

0 commit comments

Comments
 (0)