Skip to content

Conversation

@ojhunt
Copy link
Contributor

@ojhunt ojhunt commented Mar 2, 2025

Simply add awareness of BindingDecl to the logic for identifying local assignments.

…gsChecker

Simply add awareness of BindingDecl to the logic for identifying local
assignments.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Mar 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 2, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Oliver Hunt (ojhunt)

Changes

Simply add awareness of BindingDecl to the logic for identifying local assignments.


Full diff: https://github.com/llvm/llvm-project/pull/129424.diff

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+6-1)
  • (added) clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp (+34)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index dc86c4fcc64b1..58020ec4e084d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -140,9 +140,14 @@ bool tryToFindPtrOrigin(
 bool isASafeCallArg(const Expr *E) {
   assert(E);
   if (auto *Ref = dyn_cast<DeclRefExpr>(E)) {
-    if (auto *D = dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
+    auto *FoundDecl = Ref->getFoundDecl();
+    if (auto *D = dyn_cast_or_null<VarDecl>(FoundDecl)) {
       if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
         return true;
+    } else if (auto *BD = dyn_cast_or_null<BindingDecl>(FoundDecl)) {
+      VarDecl *VD = BD->getHoldingVar();
+      if (VD && (isa<ParmVarDecl>(VD) || VD->isLocalVarDecl()))
+        return true;
     }
   }
   if (isConstOwnerPtrMemberExpr(E))
diff --git a/clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp b/clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp
new file mode 100644
index 0000000000000..e3006d5f31372
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s -std=c++2c
+// expected-no-diagnostics
+
+#include "mock-types.h"
+
+class Node {
+public:
+    Node* nextSibling() const;
+
+    void ref() const;
+    void deref() const;
+};
+
+template <class A, class B> struct pair {
+  A a;
+  B b;
+  template <unsigned I> requires ( I == 0 ) A& get();
+  template <unsigned I> requires ( I == 1 ) B& get();
+};
+
+namespace std {
+    template <class> struct tuple_size;
+    template <unsigned, class> struct tuple_element;
+    template <class A, class B> struct tuple_size<::pair<A, B>> { static constexpr int value = 2; };
+    template <class A, class B> struct tuple_element<0, ::pair<A, B>> { using type = A; };
+    template <class A, class B> struct tuple_element<1, ::pair<A, B>> { using type = B; };
+}
+
+pair<RefPtr<Node>, RefPtr<Node>> &getPair();
+
+static void testUnpackedAssignment() {
+    auto [a, b] = getPair();
+    a->nextSibling();
+}

@ojhunt
Copy link
Contributor Author

ojhunt commented Mar 2, 2025

@rniwa Hi! I did a thing!

pair<RefPtr<Node>, RefPtr<Node>> &getPair();

static void testUnpackedAssignment() {
auto [a, b] = getPair();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add a test for local vars checker as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

Copy link
Contributor

@rniwa rniwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@rniwa
Copy link
Contributor

rniwa commented Mar 3, 2025

Thanks for the fix!

@rniwa rniwa merged commit e6aae2a into llvm:main Mar 3, 2025
8 of 11 checks passed
@rniwa rniwa deleted the users/ojhunt/webkit-refptr-binding branch March 3, 2025 04:30
rniwa pushed a commit to rniwa/llvm-project that referenced this pull request Apr 22, 2025
…gsChecker (llvm#129424)

Simply add awareness of BindingDecl to the logic for identifying local
assignments.
rniwa pushed a commit to rniwa/llvm-project that referenced this pull request Aug 21, 2025
…gsChecker (llvm#129424)

Simply add awareness of BindingDecl to the logic for identifying local
assignments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants