[llvm-branch-commits] [llvm] b6a29f4 - Revert "[analyzer] Accept C library functions from the `std` namespace (#84469)"

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Mar 12 08:00:22 PDT 2024


Author: NagyDonat
Date: 2024-03-12T16:00:19+01:00
New Revision: b6a29f40aebde93977368979710ca1903482d4b4

URL: https://github.com/llvm/llvm-project/commit/b6a29f40aebde93977368979710ca1903482d4b4
DIFF: https://github.com/llvm/llvm-project/commit/b6a29f40aebde93977368979710ca1903482d4b4.diff

LOG: Revert "[analyzer] Accept C library functions from the `std` namespace (#84469)"

This reverts commit 80ab8234ac309418637488b97e0a62d8377b2ecf.

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 19c66cc6bee1eb..00000000000000
--- a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
+++ /dev/null
@@ -1,89 +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;
-
-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].getNodeAs<FunctionDecl>("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, RejectsNoInlineNoExternalLinkage) {
-  // Functions that are neither inlined nor externally visible cannot be C library functions.
-  const FunctionDecl *Result;
-  ASSERT_TRUE(extractFunctionDecl(R"cpp(static void fun();)cpp", Result));
-  EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
-}
-
-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));
-  EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
-}

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",


        


More information about the llvm-branch-commits mailing list