Skip to content

[analyzer] Handle structured bindings in alpha.webkit.UncountedCallArgsChecker #129424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 3, 2025

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 Mar 11, 2025
…gsChecker (llvm#129424)

Simply add awareness of BindingDecl to the logic for identifying local
assignments.
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 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 Apr 22, 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