[clang] 32ac21d - [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs
Kristóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 1 08:13:19 PST 2022
Author: Kristóf Umann
Date: 2022-03-01T17:13:04+01:00
New Revision: 32ac21d04909da0d50d3b24100d5d9ab30b29a95
URL: https://github.com/llvm/llvm-project/commit/32ac21d04909da0d50d3b24100d5d9ab30b29a95
DIFF: https://github.com/llvm/llvm-project/commit/32ac21d04909da0d50d3b24100d5d9ab30b29a95.diff
LOG: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs
Since CallDescriptions can only be matched against CallEvents that are created
during symbolic execution, it was not possible to use it in syntactic-only
contexts. For example, even though InnerPointerChecker can check with its set of
CallDescriptions whether a function call is interested during analysis, its
unable to check without hassle whether a non-analyzer piece of code also calls
such a function.
The patch adds the ability to use CallDescriptions in syntactic contexts as
well. While we already have that in Signature, we still want to leverage the
ability to use dynamic information when we have it (function pointers, for
example). This could be done with Signature as well (StdLibraryFunctionsChecker
does it), but it makes it even less of a drop-in replacement.
Differential Revision: https://reviews.llvm.org/D119004
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
clang/lib/StaticAnalyzer/Core/CallDescription.cpp
clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index cd972b7837d00..e552b833b6f25 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -108,13 +108,56 @@ class CallDescription {
return CD1.matches(Call);
}
- /// \copydoc clang::ento::matchesAny(const CallEvent &, const CallDescription &)
+ /// \copydoc clang::ento::CallDescription::matchesAny(const CallEvent &, const CallDescription &)
template <typename... Ts>
friend bool matchesAny(const CallEvent &Call, const CallDescription &CD1,
const Ts &...CDs) {
return CD1.matches(Call) || matchesAny(Call, CDs...);
}
/// @}
+
+ /// @name Matching CallDescriptions against a CallExpr
+ /// @{
+
+ /// Returns true if the CallExpr is a call to a function that matches the
+ /// CallDescription.
+ ///
+ /// When available, always prefer matching with a CallEvent! This function
+ /// exists only when that is not available, for example, when _only_
+ /// syntactic check is done on a piece of code.
+ ///
+ /// Also, StdLibraryFunctionsChecker::Signature is likely a better candicade
+ /// for syntactic only matching if you are writing a new checker. This is
+ /// handy if a CallDescriptionMap is already there.
+ ///
+ /// The function is imprecise because CallEvent may know path sensitive
+ /// information, such as the precise argument count (see comments for
+ /// CallEvent::getNumArgs), the called function if it was called through a
+ /// function pointer, and other information not available syntactically.
+ bool matchesAsWritten(const CallExpr &CE) const;
+
+ /// Returns true whether the CallExpr matches on any of the CallDescriptions
+ /// supplied.
+ ///
+ /// \note This function is not intended to be used to match Obj-C method
+ /// calls.
+ friend bool matchesAnyAsWritten(const CallExpr &CE,
+ const CallDescription &CD1) {
+ return CD1.matchesAsWritten(CE);
+ }
+
+ /// \copydoc clang::ento::CallDescription::matchesAnyAsWritten(const CallExpr &, const CallDescription &)
+ template <typename... Ts>
+ friend bool matchesAnyAsWritten(const CallExpr &CE,
+ const CallDescription &CD1,
+ const Ts &...CDs) {
+ return CD1.matchesAsWritten(CE) || matchesAnyAsWritten(CE, CDs...);
+ }
+ /// @}
+
+private:
+ bool matchesImpl(const FunctionDecl *Callee, size_t ArgCount,
+ size_t ParamCount) const;
};
/// An immutable map from CallDescriptions to arbitrary data. Provides a unified
@@ -156,6 +199,28 @@ template <typename T> class CallDescriptionMap {
return nullptr;
}
+
+ /// When available, always prefer lookup with a CallEvent! This function
+ /// exists only when that is not available, for example, when _only_
+ /// syntactic check is done on a piece of code.
+ ///
+ /// Also, StdLibraryFunctionsChecker::Signature is likely a better candicade
+ /// for syntactic only matching if you are writing a new checker. This is
+ /// handy if a CallDescriptionMap is already there.
+ ///
+ /// The function is imprecise because CallEvent may know path sensitive
+ /// information, such as the precise argument count (see comments for
+ /// CallEvent::getNumArgs), the called function if it was called through a
+ /// function pointer, and other information not available syntactically.
+ LLVM_NODISCARD const T *lookupAsWritten(const CallExpr &Call) const {
+ // Slow path: linear lookup.
+ // TODO: Implement some sort of fast path.
+ for (const std::pair<CallDescription, T> &I : LinearMap)
+ if (I.first.matchesAsWritten(Call))
+ return &I.second;
+
+ return nullptr;
+ }
};
/// An immutable set of CallDescriptions.
@@ -171,6 +236,20 @@ class CallDescriptionSet {
CallDescriptionSet &operator=(const CallDescription &) = delete;
LLVM_NODISCARD bool contains(const CallEvent &Call) const;
+
+ /// When available, always prefer lookup with a CallEvent! This function
+ /// exists only when that is not available, for example, when _only_
+ /// syntactic check is done on a piece of code.
+ ///
+ /// Also, StdLibraryFunctionsChecker::Signature is likely a better candicade
+ /// for syntactic only matching if you are writing a new checker. This is
+ /// handy if a CallDescriptionMap is already there.
+ ///
+ /// The function is imprecise because CallEvent may know path sensitive
+ /// information, such as the precise argument count (see comments for
+ /// CallEvent::getNumArgs), the called function if it was called through a
+ /// function pointer, and other information not available syntactically.
+ LLVM_NODISCARD bool containsAsWritten(const CallExpr &CE) const;
};
} // namespace ento
diff --git a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
index 810fe365d021d..d4d7672e0959b 100644
--- a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
+#include "clang/AST/Decl.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "llvm/ADT/ArrayRef.h"
@@ -61,15 +62,32 @@ bool ento::CallDescription::matches(const CallEvent &Call) const {
if (!FD)
return false;
+ return matchesImpl(FD, Call.getNumArgs(), Call.parameters().size());
+}
+
+bool ento::CallDescription::matchesAsWritten(const CallExpr &CE) const {
+ const auto *FD = dyn_cast_or_null<FunctionDecl>(CE.getCalleeDecl());
+ if (!FD)
+ return false;
+
+ return matchesImpl(FD, CE.getNumArgs(), FD->param_size());
+}
+
+bool ento::CallDescription::matchesImpl(const FunctionDecl *Callee,
+ size_t ArgCount,
+ size_t ParamCount) const {
+ const auto *FD = Callee;
+ if (!FD)
+ return false;
+
if (Flags & CDF_MaybeBuiltin) {
return CheckerContext::isCLibraryFunction(FD, getFunctionName()) &&
- (!RequiredArgs || *RequiredArgs <= Call.getNumArgs()) &&
- (!RequiredParams || *RequiredParams <= Call.parameters().size());
+ (!RequiredArgs || *RequiredArgs <= ArgCount) &&
+ (!RequiredParams || *RequiredParams <= ParamCount);
}
if (!II.hasValue()) {
- II = &Call.getState()->getStateManager().getContext().Idents.get(
- getFunctionName());
+ II = &FD->getASTContext().Idents.get(getFunctionName());
}
const auto MatchNameOnly = [](const CallDescription &CD,
@@ -86,11 +104,11 @@ bool ento::CallDescription::matches(const CallEvent &Call) const {
};
const auto ExactMatchArgAndParamCounts =
- [](const CallEvent &Call, const CallDescription &CD) -> bool {
- const bool ArgsMatch =
- !CD.RequiredArgs || *CD.RequiredArgs == Call.getNumArgs();
+ [](size_t ArgCount, size_t ParamCount,
+ const CallDescription &CD) -> bool {
+ const bool ArgsMatch = !CD.RequiredArgs || *CD.RequiredArgs == ArgCount;
const bool ParamsMatch =
- !CD.RequiredParams || *CD.RequiredParams == Call.parameters().size();
+ !CD.RequiredParams || *CD.RequiredParams == ParamCount;
return ArgsMatch && ParamsMatch;
};
@@ -122,7 +140,7 @@ bool ento::CallDescription::matches(const CallEvent &Call) const {
};
// Let's start matching...
- if (!ExactMatchArgAndParamCounts(Call, *this))
+ if (!ExactMatchArgAndParamCounts(ArgCount, ParamCount, *this))
return false;
if (!MatchNameOnly(*this, FD))
@@ -144,3 +162,7 @@ ento::CallDescriptionSet::CallDescriptionSet(
bool ento::CallDescriptionSet::contains(const CallEvent &Call) const {
return static_cast<bool>(Impl.lookup(Call));
}
+
+bool ento::CallDescriptionSet::containsAsWritten(const CallExpr &CE) const {
+ return static_cast<bool>(Impl.lookupAsWritten(CE));
+}
diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
index f2d85b3bc17c7..bfdb357d60aab 100644
--- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -6,11 +6,18 @@
//
//===----------------------------------------------------------------------===//
+#include "CheckerRegistration.h"
#include "Reusables.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/Analysis/PathDiagnostic.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
#include <type_traits>
@@ -543,6 +550,77 @@ TEST(CallDescription, MatchBuiltins) {
}
}
+//===----------------------------------------------------------------------===//
+// Testing through a checker interface.
+//
+// Above, the static analyzer isn't run properly, only the bare minimum to
+// create CallEvents. This causes CallEvents through function pointers to not
+// refer to the pointee function, but this works fine if we run
+// AnalysisASTConsumer.
+//===----------------------------------------------------------------------===//
+
+class CallDescChecker
+ : public Checker<check::PreCall, check::PreStmt<CallExpr>> {
+ CallDescriptionSet Set = {{"bar", 0}};
+
+public:
+ void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
+ if (Set.contains(Call)) {
+ C.getBugReporter().EmitBasicReport(
+ Call.getDecl(), this, "CallEvent match", categories::LogicError,
+ "CallEvent match",
+ PathDiagnosticLocation{Call.getDecl(), C.getSourceManager()});
+ }
+ }
+
+ void checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
+ if (Set.containsAsWritten(*CE)) {
+ C.getBugReporter().EmitBasicReport(
+ CE->getCalleeDecl(), this, "CallExpr match", categories::LogicError,
+ "CallExpr match",
+ PathDiagnosticLocation{CE->getCalleeDecl(), C.getSourceManager()});
+ }
+ }
+};
+
+void addCallDescChecker(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+ AnOpts.CheckersAndPackages = {{"test.CallDescChecker", true}};
+ AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+ Registry.addChecker<CallDescChecker>("test.CallDescChecker", "Description",
+ "");
+ });
+}
+
+TEST(CallDescription, CheckCallExprMatching) {
+ // Imprecise matching shouldn't catch the call to bar, because its obscured
+ // by a function pointer.
+ constexpr StringRef FnPtrCode = R"code(
+ void bar();
+ void foo() {
+ void (*fnptr)() = bar;
+ fnptr();
+ })code";
+ std::string Diags;
+ EXPECT_TRUE(runCheckerOnCode<addCallDescChecker>(FnPtrCode.str(), Diags,
+ /*OnlyEmitWarnings*/ true));
+ EXPECT_EQ("test.CallDescChecker: CallEvent match\n", Diags);
+
+ // This should be caught properly by imprecise matching, as the call is done
+ // purely through syntactic means.
+ constexpr StringRef Code = R"code(
+ void bar();
+ void foo() {
+ bar();
+ })code";
+ Diags.clear();
+ EXPECT_TRUE(runCheckerOnCode<addCallDescChecker>(Code.str(), Diags,
+ /*OnlyEmitWarnings*/ true));
+ EXPECT_EQ("test.CallDescChecker: CallEvent match\n"
+ "test.CallDescChecker: CallExpr match\n",
+ Diags);
+}
+
} // namespace
} // namespace ento
} // namespace clang
More information about the cfe-commits
mailing list