[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)
Balazs Benics via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 12 11:27:06 PDT 2024
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 <benicsbalazs at gmail.com>
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 <cstdlib>.
+ /// (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 <cstdlib>).
+ 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 00000000000000..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 <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 01c2b6ced3366f..9c240cff181634 100644
--- a/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn
@@ -19,6 +19,7 @@ unittest("StaticAnalysisTests") {
"CallEventTest.cpp",
"ConflictingEvalCallsTest.cpp",
"FalsePositiveRefutationBRVisitorTest.cpp",
+ "IsCLibraryFunctionTest.cpp",
"NoStateChangeFuncVisitorTest.cpp",
"ParamRegionTest.cpp",
"RangeSetTest.cpp",
>From ca45c672e32372d0144720f2ee47d5a5c132ced0 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Tue, 12 Mar 2024 18:50:21 +0100
Subject: [PATCH 2/3] Fix use-after-free of ASTUnit in the unittest
https://github.com/llvm/llvm-project/pull/84469#issuecomment-1992163439
---
.../StaticAnalyzer/IsCLibraryFunctionTest.cpp | 111 +++++++++---------
1 file changed, 53 insertions(+), 58 deletions(-)
diff --git a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
index 19c66cc6bee1eb..31ff13f428da36 100644
--- a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
@@ -12,78 +12,73 @@ 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();
-}
+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(IsCLibraryFunctionTest, AcceptsGlobal) {
- const FunctionDecl *Result;
- ASSERT_TRUE(extractFunctionDecl(R"cpp(void fun();)cpp", Result));
- EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, AcceptsGlobal) {
+ ASSERT_TRUE(buildAST(R"cpp(void fun();)cpp"));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
- const FunctionDecl *Result;
- ASSERT_TRUE(
- extractFunctionDecl(R"cpp(extern "C" { void fun(); })cpp", Result));
- EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
+ ASSERT_TRUE(buildAST(R"cpp(extern "C" { void fun(); })cpp"));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, RejectsNoInlineNoExternalLinkage) {
+TEST_F(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));
+ ASSERT_TRUE(buildAST(R"cpp(static void fun();)cpp"));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, RejectsAnonymousNamespace) {
- const FunctionDecl *Result;
- ASSERT_TRUE(
- extractFunctionDecl(R"cpp(namespace { void fun(); })cpp", Result));
- EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, RejectsAnonymousNamespace) {
+ ASSERT_TRUE(buildAST(R"cpp(namespace { void fun(); })cpp"));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, AcceptsStdNamespace) {
- const FunctionDecl *Result;
- ASSERT_TRUE(
- extractFunctionDecl(R"cpp(namespace std { void fun(); })cpp", Result));
- EXPECT_TRUE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, AcceptsStdNamespace) {
+ ASSERT_TRUE(buildAST(R"cpp(namespace std { void fun(); })cpp"));
+ EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, RejectsOtherNamespaces) {
- const FunctionDecl *Result;
- ASSERT_TRUE(
- extractFunctionDecl(R"cpp(namespace stdx { void fun(); })cpp", Result));
- EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, RejectsOtherNamespaces) {
+ ASSERT_TRUE(buildAST(R"cpp(namespace stdx { void fun(); })cpp"));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, RejectsClassStatic) {
- const FunctionDecl *Result;
- ASSERT_TRUE(
- extractFunctionDecl(R"cpp(class A { static void fun(); };)cpp", Result));
- EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, RejectsClassStatic) {
+ ASSERT_TRUE(buildAST(R"cpp(class A { static void fun(); };)cpp"));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
-TEST(IsCLibraryFunctionTest, RejectsClassMember) {
- const FunctionDecl *Result;
- ASSERT_TRUE(extractFunctionDecl(R"cpp(class A { void fun(); };)cpp", Result));
- EXPECT_FALSE(CheckerContext::isCLibraryFunction(Result));
+TEST_F(IsCLibraryFunctionTest, RejectsClassMember) {
+ ASSERT_TRUE(buildAST(R"cpp(class A { void fun(); };)cpp"));
+ EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
}
>From 0858f5e88afaf2f6fb6f8b696e877b3bb7470f72 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Tue, 12 Mar 2024 19:26:58 +0100
Subject: [PATCH 3/3] clang-format test file
---
clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp b/clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
index 31ff13f428da36..a5ec26bdb13068 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()));
}
More information about the llvm-commits
mailing list