Author: Philip Reames Date: 2024-03-13T10:19:42-07:00 New Revision: 13ccaf9b9d4400bb128b35ff4ac733e4afc3ad1c
URL: https://github.com/llvm/llvm-project/commit/13ccaf9b9d4400bb128b35ff4ac733e4afc3ad1c DIFF: https://github.com/llvm/llvm-project/commit/13ccaf9b9d4400bb128b35ff4ac733e4afc3ad1c.diff LOG: Revert "Reapply "[analyzer] Accept C library functions from the `std` namespace"" This reverts commit e48d5a838f69e0a8e0ae95a8aed1a8809f45465a. Fails to build on x86-64 w/gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) with the following message: ../llvm-project/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp:41:28: error: declaration of ‘std::unique_ptr<clang::ASTUnit> IsCLibraryFunctionTest::ASTUnit’ changes meaning of ‘ASTUnit’ [-fpermissive] 41 | std::unique_ptr<ASTUnit> ASTUnit; | ^~~~~~~ In file included from ../llvm-project/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp:4: ../llvm-project/clang/include/clang/Frontend/ASTUnit.h:89:7: note: ‘ASTUnit’ declared here as ‘class clang::ASTUnit’ 89 | class ASTUnit { | ^~~~~~~ Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h clang/lib/StaticAnalyzer/Core/CheckerContext.cpp clang/unittests/StaticAnalyzer/CMakeLists.txt llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn Removed: 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 b4e1636130ca7c..3432d2648633c2 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h @@ -41,8 +41,12 @@ 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(). - /// (This mode only matches functions that are declared either directly - /// within a TU or in the namespace `std`.) + /// 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 <cstdlib>. 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 1a9bff529e9bb1..d6d4cec9dd3d4d 100644 --- a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp +++ b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp @@ -87,11 +87,9 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD, if (!II) return false; - // 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 <cstdlib>). - const DeclContext *DC = FD->getDeclContext()->getRedeclContext(); - if (!(DC->isTranslationUnit() || DC->isStdNamespace())) + // 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()) 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 db56e77331b821..775f0f8486b8f9 100644 --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -11,7 +11,6 @@ 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 deleted file mode 100644 index 31ff13f428da36..00000000000000 --- a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp +++ /dev/null @@ -1,84 +0,0 @@ -#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 <memory> - -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<FunctionDecl>("fn"); - return testing::AssertionSuccess(); - } - -private: - std::unique_ptr<ASTUnit> ASTUnit; - const FunctionDecl *Result = nullptr; -}; - -TEST_F(IsCLibraryFunctionTest, AcceptsGlobal) { - ASSERT_TRUE(buildAST(R"cpp(void fun();)cpp")); - EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} - -TEST_F(IsCLibraryFunctionTest, AcceptsExternCGlobal) { - ASSERT_TRUE(buildAST(R"cpp(extern "C" { void fun(); })cpp")); - EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} - -TEST_F(IsCLibraryFunctionTest, RejectsNoInlineNoExternalLinkage) { - // 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())); -} - -TEST_F(IsCLibraryFunctionTest, RejectsAnonymousNamespace) { - ASSERT_TRUE(buildAST(R"cpp(namespace { void fun(); })cpp")); - EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} - -TEST_F(IsCLibraryFunctionTest, AcceptsStdNamespace) { - ASSERT_TRUE(buildAST(R"cpp(namespace std { void fun(); })cpp")); - EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} - -TEST_F(IsCLibraryFunctionTest, RejectsOtherNamespaces) { - ASSERT_TRUE(buildAST(R"cpp(namespace stdx { void fun(); })cpp")); - EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} - -TEST_F(IsCLibraryFunctionTest, RejectsClassStatic) { - ASSERT_TRUE(buildAST(R"cpp(class A { static void fun(); };)cpp")); - EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} - -TEST_F(IsCLibraryFunctionTest, RejectsClassMember) { - ASSERT_TRUE(buildAST(R"cpp(class A { void fun(); };)cpp")); - EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl())); -} diff --git a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn index 9c240cff181634..01c2b6ced3366f 100644 --- a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn @@ -19,7 +19,6 @@ unittest("StaticAnalysisTests") { "CallEventTest.cpp", "ConflictingEvalCallsTest.cpp", "FalsePositiveRefutationBRVisitorTest.cpp", - "IsCLibraryFunctionTest.cpp", "NoStateChangeFuncVisitorTest.cpp", "ParamRegionTest.cpp", "RangeSetTest.cpp", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
