[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