This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG32ac21d04909: [NFC][analyzer] Allow CallDescriptions to be
matched with CallExprs (authored by Szelethus).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119004/new/
https://reviews.llvm.org/D119004
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
clang/lib/StaticAnalyzer/Core/CallDescription.cpp
clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===================================================================
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ 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 @@
}
}
+//===----------------------------------------------------------------------===//
+// 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
Index: clang/lib/StaticAnalyzer/Core/CallDescription.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallDescription.cpp
+++ 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 @@
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 @@
};
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 @@
};
// Let's start matching...
- if (!ExactMatchArgAndParamCounts(Call, *this))
+ if (!ExactMatchArgAndParamCounts(ArgCount, ParamCount, *this))
return false;
if (!MatchNameOnly(*this, FD))
@@ -144,3 +162,7 @@
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));
+}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -108,13 +108,56 @@
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 @@
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 @@
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
