Skip to content

Commit 763bb3a

Browse files
committed
Fix a regression that alpha.webkit.RetainPtrCtorAdoptChecker emits a warning for non-retained types.
This was caused by the checkCreateOrCopyFunction not checking RTC.isUnretained. Fixed this bug by adding the check. This in turn revealed a bug that we weren't properly recognizing CFTypeRef as the pointee type is "void" and therefore does not have a corresponding Record object. Added a new dense set, RecordlessTypes, to discover these types. We only use as a fallback since looping over ElaboratedType and checking its existence in the set is slower than just checking the presence of the canonical type in the CFPointees set. Finally, skip the analyses of smart pointer template classes in RawPtrRefCallArgsChecker and RawPtrRefLocalVarsChecker now that we properly recognize CFTypeRef as a CF type and this type is used internally inside the mock RetainPtr.
1 parent 9dde1db commit 763bb3a

File tree

6 files changed

+42
-2
lines changed

6 files changed

+42
-2
lines changed

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,16 @@ bool isCheckedPtr(const std::string &Name) {
125125
return Name == "CheckedPtr" || Name == "CheckedRef";
126126
}
127127

128+
bool isSmartPtrClass(const std::string &Name) {
129+
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) ||
130+
Name == "WeakPtr" || Name == "WeakPtr" || Name == "WeakPtrFactory" ||
131+
Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
132+
Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
133+
Name == "ThreadSafeWeakOrStrongPtr" ||
134+
Name == "ThreadSafeWeakPtrControlBlock" ||
135+
Name == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr";
136+
}
137+
128138
bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
129139
assert(F);
130140
const std::string &FunctionName = safeGetName(F);
@@ -222,8 +232,13 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) {
222232

223233
auto PointeeQT = QT->getPointeeType();
224234
const RecordType *RT = PointeeQT->getAs<RecordType>();
225-
if (!RT)
235+
if (!RT) {
236+
if (TD->hasAttr<ObjCBridgeAttr>() || TD->hasAttr<ObjCBridgeMutableAttr>()) {
237+
if (auto *Type = TD->getTypeForDecl())
238+
RecordlessTypes.insert(Type);
239+
}
226240
return;
241+
}
227242

228243
for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) {
229244
if (Redecl->getAttr<ObjCBridgeAttr>() ||
@@ -240,6 +255,17 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
240255
auto CanonicalType = QT.getCanonicalType();
241256
auto PointeeType = CanonicalType->getPointeeType();
242257
auto *RT = dyn_cast_or_null<RecordType>(PointeeType.getTypePtrOrNull());
258+
if (!RT) {
259+
auto *Type = QT.getTypePtrOrNull();
260+
while (Type) {
261+
if (RecordlessTypes.contains(Type))
262+
return true;
263+
auto *ET = dyn_cast_or_null<ElaboratedType>(Type);
264+
if (!ET)
265+
break;
266+
Type = ET->desugar().getTypePtrOrNull();
267+
}
268+
}
243269
return RT && CFPointees.contains(RT);
244270
}
245271

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ std::optional<bool> isUnchecked(const clang::QualType T);
7070
/// underlying pointer type.
7171
class RetainTypeChecker {
7272
llvm::DenseSet<const RecordType *> CFPointees;
73+
llvm::DenseSet<const Type *> RecordlessTypes;
7374
bool IsARCEnabled{false};
7475

7576
public:
@@ -135,6 +136,9 @@ bool isCheckedPtr(const std::string &Name);
135136
/// \returns true if \p Name is RetainPtr or its variant, false if not.
136137
bool isRetainPtr(const std::string &Name);
137138

139+
/// \returns true if \p Name is a smart pointer type name, false if not.
140+
bool isSmartPtrClass(const std::string &Name);
141+
138142
/// \returns true if \p M is getter of a ref-counted class, false if not.
139143
std::optional<bool> isGetterOfSafePtr(const clang::CXXMethodDecl *Method);
140144

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class RawPtrRefCallArgsChecker
6868
}
6969

7070
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) override {
71-
if (isRefType(safeGetName(Decl)))
71+
if (isSmartPtrClass(safeGetName(Decl)))
7272
return true;
7373
return DynamicRecursiveASTVisitor::TraverseClassTemplateDecl(Decl);
7474
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,12 @@ class RawPtrRefLocalVarsChecker
260260
return DynamicRecursiveASTVisitor::TraverseCompoundStmt(CS);
261261
return true;
262262
}
263+
264+
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) override {
265+
if (isSmartPtrClass(safeGetName(Decl)))
266+
return true;
267+
return DynamicRecursiveASTVisitor::TraverseClassTemplateDecl(Decl);
268+
}
263269
};
264270

265271
LocalVisitor visitor(this);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ class RetainPtrCtorAdoptChecker
172172
CreateOrCopyOutArguments.insert(Decl);
173173
hasOutArgument = true;
174174
}
175+
if (!RTC.isUnretained(Callee->getReturnType()))
176+
return;
175177
if (!hasOutArgument && !CreateOrCopyFnCall.contains(CE))
176178
reportLeak(CE, DeclWithIssue);
177179
}

clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "objc-mock-types.h"
55

66
CFTypeRef CFCopyArray(CFArrayRef);
7+
void* CreateCopy();
78

89
void basic_correct() {
910
auto ns1 = adoptNS([SomeObj alloc]);
@@ -15,6 +16,7 @@ void basic_correct() {
1516
CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10));
1617
auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault));
1718
auto cf3 = adoptCF(checked_cf_cast<CFArrayRef>(CFCopyArray(cf1)));
19+
CreateCopy();
1820
}
1921

2022
CFMutableArrayRef provide_cf();

0 commit comments

Comments
 (0)