[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/NagyDonat commented: Thanks for finding the root cause and fixing the commit! The change LGTM, but I don't think that it is fair to give a formal approval on what's essentially my own commit. https://github.com/llvm/llvm-project/pull/84963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/84963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
steakhal wrote: You gave approval for the fix for the buggy tests I suggested. I think there is nothing wrong with this. Thanks for having a look at my fix. Ill recommit this as you authored, and I coauthored. https://github.com/llvm/llvm-project/pull/84963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/Szelethus approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/84963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/84963
>From bca501228e3a146d3ca6c05b7de15fd914fb16b9 Mon Sep 17 00:00:00 2001
From: NagyDonat
Date: Tue, 12 Mar 2024 18:24:26 +0100
Subject: [PATCH] Reapply "[analyzer] Accept C library functions from the `std`
namespace"
This reapplies f32b04d4ea91ad1018c25a1d4178cc4392d34968i, after fixing
the use-after-free of ASTUnit in the unittest.
https://github.com/llvm/llvm-project/pull/84469#issuecomment-1992163439
Co-authored-by: Balazs Benics
---
.../Core/PathSensitive/CallDescription.h | 8 +-
.../StaticAnalyzer/Core/CheckerContext.cpp| 8 +-
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
.../StaticAnalyzer/IsCLibraryFunctionTest.cpp | 84 +++
.../clang/unittests/StaticAnalyzer/BUILD.gn | 1 +
5 files changed, 93 insertions(+), 9 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..b4e1636130ca7c 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 namespace `std`.)
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..1a9bff529e9bb1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -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 ).
+ const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
+ if (!(DC->isTranslationUnit() || DC->isStdNamespace()))
return false;
// If this function is not externally visible, it is not a C library
function.
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt
b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 775f0f8486b8f9..db56e77331b821 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 00..31ff13f428da36
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
@@ -0,0 +1,84 @@
+#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;
+
+class IsCLibraryFunctionTest : public testing::Test {
+public:
+ const FunctionDecl *getFunctionDecl() const { return Result; }
+
+ testing::AssertionResult buildAST(StringRef Code) {
+ASTUnit = tooling::buildASTFromCode(Code);
+if (!ASTUnit)
+ return testing::AssertionFailure() << "AST construction failed";
+
+ASTContext &Context = 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::Asser
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/84963
>From e48d5a838f69e0a8e0ae95a8aed1a8809f45465a Mon Sep 17 00:00:00 2001
From: NagyDonat
Date: Tue, 12 Mar 2024 18:24:26 +0100
Subject: [PATCH] Reapply "[analyzer] Accept C library functions from the `std`
namespace"
This reapplies f32b04d4ea91ad1018c25a1d4178cc4392d34968i, after fixing
the use-after-free of ASTUnit in the unittest.
https://github.com/llvm/llvm-project/pull/84469#issuecomment-1992163439
Co-authored-by: Balazs Benics
---
.../Core/PathSensitive/CallDescription.h | 8 +-
.../StaticAnalyzer/Core/CheckerContext.cpp| 8 +-
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
.../StaticAnalyzer/IsCLibraryFunctionTest.cpp | 84 +++
.../clang/unittests/StaticAnalyzer/BUILD.gn | 1 +
5 files changed, 93 insertions(+), 9 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..b4e1636130ca7c 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 namespace `std`.)
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..1a9bff529e9bb1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -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 ).
+ const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
+ if (!(DC->isTranslationUnit() || DC->isStdNamespace()))
return false;
// If this function is not externally visible, it is not a C library
function.
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt
b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 775f0f8486b8f9..db56e77331b821 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 00..31ff13f428da36
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
@@ -0,0 +1,84 @@
+#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;
+
+class IsCLibraryFunctionTest : public testing::Test {
+public:
+ const FunctionDecl *getFunctionDecl() const { return Result; }
+
+ testing::AssertionResult buildAST(StringRef Code) {
+ASTUnit = tooling::buildASTFromCode(Code);
+if (!ASTUnit)
+ return testing::AssertionFailure() << "AST construction failed";
+
+ASTContext &Context = 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::Asser
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/steakhal closed https://github.com/llvm/llvm-project/pull/84963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
steakhal wrote: Merged to main as e48d5a838f69e0a8e0ae95a8aed1a8809f45465a https://github.com/llvm/llvm-project/pull/84963 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/steakhal created
https://github.com/llvm/llvm-project/pull/84963
Relands PR #84926, after resolving use-after-free of the AST of the unittest xD
Pretty silly bug, I must admit.
>From a88c479c8c1141af65887829e27194b2715ebfb2 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Tue, 12 Mar 2024 18:24:26 +0100
Subject: [PATCH 1/2] Reapply "[analyzer] Accept C library functions from the
`std` namespace" (#84926)
This reverts commit f32b04d4ea91ad1018c25a1d4178cc4392d34968.
---
.../Core/PathSensitive/CallDescription.h | 8 +-
.../StaticAnalyzer/Core/CheckerContext.cpp| 8 +-
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
.../StaticAnalyzer/IsCLibraryFunctionTest.cpp | 89 +++
.../clang/unittests/StaticAnalyzer/BUILD.gn | 1 +
5 files changed, 98 insertions(+), 9 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..b4e1636130ca7c 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 namespace `std`.)
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..1a9bff529e9bb1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -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 ).
+ const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
+ if (!(DC->isTranslationUnit() || DC->isStdNamespace()))
return false;
// If this function is not externally visible, it is not a C library
function.
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt
b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 775f0f8486b8f9..db56e77331b821 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 00..19c66cc6bee1eb
--- /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 *&Result) {
+ auto ASTUnit = tooling::buildASTFromCode(Code);
+ if (!ASTUnit)
+return testing::AssertionFailure() << "AST construction failed";
+
+ ASTContext &Context = 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
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balazs Benics (steakhal)
Changes
Relands PR #84926, after resolving use-after-free of the AST of the
unittest xD
Pretty silly bug, I must admit.
---
Full diff: https://github.com/llvm/llvm-project/pull/84963.diff
5 Files Affected:
- (modified)
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h (+2-6)
- (modified) clang/lib/StaticAnalyzer/Core/CheckerContext.cpp (+5-3)
- (modified) clang/unittests/StaticAnalyzer/CMakeLists.txt (+1)
- (added) clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp (+84)
- (modified) llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn
(+1)
``diff
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..b4e1636130ca7c 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 namespace `std`.)
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..1a9bff529e9bb1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -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 ).
+ const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
+ if (!(DC->isTranslationUnit() || DC->isStdNamespace()))
return false;
// If this function is not externally visible, it is not a C library
function.
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt
b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 775f0f8486b8f9..db56e77331b821 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 00..31ff13f428da36
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
@@ -0,0 +1,84 @@
+#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;
+
+class IsCLibraryFunctionTest : public testing::Test {
+public:
+ const FunctionDecl *getFunctionDecl() const { return Result; }
+
+ testing::AssertionResult buildAST(StringRef Code) {
+ASTUnit = tooling::buildASTFromCode(Code);
+if (!ASTUnit)
+ return testing::AssertionFailure() << "AST construction failed";
+
+ASTContext &Context = 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");
+retur
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
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 c1af6ab505a83bfb4fc8752591ad333190bc9389
ca45c672e32372d0144720f2ee47d5a5c132ced0 --
clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
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/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
index 31ff13f428..1a7b481a4c 100644
--- a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
@@ -53,7 +53,8 @@ TEST_F(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
}
TEST_F(IsCLibraryFunctionTest, RejectsNoInlineNoExternalLinkage) {
- // Functions that are neither inlined nor externally visible cannot be C
library functions.
+ // Functions that are neither inlined nor externally visible cannot be C
+ // library functions.
ASSERT_TRUE(buildAST(R"cpp(static void fun();)cpp"));
EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
``
https://github.com/llvm/llvm-project/pull/84963
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/84963
>From a88c479c8c1141af65887829e27194b2715ebfb2 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Tue, 12 Mar 2024 18:24:26 +0100
Subject: [PATCH 1/3] Reapply "[analyzer] Accept C library functions from the
`std` namespace" (#84926)
This reverts commit f32b04d4ea91ad1018c25a1d4178cc4392d34968.
---
.../Core/PathSensitive/CallDescription.h | 8 +-
.../StaticAnalyzer/Core/CheckerContext.cpp| 8 +-
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
.../StaticAnalyzer/IsCLibraryFunctionTest.cpp | 89 +++
.../clang/unittests/StaticAnalyzer/BUILD.gn | 1 +
5 files changed, 98 insertions(+), 9 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 3432d2648633c2..b4e1636130ca7c 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 namespace `std`.)
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..1a9bff529e9bb1 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -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 ).
+ const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
+ if (!(DC->isTranslationUnit() || DC->isStdNamespace()))
return false;
// If this function is not externally visible, it is not a C library
function.
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt
b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 775f0f8486b8f9..db56e77331b821 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 00..19c66cc6bee1eb
--- /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 *&Result) {
+ auto ASTUnit = tooling::buildASTFromCode(Code);
+ if (!ASTUnit)
+return testing::AssertionFailure() << "AST construction failed";
+
+ ASTContext &Context = 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].getNo
