[clang] Skip over std namespace in WebKit checkers. (PR #90552)
rniwa wrote: Closing this in favor of https://github.com/llvm/llvm-project/pull/91103. https://github.com/llvm/llvm-project/pull/90552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
https://github.com/rniwa closed https://github.com/llvm/llvm-project/pull/90552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
haoNoQ wrote: Hmm it doesn't look like we actually have that other suppression. It looks like the checker simply never warned in the standard library because there aren't WebKit classes in the standard library. But it looks like you've found a few cases where it actually happens? How does this happen? Is this because templates get instantiated with WTF classes as type arguments? Maybe a something completely different solution should be considered? If we think it's actually a good idea to suppress warnings about standard functions, typically these suppressions are implemented with `getSourceManager().isInSystemHeader(SLoc)` checks, such as [the one we actually have](https://github.com/llvm/llvm-project/blob/llvmorg-19-init/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp#L110). This covers not only the standard library, but also other code outside of your project (basically everything that goes with `-isystem` as opposed to `-I`). Which may or may not be what we want. I think it's likely that your solution is optimal but I still want to see some examples to understand what exactly we're dealing with. https://github.com/llvm/llvm-project/pull/90552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
rniwa wrote: > Ok am I reading this right: this is about testing code that lives directly in > namespace `std::` but somehow still lives in your project right? Such as your > custom modifications to standard classes, various overloads and > specializations of standard things. No, this is about standard library code getting warnings. > This patch does not affect the behavior of checkers when you're just passing > raw pointers from user code to standard functions such as `std::memcpy()`. > You still want to have a warning about those. Right. But we don't want code within standard library to give warnings. > And generally you already don't have any warnings about things that's going > on in system headers themselves. There's a separate suppression about that. Hm... it's odd that we're getting warnings there then. https://github.com/llvm/llvm-project/pull/90552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
haoNoQ wrote: (If yes, LGTM!) https://github.com/llvm/llvm-project/pull/90552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
haoNoQ wrote: Ok am I reading this right: this is about testing code that lives directly in namespace `std::` but somehow still lives in your project right? Such as your custom modifications to standard classes, various overloads and specializations of standard things. This patch does not affect the behavior of checkers when you're just passing raw pointers from user code to standard functions such as `std::memcpy()`. You still want to have a warning about those. And generally you already don't have any warnings about things that's going on in system headers themselves. There's a separate suppression about that. https://github.com/llvm/llvm-project/pull/90552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Ryosuke Niwa (rniwa)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/90552.diff
4 Files Affected:
- (modified)
clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp (+6)
- (modified)
clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp (+6)
- (modified) clang/test/Analysis/Checkers/WebKit/call-args.cpp (+27)
- (modified) clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp (+26)
``diff
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index ae494de58da3da..0ff1a27ff412b9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -53,6 +53,12 @@ class UncountedCallArgsChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
+ bool TraverseNamespaceDecl(NamespaceDecl *Decl) {
+if (safeGetName(Decl) == "std")
+ return true;
+return RecursiveASTVisitor::TraverseNamespaceDecl(Decl);
+ }
+
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
if (isRefType(safeGetName(Decl)))
return true;
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 6036ad58cf253c..b27400678a5790 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -134,6 +134,12 @@ class UncountedLocalVarsChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
+ bool TraverseNamespaceDecl(NamespaceDecl *Decl) {
+if (safeGetName(Decl) == "std")
+ return true;
+return RecursiveASTVisitor::TraverseNamespaceDecl(Decl);
+ }
+
bool VisitVarDecl(VarDecl *V) {
Checker->visitVarDecl(V);
return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index 2a4b6bb1f1063a..3a1a2f9e92289e 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -333,3 +333,30 @@ namespace cxx_member_operator_call {
// expected-warning@-1{{Call argument for parameter 'bad' is uncounted and
unsafe}}
}
}
+
+namespace std {
+
+template
+T* other_function();
+
+template
+void another_function(T*, T*);
+
+template
+void some_function(T* a)
+{
+ another_function(other_function(), a);
+}
+
+} // std
+
+namespace ignore_std_namespace {
+
+RefCountable *ref_counted();
+
+void foo() {
+ std::some_function(ref_counted());
+ // expected-warning@-1{{Call argument for parameter 'a' is uncounted and
unsafe}}
+}
+
+} // ignore_std_namespace
\ No newline at end of file
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 00673e91f471ea..c8550b7e2e77f1 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -187,3 +187,29 @@ void bar() {
}
} // namespace ignore_for_if
+
+namespace std {
+
+void memcpy(void*, void*, unsigned long);
+
+template
+void some_function(T* a, T* b)
+{
+ T* temp = new T;
+ memcpy(temp, a, sizeof(T));
+ memcpy(a, b, sizeof(T));
+ memcpy(b, temp, sizeof(T));
+ delete temp;
+}
+
+} // std
+
+namespace ignore_std_namespace {
+
+RefCountable *ref_counted();
+
+void foo() {
+ std::some_function(ref_counted(), ref_counted());
+}
+
+} // ignore_std_namespace
``
https://github.com/llvm/llvm-project/pull/90552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Skip over std namespace in WebKit checkers. (PR #90552)
https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/90552
None
>From fa7a6e376b07ae6262dd06920c87dc69705a646d Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa
Date: Mon, 29 Apr 2024 20:24:24 -0700
Subject: [PATCH] Skip over std namespace in WebKit checkers.
---
.../WebKit/UncountedCallArgsChecker.cpp | 6 +
.../WebKit/UncountedLocalVarsChecker.cpp | 6 +
.../Analysis/Checkers/WebKit/call-args.cpp| 27 +++
.../Checkers/WebKit/uncounted-local-vars.cpp | 26 ++
4 files changed, 65 insertions(+)
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index ae494de58da3da..0ff1a27ff412b9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -53,6 +53,12 @@ class UncountedCallArgsChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
+ bool TraverseNamespaceDecl(NamespaceDecl *Decl) {
+if (safeGetName(Decl) == "std")
+ return true;
+return RecursiveASTVisitor::TraverseNamespaceDecl(Decl);
+ }
+
bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) {
if (isRefType(safeGetName(Decl)))
return true;
diff --git
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 6036ad58cf253c..b27400678a5790 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -134,6 +134,12 @@ class UncountedLocalVarsChecker
bool shouldVisitTemplateInstantiations() const { return true; }
bool shouldVisitImplicitCode() const { return false; }
+ bool TraverseNamespaceDecl(NamespaceDecl *Decl) {
+if (safeGetName(Decl) == "std")
+ return true;
+return RecursiveASTVisitor::TraverseNamespaceDecl(Decl);
+ }
+
bool VisitVarDecl(VarDecl *V) {
Checker->visitVarDecl(V);
return true;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index 2a4b6bb1f1063a..3a1a2f9e92289e 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -333,3 +333,30 @@ namespace cxx_member_operator_call {
// expected-warning@-1{{Call argument for parameter 'bad' is uncounted and
unsafe}}
}
}
+
+namespace std {
+
+template
+T* other_function();
+
+template
+void another_function(T*, T*);
+
+template
+void some_function(T* a)
+{
+ another_function(other_function(), a);
+}
+
+} // std
+
+namespace ignore_std_namespace {
+
+RefCountable *ref_counted();
+
+void foo() {
+ std::some_function(ref_counted());
+ // expected-warning@-1{{Call argument for parameter 'a' is uncounted and
unsafe}}
+}
+
+} // ignore_std_namespace
\ No newline at end of file
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
index 00673e91f471ea..c8550b7e2e77f1 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
@@ -187,3 +187,29 @@ void bar() {
}
} // namespace ignore_for_if
+
+namespace std {
+
+void memcpy(void*, void*, unsigned long);
+
+template
+void some_function(T* a, T* b)
+{
+ T* temp = new T;
+ memcpy(temp, a, sizeof(T));
+ memcpy(a, b, sizeof(T));
+ memcpy(b, temp, sizeof(T));
+ delete temp;
+}
+
+} // std
+
+namespace ignore_std_namespace {
+
+RefCountable *ref_counted();
+
+void foo() {
+ std::some_function(ref_counted(), ref_counted());
+}
+
+} // ignore_std_namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
