https://github.com/rniwa created
https://github.com/llvm/llvm-project/pull/137566
None
>From a04f65339530bea920c80dc1b2b0f3c9bab488c3 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Sun, 20 Apr 2025 18:38:31 -0700
Subject: [PATCH 1/2] [RawPtrRefMemberChecker] Make RawPtrRefMemberChecker
consistent with other checkers
Refactor RawPtrRefMemberChecker so that each subclass override isUnsafePtr like
other
WebKit checkers instead of overriding isPtrCompatible.
---
.../WebKit/RawPtrRefMemberChecker.cpp | 98 +++++++++----------
1 file changed, 45 insertions(+), 53 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index 10b9749319a57..e8992f58273bb 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;
@@ -93,22 +91,30 @@ class RawPtrRefMemberChecker
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);
- }
- }
+ auto IsUnsafePtr = isUnsafePtr(QT);
+ if (!IsUnsafePtr || !*IsUnsafePtr)
+ continue;
+
+ 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 {
if (BR->getSourceManager().isInSystemHeader(CD->getLocation()))
return;
@@ -138,19 +144,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 +163,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 {
@@ -263,10 +261,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 +278,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 +298,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);
}
>From 0281e11cd8ba878e1d755fdef781250652fd6e91 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Sun, 27 Apr 2025 16:52:13 -0700
Subject: [PATCH 2/2] [RawPtrRefMemberChecker] Add the support for union and
pointers to unsafe pointers.
This PR adds support for detecting unsafe union members and pointers to unsafe
pointers
(e.g. T** where T* is an unsafe pointer type).
---
.../WebKit/RawPtrRefMemberChecker.cpp | 42 ++++++++++++-------
.../Checkers/WebKit/unchecked-members.cpp | 26 +++++++++++-
.../Checkers/WebKit/uncounted-members.cpp | 30 +++++++++++--
.../Checkers/WebKit/unretained-members-arc.mm | 27 ++++++++++++
.../Checkers/WebKit/unretained-members.mm | 34 +++++++++++++--
5 files changed, 136 insertions(+), 23 deletions(-)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
index e8992f58273bb..d1a4f203d7a49 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp
@@ -85,21 +85,31 @@ class RawPtrRefMemberChecker
if (shouldSkipDecl(RD))
return;
- for (auto *Member : RD->fields()) {
- auto QT = Member->getType();
- const Type *MemberType = QT.getTypePtrOrNull();
- if (!MemberType)
- continue;
+ for (auto *Member : RD->fields())
+ visitMember(Member, RD);
+ }
- auto IsUnsafePtr = isUnsafePtr(QT);
- if (!IsUnsafePtr || !*IsUnsafePtr)
- continue;
+ void visitMember(const FieldDecl *Member, const RecordDecl *RD) const {
+ auto QT = Member->getType();
+ const Type *MemberType = QT.getTypePtrOrNull();
- if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl())
- reportBug(Member, MemberType, MemberCXXRD, RD);
- else if (auto *ObjCDecl = getObjCDecl(MemberType))
- reportBug(Member, MemberType, ObjCDecl, RD);
+ 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 {
@@ -192,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.
@@ -229,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);
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@-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@-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@-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@-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@-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@-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@-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@-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@-1{{Member variable 'e' in 'members::Foo' is a retainable
type 'CFMutableArrayRef'}}
};
+
+ union FooUnion {
+ SomeObj* a;
+ CFMutableArrayRef b;
+ // expected-warning@-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@-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@-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@-1{{Member variable 'a' in 'unions::Foo' is a raw
pointer to retainable type 'SomeObj'}}
RetainPtr<SomeObj> b;
CFMutableArrayRef c;
+ // expected-warning@-1{{Member variable 'c' in 'unions::Foo' is a
retainable type 'CFMutableArrayRef'}}
};
template<class T>
- union RefPtr {
+ union FooTempl {
T* a;
+ // expected-warning@-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@-1{{Member variable 'elements1' in
'ptr_to_ptr_to_retained::List' contains a raw pointer to retainable type
'SomeObj'}}
+ CFMutableArrayRef* elements2;
+ // expected-warning@-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@-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@-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@-1{{Instance variable 'ns_string' in 'AnotherObject' is
a raw pointer to retainable type 'NSString'; member variables must be a
RetainPtr}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
