[clang] 91ec9cb - [alpha.webkit.UncountedCallArgsChecker] Use canonical type (#109393)

[via cfe-commits]

Author: Ryosuke Niwa
Date: 2024-09-26T22:18:07-07:00
New Revision: 91ec9cb96051e4c9044fc47c42732a5f5528e6c8

URL: https://github.com/llvm/llvm-project/commit/91ec9cb96051e4c9044fc47c42732a5f5528e6c8
DIFF: https://github.com/llvm/llvm-project/commit/91ec9cb96051e4c9044fc47c42732a5f5528e6c8.diff

LOG: [alpha.webkit.UncountedCallArgsChecker] Use canonical type (#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.

Added: 
    clang/test/Analysis/Checkers/WebKit/uncounted-obj-const-v-muable.cpp

Modified: 
    clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
    clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp

Removed: 
    


################################################################################
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 at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+    m_obj->mutableFunc();
+    // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}