Skip to content

Commit e48d5a8

Browse files
NagyDonatsteakhal
andcommitted
Reapply "[analyzer] Accept C library functions from the std namespace"
This reapplies f32b04d4ea91ad1018c25a1d4178cc4392d34968i, after fixing the use-after-free of ASTUnit in the unittest. #84469 (comment) Co-authored-by: Balazs Benics <[email protected]>
1 parent 9fa8660 commit e48d5a8

File tree

5 files changed

+93
-9
lines changed

5 files changed

+93
-9
lines changed

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,8 @@ class CallDescription {
4141
/// - We also accept calls where the number of arguments or parameters is
4242
/// greater than the specified value.
4343
/// For the exact heuristics, see CheckerContext::isCLibraryFunction().
44-
/// Note that functions whose declaration context is not a TU (e.g.
45-
/// methods, functions in namespaces) are not accepted as C library
46-
/// functions.
47-
/// FIXME: If I understand it correctly, this discards calls where C++ code
48-
/// refers a C library function through the namespace `std::` via headers
49-
/// like <cstdlib>.
44+
/// (This mode only matches functions that are declared either directly
45+
/// within a TU or in the namespace `std`.)
5046
CLibrary,
5147

5248
/// Matches "simple" functions that are not methods. (Static methods are

clang/lib/StaticAnalyzer/Core/CheckerContext.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD,
8787
if (!II)
8888
return false;
8989

90-
// Look through 'extern "C"' and anything similar invented in the future.
91-
// If this function is not in TU directly, it is not a C library function.
92-
if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
90+
// C library functions are either declared directly within a TU (the common
91+
// case) or they are accessed through the namespace `std` (when they are used
92+
// in C++ via headers like <cstdlib>).
93+
const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
94+
if (!(DC->isTranslationUnit() || DC->isStdNamespace()))
9395
return false;
9496

9597
// If this function is not externally visible, it is not a C library function.

clang/unittests/StaticAnalyzer/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ add_clang_unittest(StaticAnalysisTests
1111
CallEventTest.cpp
1212
ConflictingEvalCallsTest.cpp
1313
FalsePositiveRefutationBRVisitorTest.cpp
14+
IsCLibraryFunctionTest.cpp
1415
NoStateChangeFuncVisitorTest.cpp
1516
ParamRegionTest.cpp
1617
RangeSetTest.cpp
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
#include "clang/ASTMatchers/ASTMatchFinder.h"
2+
#include "clang/ASTMatchers/ASTMatchers.h"
3+
#include "clang/Analysis/AnalysisDeclContext.h"
4+
#include "clang/Frontend/ASTUnit.h"
5+
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
6+
#include "clang/Tooling/Tooling.h"
7+
#include "gtest/gtest.h"
8+
9+
#include <memory>
10+
11+
using namespace clang;
12+
using namespace ento;
13+
using namespace ast_matchers;
14+
15+
class IsCLibraryFunctionTest : public testing::Test {
16+
public:
17+
const FunctionDecl *getFunctionDecl() const { return Result; }
18+
19+
testing::AssertionResult buildAST(StringRef Code) {
20+
ASTUnit = tooling::buildASTFromCode(Code);
21+
if (!ASTUnit)
22+
return testing::AssertionFailure() << "AST construction failed";
23+
24+
ASTContext &Context = ASTUnit->getASTContext();
25+
if (Context.getDiagnostics().hasErrorOccurred())
26+
return testing::AssertionFailure() << "Compilation error";
27+
28+
auto Matches = ast_matchers::match(functionDecl().bind("fn"), Context);
29+
if (Matches.empty())
30+
return testing::AssertionFailure() << "No function declaration found";
31+
32+
if (Matches.size() > 1)
33+
return testing::AssertionFailure()
34+
<< "Multiple function declarations found";
35+
36+
Result = Matches[0].getNodeAs<FunctionDecl>("fn");
37+
return testing::AssertionSuccess();
38+
}
39+
40+
private:
41+
std::unique_ptr<ASTUnit> ASTUnit;
42+
const FunctionDecl *Result = nullptr;
43+
};
44+
45+
TEST_F(IsCLibraryFunctionTest, AcceptsGlobal) {
46+
ASSERT_TRUE(buildAST(R"cpp(void fun();)cpp"));
47+
EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
48+
}
49+
50+
TEST_F(IsCLibraryFunctionTest, AcceptsExternCGlobal) {
51+
ASSERT_TRUE(buildAST(R"cpp(extern "C" { void fun(); })cpp"));
52+
EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
53+
}
54+
55+
TEST_F(IsCLibraryFunctionTest, RejectsNoInlineNoExternalLinkage) {
56+
// Functions that are neither inlined nor externally visible cannot be C library functions.
57+
ASSERT_TRUE(buildAST(R"cpp(static void fun();)cpp"));
58+
EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
59+
}
60+
61+
TEST_F(IsCLibraryFunctionTest, RejectsAnonymousNamespace) {
62+
ASSERT_TRUE(buildAST(R"cpp(namespace { void fun(); })cpp"));
63+
EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
64+
}
65+
66+
TEST_F(IsCLibraryFunctionTest, AcceptsStdNamespace) {
67+
ASSERT_TRUE(buildAST(R"cpp(namespace std { void fun(); })cpp"));
68+
EXPECT_TRUE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
69+
}
70+
71+
TEST_F(IsCLibraryFunctionTest, RejectsOtherNamespaces) {
72+
ASSERT_TRUE(buildAST(R"cpp(namespace stdx { void fun(); })cpp"));
73+
EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
74+
}
75+
76+
TEST_F(IsCLibraryFunctionTest, RejectsClassStatic) {
77+
ASSERT_TRUE(buildAST(R"cpp(class A { static void fun(); };)cpp"));
78+
EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
79+
}
80+
81+
TEST_F(IsCLibraryFunctionTest, RejectsClassMember) {
82+
ASSERT_TRUE(buildAST(R"cpp(class A { void fun(); };)cpp"));
83+
EXPECT_FALSE(CheckerContext::isCLibraryFunction(getFunctionDecl()));
84+
}

llvm/utils/gn/secondary/clang/unittests/StaticAnalyzer/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ unittest("StaticAnalysisTests") {
1919
"CallEventTest.cpp",
2020
"ConflictingEvalCallsTest.cpp",
2121
"FalsePositiveRefutationBRVisitorTest.cpp",
22+
"IsCLibraryFunctionTest.cpp",
2223
"NoStateChangeFuncVisitorTest.cpp",
2324
"ParamRegionTest.cpp",
2425
"RangeSetTest.cpp",

0 commit comments

Comments
 (0)