Skip to content

Commit bef9d8b

Browse files
rniwarniwa
committed
[RawPtrRefMemberChecker] Add the support for union and pointers to unsafe pointers. (llvm#138042)
This PR adds support for detecting unsafe union members and pointers to unsafe pointers (e.g. T** where T* is an unsafe pointer type).
1 parent d236bc8 commit bef9d8b

File tree

5 files changed

+136
-24
lines changed

5 files changed

+136
-24
lines changed

‎clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -86,21 +86,31 @@ class RawPtrRefMemberChecker
8686
if (shouldSkipDecl(RD))
8787
return;
8888

89-
for (auto *Member : RD->fields()) {
90-
auto QT = Member->getType();
91-
const Type *MemberType = QT.getTypePtrOrNull();
92-
if (!MemberType)
93-
continue;
89+
for (auto *Member : RD->fields())
90+
visitMember(Member, RD);
91+
}
9492

95-
auto IsUnsafePtr = isUnsafePtr(QT);
96-
if (!IsUnsafePtr || !*IsUnsafePtr)
97-
continue;
93+
void visitMember(const FieldDecl *Member, const RecordDecl *RD) const {
94+
auto QT = Member->getType();
95+
const Type *MemberType = QT.getTypePtrOrNull();
9896

99-
if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl())
100-
reportBug(Member, MemberType, MemberCXXRD, RD);
101-
else if (auto *ObjCDecl = getObjCDecl(MemberType))
102-
reportBug(Member, MemberType, ObjCDecl, RD);
97+
while (MemberType) {
98+
auto IsUnsafePtr = isUnsafePtr(QT);
99+
if (IsUnsafePtr && *IsUnsafePtr)
100+
break;
101+
if (!MemberType->isPointerType())
102+
return;
103+
QT = MemberType->getPointeeType();
104+
MemberType = QT.getTypePtrOrNull();
103105
}
106+
107+
if (!MemberType)
108+
return;
109+
110+
if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl())
111+
reportBug(Member, MemberType, MemberCXXRD, RD);
112+
else if (auto *ObjCDecl = getObjCDecl(MemberType))
113+
reportBug(Member, MemberType, ObjCDecl, RD);
104114
}
105115

106116
ObjCInterfaceDecl *getObjCDecl(const Type *TypePtr) const {
@@ -192,8 +202,8 @@ class RawPtrRefMemberChecker
192202
return true;
193203

194204
const auto Kind = RD->getTagKind();
195-
// FIMXE: Should we check union members too?
196-
if (Kind != TagTypeKind::Struct && Kind != TagTypeKind::Class)
205+
if (Kind != TagTypeKind::Struct && Kind != TagTypeKind::Class &&
206+
Kind != TagTypeKind::Union)
197207
return true;
198208

199209
// Ignore CXXRecords that come from system headers.
@@ -230,7 +240,10 @@ class RawPtrRefMemberChecker
230240
printQuotedName(Os, Member);
231241
Os << " in ";
232242
printQuotedQualifiedName(Os, ClassCXXRD);
233-
Os << " is a ";
243+
if (Member->getType().getTypePtrOrNull() == MemberType)
244+
Os << " is a ";
245+
else
246+
Os << " contains a ";
234247
if (printPointer(Os, MemberType) == PrintDeclKind::Pointer) {
235248
auto Typedef = MemberType->getAs<TypedefType>();
236249
assert(Typedef);

‎clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,24 @@ namespace members {
3434

3535
} // namespace members
3636

37-
namespace ignore_unions {
37+
namespace unions {
3838

3939
union Foo {
4040
CheckedObj* a;
41+
// expected-warning@-1{{Member variable 'a' in 'unions::Foo' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
4142
CheckedPtr<CheckedObj> c;
4243
CheckedRef<CheckedObj> d;
4344
};
4445

4546
template<class T>
4647
union FooTmpl {
4748
T* a;
49+
// expected-warning@-1{{Member variable 'a' in 'unions::FooTmpl<CheckedObj>' is a raw pointer to CheckedPtr capable type 'CheckedObj'}}
4850
};
4951

5052
void forceTmplToInstantiate(FooTmpl<CheckedObj>) { }
5153

52-
} // namespace ignore_unions
54+
} // namespace unions
5355

5456
namespace checked_ptr_ref_ptr_capable {
5557

@@ -59,3 +61,23 @@ namespace checked_ptr_ref_ptr_capable {
5961
}
6062

6163
} // checked_ptr_ref_ptr_capable
64+
65+
namespace ptr_to_ptr_to_checked_ptr_capable {
66+
67+
struct List {
68+
CheckedObj** elements;
69+
// expected-warning@-1{{Member variable 'elements' in 'ptr_to_ptr_to_checked_ptr_capable::List' contains a raw pointer to CheckedPtr capable type 'CheckedObj'}}
70+
};
71+
72+
template <typename T>
73+
struct TemplateList {
74+
T** elements;
75+
// 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'}}
76+
};
77+
TemplateList<CheckedObj> list;
78+
79+
struct SafeList {
80+
CheckedPtr<CheckedObj>* elements;
81+
};
82+
83+
} // namespace ptr_to_ptr_to_checked_ptr_capable

‎clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,22 @@ namespace members {
3636
};
3737
} // members
3838

39-
namespace ignore_unions {
39+
namespace unions {
4040
union Foo {
4141
RefCountable* a;
42+
// expected-warning@-1{{Member variable 'a' in 'unions::Foo' is a raw pointer to ref-countable type 'RefCountable'}}
4243
RefPtr<RefCountable> b;
4344
Ref<RefCountable> c;
4445
};
4546

4647
template<class T>
47-
union RefPtr {
48+
union FooTmpl {
4849
T* a;
50+
// expected-warning@-1{{Member variable 'a' in 'unions::FooTmpl<RefCountable>' is a raw pointer to ref-countable type 'RefCountable'}}
4951
};
5052

51-
void forceTmplToInstantiate(RefPtr<RefCountable>) {}
52-
} // ignore_unions
53+
void forceTmplToInstantiate(FooTmpl<RefCountable>) {}
54+
} // unions
5355

5456
namespace ignore_system_header {
5557

@@ -77,3 +79,23 @@ namespace checked_ptr_ref_ptr_capable {
7779
}
7880

7981
} // checked_ptr_ref_ptr_capable
82+
83+
namespace ptr_to_ptr_to_ref_counted {
84+
85+
struct List {
86+
RefCountable** elements;
87+
// expected-warning@-1{{Member variable 'elements' in 'ptr_to_ptr_to_ref_counted::List' contains a raw pointer to ref-countable type 'RefCountable'}}
88+
};
89+
90+
template <typename T>
91+
struct TemplateList {
92+
T** elements;
93+
// expected-warning@-1{{Member variable 'elements' in 'ptr_to_ptr_to_ref_counted::TemplateList<RefCountable>' contains a raw pointer to ref-countable type 'RefCountable'}}
94+
};
95+
TemplateList<RefCountable> list;
96+
97+
struct SafeList {
98+
RefPtr<RefCountable>* elements;
99+
};
100+
101+
} // namespace ptr_to_ptr_to_ref_counted

‎clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
CFMutableArrayRef e = nullptr;
2222
// expected-warning@-1{{Member variable 'e' in 'members::Foo' is a retainable type 'CFMutableArrayRef'}}
2323
};
24+
25+
union FooUnion {
26+
SomeObj* a;
27+
CFMutableArrayRef b;
28+
// expected-warning@-1{{Member variable 'b' in 'members::FooUnion' is a retainable type 'CFMutableArrayRef'}}
29+
};
2430

2531
template<class T, class S>
2632
struct FooTmpl {
@@ -37,3 +43,24 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
3743
};
3844

3945
}
46+
47+
namespace ptr_to_ptr_to_retained {
48+
49+
struct List {
50+
CFMutableArrayRef* elements2;
51+
// expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'CFMutableArrayRef'}}
52+
};
53+
54+
template <typename T, typename S>
55+
struct TemplateList {
56+
S* elements2;
57+
// expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList<SomeObj, __CFArray *>' contains a raw pointer to retainable type '__CFArray'}}
58+
};
59+
TemplateList<SomeObj, CFMutableArrayRef> list;
60+
61+
struct SafeList {
62+
RetainPtr<SomeObj>* elements1;
63+
RetainPtr<CFMutableArrayRef>* elements2;
64+
};
65+
66+
} // namespace ptr_to_ptr_to_retained

‎clang/test/Analysis/Checkers/WebKit/unretained-members.mm‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,49 @@ void forceTmplToInstantiate(FooTmpl<SomeObj, CFMutableArrayRef>) {}
4747

4848
}
4949

50-
namespace ignore_unions {
50+
namespace unions {
5151
union Foo {
5252
SomeObj* a;
53+
// expected-warning@-1{{Member variable 'a' in 'unions::Foo' is a raw pointer to retainable type 'SomeObj'}}
5354
RetainPtr<SomeObj> b;
5455
CFMutableArrayRef c;
56+
// expected-warning@-1{{Member variable 'c' in 'unions::Foo' is a retainable type 'CFMutableArrayRef'}}
5557
};
5658

5759
template<class T>
58-
union RefPtr {
60+
union FooTempl {
5961
T* a;
62+
// expected-warning@-1{{Member variable 'a' in 'unions::FooTempl<SomeObj>' is a raw pointer to retainable type 'SomeObj'}}
6063
};
6164

62-
void forceTmplToInstantiate(RefPtr<SomeObj>) {}
65+
void forceTmplToInstantiate(FooTempl<SomeObj>) {}
6366
}
6467

68+
namespace ptr_to_ptr_to_retained {
69+
70+
struct List {
71+
SomeObj** elements1;
72+
// expected-warning@-1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::List' contains a raw pointer to retainable type 'SomeObj'}}
73+
CFMutableArrayRef* elements2;
74+
// expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::List' contains a retainable type 'CFMutableArrayRef'}}
75+
};
76+
77+
template <typename T, typename S>
78+
struct TemplateList {
79+
T** elements1;
80+
// expected-warning@-1{{Member variable 'elements1' in 'ptr_to_ptr_to_retained::TemplateList<SomeObj, __CFArray *>' contains a raw pointer to retainable type 'SomeObj'}}
81+
S* elements2;
82+
// expected-warning@-1{{Member variable 'elements2' in 'ptr_to_ptr_to_retained::TemplateList<SomeObj, __CFArray *>' contains a raw pointer to retainable type '__CFArray'}}
83+
};
84+
TemplateList<SomeObj, CFMutableArrayRef> list;
85+
86+
struct SafeList {
87+
RetainPtr<SomeObj>* elements1;
88+
RetainPtr<CFMutableArrayRef>* elements2;
89+
};
90+
91+
} // namespace ptr_to_ptr_to_retained
92+
6593
@interface AnotherObject : NSObject {
6694
NSString *ns_string;
6795
// expected-warning@-1{{Instance variable 'ns_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}}

0 commit comments

Comments
 (0)