Skip to content

Commit cec3507

Browse files
authored
[alpha.webkit.UnretainedLocalVarsChecker] Add a checker for local variables to NS and CF types. (#127554)
This PR adds alpha.webkit.UnretainedLocalVarsChecker by generalizing RawPtrRefLocalVarsChecker. It checks local variables to NS or CF types are guarded with a RetainPtr or not. The new checker is effective for NS and CF types in Objective-C++ code without ARC, and it's effective for CF types in code with ARC.
1 parent 9884803 commit cec3507

File tree

12 files changed

+856
-40
lines changed

12 files changed

+856
-40
lines changed

Diff for: clang/docs/analyzer/checkers.rst

+45
Original file line numberDiff line numberDiff line change
@@ -3668,6 +3668,51 @@ Here are some examples of situations that we warn about as they *might* be poten
36683668
RefCountable* uncounted = counted.get(); // warn
36693669
}
36703670
3671+
alpha.webkit.UnretainedLocalVarsChecker
3672+
"""""""""""""""""""""""""""""""""""""""
3673+
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.
3674+
3675+
The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.
3676+
3677+
These are examples of cases that we consider safe:
3678+
3679+
.. code-block:: cpp
3680+
3681+
void foo1() {
3682+
RetainPtr<NSObject> retained;
3683+
// The scope of unretained is EMBEDDED in the scope of retained.
3684+
{
3685+
NSObject* unretained = retained.get(); // ok
3686+
}
3687+
}
3688+
3689+
void foo2(RetainPtr<NSObject> retained_param) {
3690+
NSObject* unretained = retained_param.get(); // ok
3691+
}
3692+
3693+
void FooClass::foo_method() {
3694+
NSObject* unretained = this; // ok
3695+
}
3696+
3697+
Here are some examples of situations that we warn about as they *might* be potentially unsafe. The logic is that either we're able to guarantee that a local variable is safe or it's considered unsafe.
3698+
3699+
.. code-block:: cpp
3700+
3701+
void foo1() {
3702+
NSObject* unretained = [[NSObject alloc] init]; // warn
3703+
}
3704+
3705+
NSObject* global_unretained;
3706+
void foo2() {
3707+
NSObject* unretained = global_unretained; // warn
3708+
}
3709+
3710+
void foo3() {
3711+
RetainPtr<NSObject> retained;
3712+
// The scope of unretained is not EMBEDDED in the scope of retained.
3713+
NSObject* unretained = retained.get(); // warn
3714+
}
3715+
36713716
Debug Checkers
36723717
---------------
36733718

Diff for: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

+4
Original file line numberDiff line numberDiff line change
@@ -1782,4 +1782,8 @@ def UncheckedLocalVarsChecker : Checker<"UncheckedLocalVarsChecker">,
17821782
HelpText<"Check unchecked local variables.">,
17831783
Documentation<HasDocumentation>;
17841784

1785+
def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">,
1786+
HelpText<"Check unretained local variables.">,
1787+
Documentation<HasDocumentation>;
1788+
17851789
} // end alpha.webkit

Diff for: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ bool isSafePtr(clang::CXXRecordDecl *Decl) {
2424

2525
bool tryToFindPtrOrigin(
2626
const Expr *E, bool StopAtFirstRefCountedObj,
27+
std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
28+
std::function<bool(const clang::QualType)> isSafePtrType,
2729
std::function<bool(const clang::Expr *, bool)> callback) {
2830
while (E) {
2931
if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
@@ -53,9 +55,9 @@ bool tryToFindPtrOrigin(
5355
}
5456
if (auto *Expr = dyn_cast<ConditionalOperator>(E)) {
5557
return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj,
56-
callback) &&
58+
isSafePtr, isSafePtrType, callback) &&
5759
tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj,
58-
callback);
60+
isSafePtr, isSafePtrType, callback);
5961
}
6062
if (auto *cast = dyn_cast<CastExpr>(E)) {
6163
if (StopAtFirstRefCountedObj) {
@@ -92,7 +94,7 @@ bool tryToFindPtrOrigin(
9294
}
9395

9496
if (auto *callee = call->getDirectCallee()) {
95-
if (isCtorOfRefCounted(callee) || isCtorOfCheckedPtr(callee)) {
97+
if (isCtorOfSafePtr(callee)) {
9698
if (StopAtFirstRefCountedObj)
9799
return callback(E, true);
98100

Diff for: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h

+2
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ class Expr;
5454
/// Returns false if any of calls to callbacks returned false. Otherwise true.
5555
bool tryToFindPtrOrigin(
5656
const clang::Expr *E, bool StopAtFirstRefCountedObj,
57+
std::function<bool(const clang::CXXRecordDecl *)> isSafePtr,
58+
std::function<bool(const clang::QualType)> isSafePtrType,
5759
std::function<bool(const clang::Expr *, bool)> callback);
5860

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

Diff for: clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

+118-11
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88

99
#include "PtrTypesSemantics.h"
1010
#include "ASTUtils.h"
11+
#include "clang/AST/Attr.h"
1112
#include "clang/AST/CXXInheritance.h"
1213
#include "clang/AST/Decl.h"
1314
#include "clang/AST/DeclCXX.h"
1415
#include "clang/AST/ExprCXX.h"
1516
#include "clang/AST/StmtVisitor.h"
17+
#include "clang/Analysis/DomainSpecific/CocoaConventions.h"
1618
#include <optional>
1719

1820
using namespace clang;
@@ -117,6 +119,8 @@ bool isRefType(const std::string &Name) {
117119
Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
118120
}
119121

122+
bool isRetainPtr(const std::string &Name) { return Name == "RetainPtr"; }
123+
120124
bool isCheckedPtr(const std::string &Name) {
121125
return Name == "CheckedPtr" || Name == "CheckedRef";
122126
}
@@ -140,8 +144,14 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
140144
return isCheckedPtr(safeGetName(F));
141145
}
142146

147+
bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
148+
const std::string &FunctionName = safeGetName(F);
149+
return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
150+
FunctionName == "adoptCF";
151+
}
152+
143153
bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
144-
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F);
154+
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || isCtorOfRetainPtr(F);
145155
}
146156

147157
template <typename Predicate>
@@ -163,11 +173,15 @@ static bool isPtrOfType(const clang::QualType T, Predicate Pred) {
163173
return false;
164174
}
165175

166-
bool isSafePtrType(const clang::QualType T) {
176+
bool isRefOrCheckedPtrType(const clang::QualType T) {
167177
return isPtrOfType(
168178
T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); });
169179
}
170180

181+
bool isRetainPtrType(const clang::QualType T) {
182+
return isPtrOfType(T, [](auto Name) { return Name == "RetainPtr"; });
183+
}
184+
171185
bool isOwnerPtrType(const clang::QualType T) {
172186
return isPtrOfType(T, [](auto Name) {
173187
return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
@@ -195,6 +209,69 @@ std::optional<bool> isUnchecked(const QualType T) {
195209
return isUnchecked(T->getAsCXXRecordDecl());
196210
}
197211

212+
void RetainTypeChecker::visitTranslationUnitDecl(
213+
const TranslationUnitDecl *TUD) {
214+
IsARCEnabled = TUD->getLangOpts().ObjCAutoRefCount;
215+
}
216+
217+
void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
218+
auto QT = TD->getUnderlyingType();
219+
if (!QT->isPointerType())
220+
return;
221+
222+
auto PointeeQT = QT->getPointeeType();
223+
const RecordType *RT = PointeeQT->getAs<RecordType>();
224+
if (!RT)
225+
return;
226+
227+
for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
228+
if (Redecl->getAttr<ObjCBridgeAttr>()) {
229+
CFPointees.insert(RT);
230+
return;
231+
}
232+
}
233+
}
234+
235+
bool RetainTypeChecker::isUnretained(const QualType QT) {
236+
if (ento::cocoa::isCocoaObjectRef(QT) && !IsARCEnabled)
237+
return true;
238+
auto CanonicalType = QT.getCanonicalType();
239+
auto PointeeType = CanonicalType->getPointeeType();
240+
auto *RT = dyn_cast_or_null<RecordType>(PointeeType.getTypePtrOrNull());
241+
return RT && CFPointees.contains(RT);
242+
}
243+
244+
std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) {
245+
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
246+
if (auto *Decl = Subst->getAssociatedDecl()) {
247+
if (isRetainPtr(safeGetName(Decl)))
248+
return false;
249+
}
250+
}
251+
if ((ento::cocoa::isCocoaObjectRef(T) && !IsARCEnabled) ||
252+
ento::coreFoundation::isCFObjectRef(T))
253+
return true;
254+
255+
// RetainPtr strips typedef for CF*Ref. Manually check for struct __CF* types.
256+
auto CanonicalType = T.getCanonicalType();
257+
auto *Type = CanonicalType.getTypePtrOrNull();
258+
if (!Type)
259+
return false;
260+
auto Pointee = Type->getPointeeType();
261+
auto *PointeeType = Pointee.getTypePtrOrNull();
262+
if (!PointeeType)
263+
return false;
264+
auto *Record = PointeeType->getAsStructureType();
265+
if (!Record)
266+
return false;
267+
auto *Decl = Record->getDecl();
268+
if (!Decl)
269+
return false;
270+
auto TypeName = Decl->getName();
271+
return TypeName.starts_with("__CF") || TypeName.starts_with("__CG") ||
272+
TypeName.starts_with("__CM");
273+
}
274+
198275
std::optional<bool> isUncounted(const CXXRecordDecl* Class)
199276
{
200277
// Keep isRefCounted first as it's cheaper.
@@ -230,16 +307,20 @@ std::optional<bool> isUncheckedPtr(const QualType T) {
230307
return false;
231308
}
232309

233-
std::optional<bool> isUnsafePtr(const QualType T) {
310+
std::optional<bool> isUnsafePtr(const QualType T, bool IsArcEnabled) {
234311
if (T->isPointerType() || T->isReferenceType()) {
235312
if (auto *CXXRD = T->getPointeeCXXRecordDecl()) {
236313
auto isUncountedPtr = isUncounted(CXXRD);
237314
auto isUncheckedPtr = isUnchecked(CXXRD);
238-
if (isUncountedPtr && isUncheckedPtr)
239-
return *isUncountedPtr || *isUncheckedPtr;
315+
auto isUnretainedPtr = isUnretained(T, IsArcEnabled);
316+
std::optional<bool> result;
240317
if (isUncountedPtr)
241-
return *isUncountedPtr;
242-
return isUncheckedPtr;
318+
result = *isUncountedPtr;
319+
if (isUncheckedPtr)
320+
result = result ? *result || *isUncheckedPtr : *isUncheckedPtr;
321+
if (isUnretainedPtr)
322+
result = result ? *result || *isUnretainedPtr : *isUnretainedPtr;
323+
return result;
243324
}
244325
}
245326
return false;
@@ -248,6 +329,8 @@ std::optional<bool> isUnsafePtr(const QualType T) {
248329
std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
249330
assert(M);
250331

332+
std::optional<RetainTypeChecker> RTC;
333+
251334
if (isa<CXXMethodDecl>(M)) {
252335
const CXXRecordDecl *calleeMethodsClass = M->getParent();
253336
auto className = safeGetName(calleeMethodsClass);
@@ -263,16 +346,33 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
263346
method == "impl"))
264347
return true;
265348

349+
if (className == "RetainPtr" && method == "get")
350+
return true;
351+
266352
// Ref<T> -> T conversion
267353
// FIXME: Currently allowing any Ref<T> -> whatever cast.
268354
if (isRefType(className)) {
269-
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M))
270-
return isUnsafePtr(maybeRefToRawOperator->getConversionType());
355+
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
356+
auto QT = maybeRefToRawOperator->getConversionType();
357+
auto *T = QT.getTypePtrOrNull();
358+
return T && (T->isPointerType() || T->isReferenceType());
359+
}
271360
}
272361

273362
if (isCheckedPtr(className)) {
274-
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M))
275-
return isUnsafePtr(maybeRefToRawOperator->getConversionType());
363+
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
364+
auto QT = maybeRefToRawOperator->getConversionType();
365+
auto *T = QT.getTypePtrOrNull();
366+
return T && (T->isPointerType() || T->isReferenceType());
367+
}
368+
}
369+
370+
if (className == "RetainPtr") {
371+
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
372+
auto QT = maybeRefToRawOperator->getConversionType();
373+
auto *T = QT.getTypePtrOrNull();
374+
return T && (T->isPointerType() || T->isReferenceType());
375+
}
276376
}
277377
}
278378
return false;
@@ -297,6 +397,13 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
297397
return false;
298398
}
299399

400+
bool isRetainPtr(const CXXRecordDecl *R) {
401+
assert(R);
402+
if (auto *TmplR = R->getTemplateInstantiationPattern())
403+
return safeGetName(TmplR) == "RetainPtr";
404+
return false;
405+
}
406+
300407
bool isPtrConversion(const FunctionDecl *F) {
301408
assert(F);
302409
if (isCtorOfRefCounted(F))

Diff for: clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

+28-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "llvm/ADT/APInt.h"
1313
#include "llvm/ADT/DenseMap.h"
14+
#include "llvm/ADT/DenseSet.h"
1415
#include "llvm/ADT/PointerUnion.h"
1516
#include <optional>
1617

@@ -21,8 +22,11 @@ class CXXRecordDecl;
2122
class Decl;
2223
class FunctionDecl;
2324
class QualType;
25+
class RecordType;
2426
class Stmt;
27+
class TranslationUnitDecl;
2528
class Type;
29+
class TypedefDecl;
2630

2731
// Ref-countability of a type is implicitly defined by Ref<T> and RefPtr<T>
2832
// implementation. It can be modeled as: type T having public methods ref() and
@@ -51,6 +55,9 @@ bool isRefCounted(const clang::CXXRecordDecl *Class);
5155
/// \returns true if \p Class is a CheckedPtr / CheckedRef, false if not.
5256
bool isCheckedPtr(const clang::CXXRecordDecl *Class);
5357

58+
/// \returns true if \p Class is a RetainPtr, false if not.
59+
bool isRetainPtr(const clang::CXXRecordDecl *Class);
60+
5461
/// \returns true if \p Class is ref-countable AND not ref-counted, false if
5562
/// not, std::nullopt if inconclusive.
5663
std::optional<bool> isUncounted(const clang::QualType T);
@@ -59,6 +66,22 @@ std::optional<bool> isUncounted(const clang::QualType T);
5966
/// not, std::nullopt if inconclusive.
6067
std::optional<bool> isUnchecked(const clang::QualType T);
6168

69+
/// An inter-procedural analysis facility that detects CF types with the
70+
/// underlying pointer type.
71+
class RetainTypeChecker {
72+
llvm::DenseSet<const RecordType *> CFPointees;
73+
bool IsARCEnabled{false};
74+
75+
public:
76+
void visitTranslationUnitDecl(const TranslationUnitDecl *);
77+
void visitTypedef(const TypedefDecl *);
78+
bool isUnretained(const QualType);
79+
};
80+
81+
/// \returns true if \p Class is NS or CF objects AND not retained, false if
82+
/// not, std::nullopt if inconclusive.
83+
std::optional<bool> isUnretained(const clang::QualType T, bool IsARCEnabled);
84+
6285
/// \returns true if \p Class is ref-countable AND not ref-counted, false if
6386
/// not, std::nullopt if inconclusive.
6487
std::optional<bool> isUncounted(const clang::CXXRecordDecl* Class);
@@ -77,11 +100,14 @@ std::optional<bool> isUncheckedPtr(const clang::QualType T);
77100

78101
/// \returns true if \p T is either a raw pointer or reference to an uncounted
79102
/// or unchecked class, false if not, std::nullopt if inconclusive.
80-
std::optional<bool> isUnsafePtr(const QualType T);
103+
std::optional<bool> isUnsafePtr(const QualType T, bool IsArcEnabled);
81104

82105
/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its
83106
/// variant, false if not.
84-
bool isSafePtrType(const clang::QualType T);
107+
bool isRefOrCheckedPtrType(const clang::QualType T);
108+
109+
/// \returns true if \p T is a RetainPtr, false if not.
110+
bool isRetainPtrType(const clang::QualType T);
85111

86112
/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or
87113
/// unique_ptr, false if not.

0 commit comments

Comments
 (0)