[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
Xazax-hun wrote: Once the other reviewers are happy, this looks good to me! https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
steakhal wrote:
> I feel that I'm getting bogged down in this area with changes that are more
> general than what's strictly necessary and only tangentially related to my
> real goals...
>
> I'm not familiar with the unittest framework and if unittests were necessary
> for this change, then I'd sooner abandon it, because it's not worth the
> effort.
>
> (Disclaimer: this comment reflects my current annoyance, and is not
> necessarily aligned with my long-term opinion.)
No worries!
Note the `IsCLibraryFunctionTest.AcceptsStdNamespace` test case which would
fail on llvm/main, but would pass with the PR.
Here is what I had in mind:
(I could not push to your branch, neither upload the patch file, so here is the
verbatim content instead:
```
commit 1cfbeec724d758b136d36966696df29cf850ca5b
Author: Balazs Benics
Date: Fri Mar 8 17:26:49 2024 +0100
Add unittests
FYI IsCLibraryFunctionTest.AcceptsStdNamespace would have been FALSE
prior this PR.
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt
b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 775f0f8486b8..db56e77331b8 100644
--- a/clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_unittest(StaticAnalysisTests
CallEventTest.cpp
ConflictingEvalCallsTest.cpp
FalsePositiveRefutationBRVisitorTest.cpp
+ IsCLibraryFunctionTest.cpp
NoStateChangeFuncVisitorTest.cpp
ParamRegionTest.cpp
RangeSetTest.cpp
diff --git a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
new file mode 100644
index ..e1dea3458bc1
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
@@ -0,0 +1,89 @@
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+#include
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+testing::AssertionResult extractFunctionDecl(StringRef Code,
+ const FunctionDecl *) {
+ auto ASTUnit = tooling::buildASTFromCode(Code);
+ if (!ASTUnit)
+return testing::AssertionFailure() << "AST construction failed";
+
+ ASTContext = ASTUnit->getASTContext();
+ if (Context.getDiagnostics().hasErrorOccurred())
+return testing::AssertionFailure() << "Compilation error";
+
+ auto Matches = ast_matchers::match(functionDecl().bind("fn"), Context);
+ if (Matches.empty())
+return testing::AssertionFailure() << "No function declaration found";
+
+ if (Matches.size() > 1)
+return testing::AssertionFailure()
+ << "Multiple function declarations found";
+
+ Result = Matches[0].getNodeAs("fn");
+ return testing::AssertionSuccess();
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(void fun();)cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(extern "C" { void fun(); })cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsStaticGlobal) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+ // FIXME: Shouldn't this be TRUE?
+}
+
+TEST(IsCLibraryFunctionTest, RejectsAnonymousNamespace) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(namespace { void fun(); })cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, AcceptsStdNamespace) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(namespace std { void fun(); })cpp", Result));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, RejectsOtherNamespaces) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(namespace stdx { void fun(); })cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, RejectsClassStatic) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(
+ extractFunctionDecl(R"cpp(class A { static void fun(); };)cpp", Result));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+}
+
+TEST(IsCLibraryFunctionTest, RejectsClassMember) {
+ const FunctionDecl *Result;
+ ASSERT_TRUE(extractFunctionDecl(R"cpp(class A { void fun(); };)cpp",
Result));
+
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: NagyDonat wrote: I feel that I'm getting bogged down in this area with changes that are more general than what's strictly necessary... I'm not familiar with the unittest framework and if unittests were necessary for this change, then I'd sooner abandon it, because it's not worth the effort. https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: https://github.com/steakhal commented: Looks good to me, but I'd be more eased if had seen some unittests for this matter. You could use AST-matchers selecting different FunctionDecls and query them using this free-function. I think this deserves a test, thus I vote weak reject. https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the namespace `std`.)
NagyDonat wrote:
I think saying "functions from the C standard library" (at the beginning of
this comment block) is clear enough, mentioning `malloc` is redundant after it.
Also, technically, we are not using this mode for `malloc` _yet_.
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy , =?utf-8?q?Donát?= Nagy Message-ID: In-Reply-To: NagyDonat wrote: Currently MallocChecker doesn't use this `CDM::CLibrary` mode in the `CallDescription` for `malloc` -- because before this commit it would've been incorrect to use it. I'm planning to ensure that functions like `malloc` are matched in this mode; but I would like to do it in separate commits. This means that I cannot create a simple testcase that demonstrates the advantage of this commit: the limitations of `isCLibraryFunction` meant that many checkers just didn't use it even if they were handling C library functions; but I don't know about a case where it was used and incorrect. However, my followup commits will soon add testcases that demonstrate the effects of this change, so I think it's a forgivable sin to merge this without a TC. https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
https://github.com/Szelethus edited https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the namespace `std`.)
Szelethus wrote:
Might as well go the extra mile and mention an example, like `std::malloc`.
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
@@ -87,9 +87,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD, if (!II) return false; - // Look through 'extern "C"' and anything similar invented in the future. - // If this function is not in TU directly, it is not a C library function. - if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit()) + // C library functions are either declared directly within a TU (the common + // case) or they are accessed through the namespace `std` (when they are used + // in C++ via headers like ). Szelethus wrote: Same here. https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
https://github.com/Szelethus commented: Makes perfect sense to me. Can you add a testcase for `std::malloc` or something similar? https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/84469
>From c357aa998204e6693430c801f5b7d3a9e5e09e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:06:01 +0100
Subject: [PATCH 1/3] [analyzer] Accept C library functions from the `std`
namespace
Previously, the function `isCLibraryFunction()` and logic relying on it
only accepted functions that are declared directly within a TU (i.e. not
in a namespace or a class). However C++ headers like declare
many C standard library functions within the namespace `std`, so this
commit ensures that functions within the namespace `std` are also
accepted.
After this commit it will be possible to match functions like `malloc` or
`free` with `CallDescription::Mode::CLibrary`.
---
.../StaticAnalyzer/Core/PathSensitive/CallDescription.h | 8 ++--
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 9 ++---
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..7da65734a44cf3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the `std::` namespace.)
CLibrary,
/// Matches "simple" functions that are not methods. (Static methods are
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index d6d4cec9dd3d4d..c1ae9b441d98d1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -87,9 +88,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
if (!II)
return false;
- // Look through 'extern "C"' and anything similar invented in the future.
- // If this function is not in TU directly, it is not a C library function.
- if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
+ // C library functions are either declared directly within a TU (the common
+ // case) or they are accessed through the namespace `std::` (when they are
+ // used in C++ via headers like ).
+ if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
+ !AnalysisDeclContext::isInStdNamespace(FD))
return false;
// If this function is not externally visible, it is not a C library
function.
>From 6afadd86a6ee790f58c7339dc019d8c8eac8a6b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:22:27 +0100
Subject: [PATCH 2/3] Don't match methods from the namespace `std`
Only accept those functions whose declaration is _directly_ within the
namespace `std` (that is, not within a class or a sub-namespace).
Transparent declaration contexts (e.g. `extern "C"`) are still allowed,
but this prevents matching methods.
---
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index c1ae9b441d98d1..5e706d17eeae14 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,7 +12,6 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -91,8 +90,8 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
// C library functions are either declared directly within a TU (the common
// case) or they are accessed through the namespace `std::` (when
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
=?utf-8?q?Donát?= Nagy
Message-ID:
In-Reply-To:
https://github.com/NagyDonat updated
https://github.com/llvm/llvm-project/pull/84469
>From c357aa998204e6693430c801f5b7d3a9e5e09e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:06:01 +0100
Subject: [PATCH 1/2] [analyzer] Accept C library functions from the `std`
namespace
Previously, the function `isCLibraryFunction()` and logic relying on it
only accepted functions that are declared directly within a TU (i.e. not
in a namespace or a class). However C++ headers like declare
many C standard library functions within the namespace `std`, so this
commit ensures that functions within the namespace `std` are also
accepted.
After this commit it will be possible to match functions like `malloc` or
`free` with `CallDescription::Mode::CLibrary`.
---
.../StaticAnalyzer/Core/PathSensitive/CallDescription.h | 8 ++--
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 9 ++---
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..7da65734a44cf3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the `std::` namespace.)
CLibrary,
/// Matches "simple" functions that are not methods. (Static methods are
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index d6d4cec9dd3d4d..c1ae9b441d98d1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -87,9 +88,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
if (!II)
return false;
- // Look through 'extern "C"' and anything similar invented in the future.
- // If this function is not in TU directly, it is not a C library function.
- if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
+ // C library functions are either declared directly within a TU (the common
+ // case) or they are accessed through the namespace `std::` (when they are
+ // used in C++ via headers like ).
+ if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
+ !AnalysisDeclContext::isInStdNamespace(FD))
return false;
// If this function is not externally visible, it is not a C library
function.
>From 6afadd86a6ee790f58c7339dc019d8c8eac8a6b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:22:27 +0100
Subject: [PATCH 2/2] Don't match methods from the namespace `std`
Only accept those functions whose declaration is _directly_ within the
namespace `std` (that is, not within a class or a sub-namespace).
Transparent declaration contexts (e.g. `extern "C"`) are still allowed,
but this prevents matching methods.
---
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index c1ae9b441d98d1..5e706d17eeae14 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,7 +12,6 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -91,8 +90,8 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
// C library functions are either declared directly within a TU (the common
// case) or they are accessed through the namespace `std::` (when they are
// used in C++
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 6a0618a0289cb0c23ef3e5c820418650cc1d0fdc c357aa998204e6693430c801f5b7d3a9e5e09e37 -- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h clang/lib/StaticAnalyzer/Core/CheckerContext.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp index c1ae9b441d..a67b44724b 100644 --- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp +++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp @@ -92,7 +92,7 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD, // case) or they are accessed through the namespace `std::` (when they are // used in C++ via headers like ). if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit() && - !AnalysisDeclContext::isInStdNamespace(FD)) + !AnalysisDeclContext::isInStdNamespace(FD)) return false; // If this function is not externally visible, it is not a C library function. `` https://github.com/llvm/llvm-project/pull/84469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (NagyDonat)
Changes
Previously, the function `isCLibraryFunction()` and logic relying on it only
accepted functions that are declared directly within a TU (i.e. not in a
namespace or a class). However C++ headers like cstdlib declare many C
standard library functions within the namespace `std`, so this commit ensures
that functions within the namespace `std` are also accepted.
After this commit it will be possible to match functions like `malloc` or
`free` with `CallDescription::Mode::CLibrary`.
---
Full diff: https://github.com/llvm/llvm-project/pull/84469.diff
2 Files Affected:
- (modified)
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h (+2-6)
- (modified) clang/lib/StaticAnalyzer/Core/CheckerContext.cpp (+6-3)
``diff
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..7da65734a44cf3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the `std::` namespace.)
CLibrary,
/// Matches "simple" functions that are not methods. (Static methods are
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index d6d4cec9dd3d4d..c1ae9b441d98d1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -87,9 +88,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
if (!II)
return false;
- // Look through 'extern "C"' and anything similar invented in the future.
- // If this function is not in TU directly, it is not a C library function.
- if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
+ // C library functions are either declared directly within a TU (the common
+ // case) or they are accessed through the namespace `std::` (when they are
+ // used in C++ via headers like ).
+ if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
+ !AnalysisDeclContext::isInStdNamespace(FD))
return false;
// If this function is not externally visible, it is not a C library
function.
``
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)
https://github.com/NagyDonat created
https://github.com/llvm/llvm-project/pull/84469
Previously, the function `isCLibraryFunction()` and logic relying on it only
accepted functions that are declared directly within a TU (i.e. not in a
namespace or a class). However C++ headers like declare many C
standard library functions within the namespace `std`, so this commit ensures
that functions within the namespace `std` are also accepted.
After this commit it will be possible to match functions like `malloc` or
`free` with `CallDescription::Mode::CLibrary`.
>From c357aa998204e6693430c801f5b7d3a9e5e09e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?=
Date: Fri, 8 Mar 2024 13:06:01 +0100
Subject: [PATCH] [analyzer] Accept C library functions from the `std`
namespace
Previously, the function `isCLibraryFunction()` and logic relying on it
only accepted functions that are declared directly within a TU (i.e. not
in a namespace or a class). However C++ headers like declare
many C standard library functions within the namespace `std`, so this
commit ensures that functions within the namespace `std` are also
accepted.
After this commit it will be possible to match functions like `malloc` or
`free` with `CallDescription::Mode::CLibrary`.
---
.../StaticAnalyzer/Core/PathSensitive/CallDescription.h | 8 ++--
clang/lib/StaticAnalyzer/Core/CheckerContext.cpp | 9 ++---
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..7da65734a44cf3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -41,12 +41,8 @@ class CallDescription {
/// - We also accept calls where the number of arguments or parameters is
///greater than the specified value.
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the `std::` namespace.)
CLibrary,
/// Matches "simple" functions that are not methods. (Static methods are
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
index d6d4cec9dd3d4d..c1ae9b441d98d1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/Builtins.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/StringExtras.h"
@@ -87,9 +88,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl
*FD,
if (!II)
return false;
- // Look through 'extern "C"' and anything similar invented in the future.
- // If this function is not in TU directly, it is not a C library function.
- if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
+ // C library functions are either declared directly within a TU (the common
+ // case) or they are accessed through the namespace `std::` (when they are
+ // used in C++ via headers like ).
+ if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit() &&
+ !AnalysisDeclContext::isInStdNamespace(FD))
return false;
// If this function is not externally visible, it is not a C library
function.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
