[clang] Support ptr to ptr and union in webkit member checker (PR #137565)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 27 16:55:14 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Ryosuke Niwa (rniwa)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/137565.diff
5 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+67-61)
- (modified) clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp (+24-2)
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp (+26-4)
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm (+27)
- (modified) clang/test/Analysis/Checkers/WebKit/unretained-members.mm (+31-3)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 10b9749319a57..b5395ca7e34ff 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -38,9 +38,7 @@ class RawPtrRefMemberChecker
RawPtrRefMemberChecker(const char *description)
: Bug(this, description, "WebKit coding guidelines") {}
- virtual std::optional<bool>
- isPtrCompatible(const clang::QualType,
- const clang::CXXRecordDecl *R) const = 0;
+ virtual std::optional<bool> isUnsafePtr(QualType) const = 0;
virtual const char *typeName() const = 0;
virtual const char *invariant() const = 0;
@@ -87,26 +85,44 @@ class RawPtrRefMemberChecker
if (shouldSkipDecl(RD))
return;
- for (auto *Member : RD->fields()) {
- auto QT = Member->getType();
- const Type *MemberType = QT.getTypePtrOrNull();
- if (!MemberType)
- continue;
-
- if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
- std::optional<bool> IsCompatible = isPtrCompatible(QT, MemberCXXRD);
- if (IsCompatible && *IsCompatible)
- reportBug(Member, MemberType, MemberCXXRD, RD);
- } else {
- std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr);
- auto *PointeeType = MemberType->getPointeeType().getTypePtrOrNull();
- if (IsCompatible && *IsCompatible) {
- auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
- if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared))
- reportBug(Member, MemberType, ObjCType->getDecl(), RD);
- }
- }
+ for (auto *Member : RD->fields())
+ visitMember(Member, RD);
+ }
+
+ void visitMember(const FieldDecl* Member, const RecordDecl *RD) const {
+ auto QT = Member->getType();
+ const Type *MemberType = QT.getTypePtrOrNull();
+
+ while (MemberType) {
+ auto IsUnsafePtr = isUnsafePtr(QT);
+ if (IsUnsafePtr && *IsUnsafePtr)
+ break;
+ if (!MemberType->isPointerType())
+ return;
+ QT = MemberType->getPointeeType();
+ MemberType = QT.getTypePtrOrNull();
}
+
+ if (!MemberType)
+ return;
+
+ if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl())
+ reportBug(Member, MemberType, MemberCXXRD, RD);
+ else if (auto* ObjCDecl = getObjCDecl(MemberType))
+ reportBug(Member, MemberType, ObjCDecl, RD);
+ }
+
+ ObjCInterfaceDecl* getObjCDecl(const Type *TypePtr) const {
+ auto *PointeeType = TypePtr->getPointeeType().getTypePtrOrNull();
+ if (!PointeeType)
+ return nullptr;
+ auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
+ if (!Desugared)
+ return nullptr;
+ auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared);
+ if (!ObjCType)
+ return nullptr;
+ return ObjCType->getDecl();
}
void visitObjCDecl(const ObjCContainerDecl *CD) const {
@@ -138,19 +154,15 @@ class RawPtrRefMemberChecker
const Type *IvarType = QT.getTypePtrOrNull();
if (!IvarType)
return;
- if (auto *IvarCXXRD = IvarType->getPointeeCXXRecordDecl()) {
- std::optional<bool> IsCompatible = isPtrCompatible(QT, IvarCXXRD);
- if (IsCompatible && *IsCompatible)
- reportBug(Ivar, IvarType, IvarCXXRD, CD);
- } else {
- std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr);
- auto *PointeeType = IvarType->getPointeeType().getTypePtrOrNull();
- if (IsCompatible && *IsCompatible) {
- auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
- if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared))
- reportBug(Ivar, IvarType, ObjCType->getDecl(), CD);
- }
- }
+
+ auto IsUnsafePtr = isUnsafePtr(QT);
+ if (!IsUnsafePtr || !*IsUnsafePtr)
+ return;
+
+ if (auto *MemberCXXRD = IvarType->getPointeeCXXRecordDecl())
+ reportBug(Ivar, IvarType, MemberCXXRD, CD);
+ else if (auto* ObjCDecl = getObjCDecl(IvarType))
+ reportBug(Ivar, IvarType, ObjCDecl, CD);
}
void visitObjCPropertyDecl(const ObjCContainerDecl *CD,
@@ -161,19 +173,15 @@ class RawPtrRefMemberChecker
const Type *PropType = QT.getTypePtrOrNull();
if (!PropType)
return;
- if (auto *PropCXXRD = PropType->getPointeeCXXRecordDecl()) {
- std::optional<bool> IsCompatible = isPtrCompatible(QT, PropCXXRD);
- if (IsCompatible && *IsCompatible)
- reportBug(PD, PropType, PropCXXRD, CD);
- } else {
- std::optional<bool> IsCompatible = isPtrCompatible(QT, nullptr);
- auto *PointeeType = PropType->getPointeeType().getTypePtrOrNull();
- if (IsCompatible && *IsCompatible) {
- auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
- if (auto *ObjCType = dyn_cast_or_null<ObjCInterfaceType>(Desugared))
- reportBug(PD, PropType, ObjCType->getDecl(), CD);
- }
- }
+
+ auto IsUnsafePtr = isUnsafePtr(QT);
+ if (!IsUnsafePtr || !*IsUnsafePtr)
+ return;
+
+ if (auto *MemberCXXRD = PropType->getPointeeCXXRecordDecl())
+ reportBug(PD, PropType, MemberCXXRD, CD);
+ else if (auto* ObjCDecl = getObjCDecl(PropType))
+ reportBug(PD, PropType, ObjCDecl, CD);
}
bool shouldSkipDecl(const RecordDecl *RD) const {
@@ -194,7 +202,8 @@ class RawPtrRefMemberChecker
const auto Kind = RD->getTagKind();
// FIMXE: Should we check union members too?
- if (Kind != TagTypeKind::Struct && Kind != TagTypeKind::Class)
+ if (Kind != TagTypeKind::Struct && Kind != TagTypeKind::Class &&
+ Kind != TagTypeKind::Union)
return true;
// Ignore CXXRecords that come from system headers.
@@ -231,7 +240,10 @@ class RawPtrRefMemberChecker
printQuotedName(Os, Member);
Os << " in ";
printQuotedQualifiedName(Os, ClassCXXRD);
- Os << " is a ";
+ if (Member->getType().getTypePtrOrNull() == MemberType)
+ Os << " is a ";
+ else
+ Os << " contains a ";
if (printPointer(Os, MemberType) == PrintDeclKind::Pointer) {
auto Typedef = MemberType->getAs<TypedefType>();
assert(Typedef);
@@ -263,10 +275,8 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker {
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
"reference-countable type") {}
- std::optional<bool>
- isPtrCompatible(const clang::QualType,
- const clang::CXXRecordDecl *R) const final {
- return R ? isRefCountable(R) : std::nullopt;
+ std::optional<bool> isUnsafePtr(QualType QT) const final {
+ return isUncountedPtr(QT.getCanonicalType());
}
const char *typeName() const final { return "ref-countable type"; }
@@ -282,10 +292,8 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker {
: RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to "
"checked-pointer capable type") {}
- std::optional<bool>
- isPtrCompatible(const clang::QualType,
- const clang::CXXRecordDecl *R) const final {
- return R ? isCheckedPtrCapable(R) : std::nullopt;
+ std::optional<bool> isUnsafePtr(QualType QT) const final {
+ return isUncheckedPtr(QT.getCanonicalType());
}
const char *typeName() const final { return "CheckedPtr capable type"; }
@@ -304,9 +312,7 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
RTC = RetainTypeChecker();
}
- std::optional<bool>
- isPtrCompatible(const clang::QualType QT,
- const clang::CXXRecordDecl *) const final {
+ std::optional<bool> isUnsafePtr(QualType QT) const final {
return RTC->isUnretained(QT);
}
diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
index 048ffbffcdefb..3fe15d88ff312 100644
--- a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp
@@ -34,10 +34,11 @@ namespace members {
} // namespace members
-namespace ignore_unions {
+namespace unions {
union Foo {
CheckedObj* a;
+ // expected-warning at -1{{Member variable 'a' in 'unions::Foo' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
CheckedPtr<CheckedObj> c;
CheckedRef<CheckedObj> d;
};
@@ -45,11 +46,12 @@ namespace ignore_unions {
template<class T>
union FooTmpl {
T* a;
+ // expected-warning at -1{{Member variable 'a' in 'unions::FooTmpl<CheckedObj>' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
};
void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
-} // namespace ignore_unions
+} // namespace unions
namespace checked_ptr_ref_ptr_capable {
@@ -59,3 +61,23 @@ namespace checked_ptr_ref_ptr_capable {
}
} // checked_ptr_ref_ptr_capable
+
+namespace ptr_to_ptr_to_checked_ptr_capable {
+
+ struct List {
+ CheckedObj** elements;
+ // expected-warning at -1{{Member variable 'elements' in 'ptr_to_ptr_to_checked_ptr_capable::List' contains a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+ };
+
+ template <typename T>
+ struct TemplateList {
+ T** elements;
+ // expected-warning at -1{{Member variable 'elements' in 'ptr_to_ptr_to_checked_ptr_capable::TemplateList<CheckedObj>' contains a raw pointer to CheckedPtr capable type 'CheckedObj'}}
+ };
+ TemplateList<CheckedObj> list;
+
+ struct SafeList {
+ CheckedPtr<CheckedObj>* elements;
+ };
+
+} // namespace ptr_to_ptr_to_checked_ptr_capable
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
index 130777a9a5fee..b8c443cda4f8e 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
@@ -36,20 +36,22 @@ namespace members {
};
} // members
-namespace ignore_unions {
+namespace unions {
union Foo {
RefCountable* a;
+ // expected-warning at -1{{Member variable 'a' in 'unions::Foo' is a raw pointer to ref-countable type 'RefCountable'}}
RefPtr<RefCountable> b;
Ref<RefCountable> c;
};
template<class T>
- union RefPtr {
+ union FooTmpl {
T* a;
+ // expected-warning at -1{{Member variable 'a' in 'unions::FooTmpl<RefCountable>' is a raw pointer to ref-countable type 'RefCountable'}}
};
- void forceTmplToInstantiate(RefPtr<RefCountable>) {}
-} // ignore_unions
+ void forceTmplToInstantiate(FooTmpl<RefCountable>) {}
+} // unions
namespace ignore_system_header {
@@ -77,3 +79,23 @@ namespace checked_ptr_ref_ptr_capable {
}
} // checked_ptr_ref_ptr_capable
+
+namespace ptr_to_ptr_to_ref_counted {
+
+ struct List {
+ RefCountable** elements;
+ // expected-warning at -1{{Member variable 'elements' in 'ptr_to_ptr_to_ref_counted::List' contains a raw pointer to ref-countable type 'RefCountable'}}
+ };
+
+ template <typename T>
+ struct TemplateList {
+ T** elements;
+ // expected-warning at -1{{Member variable 'elements' in 'ptr_to_ptr_to_ref_counted::TemplateList<RefCountable>' contains a raw pointer to ref-countable type 'RefCountable'}}
+ };
+ TemplateList<RefCountable> list;
+
+ struct SafeList {
+ RefPtr<RefCountable>* elements;
+ };
+
+} // namespace ptr_to_ptr_to_ref_counted
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
index 9820c875b87c0..3491bc93ed98a 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm
@@ -21,6 +21,12 @@
CFMutableArrayRef e = nullptr;
// expected-warning at -1{{Member variable 'e' in 'members::Foo' is a retainable type 'CFMutableArrayRef'}}
};
+
+ union FooUnion {
+ SomeObj* a;
+ CFMutableArrayRef b;
+ // expected-warning at -1{{Member variable 'b' in 'members::FooUnion' is a retainable type 'CFMutableArrayRef'}}
+ };
template<class T, class S>
struct FooTmpl {
@@ -37,3 +43,24 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
};
}
+
+namespace ptr_to_ptr_to_retained {
+
+ struct List {
+ CFMutableArrayRef* elements2;
+ // expected-warning at -1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'CFMutableArrayRef'}}
+ };
+
+ template <typename T, typename S>
+ struct TemplateList {
+ S* elements2;
+ // expected-warning at -1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList<SomeObj, __CFArray *>' contains a raw pointer to retainable type '__CFArray'}}
+ };
+ TemplateList<SomeObj, CFMutableArrayRef> list;
+
+ struct SafeList {
+ RetainPtr<SomeObj>* elements1;
+ RetainPtr<CFMutableArrayRef>* elements2;
+ };
+
+} // namespace ptr_to_ptr_to_retained
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
index fff1f8ede091b..0cb4c4ac0f6a0 100644
--- a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
+++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm
@@ -47,21 +47,49 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
}
-namespace ignore_unions {
+namespace unions {
union Foo {
SomeObj* a;
+ // expected-warning at -1{{Member variable 'a' in 'unions::Foo' is a raw pointer to retainable type 'SomeObj'}}
RetainPtr<SomeObj> b;
CFMutableArrayRef c;
+ // expected-warning at -1{{Member variable 'c' in 'unions::Foo' is a retainable type 'CFMutableArrayRef'}}
};
template<class T>
- union RefPtr {
+ union FooTempl {
T* a;
+ // expected-warning at -1{{Member variable 'a' in 'unions::FooTempl<SomeObj>' is a raw pointer to retainable type 'SomeObj'}}
};
- void forceTmplToInstantiate(RefPtr<SomeObj>) {}
+ void forceTmplToInstantiate(FooTempl<SomeObj>) {}
}
+namespace ptr_to_ptr_to_retained {
+
+ struct List {
+ SomeObj** elements1;
+ // expected-warning at -1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::List' contains a raw pointer to retainable type 'SomeObj'}}
+ CFMutableArrayRef* elements2;
+ // expected-warning at -1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'CFMutableArrayRef'}}
+ };
+
+ template <typename T, typename S>
+ struct TemplateList {
+ T** elements1;
+ // expected-warning at -1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::TemplateList<SomeObj, __CFArray *>' contains a raw pointer to retainable type 'SomeObj'}}
+ S* elements2;
+ // expected-warning at -1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList<SomeObj, __CFArray *>' contains a raw pointer to retainable type '__CFArray'}}
+ };
+ TemplateList<SomeObj, CFMutableArrayRef> list;
+
+ struct SafeList {
+ RetainPtr<SomeObj>* elements1;
+ RetainPtr<CFMutableArrayRef>* elements2;
+ };
+
+} // namespace ptr_to_ptr_to_retained
+
@interface AnotherObject : NSObject {
NSString *ns_string;
// expected-warning at -1{{Instance variable 'ns_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}
``````````
</details>
https://github.com/llvm/llvm-project/pull/137565
More information about the cfe-commits
mailing list