diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h +++ b/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 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 + 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 &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 diff --git a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp --- 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 @@ 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(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(Impl.lookup(Call)); } + +bool ento::CallDescriptionSet::containsAsWritten(const CallExpr &CE) const { + return static_cast(Impl.lookupAsWritten(CE)); +} diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp --- 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 @@ -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> { + 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("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(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(Code.str(), Diags, + /*OnlyEmitWarnings*/ true)); + EXPECT_EQ("test.CallDescChecker: CallEvent match\n" + "test.CallDescChecker: CallExpr match\n", + Diags); +} + } // namespace } // namespace ento } // namespace clang