[analyzer] NFC: Change evalCall() to provide a CallEvent.

This changes the checker callback signature to use the modern, easy to
use interface. Additionally, this unblocks future work on allowing
checkers to implement evalCall() for calls that don't correspond to any
call-expression or require additional information that's only available
as part of the CallEvent, such as C++ constructors and destructors.

Differential Revision: https://reviews.llvm.org/D62440

llvm-svn: 363893

clang/include/clang/StaticAnalyzer/Core/Checker.h

@@ -474,8 +474,9 @@ class Assume {
 
 class Call {
   template <typename CHECKER>
-  static bool _evalCall(void *checker, const CallExpr *CE, CheckerContext &C) {
-    return ((const CHECKER *)checker)->evalCall(CE, C);
+  static bool _evalCall(void *checker, const CallEvent &Call,
+                        CheckerContext &C) {
+    return ((const CHECKER *)checker)->evalCall(Call, C);
   }
 
 public:

clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

@@ -490,7 +490,7 @@ class CheckerManager {
       CheckerFn<ProgramStateRef (ProgramStateRef, const SVal &cond,
                                  bool assumption)>;
 
-  using EvalCallFunc = CheckerFn<bool (const CallExpr *, CheckerContext &)>;
+  using EvalCallFunc = CheckerFn<bool (const CallEvent &, CheckerContext &)>;
 
   using CheckEndOfTranslationUnit =
       CheckerFn<void (const TranslationUnitDecl *, AnalysisManager &,

clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp

@@ -14,6 +14,7 @@
 #include "clang/Basic/Builtins.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 using namespace clang;
@@ -23,30 +24,32 @@ namespace {
 
 class BuiltinFunctionChecker : public Checker<eval::Call> {
 public:
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 };
 
 }
 
-bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
+bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
                                       CheckerContext &C) const {
   ProgramStateRef state = C.getState();
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
-  const LocationContext *LCtx = C.getLocationContext();
+  const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
   if (!FD)
     return false;
 
+  const LocationContext *LCtx = C.getLocationContext();
+  const Expr *CE = Call.getOriginExpr();
+
   switch (FD->getBuiltinID()) {
   default:
     return false;
 
   case Builtin::BI__builtin_assume: {
-    assert (CE->arg_begin() != CE->arg_end());
-    SVal ArgSVal = C.getSVal(CE->getArg(0));
-    if (ArgSVal.isUndef())
+    assert (Call.getNumArgs() > 0);
+    SVal Arg = Call.getArgSVal(0);
+    if (Arg.isUndef())
       return true; // Return true to model purity.
 
-    state = state->assume(ArgSVal.castAs<DefinedOrUnknownSVal>(), true);
+    state = state->assume(Arg.castAs<DefinedOrUnknownSVal>(), true);
     // FIXME: do we want to warn here? Not right now. The most reports might
     // come from infeasible paths, thus being false positives.
     if (!state) {
@@ -66,9 +69,9 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
     // __builtin_assume_aligned, just return the value of the subexpression.
     // __builtin_addressof is going from a reference to a pointer, but those
     // are represented the same way in the analyzer.
-    assert (CE->arg_begin() != CE->arg_end());
-    SVal X = C.getSVal(*(CE->arg_begin()));
-    C.addTransition(state->BindExpr(CE, LCtx, X));
+    assert (Call.getNumArgs() > 0);
+    SVal Arg = Call.getArgSVal(0);
+    C.addTransition(state->BindExpr(CE, LCtx, Arg));
     return true;
   }
 
@@ -82,12 +85,14 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
     // Set the extent of the region in bytes. This enables us to use the
     // SVal of the argument directly. If we save the extent in bits, we
     // cannot represent values like symbol*8.
-    auto Size = C.getSVal(*(CE->arg_begin())).castAs<DefinedOrUnknownSVal>();
+    auto Size = Call.getArgSVal(0);
+    if (Size.isUndef())
+      return true; // Return true to model purity.
 
     SValBuilder& svalBuilder = C.getSValBuilder();
     DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder);
     DefinedOrUnknownSVal extentMatchesSizeArg =
-      svalBuilder.evalEQ(state, Extent, Size);
+      svalBuilder.evalEQ(state, Extent, Size.castAs<DefinedOrUnknownSVal>());
     state = state->assume(extentMatchesSizeArg, true);
     assert(state && "The region should not have any previous constraints");
 

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

@@ -17,6 +17,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "llvm/ADT/STLExtras.h"
@@ -57,7 +58,7 @@ class CStringChecker : public Checker< eval::Call,
 
   static void *getTag() { static int tag; return &tag; }
 
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef state, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
@@ -2334,8 +2335,8 @@ static CStringChecker::FnCheck identifyCall(const CallExpr *CE,
   return nullptr;
 }
 
-bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
-
+bool CStringChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+  const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   FnCheck evalFunction = identifyCall(CE, C);
 
   // If the callee isn't a string function, let another checker handle it.

clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp

@@ -14,6 +14,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
@@ -37,53 +38,44 @@ bool isRootChanged(intptr_t k) { return k == ROOT_CHANGED; }
 //         ROOT_CHANGED<--chdir(..)--      JAIL_ENTERED<--chdir(..)--
 //                                  |                               |
 //                      bug<--foo()--          JAIL_ENTERED<--foo()--
-class ChrootChecker : public Checker<eval::Call, check::PreStmt<CallExpr> > {
-  mutable IdentifierInfo *II_chroot, *II_chdir;
+class ChrootChecker : public Checker<eval::Call, check::PreCall> {
   // This bug refers to possibly break out of a chroot() jail.
   mutable std::unique_ptr<BuiltinBug> BT_BreakJail;
 
+  const CallDescription Chroot{"chroot", 1}, Chdir{"chdir", 1};
+
 public:
-  ChrootChecker() : II_chroot(nullptr), II_chdir(nullptr) {}
+  ChrootChecker() {}
 
   static void *getTag() {
     static int x;
     return &x;
   }
 
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
-  void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
 private:
-  void Chroot(CheckerContext &C, const CallExpr *CE) const;
-  void Chdir(CheckerContext &C, const CallExpr *CE) const;
+  void evalChroot(const CallEvent &Call, CheckerContext &C) const;
+  void evalChdir(const CallEvent &Call, CheckerContext &C) const;
 };
 
 } // end anonymous namespace
 
-bool ChrootChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD)
-    return false;
-
-  ASTContext &Ctx = C.getASTContext();
-  if (!II_chroot)
-    II_chroot = &Ctx.Idents.get("chroot");
-  if (!II_chdir)
-    II_chdir = &Ctx.Idents.get("chdir");
-
-  if (FD->getIdentifier() == II_chroot) {
-    Chroot(C, CE);
+bool ChrootChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+  if (Call.isCalled(Chroot)) {
+    evalChroot(Call, C);
     return true;
   }
-  if (FD->getIdentifier() == II_chdir) {
-    Chdir(C, CE);
+  if (Call.isCalled(Chdir)) {
+    evalChdir(Call, C);
     return true;
   }
 
   return false;
 }
 
-void ChrootChecker::Chroot(CheckerContext &C, const CallExpr *CE) const {
+void ChrootChecker::evalChroot(const CallEvent &Call, CheckerContext &C) const {
   ProgramStateRef state = C.getState();
   ProgramStateManager &Mgr = state->getStateManager();
 
@@ -93,7 +85,7 @@ void ChrootChecker::Chroot(CheckerContext &C, const CallExpr *CE) const {
   C.addTransition(state);
 }
 
-void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) const {
+void ChrootChecker::evalChdir(const CallEvent &Call, CheckerContext &C) const {
   ProgramStateRef state = C.getState();
   ProgramStateManager &Mgr = state->getStateManager();
 
@@ -103,7 +95,7 @@ void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) const {
     return;
 
   // After chdir("/"), enter the jail, set the enum value JAIL_ENTERED.
-  const Expr *ArgExpr = CE->getArg(0);
+  const Expr *ArgExpr = Call.getArgExpr(0);
   SVal ArgVal = C.getSVal(ArgExpr);
 
   if (const MemRegion *R = ArgVal.getAsRegion()) {
@@ -120,19 +112,10 @@ void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) const {
 }
 
 // Check the jail state before any function call except chroot and chdir().
-void ChrootChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD)
-    return;
-
-  ASTContext &Ctx = C.getASTContext();
-  if (!II_chroot)
-    II_chroot = &Ctx.Idents.get("chroot");
-  if (!II_chdir)
-    II_chdir = &Ctx.Idents.get("chdir");
-
+void ChrootChecker::checkPreCall(const CallEvent &Call,
+                                 CheckerContext &C) const {
   // Ignore chroot and chdir.
-  if (FD->getIdentifier() == II_chroot || FD->getIdentifier() == II_chdir)
+  if (Call.isCalled(Chroot) || Call.isCalled(Chdir))
     return;
 
   // If jail state is ROOT_CHANGED, generate BugReport.

clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

@@ -11,6 +11,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/IssueHash.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -53,7 +54,7 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
                           ExplodedNode *N) const;
 
 public:
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
   void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR,
                         ExprEngine &Eng) const;
@@ -63,8 +64,12 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
 REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, SymbolRef)
 REGISTER_MAP_WITH_PROGRAMSTATE(DenotedSymbols, SymbolRef, const StringLiteral *)
 
-bool ExprInspectionChecker::evalCall(const CallExpr *CE,
+bool ExprInspectionChecker::evalCall(const CallEvent &Call,
                                      CheckerContext &C) const {
+  const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return false;
+
   // These checks should have no effect on the surrounding environment
   // (globals should not be invalidated, etc), hence the use of evalCall.
   FnCheck Handler = llvm::StringSwitch<FnCheck>(C.getCalleeName(CE))

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

@@ -886,14 +886,19 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St,
 // Handle the return values of retain-count-related functions.
 //===----------------------------------------------------------------------===//
 
-bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
+bool RetainCountChecker::evalCall(const CallEvent &Call,
+                                  CheckerContext &C) const {
   ProgramStateRef state = C.getState();
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
   if (!FD)
     return false;
 
+  const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return false;
+
   RetainSummaryManager &SmrMgr = getSummaryManager(C);
-  QualType ResultTy = CE->getCallReturnType(C.getASTContext());
+  QualType ResultTy = Call.getResultType();
 
   // See if the function has 'rc_ownership_trusted_implementation'
   // annotate attribute. If it does, we will not inline it.

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h

@@ -310,7 +310,7 @@ class RetainCountChecker
                                const CallEvent &Call,
                                CheckerContext &C) const;
 
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 
   ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
                                  bool Assumption) const;

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

@@ -26,32 +26,30 @@ using namespace ento;
 
 namespace {
 class SmartPtrModeling : public Checker<eval::Call> {
-  bool isNullAfterMoveMethod(const CXXInstanceCall *Call) const;
+  bool isNullAfterMoveMethod(const CallEvent &Call) const;
 
 public:
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 };
 } // end of anonymous namespace
 
-bool SmartPtrModeling::isNullAfterMoveMethod(
-    const CXXInstanceCall *Call) const {
+bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
   // TODO: Update CallDescription to support anonymous calls?
   // TODO: Handle other methods, such as .get() or .release().
   // But once we do, we'd need a visitor to explain null dereferences
   // that are found via such modeling.
-  const auto *CD = dyn_cast_or_null<CXXConversionDecl>(Call->getDecl());
+  const auto *CD = dyn_cast_or_null<CXXConversionDecl>(Call.getDecl());
   return CD && CD->getConversionType()->isBooleanType();
 }
 
-bool SmartPtrModeling::evalCall(const CallExpr *CE, CheckerContext &C) const {
-  CallEventRef<> CallRef = C.getStateManager().getCallEventManager().getCall(
-      CE, C.getState(), C.getLocationContext());
-  const auto *Call = dyn_cast_or_null<CXXInstanceCall>(CallRef);
-  if (!Call || !isNullAfterMoveMethod(Call))
+bool SmartPtrModeling::evalCall(const CallEvent &Call,
+                                CheckerContext &C) const {
+  if (!isNullAfterMoveMethod(Call))
     return false;
 
   ProgramStateRef State = C.getState();
-  const MemRegion *ThisR = Call->getCXXThisVal().getAsRegion();
+  const MemRegion *ThisR =
+      cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
 
   if (!move::isMovedFrom(State, ThisR)) {
     // TODO: Model this case as well. At least, avoid invalidation of globals.
@@ -60,8 +58,8 @@ bool SmartPtrModeling::evalCall(const CallExpr *CE, CheckerContext &C) const {
 
   // TODO: Add a note to bug reports describing this decision.
   C.addTransition(
-      State->BindExpr(CE, C.getLocationContext(),
-                      C.getSValBuilder().makeZeroVal(CE->getType())));
+      State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+                      C.getSValBuilder().makeZeroVal(Call.getResultType())));
   return true;
 }
 

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

@@ -224,7 +224,7 @@ class StdLibraryFunctionsChecker : public Checker<check::PostCall, eval::Call> {
 
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
-  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
+  bool evalCall(const CallEvent &Call, CheckerContext &C) const;
 
 private:
   Optional<FunctionSummaryTy> findFunctionSummary(const FunctionDecl *FD,
@@ -367,12 +367,16 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
   }
 }
 
-bool StdLibraryFunctionsChecker::evalCall(const CallExpr *CE,
+bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
                                           CheckerContext &C) const {
-  const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CE->getCalleeDecl());
+  const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
   if (!FD)
     return false;
 
+  const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return false;
+
   Optional<FunctionSummaryTy> FoundSummary = findFunctionSummary(FD, CE, C);
   if (!FoundSummary)
     return false;