Skip to content

Commit

Permalink
[NFC][analyzer] Allow CallDescriptions to be matched with CallExprs
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@Szelethus
Szelethus committed Mar 1, 2022
1 parent fa55ac6 commit 32ac21d
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 10 deletions.
81 changes: 80 additions & 1 deletion clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
40 changes: 31 additions & 9 deletions clang/lib/StaticAnalyzer/Core/CallDescription.cpp
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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;
};

Expand Down Expand Up @@ -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))
Expand All @@ -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));
}
78 changes: 78 additions & 0 deletions clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
View file
Expand Up @@ -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>
Expand Down Expand Up @@ -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

0 comments on commit 32ac21d

Please sign in to comment.