[clang] 6be567b - [Webkit Checkers] Treat const member variables as a safe origin (#115594)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 14 21:11:12 PST 2024
Author: Ryosuke Niwa
Date: 2024-11-14T21:11:08-08:00
New Revision: 6be567bfc22e0d165a4b927beab3933be7ef98e6
URL: https://github.com/llvm/llvm-project/commit/6be567bfc22e0d165a4b927beab3933be7ef98e6
DIFF: https://github.com/llvm/llvm-project/commit/6be567bfc22e0d165a4b927beab3933be7ef98e6.diff
LOG: [Webkit Checkers] Treat const member variables as a safe origin (#115594)
Treat const Ref, RefPtr, CheckedRef, CheckedPtr member variables as safe
pointer origin in WebKit's local variable and call arguments checkers.
Added:
clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp
clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
Modified:
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
clang/test/Analysis/Checkers/WebKit/mock-types.h
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 9d34dfd3cea636..b3cd594a0f3529 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -142,9 +142,31 @@ bool isASafeCallArg(const Expr *E) {
return true;
}
}
+ if (isConstOwnerPtrMemberExpr(E))
+ return true;
// TODO: checker for method calls on non-refcounted objects
return isa<CXXThisExpr>(E);
}
+bool isConstOwnerPtrMemberExpr(const clang::Expr *E) {
+ if (auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
+ if (auto *Callee = MCE->getDirectCallee()) {
+ auto Name = safeGetName(Callee);
+ if (Name == "get" || Name == "ptr") {
+ auto *ThisArg = MCE->getImplicitObjectArgument();
+ E = ThisArg;
+ }
+ }
+ }
+ auto *ME = dyn_cast<MemberExpr>(E);
+ if (!ME)
+ return false;
+ auto *D = ME->getMemberDecl();
+ if (!D)
+ return false;
+ auto T = D->getType();
+ return isOwnerPtrType(T) && T.isConstQualified();
+}
+
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
index e972924e0c523d..ddbef0fba04489 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -64,6 +64,9 @@ bool tryToFindPtrOrigin(
/// \returns Whether \p E is a safe call arugment.
bool isASafeCallArg(const clang::Expr *E);
+/// \returns true if E is a MemberExpr accessing a const smart pointer type.
+bool isConstOwnerPtrMemberExpr(const clang::Expr *E);
+
/// \returns name of AST node or empty string.
template <typename T> std::string safeGetName(const T *ASTNode) {
const auto *const ND = llvm::dyn_cast_or_null<clang::NamedDecl>(ASTNode);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 94419cfd9e73eb..797f3e1f3fba5a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -145,25 +145,37 @@ bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F);
}
-bool isSafePtrType(const clang::QualType T) {
+template <typename Predicate>
+static bool isPtrOfType(const clang::QualType T, Predicate Pred) {
QualType type = T;
while (!type.isNull()) {
if (auto *elaboratedT = type->getAs<ElaboratedType>()) {
type = elaboratedT->desugar();
continue;
}
- if (auto *specialT = type->getAs<TemplateSpecializationType>()) {
- if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) {
- auto name = decl->getNameAsString();
- return isRefType(name) || isCheckedPtr(name);
- }
+ auto *SpecialT = type->getAs<TemplateSpecializationType>();
+ if (!SpecialT)
return false;
- }
- return false;
+ auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl();
+ if (!Decl)
+ return false;
+ return Pred(Decl->getNameAsString());
}
return false;
}
+bool isSafePtrType(const clang::QualType T) {
+ return isPtrOfType(
+ T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); });
+}
+
+bool isOwnerPtrType(const clang::QualType T) {
+ return isPtrOfType(T, [](auto Name) {
+ return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" ||
+ Name == "UniqueRef" || Name == "LazyUniqueRef";
+ });
+}
+
std::optional<bool> isUncounted(const QualType T) {
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
if (auto *Decl = Subst->getAssociatedDecl()) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index 681ea55c613553..9adfc0f1f57260 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -83,6 +83,10 @@ std::optional<bool> isUnsafePtr(const QualType T);
/// variant, false if not.
bool isSafePtrType(const clang::QualType T);
+/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or
+/// unique_ptr, false if not.
+bool isOwnerPtrType(const clang::QualType T);
+
/// \returns true if \p F creates ref-countable object from uncounted parameter,
/// false if not.
bool isCtorOfRefCounted(const clang::FunctionDecl *F);
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
index 06f8f43cee8151..48c3dc4c94477a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp
@@ -281,6 +281,9 @@ class RawPtrRefLocalVarsChecker
if (isa<IntegerLiteral>(InitArgOrigin))
return true;
+ if (isConstOwnerPtrMemberExpr(InitArgOrigin))
+ return true;
+
if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
if (auto *MaybeGuardian =
dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
new file mode 100644
index 00000000000000..76f80a12c1703c
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked-const-member.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+namespace call_args_const_checkedptr_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const CheckedPtr<CheckedObj> m_obj1;
+ CheckedPtr<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is unchecked and unsafe}}
+}
+
+} // namespace call_args_const_checkedptr_member
+
+namespace call_args_const_checkedref_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const CheckedRef<CheckedObj> m_obj1;
+ CheckedRef<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is unchecked and unsafe}}
+}
+
+} // namespace call_args_const_checkedref_member
+
+namespace call_args_const_unique_ptr {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const std::unique_ptr<CheckedObj> m_obj1;
+ std::unique_ptr<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is unchecked and unsafe}}
+}
+
+} // namespace call_args_const_unique_ptr
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp
new file mode 100644
index 00000000000000..b3296507a5c92d
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-counted-const-member.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+namespace std {
+}
+
+namespace call_args_const_refptr_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const RefPtr<RefCountable> m_obj1;
+ RefPtr<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
+
+} // namespace call_args_const_refptr_member
+
+namespace call_args_const_ref_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const Ref<RefCountable> m_obj1;
+ Ref<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
+
+} // namespace call_args_const_ref_member
+
+namespace call_args_const_unique_ptr {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const std::unique_ptr<RefCountable> m_obj1;
+ std::unique_ptr<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
+
+} // namespace call_args_const_unique_ptr
+
+namespace call_args_const_unique_ref {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const UniqueRef<RefCountable> m_obj1;
+ UniqueRef<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ m_obj1->method();
+ m_obj2->method();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
+
+} // namespace call_args_const_unique_ref
diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
new file mode 100644
index 00000000000000..e52d1e735f6379
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedLocalVarsChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
+
+void someFunction();
+
+namespace local_vars_const_checkedptr_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const CheckedPtr<CheckedObj> m_obj1;
+ CheckedPtr<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+ auto* obj1 = m_obj1.get();
+ obj1->method();
+ auto* obj2 = m_obj2.get();
+ // expected-warning at -1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+ obj2->method();
+}
+
+} // namespace local_vars_const_checkedptr_member
+
+namespace local_vars_const_checkedref_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const CheckedRef<CheckedObj> m_obj1;
+ CheckedRef<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+ auto& obj1 = m_obj1.get();
+ obj1.method();
+ auto& obj2 = m_obj2.get();
+ // expected-warning at -1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+ obj2.method();
+}
+
+} // namespace local_vars_const_ref_member
+
+namespace call_args_const_unique_ptr {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const std::unique_ptr<CheckedObj> m_obj1;
+ std::unique_ptr<CheckedObj> m_obj2;
+};
+
+void Foo::bar() {
+ auto* obj1 = m_obj1.get();
+ obj1->method();
+ auto* obj2 = m_obj2.get();
+ // expected-warning at -1{{Local variable 'obj2' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}}
+ obj2->method();
+}
+
+} // namespace call_args_const_unique_ptr
diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
new file mode 100644
index 00000000000000..03d16285f88b53
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedLocalVarsChecker -verify %s
+
+#include "mock-types.h"
+#include "mock-system-header.h"
+
+void someFunction();
+
+namespace local_vars_const_refptr_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const RefPtr<RefCountable> m_obj1;
+ RefPtr<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ auto* obj1 = m_obj1.get();
+ obj1->method();
+ auto* obj2 = m_obj2.get();
+ // expected-warning at -1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ obj2->method();
+}
+
+} // namespace local_vars_const_refptr_member
+
+namespace local_vars_const_ref_member {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const Ref<RefCountable> m_obj1;
+ Ref<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ auto& obj1 = m_obj1.get();
+ obj1.method();
+ auto& obj2 = m_obj2.get();
+ // expected-warning at -1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ obj2.method();
+}
+
+} // namespace local_vars_const_ref_member
+
+namespace call_args_const_unique_ptr {
+
+class Foo {
+public:
+ Foo();
+ void bar();
+
+private:
+ const std::unique_ptr<RefCountable> m_obj1;
+ std::unique_ptr<RefCountable> m_obj2;
+};
+
+void Foo::bar() {
+ auto* obj1 = m_obj1.get();
+ obj1->method();
+ auto* obj2 = m_obj2.get();
+ // expected-warning at -1{{Local variable 'obj2' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}}
+ obj2->method();
+}
+
+} // namespace call_args_const_unique_ptr
diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h
index 4a55ddab64d804..fb1ee51c7ec1de 100644
--- a/clang/test/Analysis/Checkers/WebKit/mock-types.h
+++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -66,11 +66,10 @@ template <typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTra
t = o.t;
o.t = tmp;
}
- T &get() { return *PtrTraits::unwrap(t); }
- T *ptr() { return PtrTraits::unwrap(t); }
- T *operator->() { return PtrTraits::unwrap(t); }
- operator const T &() const { return *PtrTraits::unwrap(t); }
- operator T &() { return *PtrTraits::unwrap(t); }
+ T &get() const { return *PtrTraits::unwrap(t); }
+ T *ptr() const { return PtrTraits::unwrap(t); }
+ T *operator->() const { return PtrTraits::unwrap(t); }
+ operator T &() const { return *PtrTraits::unwrap(t); }
T* leakRef() { return PtrTraits::exchange(t, nullptr); }
};
@@ -102,9 +101,8 @@ template <typename T> struct RefPtr {
t = o.t;
o.t = tmp;
}
- T *get() { return t; }
- T *operator->() { return t; }
- const T *operator->() const { return t; }
+ T *get() const { return t; }
+ T *operator->() const { return t; }
T &operator*() { return *t; }
RefPtr &operator=(T *t) {
RefPtr o(t);
@@ -151,9 +149,9 @@ template <typename T> struct CheckedRef {
CheckedRef(T &t) : t(&t) { t.incrementCheckedPtrCount(); }
CheckedRef(const CheckedRef &o) : t(o.t) { if (t) t->incrementCheckedPtrCount(); }
~CheckedRef() { if (t) t->decrementCheckedPtrCount(); }
- T &get() { return *t; }
- T *ptr() { return t; }
- T *operator->() { return t; }
+ T &get() const { return *t; }
+ T *ptr() const { return t; }
+ T *operator->() const { return t; }
operator const T &() const { return *t; }
operator T &() { return *t; }
};
@@ -176,9 +174,8 @@ template <typename T> struct CheckedPtr {
if (t)
t->decrementCheckedPtrCount();
}
- T *get() { return t; }
- T *operator->() { return t; }
- const T *operator->() const { return t; }
+ T *get() const { return t; }
+ T *operator->() const { return t; }
T &operator*() { return *t; }
CheckedPtr &operator=(T *) { return *this; }
operator bool() const { return t; }
@@ -202,4 +199,52 @@ class RefCountableAndCheckable {
int trivial() { return 0; }
};
+template <typename T>
+class UniqueRef {
+private:
+ T *t;
+
+public:
+ UniqueRef(T &t) : t(&t) { }
+ ~UniqueRef() {
+ if (t)
+ delete t;
+ }
+ template <typename U> UniqueRef(UniqueRef<U>&& u)
+ : t(u.t)
+ {
+ u.t = nullptr;
+ }
+ T &get() const { return *t; }
+ T *operator->() const { return t; }
+ UniqueRef &operator=(T &) { return *this; }
+};
+
+namespace std {
+
+template <typename T>
+class unique_ptr {
+private:
+ T *t;
+
+public:
+ unique_ptr() : t(nullptr) { }
+ unique_ptr(T *t) : t(t) { }
+ ~unique_ptr() {
+ if (t)
+ delete t;
+ }
+ template <typename U> unique_ptr(unique_ptr<U>&& u)
+ : t(u.t)
+ {
+ u.t = nullptr;
+ }
+ T *get() const { return t; }
+ T *operator->() const { return t; }
+ T &operator*() { return *t; }
+ unique_ptr &operator=(T *) { return *this; }
+};
+
+};
+
#endif
More information about the cfe-commits
mailing list