llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) <details> <summary>Changes</summary> This PR adds support for detecting unsafe union members and pointers to unsafe pointers (e.g. T** where T* is an unsafe pointer type). --- Full diff: https://github.com/llvm/llvm-project/pull/138042.diff 5 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+28-14) - (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 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}} `````````` </details> https://github.com/llvm/llvm-project/pull/138042 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
