[clang] [analyzer] Refactor CallDescription match mode (NFC) (PR #83432)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 29 07:23:16 PST 2024
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/83432 at github.com>
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/83432
>From 7a72174f9df2211febf789941ed0adb75ebacc89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 22 Feb 2024 18:09:15 +0100
Subject: [PATCH 1/2] [analyzer] Refactor CallDescription match mode (NFC)
The class `CallDescription` is used to define patterns that are used for
matching `CallEvent`s. For example, a `CallEvent{{"std", "find_if"}, 3}`
matches a call to `std::find_if` with 3 arguments.
However, these patterns are somewhat fuzzy, so this pattern could also
match something like `std::__1::find_if` (with an additional namespace
layer), or, unfortunately, a `CallDescription` for the well-known
function `free()` can match a C++ method named `free()`:
https://github.com/llvm/llvm-project/issues/81597
To prevent this kind of ambiguity this commit introduces the enum
`CallDescription::Mode` which can limit the pattern matching to
non-method function calls (or method calls etc.). After this NFC change,
one or more follow-up commits will apply the right pattern matching
modes in the ~30 checkers that use `CallDescription`s.
Note that `CallDescription` previously had a `Flags` field which had
only two supported values:
- CDF_None was the default "match anything" mode,
- CDF_MaybeBuiltin was a "match only C library functions and accept
some inexact matches" mode.
This commit preserves `CDF_MaybeBuiltin` under the more descriptive name
`CallDescription::Mode::CLibrary` (or `CDM::CLibrary`).
Instead of this "Flags" model I'm switching to a plain enumeration
becasue I don't think that there is a natural usecase to combine the
different matching modes. (Except for the default "match anything" mode,
which is currently kept for compatibility, but will be phased out in the
follow-up commits.)
---
.../Core/PathSensitive/CallDescription.h | 72 ++++++++++++++-----
.../Checkers/CStringChecker.cpp | 64 ++++++++---------
.../Checkers/GenericTaintChecker.cpp | 44 ++++++------
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 8 +--
.../StaticAnalyzer/Core/CallDescription.cpp | 19 +++--
.../StaticAnalyzer/CallDescriptionTest.cpp | 16 ++---
6 files changed, 133 insertions(+), 90 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index 965838a4408c23..d4985238a3c73b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -27,20 +27,46 @@ class IdentifierInfo;
namespace clang {
namespace ento {
-
-enum CallDescriptionFlags : unsigned {
- CDF_None = 0,
-
- /// Describes a C standard function that is sometimes implemented as a macro
- /// that expands to a compiler builtin with some __builtin prefix.
- /// The builtin may as well have a few extra arguments on top of the requested
- /// number of arguments.
- CDF_MaybeBuiltin = 1 << 0,
-};
-
-/// This class represents a description of a function call using the number of
-/// arguments and the name of the function.
+/// A `CallDescription` is a pattern that can be used to _match_ calls
+/// based on the qualified name and the argument/parameter counts.
class CallDescription {
+public:
+ enum class Mode {
+ /// Match calls to functions from the C standard library. On some platforms
+ /// some functions may be implemented as macros that expand to calls to
+ /// built-in variants of the given functions, so in this mode we use some
+ /// heuristics to recognize these implementation-defined variants:
+ /// - We also accept calls where the name is derived from the specified
+ /// name by adding "__builtin" or similar prefixes/suffixes.
+ /// - 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>.
+ CLibrary,
+
+ /// Matches "simple" functions that are not methods. (Static methods are
+ /// methods.)
+ SimpleFunc,
+
+ /// Matches a C+ method (may be static, may be virtual, may be an
+ /// overloaded operator, a constructor or a destructor).
+ CXXMethod,
+
+ /// Match any CallEvent that is not an ObjCMethodCall.
+ /// FIXME: Previously this was the default behavior of CallDescription, but
+ /// its use should be replaced by a more specific mode almost everywhere.
+ Unspecified,
+
+ /// FIXME: Add support for ObjCMethodCall events (I'm not adding it because
+ /// I'm not familiar with Objective-C). Note that currently an early return
+ /// in `bool matches(const CallEvent &Call) const;` discards all
+ /// Objective-C method calls.
+ };
+private:
friend class CallEvent;
using MaybeCount = std::optional<unsigned>;
@@ -50,20 +76,26 @@ class CallDescription {
std::vector<std::string> QualifiedName;
MaybeCount RequiredArgs;
MaybeCount RequiredParams;
- int Flags;
+ Mode MatchAs;
public:
/// Constructs a CallDescription object.
///
+ /// @param MatchAs Specifies the kind of the call that should be matched.
+ ///
/// @param QualifiedName The list of the name qualifiers of the function that
/// will be matched. The user is allowed to skip any of the qualifiers.
/// For example, {"std", "basic_string", "c_str"} would match both
/// std::basic_string<...>::c_str() and std::__1::basic_string<...>::c_str().
///
- /// @param RequiredArgs The number of arguments that is expected to match a
- /// call. Omit this parameter to match every occurrence of call with a given
- /// name regardless the number of arguments.
- CallDescription(CallDescriptionFlags Flags, ArrayRef<StringRef> QualifiedName,
+ /// @param RequiredArgs The expected number of arguments that are passed to
+ /// the function. Omit this parameter (or pass std::nullopt) to match every
+ /// occurrence without checking the argument count in the call.
+ ///
+ /// @param RequiredParams The expected number of parameters in the function
+ /// definition that is called. Omit this parameter to match every occurrence
+ /// without checking the parameter count in the definition.
+ CallDescription(Mode MatchAs, ArrayRef<StringRef> QualifiedName,
MaybeCount RequiredArgs = std::nullopt,
MaybeCount RequiredParams = std::nullopt);
@@ -222,6 +254,10 @@ template <typename T> class CallDescriptionMap {
}
};
+/// Enumerators of this enum class are used to construct CallDescription
+/// objects; in that context the fully qualified name is needlessly verbose.
+using CDM = CallDescription::Mode;
+
/// An immutable set of CallDescriptions.
/// Checkers can efficiently decide if a given CallEvent matches any
/// CallDescription in the set.
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index b7b64c3da4f6c8..cd49a44a748a9c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -124,48 +124,48 @@ class CStringChecker : public Checker< eval::Call,
const CallEvent &)>;
CallDescriptionMap<FnCheck> Callbacks = {
- {{CDF_MaybeBuiltin, {"memcpy"}, 3},
+ {{CDM::CLibrary, {"memcpy"}, 3},
std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Regular)},
- {{CDF_MaybeBuiltin, {"wmemcpy"}, 3},
+ {{CDM::CLibrary, {"wmemcpy"}, 3},
std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, CK_Wide)},
- {{CDF_MaybeBuiltin, {"mempcpy"}, 3},
+ {{CDM::CLibrary, {"mempcpy"}, 3},
std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Regular)},
- {{CDF_None, {"wmempcpy"}, 3},
+ {{CDM::Unspecified, {"wmempcpy"}, 3},
std::bind(&CStringChecker::evalMempcpy, _1, _2, _3, CK_Wide)},
- {{CDF_MaybeBuiltin, {"memcmp"}, 3},
+ {{CDM::CLibrary, {"memcmp"}, 3},
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
- {{CDF_MaybeBuiltin, {"wmemcmp"}, 3},
+ {{CDM::CLibrary, {"wmemcmp"}, 3},
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Wide)},
- {{CDF_MaybeBuiltin, {"memmove"}, 3},
+ {{CDM::CLibrary, {"memmove"}, 3},
std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Regular)},
- {{CDF_MaybeBuiltin, {"wmemmove"}, 3},
+ {{CDM::CLibrary, {"wmemmove"}, 3},
std::bind(&CStringChecker::evalMemmove, _1, _2, _3, CK_Wide)},
- {{CDF_MaybeBuiltin, {"memset"}, 3}, &CStringChecker::evalMemset},
- {{CDF_MaybeBuiltin, {"explicit_memset"}, 3}, &CStringChecker::evalMemset},
- {{CDF_MaybeBuiltin, {"strcpy"}, 2}, &CStringChecker::evalStrcpy},
- {{CDF_MaybeBuiltin, {"strncpy"}, 3}, &CStringChecker::evalStrncpy},
- {{CDF_MaybeBuiltin, {"stpcpy"}, 2}, &CStringChecker::evalStpcpy},
- {{CDF_MaybeBuiltin, {"strlcpy"}, 3}, &CStringChecker::evalStrlcpy},
- {{CDF_MaybeBuiltin, {"strcat"}, 2}, &CStringChecker::evalStrcat},
- {{CDF_MaybeBuiltin, {"strncat"}, 3}, &CStringChecker::evalStrncat},
- {{CDF_MaybeBuiltin, {"strlcat"}, 3}, &CStringChecker::evalStrlcat},
- {{CDF_MaybeBuiltin, {"strlen"}, 1}, &CStringChecker::evalstrLength},
- {{CDF_MaybeBuiltin, {"wcslen"}, 1}, &CStringChecker::evalstrLength},
- {{CDF_MaybeBuiltin, {"strnlen"}, 2}, &CStringChecker::evalstrnLength},
- {{CDF_MaybeBuiltin, {"wcsnlen"}, 2}, &CStringChecker::evalstrnLength},
- {{CDF_MaybeBuiltin, {"strcmp"}, 2}, &CStringChecker::evalStrcmp},
- {{CDF_MaybeBuiltin, {"strncmp"}, 3}, &CStringChecker::evalStrncmp},
- {{CDF_MaybeBuiltin, {"strcasecmp"}, 2}, &CStringChecker::evalStrcasecmp},
- {{CDF_MaybeBuiltin, {"strncasecmp"}, 3},
+ {{CDM::CLibrary, {"memset"}, 3}, &CStringChecker::evalMemset},
+ {{CDM::CLibrary, {"explicit_memset"}, 3}, &CStringChecker::evalMemset},
+ {{CDM::CLibrary, {"strcpy"}, 2}, &CStringChecker::evalStrcpy},
+ {{CDM::CLibrary, {"strncpy"}, 3}, &CStringChecker::evalStrncpy},
+ {{CDM::CLibrary, {"stpcpy"}, 2}, &CStringChecker::evalStpcpy},
+ {{CDM::CLibrary, {"strlcpy"}, 3}, &CStringChecker::evalStrlcpy},
+ {{CDM::CLibrary, {"strcat"}, 2}, &CStringChecker::evalStrcat},
+ {{CDM::CLibrary, {"strncat"}, 3}, &CStringChecker::evalStrncat},
+ {{CDM::CLibrary, {"strlcat"}, 3}, &CStringChecker::evalStrlcat},
+ {{CDM::CLibrary, {"strlen"}, 1}, &CStringChecker::evalstrLength},
+ {{CDM::CLibrary, {"wcslen"}, 1}, &CStringChecker::evalstrLength},
+ {{CDM::CLibrary, {"strnlen"}, 2}, &CStringChecker::evalstrnLength},
+ {{CDM::CLibrary, {"wcsnlen"}, 2}, &CStringChecker::evalstrnLength},
+ {{CDM::CLibrary, {"strcmp"}, 2}, &CStringChecker::evalStrcmp},
+ {{CDM::CLibrary, {"strncmp"}, 3}, &CStringChecker::evalStrncmp},
+ {{CDM::CLibrary, {"strcasecmp"}, 2}, &CStringChecker::evalStrcasecmp},
+ {{CDM::CLibrary, {"strncasecmp"}, 3},
&CStringChecker::evalStrncasecmp},
- {{CDF_MaybeBuiltin, {"strsep"}, 2}, &CStringChecker::evalStrsep},
- {{CDF_MaybeBuiltin, {"bcopy"}, 3}, &CStringChecker::evalBcopy},
- {{CDF_MaybeBuiltin, {"bcmp"}, 3},
+ {{CDM::CLibrary, {"strsep"}, 2}, &CStringChecker::evalStrsep},
+ {{CDM::CLibrary, {"bcopy"}, 3}, &CStringChecker::evalBcopy},
+ {{CDM::CLibrary, {"bcmp"}, 3},
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
- {{CDF_MaybeBuiltin, {"bzero"}, 2}, &CStringChecker::evalBzero},
- {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
- {{CDF_MaybeBuiltin, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
- {{CDF_MaybeBuiltin, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
+ {{CDM::CLibrary, {"bzero"}, 2}, &CStringChecker::evalBzero},
+ {{CDM::CLibrary, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
+ {{CDM::CLibrary, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
+ {{CDM::CLibrary, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
};
// These require a bit of special handling.
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 4ceaf933d0bfc8..55e5877d694b73 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -718,28 +718,28 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
{{{"isupper"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
{{{"isxdigit"}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {BI.getName(Builtin::BIstrncat)}},
+ {{CDM::CLibrary, {BI.getName(Builtin::BIstrncat)}},
TR::Prop({{1, 2}}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {BI.getName(Builtin::BIstrlcpy)}},
+ {{CDM::CLibrary, {BI.getName(Builtin::BIstrlcpy)}},
TR::Prop({{1, 2}}, {{0}})},
- {{CDF_MaybeBuiltin, {BI.getName(Builtin::BIstrlcat)}},
+ {{CDM::CLibrary, {BI.getName(Builtin::BIstrlcat)}},
TR::Prop({{1, 2}}, {{0}})},
- {{CDF_MaybeBuiltin, {{"snprintf"}}},
+ {{CDM::CLibrary, {{"snprintf"}}},
TR::Prop({{1}, 3}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"sprintf"}}},
+ {{CDM::CLibrary, {{"sprintf"}}},
TR::Prop({{1}, 2}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"strcpy"}}},
+ {{CDM::CLibrary, {{"strcpy"}}},
TR::Prop({{1}}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"stpcpy"}}},
+ {{CDM::CLibrary, {{"stpcpy"}}},
TR::Prop({{1}}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"strcat"}}},
+ {{CDM::CLibrary, {{"strcat"}}},
TR::Prop({{1}}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"wcsncat"}}},
+ {{CDM::CLibrary, {{"wcsncat"}}},
TR::Prop({{1}}, {{0, ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"strdup"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"strdupa"}}},
+ {{CDM::CLibrary, {{"strdup"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
+ {{CDM::CLibrary, {{"strdupa"}}},
TR::Prop({{0}}, {{ReturnValueIndex}})},
- {{CDF_MaybeBuiltin, {{"wcsdup"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
+ {{CDM::CLibrary, {{"wcsdup"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
// Sinks
{{{"system"}}, TR::Sink({{0}}, MsgSanitizeSystemArgs)},
@@ -753,31 +753,31 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
{{{"execvp"}}, TR::Sink({{0, 1}}, MsgSanitizeSystemArgs)},
{{{"execvpe"}}, TR::Sink({{0, 1, 2}}, MsgSanitizeSystemArgs)},
{{{"dlopen"}}, TR::Sink({{0}}, MsgSanitizeSystemArgs)},
- {{CDF_MaybeBuiltin, {{"malloc"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {{"calloc"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {{"alloca"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {{"memccpy"}}},
+ {{CDM::CLibrary, {{"malloc"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
+ {{CDM::CLibrary, {{"calloc"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
+ {{CDM::CLibrary, {{"alloca"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
+ {{CDM::CLibrary, {{"memccpy"}}},
TR::Sink({{3}}, MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {{"realloc"}}},
+ {{CDM::CLibrary, {{"realloc"}}},
TR::Sink({{1}}, MsgTaintedBufferSize)},
{{{{"setproctitle"}}}, TR::Sink({{0}, 1}, MsgUncontrolledFormatString)},
{{{{"setproctitle_fast"}}},
TR::Sink({{0}, 1}, MsgUncontrolledFormatString)},
// SinkProps
- {{CDF_MaybeBuiltin, BI.getName(Builtin::BImemcpy)},
+ {{CDM::CLibrary, BI.getName(Builtin::BImemcpy)},
TR::SinkProp({{2}}, {{1, 2}}, {{0, ReturnValueIndex}},
MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {BI.getName(Builtin::BImemmove)}},
+ {{CDM::CLibrary, {BI.getName(Builtin::BImemmove)}},
TR::SinkProp({{2}}, {{1, 2}}, {{0, ReturnValueIndex}},
MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {BI.getName(Builtin::BIstrncpy)}},
+ {{CDM::CLibrary, {BI.getName(Builtin::BIstrncpy)}},
TR::SinkProp({{2}}, {{1, 2}}, {{0, ReturnValueIndex}},
MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {BI.getName(Builtin::BIstrndup)}},
+ {{CDM::CLibrary, {BI.getName(Builtin::BIstrndup)}},
TR::SinkProp({{1}}, {{0, 1}}, {{ReturnValueIndex}},
MsgTaintedBufferSize)},
- {{CDF_MaybeBuiltin, {{"bcopy"}}},
+ {{CDM::CLibrary, {{"bcopy"}}},
TR::SinkProp({{2}}, {{0, 2}}, {{1}}, MsgTaintedBufferSize)}};
// `getenv` returns taint only in untrusted environments.
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 79ab05f2c7866a..b27ca6a4495974 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -410,13 +410,13 @@ class MallocChecker
{{{"malloc"}, 3}, &MallocChecker::checkKernelMalloc},
{{{"calloc"}, 2}, &MallocChecker::checkCalloc},
{{{"valloc"}, 1}, &MallocChecker::checkBasicAlloc},
- {{CDF_MaybeBuiltin, {"strndup"}, 2}, &MallocChecker::checkStrdup},
- {{CDF_MaybeBuiltin, {"strdup"}, 1}, &MallocChecker::checkStrdup},
+ {{CDM::CLibrary, {"strndup"}, 2}, &MallocChecker::checkStrdup},
+ {{CDM::CLibrary, {"strdup"}, 1}, &MallocChecker::checkStrdup},
{{{"_strdup"}, 1}, &MallocChecker::checkStrdup},
{{{"kmalloc"}, 2}, &MallocChecker::checkKernelMalloc},
{{{"if_nameindex"}, 1}, &MallocChecker::checkIfNameIndex},
- {{CDF_MaybeBuiltin, {"wcsdup"}, 1}, &MallocChecker::checkStrdup},
- {{CDF_MaybeBuiltin, {"_wcsdup"}, 1}, &MallocChecker::checkStrdup},
+ {{CDM::CLibrary, {"wcsdup"}, 1}, &MallocChecker::checkStrdup},
+ {{CDM::CLibrary, {"_wcsdup"}, 1}, &MallocChecker::checkStrdup},
{{{"g_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
{{{"g_malloc0"}, 1}, &MallocChecker::checkGMalloc0},
{{{"g_try_malloc"}, 1}, &MallocChecker::checkBasicAlloc},
diff --git a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
index 94b2fde0a6f395..f1b632ea3d53bb 100644
--- a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
@@ -35,13 +35,13 @@ static MaybeCount readRequiredParams(MaybeCount RequiredArgs,
return std::nullopt;
}
-ento::CallDescription::CallDescription(CallDescriptionFlags Flags,
+ento::CallDescription::CallDescription(Mode MatchAs,
ArrayRef<StringRef> QualifiedName,
MaybeCount RequiredArgs /*= None*/,
MaybeCount RequiredParams /*= None*/)
: RequiredArgs(RequiredArgs),
RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
- Flags(Flags) {
+ MatchAs(MatchAs) {
assert(!QualifiedName.empty());
this->QualifiedName.reserve(QualifiedName.size());
llvm::transform(QualifiedName, std::back_inserter(this->QualifiedName),
@@ -52,7 +52,7 @@ ento::CallDescription::CallDescription(CallDescriptionFlags Flags,
ento::CallDescription::CallDescription(ArrayRef<StringRef> QualifiedName,
MaybeCount RequiredArgs /*= None*/,
MaybeCount RequiredParams /*= None*/)
- : CallDescription(CDF_None, QualifiedName, RequiredArgs, RequiredParams) {}
+ : CallDescription(Mode::Unspecified, QualifiedName, RequiredArgs, RequiredParams) {}
bool ento::CallDescription::matches(const CallEvent &Call) const {
// FIXME: Add ObjC Message support.
@@ -74,14 +74,21 @@ bool ento::CallDescription::matchesAsWritten(const CallExpr &CE) const {
return matchesImpl(FD, CE.getNumArgs(), FD->param_size());
}
-bool ento::CallDescription::matchesImpl(const FunctionDecl *Callee,
+bool ento::CallDescription::matchesImpl(const FunctionDecl *FD,
size_t ArgCount,
size_t ParamCount) const {
- const auto *FD = Callee;
if (!FD)
return false;
- if (Flags & CDF_MaybeBuiltin) {
+ const bool isMethod = isa<CXXMethodDecl>(FD);
+
+ if (MatchAs == Mode::SimpleFunc && isMethod)
+ return false;
+
+ if (MatchAs == Mode::CXXMethod && !isMethod)
+ return false;
+
+ if (MatchAs == Mode::CLibrary) {
return CheckerContext::isCLibraryFunction(FD, getFunctionName()) &&
(!RequiredArgs || *RequiredArgs <= ArgCount) &&
(!RequiredParams || *RequiredParams <= ParamCount);
diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
index e8b237b891ca8b..fa1cf72fd75135 100644
--- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -488,11 +488,11 @@ TEST(CallDescription, NegativeMatchQualifiedNames) {
}
TEST(CallDescription, MatchBuiltins) {
- // Test CDF_MaybeBuiltin - a flag that allows matching weird builtins.
+ // Test CDM::CLibrary - a flag that allows matching weird builtins.
EXPECT_TRUE(tooling::runToolOnCode(
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
{{{{"memset"}, 3}, false},
- {{CDF_MaybeBuiltin, {"memset"}, 3}, true}})),
+ {{CDM::CLibrary, {"memset"}, 3}, true}})),
"void foo() {"
" int x;"
" __builtin___memset_chk(&x, 0, sizeof(x),"
@@ -503,8 +503,8 @@ TEST(CallDescription, MatchBuiltins) {
SCOPED_TRACE("multiple similar builtins");
EXPECT_TRUE(tooling::runToolOnCode(
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{CDF_MaybeBuiltin, {"memcpy"}, 3}, false},
- {{CDF_MaybeBuiltin, {"wmemcpy"}, 3}, true}})),
+ {{{CDM::CLibrary, {"memcpy"}, 3}, false},
+ {{CDM::CLibrary, {"wmemcpy"}, 3}, true}})),
R"(void foo(wchar_t *x, wchar_t *y) {
__builtin_wmemcpy(x, y, sizeof(wchar_t));
})"));
@@ -513,8 +513,8 @@ TEST(CallDescription, MatchBuiltins) {
SCOPED_TRACE("multiple similar builtins reversed order");
EXPECT_TRUE(tooling::runToolOnCode(
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{CDF_MaybeBuiltin, {"wmemcpy"}, 3}, true},
- {{CDF_MaybeBuiltin, {"memcpy"}, 3}, false}})),
+ {{{CDM::CLibrary, {"wmemcpy"}, 3}, true},
+ {{CDM::CLibrary, {"memcpy"}, 3}, false}})),
R"(void foo(wchar_t *x, wchar_t *y) {
__builtin_wmemcpy(x, y, sizeof(wchar_t));
})"));
@@ -523,7 +523,7 @@ TEST(CallDescription, MatchBuiltins) {
SCOPED_TRACE("lookbehind and lookahead mismatches");
EXPECT_TRUE(tooling::runToolOnCode(
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{CDF_MaybeBuiltin, {"func"}}, false}})),
+ {{{CDM::CLibrary, {"func"}}, false}})),
R"(
void funcXXX();
void XXXfunc();
@@ -538,7 +538,7 @@ TEST(CallDescription, MatchBuiltins) {
SCOPED_TRACE("lookbehind and lookahead matches");
EXPECT_TRUE(tooling::runToolOnCode(
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{CDF_MaybeBuiltin, {"func"}}, true}})),
+ {{{CDM::CLibrary, {"func"}}, true}})),
R"(
void func();
void func_XXX();
>From 4cf5db04f1b9400e4a51484dd494c5d9f9f23572 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 29 Feb 2024 16:23:03 +0100
Subject: [PATCH 2/2] Satisfy git-clang-format
---
.../Core/PathSensitive/CallDescription.h | 4 +++-
.../StaticAnalyzer/Checkers/CStringChecker.cpp | 3 +--
.../Checkers/GenericTaintChecker.cpp | 18 ++++++------------
.../StaticAnalyzer/Core/CallDescription.cpp | 6 +++---
.../StaticAnalyzer/CallDescriptionTest.cpp | 11 +++++------
5 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index d4985238a3c73b..45e5f08405f84f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -42,7 +42,8 @@ class CallDescription {
/// 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.
+ /// 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>.
@@ -66,6 +67,7 @@ class CallDescription {
/// in `bool matches(const CallEvent &Call) const;` discards all
/// Objective-C method calls.
};
+
private:
friend class CallEvent;
using MaybeCount = std::optional<unsigned>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index cd49a44a748a9c..59be236ca1c769 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -156,8 +156,7 @@ class CStringChecker : public Checker< eval::Call,
{{CDM::CLibrary, {"strcmp"}, 2}, &CStringChecker::evalStrcmp},
{{CDM::CLibrary, {"strncmp"}, 3}, &CStringChecker::evalStrncmp},
{{CDM::CLibrary, {"strcasecmp"}, 2}, &CStringChecker::evalStrcasecmp},
- {{CDM::CLibrary, {"strncasecmp"}, 3},
- &CStringChecker::evalStrncasecmp},
+ {{CDM::CLibrary, {"strncasecmp"}, 3}, &CStringChecker::evalStrncasecmp},
{{CDM::CLibrary, {"strsep"}, 2}, &CStringChecker::evalStrsep},
{{CDM::CLibrary, {"bcopy"}, 3}, &CStringChecker::evalBcopy},
{{CDM::CLibrary, {"bcmp"}, 3},
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 55e5877d694b73..4c48fdf498f7fc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -728,17 +728,13 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
TR::Prop({{1}, 3}, {{0, ReturnValueIndex}})},
{{CDM::CLibrary, {{"sprintf"}}},
TR::Prop({{1}, 2}, {{0, ReturnValueIndex}})},
- {{CDM::CLibrary, {{"strcpy"}}},
- TR::Prop({{1}}, {{0, ReturnValueIndex}})},
- {{CDM::CLibrary, {{"stpcpy"}}},
- TR::Prop({{1}}, {{0, ReturnValueIndex}})},
- {{CDM::CLibrary, {{"strcat"}}},
- TR::Prop({{1}}, {{0, ReturnValueIndex}})},
+ {{CDM::CLibrary, {{"strcpy"}}}, TR::Prop({{1}}, {{0, ReturnValueIndex}})},
+ {{CDM::CLibrary, {{"stpcpy"}}}, TR::Prop({{1}}, {{0, ReturnValueIndex}})},
+ {{CDM::CLibrary, {{"strcat"}}}, TR::Prop({{1}}, {{0, ReturnValueIndex}})},
{{CDM::CLibrary, {{"wcsncat"}}},
TR::Prop({{1}}, {{0, ReturnValueIndex}})},
{{CDM::CLibrary, {{"strdup"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
- {{CDM::CLibrary, {{"strdupa"}}},
- TR::Prop({{0}}, {{ReturnValueIndex}})},
+ {{CDM::CLibrary, {{"strdupa"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
{{CDM::CLibrary, {{"wcsdup"}}}, TR::Prop({{0}}, {{ReturnValueIndex}})},
// Sinks
@@ -756,10 +752,8 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
{{CDM::CLibrary, {{"malloc"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
{{CDM::CLibrary, {{"calloc"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
{{CDM::CLibrary, {{"alloca"}}}, TR::Sink({{0}}, MsgTaintedBufferSize)},
- {{CDM::CLibrary, {{"memccpy"}}},
- TR::Sink({{3}}, MsgTaintedBufferSize)},
- {{CDM::CLibrary, {{"realloc"}}},
- TR::Sink({{1}}, MsgTaintedBufferSize)},
+ {{CDM::CLibrary, {{"memccpy"}}}, TR::Sink({{3}}, MsgTaintedBufferSize)},
+ {{CDM::CLibrary, {{"realloc"}}}, TR::Sink({{1}}, MsgTaintedBufferSize)},
{{{{"setproctitle"}}}, TR::Sink({{0}, 1}, MsgUncontrolledFormatString)},
{{{{"setproctitle_fast"}}},
TR::Sink({{0}, 1}, MsgUncontrolledFormatString)},
diff --git a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
index f1b632ea3d53bb..459e854cd44d9a 100644
--- a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
@@ -52,7 +52,8 @@ ento::CallDescription::CallDescription(Mode MatchAs,
ento::CallDescription::CallDescription(ArrayRef<StringRef> QualifiedName,
MaybeCount RequiredArgs /*= None*/,
MaybeCount RequiredParams /*= None*/)
- : CallDescription(Mode::Unspecified, QualifiedName, RequiredArgs, RequiredParams) {}
+ : CallDescription(Mode::Unspecified, QualifiedName, RequiredArgs,
+ RequiredParams) {}
bool ento::CallDescription::matches(const CallEvent &Call) const {
// FIXME: Add ObjC Message support.
@@ -74,8 +75,7 @@ bool ento::CallDescription::matchesAsWritten(const CallExpr &CE) const {
return matchesImpl(FD, CE.getNumArgs(), FD->param_size());
}
-bool ento::CallDescription::matchesImpl(const FunctionDecl *FD,
- size_t ArgCount,
+bool ento::CallDescription::matchesImpl(const FunctionDecl *FD, size_t ArgCount,
size_t ParamCount) const {
if (!FD)
return false;
diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
index fa1cf72fd75135..3aac1f81f523db 100644
--- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -491,8 +491,7 @@ TEST(CallDescription, MatchBuiltins) {
// Test CDM::CLibrary - a flag that allows matching weird builtins.
EXPECT_TRUE(tooling::runToolOnCode(
std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{{"memset"}, 3}, false},
- {{CDM::CLibrary, {"memset"}, 3}, true}})),
+ {{{{"memset"}, 3}, false}, {{CDM::CLibrary, {"memset"}, 3}, true}})),
"void foo() {"
" int x;"
" __builtin___memset_chk(&x, 0, sizeof(x),"
@@ -522,8 +521,8 @@ TEST(CallDescription, MatchBuiltins) {
{
SCOPED_TRACE("lookbehind and lookahead mismatches");
EXPECT_TRUE(tooling::runToolOnCode(
- std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{CDM::CLibrary, {"func"}}, false}})),
+ std::unique_ptr<FrontendAction>(
+ new CallDescriptionAction<>({{{CDM::CLibrary, {"func"}}, false}})),
R"(
void funcXXX();
void XXXfunc();
@@ -537,8 +536,8 @@ TEST(CallDescription, MatchBuiltins) {
{
SCOPED_TRACE("lookbehind and lookahead matches");
EXPECT_TRUE(tooling::runToolOnCode(
- std::unique_ptr<FrontendAction>(new CallDescriptionAction<>(
- {{{CDM::CLibrary, {"func"}}, true}})),
+ std::unique_ptr<FrontendAction>(
+ new CallDescriptionAction<>({{{CDM::CLibrary, {"func"}}, true}})),
R"(
void func();
void func_XXX();
More information about the cfe-commits
mailing list