Skip to content

Commit

Permalink
[analyzer] Accept C library functions from the std namespace (#84469)
Browse files Browse the repository at this point in the history
Previously, the function `isCLibraryFunction()` and logic relying on it
only accepted functions that are declared directly within a TU (i.e. not
in a namespace or a class). However C++ headers like <cstdlib> declare
many C standard library functions within the namespace `std`, so this
commit ensures that functions within the namespace `std` are also
accepted.

After this commit it will be possible to match functions like `malloc`
or `free` with `CallDescription::Mode::CLibrary`.

---------

Co-authored-by: Balazs Benics <benicsbalazs@gmail.com>
  • Loading branch information
NagyDonat and steakhal committed Mar 12, 2024
1 parent b5a16b6 commit 80ab823
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions clang/unittests/StaticAnalyzer/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ add_clang_unittest(StaticAnalysisTests
CallEventTest.cpp
ConflictingEvalCallsTest.cpp
FalsePositiveRefutationBRVisitorTest.cpp
IsCLibraryFunctionTest.cpp
NoStateChangeFuncVisitorTest.cpp
ParamRegionTest.cpp
RangeSetTest.cpp
Expand Down
89 changes: 89 additions & 0 deletions clang/unittests/StaticAnalyzer/IsCLibraryFunctionTest.cpp
Original file line number Diff line number Diff line change
@@ -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));
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ unittest("StaticAnalysisTests") {
"CallEventTest.cpp",
"ConflictingEvalCallsTest.cpp",
"FalsePositiveRefutationBRVisitorTest.cpp",
"IsCLibraryFunctionTest.cpp",
"NoStateChangeFuncVisitorTest.cpp",
"ParamRegionTest.cpp",
"RangeSetTest.cpp",
Expand Down

0 comments on commit 80ab823

Please sign in to comment.