[alpha.webkit.UncountedCallArgsChecker] Ignore calls to WTF's container methods #82156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesThis PR makes the checker ignore / skip calls to methods of Web Template Platform's container types such as HashMap, HashSet, WeakHashSet, WeakHashMap, Vector, etc... Full diff: https://github.com/llvm/llvm-project/pull/82156.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 17a64e1b1b8e04..33e640f4d64654 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -25,6 +25,11 @@ using namespace ento;
namespace {
+bool stringEndsWith(const std::string& str, const std::string& suffix) {
+ auto index = str.rfind(suffix);
+ return index != std::string::npos && str.size() - suffix.size() == index;
+}
+
class UncountedCallArgsChecker
: public Checker<check::ASTDecl<TranslationUnitDecl>> {
BugType Bug{this,
@@ -165,6 +170,9 @@ class UncountedCallArgsChecker
if (!Callee)
return false;
+ if (isMethodOnWTFContainerType(Callee))
+ return true;
+
auto overloadedOperatorType = Callee->getOverloadedOperator();
if (overloadedOperatorType == OO_EqualEqual ||
overloadedOperatorType == OO_ExclaimEqual ||
@@ -193,6 +201,30 @@ class UncountedCallArgsChecker
return false;
}
+ bool isMethodOnWTFContainerType(const FunctionDecl *Decl) const {
+ if (!isa<CXXMethodDecl>(Decl))
+ return false;
+ auto *ClassDecl = Decl->getParent();
+ if (!ClassDecl || !isa<CXXRecordDecl>(ClassDecl))
+ return false;
+
+ auto *NsDecl = ClassDecl->getParent();
+ if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+ return false;
+
+ auto methodName = safeGetName(Decl);
+ auto clsName = safeGetName(ClassDecl);
+ auto nsName = safeGetName(NsDecl);
+ // FIXME: These should be implemented via attributes.
+ return nsName == "WTF" &&
+ (methodName == "find" || methodName == "findIf" ||
+ methodName == "reverseFind" || methodName == "reverseFindIf" ||
+ methodName == "get" || methodName == "inlineGet" ||
+ methodName == "contains" || methodName == "containsIf") &&
+ (stringEndsWith(clsName, "Vector") || stringEndsWith(clsName, "Set") ||
+ stringEndsWith(clsName, "Map"));
+ }
+
void reportBug(const Expr *CallArg, const ParmVarDecl *Param) const {
assert(CallArg);
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp
new file mode 100644
index 00000000000000..0a63a789856127
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp
@@ -0,0 +1,146 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+namespace WTF {
+
+ template <typename T>
+ class HashSet {
+ public:
+ template <typename U> T* find(U&) const;
+ template <typename U> bool contains(U&) const;
+ unsigned size() { return m_size; }
+ template <typename U> void add(U&) const;
+ template <typename U> void remove(U&) const;
+
+ private:
+ T* m_table { nullptr };
+ unsigned m_size { 0 };
+ };
+
+ template <typename T, typename S>
+ class HashMap {
+ public:
+ struct Item {
+ T key;
+ S value;
+ };
+
+ template <typename U> Item* find(U&) const;
+ template <typename U> bool contains(U&) const;
+ template <typename U> S* get(U&) const;
+ template <typename U> S* inlineGet(U&) const;
+ template <typename U> void add(U&) const;
+ template <typename U> void remove(U&) const;
+
+ private:
+ Item* m_table { nullptr };
+ };
+
+ template <typename T>
+ class WeakHashSet {
+ public:
+ template <typename U> T* find(U&) const;
+ template <typename U> bool contains(U&) const;
+ template <typename U> void add(U&) const;
+ template <typename U> void remove(U&) const;
+ };
+
+ template <typename T>
+ class Vector {
+ public:
+ unsigned size() { return m_size; }
+ T& at(unsigned i) { return m_buffer[i]; }
+ T& operator[](unsigned i) { return m_buffer[i]; }
+ template <typename U> unsigned find(U&);
+ template <typename U> unsigned reverseFind(U&);
+ template <typename U> bool contains(U&);
+ template <typename MatchFunction> unsigned findIf(const MatchFunction& match)
+ {
+ for (unsigned i = 0; i < m_size; ++i) {
+ if (match(at(i)))
+ return i;
+ }
+ return static_cast<unsigned>(-1);
+ }
+ template <typename MatchFunction> unsigned reverseFindIf(const MatchFunction& match)
+ {
+ for (unsigned i = 0; i < m_size; ++i) {
+ if (match(at(m_size - i)))
+ return i;
+ }
+ return static_cast<unsigned>(-1);
+ }
+ template <typename MatchFunction> bool containsIf(const MatchFunction& match)
+ {
+ for (unsigned i = 0; i < m_size; ++i) {
+ if (match(at(m_size - i)))
+ return true;
+ }
+ return false;
+ }
+ template <typename U> void append(U&) const;
+ template <typename U> void remove(U&) const;
+
+ private:
+ T* m_buffer { nullptr };
+ unsigned m_size { 0 };
+ };
+
+}
+
+using WTF::HashSet;
+using WTF::HashMap;
+using WTF::WeakHashSet;
+using WTF::Vector;
+
+class RefCounted {
+public:
+ void ref() const;
+ void deref() const;
+};
+
+RefCounted* object();
+
+void test() {
+ HashSet<RefPtr<RefCounted>> set;
+ set.find(*object());
+ set.contains(*object());
+ set.add(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ set.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ HashMap<Ref<RefCounted>, unsigned> map;
+ map.find(*object());
+ map.contains(*object());
+ map.inlineGet(*object());
+ map.add(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ map.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ WeakHashSet<Ref<RefCounted>> weakSet;
+ weakSet.find(*object());
+ weakSet.contains(*object());
+ weakSet.add(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ weakSet.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ Vector<Ref<RefCounted>> vector;
+ vector.at(0);
+ vector[0];
+ vector.find(*object());
+ vector.reverseFind(*object());
+ vector.contains(*object());
+ vector.append(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ vector.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ auto* obj = object();
+ vector.findIf([&](Ref<RefCounted> key) { return key.ptr() == obj; });
+ vector.reverseFindIf([&](Ref<RefCounted> key) { return key.ptr() == obj; });
+ vector.containsIf([&](Ref<RefCounted> key) { return key.ptr() == obj; });
+}
\ No newline at end of file
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesThis PR makes the checker ignore / skip calls to methods of Web Template Platform's container types such as HashMap, HashSet, WeakHashSet, WeakHashMap, Vector, etc... Full diff: https://github.com/llvm/llvm-project/pull/82156.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 17a64e1b1b8e04..33e640f4d64654 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -25,6 +25,11 @@ using namespace ento;
namespace {
+bool stringEndsWith(const std::string& str, const std::string& suffix) {
+ auto index = str.rfind(suffix);
+ return index != std::string::npos && str.size() - suffix.size() == index;
+}
+
class UncountedCallArgsChecker
: public Checker<check::ASTDecl<TranslationUnitDecl>> {
BugType Bug{this,
@@ -165,6 +170,9 @@ class UncountedCallArgsChecker
if (!Callee)
return false;
+ if (isMethodOnWTFContainerType(Callee))
+ return true;
+
auto overloadedOperatorType = Callee->getOverloadedOperator();
if (overloadedOperatorType == OO_EqualEqual ||
overloadedOperatorType == OO_ExclaimEqual ||
@@ -193,6 +201,30 @@ class UncountedCallArgsChecker
return false;
}
+ bool isMethodOnWTFContainerType(const FunctionDecl *Decl) const {
+ if (!isa<CXXMethodDecl>(Decl))
+ return false;
+ auto *ClassDecl = Decl->getParent();
+ if (!ClassDecl || !isa<CXXRecordDecl>(ClassDecl))
+ return false;
+
+ auto *NsDecl = ClassDecl->getParent();
+ if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
+ return false;
+
+ auto methodName = safeGetName(Decl);
+ auto clsName = safeGetName(ClassDecl);
+ auto nsName = safeGetName(NsDecl);
+ // FIXME: These should be implemented via attributes.
+ return nsName == "WTF" &&
+ (methodName == "find" || methodName == "findIf" ||
+ methodName == "reverseFind" || methodName == "reverseFindIf" ||
+ methodName == "get" || methodName == "inlineGet" ||
+ methodName == "contains" || methodName == "containsIf") &&
+ (stringEndsWith(clsName, "Vector") || stringEndsWith(clsName, "Set") ||
+ stringEndsWith(clsName, "Map"));
+ }
+
void reportBug(const Expr *CallArg, const ParmVarDecl *Param) const {
assert(CallArg);
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp
new file mode 100644
index 00000000000000..0a63a789856127
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-wtf-containers.cpp
@@ -0,0 +1,146 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+namespace WTF {
+
+ template <typename T>
+ class HashSet {
+ public:
+ template <typename U> T* find(U&) const;
+ template <typename U> bool contains(U&) const;
+ unsigned size() { return m_size; }
+ template <typename U> void add(U&) const;
+ template <typename U> void remove(U&) const;
+
+ private:
+ T* m_table { nullptr };
+ unsigned m_size { 0 };
+ };
+
+ template <typename T, typename S>
+ class HashMap {
+ public:
+ struct Item {
+ T key;
+ S value;
+ };
+
+ template <typename U> Item* find(U&) const;
+ template <typename U> bool contains(U&) const;
+ template <typename U> S* get(U&) const;
+ template <typename U> S* inlineGet(U&) const;
+ template <typename U> void add(U&) const;
+ template <typename U> void remove(U&) const;
+
+ private:
+ Item* m_table { nullptr };
+ };
+
+ template <typename T>
+ class WeakHashSet {
+ public:
+ template <typename U> T* find(U&) const;
+ template <typename U> bool contains(U&) const;
+ template <typename U> void add(U&) const;
+ template <typename U> void remove(U&) const;
+ };
+
+ template <typename T>
+ class Vector {
+ public:
+ unsigned size() { return m_size; }
+ T& at(unsigned i) { return m_buffer[i]; }
+ T& operator[](unsigned i) { return m_buffer[i]; }
+ template <typename U> unsigned find(U&);
+ template <typename U> unsigned reverseFind(U&);
+ template <typename U> bool contains(U&);
+ template <typename MatchFunction> unsigned findIf(const MatchFunction& match)
+ {
+ for (unsigned i = 0; i < m_size; ++i) {
+ if (match(at(i)))
+ return i;
+ }
+ return static_cast<unsigned>(-1);
+ }
+ template <typename MatchFunction> unsigned reverseFindIf(const MatchFunction& match)
+ {
+ for (unsigned i = 0; i < m_size; ++i) {
+ if (match(at(m_size - i)))
+ return i;
+ }
+ return static_cast<unsigned>(-1);
+ }
+ template <typename MatchFunction> bool containsIf(const MatchFunction& match)
+ {
+ for (unsigned i = 0; i < m_size; ++i) {
+ if (match(at(m_size - i)))
+ return true;
+ }
+ return false;
+ }
+ template <typename U> void append(U&) const;
+ template <typename U> void remove(U&) const;
+
+ private:
+ T* m_buffer { nullptr };
+ unsigned m_size { 0 };
+ };
+
+}
+
+using WTF::HashSet;
+using WTF::HashMap;
+using WTF::WeakHashSet;
+using WTF::Vector;
+
+class RefCounted {
+public:
+ void ref() const;
+ void deref() const;
+};
+
+RefCounted* object();
+
+void test() {
+ HashSet<RefPtr<RefCounted>> set;
+ set.find(*object());
+ set.contains(*object());
+ set.add(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ set.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ HashMap<Ref<RefCounted>, unsigned> map;
+ map.find(*object());
+ map.contains(*object());
+ map.inlineGet(*object());
+ map.add(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ map.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ WeakHashSet<Ref<RefCounted>> weakSet;
+ weakSet.find(*object());
+ weakSet.contains(*object());
+ weakSet.add(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ weakSet.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ Vector<Ref<RefCounted>> vector;
+ vector.at(0);
+ vector[0];
+ vector.find(*object());
+ vector.reverseFind(*object());
+ vector.contains(*object());
+ vector.append(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+ vector.remove(*object());
+ // expected-warning@-1{{Call argument is uncounted and unsafe}}
+
+ auto* obj = object();
+ vector.findIf([&](Ref<RefCounted> key) { return key.ptr() == obj; });
+ vector.reverseFindIf([&](Ref<RefCounted> key) { return key.ptr() == obj; });
+ vector.containsIf([&](Ref<RefCounted> key) { return key.ptr() == obj; });
+}
\ No newline at end of file
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
rniwa
force-pushed
the
ignore-wtf-container-methods
branch
2 times, most recently
from
51b65a7 to
d169fdd
Compare
haoNoQ
reviewed
Feb 19, 2024
clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
Outdated
Show resolved
Hide resolved
…er methods This PR makes the checker ignore / skip calls to methods of Web Template Framework's container types such as HashMap, HashSet, WeakHashSet, WeakHashMap, Vector, etc...
rniwa
force-pushed
the
ignore-wtf-container-methods
branch
from
d421182 to
262bc46
Compare
haoNoQ
approved these changes
Feb 20, 2024
|
Yeah, indeed. Thank you for the review! |
This was referenced Mar 7, 2024
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Mar 8, 2024
…er methods (llvm#82156) This PR makes the checker ignore / skip calls to methods of Web Template Platform's container types such as HashMap, HashSet, WeakHashSet, WeakHashMap, Vector, etc...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes the checker ignore / skip calls to methods of Web Template Platform's container types such as HashMap, HashSet, WeakHashSet, WeakHashMap, Vector, etc...