https://github.com/rniwa created
https://github.com/llvm/llvm-project/pull/109393
This PR fixes a bug in UncountedCallArgsChecker that calling a function with a
member variable which is Ref/RefPtr is erroneously treated as safe by
canoniclizing the type before checking whether it's ref counted or not.
>From 7b74a32cd93f7812ec48b60ffcf379a91c7bdc6c Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rn...@webkit.org>
Date: Fri, 20 Sep 2024 02:02:41 -0700
Subject: [PATCH] [alpha.webkit.UncountedCallArgsChecker] Use canonical type
This PR fixes a bug in UncountedCallArgsChecker that calling a function
with a member variable which is Ref/RefPtr is erroneously treated as safe
by canoniclizing the type before checking whether it's ref counted or not.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
.../WebKit/UncountedCallArgsChecker.cpp | 9 ++++---
.../WebKit/uncounted-obj-const-v-muable.cpp | 27 +++++++++++++++++++
3 files changed, 33 insertions(+), 5 deletions(-)
create mode 100644
clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 9da3e54e454317..54c99c3c1b37f9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -155,7 +155,7 @@ std::optional<bool> isUncounted(const QualType T) {
std::optional<bool> isUncounted(const CXXRecordDecl* Class)
{
// Keep isRefCounted first as it's cheaper.
- if (isRefCounted(Class))
+ if (!Class || isRefCounted(Class))
return false;
std::optional<bool> IsRefCountable = isRefCountable(Class);
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 81c2434ce64775..31e9b3c4b9d412 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -86,7 +86,7 @@ class UncountedCallArgsChecker
return;
}
auto *E = MemberCallExpr->getImplicitObjectArgument();
- QualType ArgType = MemberCallExpr->getObjectType();
+ QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType();
std::optional<bool> IsUncounted = isUncounted(ArgType);
if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
reportBugOnThis(E);
@@ -102,12 +102,13 @@ class UncountedCallArgsChecker
// if ((*P)->hasAttr<SafeRefCntblRawPtrAttr>())
// continue;
- const auto *ArgType = (*P)->getType().getTypePtrOrNull();
- if (!ArgType)
+ QualType ArgType = (*P)->getType().getCanonicalType();
+ const auto *TypePtr = ArgType.getTypePtrOrNull();
+ if (!TypePtr)
continue; // FIXME? Should we bail?
// FIXME: more complex types (arrays, references to raw pointers, etc)
- std::optional<bool> IsUncounted = isUncountedPtr(ArgType);
+ std::optional<bool> IsUncounted = isUncountedPtr(TypePtr);
if (!IsUncounted || !(*IsUncounted))
continue;
diff --git
a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp
b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp
new file mode 100644
index 00000000000000..2721cd8474e1b4
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_analyze_cc1
-analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+class Object {
+public:
+ void ref() const;
+ void deref() const;
+
+ bool constFunc() const;
+ void mutableFunc();
+};
+
+class Caller {
+ void someFunction();
+ void otherFunction();
+private:
+ RefPtr<Object> m_obj;
+};
+
+void Caller::someFunction()
+{
+ m_obj->constFunc();
+ // expected-warning@-1{{Call argument for 'this' parameter is uncounted
and unsafe}}
+ m_obj->mutableFunc();
+ // expected-warning@-1{{Call argument for 'this' parameter is uncounted
and unsafe}}
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
