Skip to content

[alpha.webkit.UncountedCallArgsChecker] Allow ArrayInitLoopExpr and OpaqueValueExpr in trivial expressions #127182

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
Feb 14, 2025

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented Feb 14, 2025

Allow VisitArrayInitLoopExpr, VisitArrayInitIndexExpr, and VisitOpaqueValueExpr in trivial functions and statements.

…paqueValueExpr in trivial expressions

Allow VisitArrayInitLoopExpr, VisitArrayInitIndexExpr, and VisitOpaqueValueExpr in trivial functions and statements.
@rniwa rniwa requested review from haoNoQ and t-rasmud February 14, 2025 09:01
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Feb 14, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 14, 2025

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

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)

Changes

Allow VisitArrayInitLoopExpr, VisitArrayInitIndexExpr, and VisitOpaqueValueExpr in trivial functions and statements.


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

2 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp (+12)
  • (added) clang/test/Analysis/Checkers/WebKit/call-args-loop-init-opaque-value.cpp (+47)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index d40b4b4dbb560..edfe72c62b2c8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -589,6 +589,18 @@ class TrivialFunctionAnalysisVisitor
     return Visit(BTE->getSubExpr());
   }
 
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *AILE) {
+    return Visit(AILE->getCommonExpr()) && Visit(AILE->getSubExpr());
+  }
+
+  bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *AIIE) {
+    return true; // The current array index in VisitArrayInitLoopExpr is always trivial.
+  }
+
+  bool VisitOpaqueValueExpr(const OpaqueValueExpr *OVE) {
+    return Visit(OVE->getSourceExpr());
+  }
+
   bool VisitExprWithCleanups(const ExprWithCleanups *EWC) {
     return Visit(EWC->getSubExpr());
   }
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-loop-init-opaque-value.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-loop-init-opaque-value.cpp
new file mode 100644
index 0000000000000..69987c600eeb5
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-loop-init-opaque-value.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+// expected-no-diagnostics
+
+typedef unsigned long size_t;
+template<typename T, size_t N>
+struct Obj {
+    constexpr static size_t Size = N;
+
+    constexpr T& operator[](size_t i) { return components[i]; }
+    constexpr const T& operator[](size_t i) const { return components[i]; }
+
+    constexpr size_t size() const { return Size; }
+
+    T components[N];
+};
+
+template<typename T, size_t N>
+constexpr bool operator==(const Obj<T, N>& a, const Obj<T, N>& b)
+{
+    for (size_t i = 0; i < N; ++i) {
+        if (a[i] == b[i])
+            continue;
+        return false;
+    }
+
+    return true;
+}
+
+class Component {
+public:
+    void ref() const;
+    void deref() const;
+
+    Obj<float, 4> unresolvedComponents() const { return m_components; }
+
+    bool isEqual(const Component& other) const {
+        return unresolvedComponents() == other.unresolvedComponents();
+    }
+
+private:
+    Obj<float, 4> m_components;
+};
+
+Component* provide();
+bool someFunction(Component* other) {
+    return provide()->isEqual(*other);
+}

Copy link

github-actions bot commented Feb 14, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@t-rasmud t-rasmud left a comment

Choose a reason for hiding this comment

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

I have a small nit about tests. Otherwise, LGTM.

@@ -0,0 +1,47 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to test these expressions in non-trivial expressions? Or do such tests already exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have tests for non-trivial expressions: clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp. I guess we can add a test case here for non-trivial case 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 a non-trivial test case.

@rniwa
Copy link
Contributor Author

rniwa commented Feb 14, 2025

Thanks for the review!

@rniwa rniwa merged commit 9106ee2 into llvm:main Feb 14, 2025
8 checks passed
@rniwa rniwa deleted the trivial-loop-init-opaque-value branch February 14, 2025 22:44
rniwa added a commit to rniwa/llvm-project that referenced this pull request Feb 15, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…paqueValueExpr in trivial expressions (llvm#127182)

Allow VisitArrayInitLoopExpr, VisitArrayInitIndexExpr, and
VisitOpaqueValueExpr in trivial functions and statements.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
…paqueValueExpr in trivial expressions (llvm#127182)

Allow VisitArrayInitLoopExpr, VisitArrayInitIndexExpr, and
VisitOpaqueValueExpr in trivial functions and statements.
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.

None yet

3 participants