steakhal created this revision.
steakhal added reviewers: NoQ, Szelethus, boga95.
steakhal added a project: clang.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, whisperity.
steakhal added a parent revision: D71524: [analyzer] Support tainted objects in 
GenericTaintChecker.

Intended to be a non-functional change but it turned out **CallEvent** handles 
constructor calls unlike **CallExpr** which doesn't triggered for constructors.

All in all, this change shouldn't be observable since constructors are not yet 
propagating taintness like functions.
In the future constructors should propagate taintness as well.

This change includes:

- NFCi change all uses of the CallExpr to CallEvent
- NFC rename some functions, mark static them etc.
- NFC omit explicit TaintPropagationRule type in switches
- NFC apply some clang-tidy fixits


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72035

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -22,12 +22,15 @@
 #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/Support/Regex.h"
 #include "llvm/Support/YAMLTraits.h"
+
 #include <algorithm>
 #include <limits>
+#include <memory>
 #include <unordered_map>
 #include <utility>
 
@@ -36,17 +39,15 @@
 using namespace taint;
 
 namespace {
-class GenericTaintChecker
-    : public Checker<check::PostStmt<CallExpr>, check::PreStmt<CallExpr>> {
+class GenericTaintChecker : public Checker<check::PreCall, check::PostCall> {
 public:
   static void *getTag() {
     static int Tag;
     return &Tag;
   }
 
-  void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
-
-  void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
                   const char *Sep) const override;
@@ -82,7 +83,7 @@
 
   /// Convert SignedArgVector to ArgVector.
   ArgVector convertToArgVector(CheckerManager &Mgr, const std::string &Option,
-                               SignedArgVector Args);
+                               const SignedArgVector &Args);
 
   /// Parse the config.
   void parseConfiguration(CheckerManager &Mgr, const std::string &Option,
@@ -97,7 +98,8 @@
   mutable std::unique_ptr<BugType> BT;
   void initBugType() const {
     if (!BT)
-      BT.reset(new BugType(this, "Use of Untrusted Data", "Untrusted Data"));
+      BT = std::make_unique<BugType>(this, "Use of Untrusted Data",
+                                     "Untrusted Data");
   }
 
   struct FunctionData {
@@ -107,9 +109,10 @@
     FunctionData &operator=(const FunctionData &) = delete;
     FunctionData &operator=(FunctionData &&) = delete;
 
-    static Optional<FunctionData> create(const CallExpr *CE,
+    static Optional<FunctionData> create(const CallEvent &Call,
                                          const CheckerContext &C) {
-      const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+      assert(Call.getDecl());
+      const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();
       if (!FDecl || (FDecl->getKind() != Decl::Function &&
                      FDecl->getKind() != Decl::CXXMethod))
         return None;
@@ -133,39 +136,39 @@
 
   /// Catch taint related bugs. Check if tainted data is passed to a
   /// system call etc. Returns true on matching.
-  bool checkPre(const CallExpr *CE, const FunctionData &FData,
+  bool checkPre(const CallEvent &Call, const FunctionData &FData,
                 CheckerContext &C) const;
 
   /// Add taint sources for extraction operator on pre-visit.
-  bool addOverloadedOpPre(const CallExpr *CE, CheckerContext &C) const;
+  static bool addOverloadedOpPre(const CallEvent &Call, CheckerContext &C);
 
   /// Add taint sources on a pre-visit. Returns true on matching.
-  bool addSourcesPre(const CallExpr *CE, const FunctionData &FData,
+  bool addSourcesPre(const CallEvent &Call, const FunctionData &FData,
                      CheckerContext &C) const;
 
   /// Mark filter's arguments not tainted on a pre-visit. Returns true on
   /// matching.
-  bool addFiltersPre(const CallExpr *CE, const FunctionData &FData,
+  bool addFiltersPre(const CallEvent &Call, const FunctionData &FData,
                      CheckerContext &C) const;
 
   /// Propagate taint generated at pre-visit. Returns true on matching.
-  bool propagateFromPre(const CallExpr *CE, CheckerContext &C) const;
+  static bool propagateFromPre(const CallEvent &Call, CheckerContext &C);
 
   /// Check if the region the expression evaluates to is the standard input,
   /// and thus, is tainted.
   static bool isStdin(const Expr *E, CheckerContext &C);
 
   /// Check if the type of the variable is std::basic_istream
-  static bool isStdstream(const Expr *E, CheckerContext &C);
+  static bool isStdIstream(const Expr *E, CheckerContext &C);
 
   /// Given a pointer argument, return the value it points to.
-  static Optional<SVal> getPointedToSVal(CheckerContext &C, const Expr *Arg);
+  static Optional<SVal> getPointeeOf(CheckerContext &C, const Expr *Arg);
 
   /// Check for CWE-134: Uncontrolled Format String.
   static constexpr llvm::StringLiteral MsgUncontrolledFormatString =
       "Untrusted data is used as a format string "
       "(CWE-134: Uncontrolled Format String)";
-  bool checkUncontrolledFormatString(const CallExpr *CE,
+  bool checkUncontrolledFormatString(const CallEvent &Call,
                                      CheckerContext &C) const;
 
   /// Check for:
@@ -174,7 +177,7 @@
   static constexpr llvm::StringLiteral MsgSanitizeSystemArgs =
       "Untrusted data is passed to a system call "
       "(CERT/STR02-C. Sanitize data passed to complex subsystems)";
-  bool checkSystemCall(const CallExpr *CE, StringRef Name,
+  bool checkSystemCall(const CallEvent &Call, StringRef Name,
                        CheckerContext &C) const;
 
   /// Check if tainted data is used as a buffer size ins strn.. functions,
@@ -183,13 +186,12 @@
       "Untrusted data is used to specify the buffer size "
       "(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
       "for character data and the null terminator)";
-  bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl,
-                              CheckerContext &C) const;
+  bool checkTaintedBufferSize(const CallEvent &Call, CheckerContext &C) const;
 
   /// Check if tainted data is used as a custom sink's parameter.
   static constexpr llvm::StringLiteral MsgCustomSink =
       "Untrusted data is passed to a user-defined sink";
-  bool checkCustomSinks(const CallExpr *CE, const FunctionData &FData,
+  bool checkCustomSinks(const CallEvent &Call, const FunctionData &FData,
                         CheckerContext &C) const;
 
   /// Generate a report if the expression is tainted or points to tainted data.
@@ -219,7 +221,7 @@
   /// ReturnValueIndex is added to the dst list, the return value will be
   /// tainted.
   struct TaintPropagationRule {
-    using PropagationFuncType = bool (*)(bool IsTainted, const CallExpr *,
+    using PropagationFuncType = bool (*)(bool IsTainted, const CallEvent &Call,
                                          CheckerContext &C);
 
     /// List of arguments which can be taint sources and should be checked.
@@ -263,22 +265,23 @@
       return (llvm::find(DstArgs, ArgNum) != DstArgs.end());
     }
 
-    static bool isTaintedOrPointsToTainted(const Expr *E, ProgramStateRef State,
+    static bool isTaintedOrPointsToTainted(const Expr *E,
+                                           const ProgramStateRef &State,
                                            CheckerContext &C) {
       if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C) ||
-          isStdstream(E, C))
+          isStdIstream(E, C))
         return true;
 
-      Optional<SVal> V = getPointedToSVal(C, E);
+      Optional<SVal> V = getPointeeOf(C, E);
       return (V && isTainted(State, *V));
     }
 
     /// Pre-process a function which propagates taint according to the
     /// taint rule.
-    ProgramStateRef process(const CallExpr *CE, CheckerContext &C) const;
+    ProgramStateRef process(const CallEvent &Call, CheckerContext &C) const;
 
     // Functions for custom taintedness propagation.
-    static bool postSocket(bool IsTainted, const CallExpr *CE,
+    static bool postSocket(bool IsTainted, const CallEvent &Call,
                            CheckerContext &C);
   };
 
@@ -370,24 +373,23 @@
 REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned)
 
 /// Treat implicit this parameter as the 0. argument.
-const Expr *getArg(const CallExpr *CE, unsigned ArgNum) {
-  if (const auto *MCE = dyn_cast<CXXMemberCallExpr>(CE)) {
-    if (ArgNum == 0)
-      return MCE->getImplicitObjectArgument();
-    return MCE->getArg(ArgNum - 1);
-  }
-  return CE->getArg(ArgNum);
+const Expr *getArg(const CallEvent &Call, unsigned ArgNum) {
+  if (const auto *InstanceCall = dyn_cast<CXXInstanceCall>(&Call))
+    return ArgNum == 0 ? InstanceCall->getCXXThisExpr()
+                       : InstanceCall->getArgExpr(ArgNum - 1);
+  return Call.getArgExpr(ArgNum);
 }
 
-/// Class member functions has an implicit this parameteter.
-unsigned getNumArgs(const CallExpr *CE) {
-  if (isa<CXXMemberCallExpr>(CE))
-    return CE->getNumArgs() + 1;
-  return CE->getNumArgs();
+/// Class member functions has an implicit this parameter.
+unsigned getNumArgs(const CallEvent &Call) {
+  return Call.getNumArgs() + static_cast<unsigned>(isa<CXXInstanceCall>(Call));
 }
 
-GenericTaintChecker::ArgVector GenericTaintChecker::convertToArgVector(
-    CheckerManager &Mgr, const std::string &Option, SignedArgVector Args) {
+GenericTaintChecker::ArgVector
+GenericTaintChecker::convertToArgVector(CheckerManager &Mgr,
+                                        const std::string &Option,
+                                        const SignedArgVector &Args) {
+
   ArgVector Result;
   for (int Arg : Args) {
     if (Arg == -1)
@@ -454,128 +456,128 @@
       llvm::StringSwitch<TaintPropagationRule>(FData.FullName)
           // Source functions
           // TODO: Add support for vfscanf & family.
-          .Case("fdopen", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("fopen", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("freopen", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("getch", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("getchar", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("getchar_unlocked",
-                TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("getenv", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("gets", TaintPropagationRule({}, {0, ReturnValueIndex}))
-          .Case("scanf", TaintPropagationRule({}, {}, VariadicType::Dst, 1))
-          .Case("socket",
-                TaintPropagationRule({}, {ReturnValueIndex}, VariadicType::None,
-                                     InvalidArgIndex,
-                                     &TaintPropagationRule::postSocket))
-          .Case("wgetch", TaintPropagationRule({}, {ReturnValueIndex}))
+          .Case("fdopen", {{}, {ReturnValueIndex}})
+          .Case("fopen", {{}, {ReturnValueIndex}})
+          .Case("freopen", {{}, {ReturnValueIndex}})
+          .Case("getch", {{}, {ReturnValueIndex}})
+          .Case("getchar", {{}, {ReturnValueIndex}})
+          .Case("getchar_unlocked", {{}, {ReturnValueIndex}})
+          .Case("getenv", {{}, {ReturnValueIndex}})
+          .Case("gets", {{}, {0, ReturnValueIndex}})
+          .Case("scanf", {{}, {}, VariadicType::Dst, 1})
+          .Case("socket", {{},
+                           {ReturnValueIndex},
+                           VariadicType::None,
+                           InvalidArgIndex,
+                           &TaintPropagationRule::postSocket})
+          .Case("wgetch", {{}, {ReturnValueIndex}})
           // Propagating functions
-          .Case("atoi", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("atol", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("atoll", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("fgetc", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("fgetln", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("fgets", TaintPropagationRule({2}, {0, ReturnValueIndex}))
-          .Case("fscanf", TaintPropagationRule({0}, {}, VariadicType::Dst, 2))
-          .Case("getc", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("getc_unlocked", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("getdelim", TaintPropagationRule({3}, {0}))
-          .Case("getline", TaintPropagationRule({2}, {0}))
-          .Case("getw", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("pread",
-                TaintPropagationRule({0, 1, 2, 3}, {1, ReturnValueIndex}))
-          .Case("read", TaintPropagationRule({0, 2}, {1, ReturnValueIndex}))
-          .Case("strchr", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("strrchr", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("tolower", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Case("toupper", TaintPropagationRule({0}, {ReturnValueIndex}))
-          .Default(TaintPropagationRule());
+          .Case("atoi", {{0}, {ReturnValueIndex}})
+          .Case("atol", {{0}, {ReturnValueIndex}})
+          .Case("atoll", {{0}, {ReturnValueIndex}})
+          .Case("fgetc", {{0}, {ReturnValueIndex}})
+          .Case("fgetln", {{0}, {ReturnValueIndex}})
+          .Case("fgets", {{2}, {0, ReturnValueIndex}})
+          .Case("fscanf", {{0}, {}, VariadicType::Dst, 2})
+          .Case("sscanf", {{0}, {}, VariadicType::Dst, 2})
+          .Case("getc", {{0}, {ReturnValueIndex}})
+          .Case("getc_unlocked", {{0}, {ReturnValueIndex}})
+          .Case("getdelim", {{3}, {0}})
+          .Case("getline", {{2}, {0}})
+          .Case("getw", {{0}, {ReturnValueIndex}})
+          .Case("pread", {{0, 1, 2, 3}, {1, ReturnValueIndex}})
+          .Case("read", {{0, 2}, {1, ReturnValueIndex}})
+          .Case("strchr", {{0}, {ReturnValueIndex}})
+          .Case("strrchr", {{0}, {ReturnValueIndex}})
+          .Case("tolower", {{0}, {ReturnValueIndex}})
+          .Case("toupper", {{0}, {ReturnValueIndex}})
+          .Default({});
 
   if (!Rule.isNull())
     return Rule;
+  assert(FData.FDecl);
 
   // Check if it's one of the memory setting/copying functions.
   // This check is specialized but faster then calling isCLibraryFunction.
   const FunctionDecl *FDecl = FData.FDecl;
   unsigned BId = 0;
-  if ((BId = FDecl->getMemoryFunctionKind()))
+  if ((BId = FDecl->getMemoryFunctionKind())) {
     switch (BId) {
     case Builtin::BImemcpy:
     case Builtin::BImemmove:
     case Builtin::BIstrncpy:
     case Builtin::BIstrncat:
-      return TaintPropagationRule({1, 2}, {0, ReturnValueIndex});
+      return {{1, 2}, {0, ReturnValueIndex}};
     case Builtin::BIstrlcpy:
     case Builtin::BIstrlcat:
-      return TaintPropagationRule({1, 2}, {0});
+      return {{1, 2}, {0}};
     case Builtin::BIstrndup:
-      return TaintPropagationRule({0, 1}, {ReturnValueIndex});
+      return {{0, 1}, {ReturnValueIndex}};
 
     default:
       break;
-    };
+    }
+  }
 
   // Process all other functions which could be defined as builtins.
   if (Rule.isNull()) {
-    if (C.isCLibraryFunction(FDecl, "snprintf"))
-      return TaintPropagationRule({1}, {0, ReturnValueIndex}, VariadicType::Src,
-                                  3);
-    else if (C.isCLibraryFunction(FDecl, "sprintf"))
-      return TaintPropagationRule({}, {0, ReturnValueIndex}, VariadicType::Src,
-                                  2);
-    else if (C.isCLibraryFunction(FDecl, "strcpy") ||
-             C.isCLibraryFunction(FDecl, "stpcpy") ||
-             C.isCLibraryFunction(FDecl, "strcat"))
-      return TaintPropagationRule({1}, {0, ReturnValueIndex});
-    else if (C.isCLibraryFunction(FDecl, "bcopy"))
-      return TaintPropagationRule({0, 2}, {1});
-    else if (C.isCLibraryFunction(FDecl, "strdup") ||
-             C.isCLibraryFunction(FDecl, "strdupa"))
-      return TaintPropagationRule({0}, {ReturnValueIndex});
-    else if (C.isCLibraryFunction(FDecl, "wcsdup"))
-      return TaintPropagationRule({0}, {ReturnValueIndex});
+    const auto OneOf = [FDecl](const auto &... Name) {
+      // FIXME: use fold expression in C++17
+      using unused = int[];
+      bool ret = false;
+      static_cast<void>(unused{
+          0, (ret |= CheckerContext::isCLibraryFunction(FDecl, Name), 0)...});
+      return ret;
+    };
+    if (OneOf("snprintf"))
+      return {{1}, {0, ReturnValueIndex}, VariadicType::Src, 3};
+    if (OneOf("sprintf"))
+      return {{}, {0, ReturnValueIndex}, VariadicType::Src, 2};
+    if (OneOf("strcpy", "stpcpy", "strcat"))
+      return {{1}, {0, ReturnValueIndex}};
+    if (OneOf("bcopy"))
+      return {{0, 2}, {1}};
+    if (OneOf("strdup", "strdupa", "wcsdup"))
+      return {{0}, {ReturnValueIndex}};
   }
 
-  // Skipping the following functions, since they might be used for cleansing
-  // or smart memory copy:
+  // Skipping the following functions, since they might be used for cleansing or
+  // smart memory copy:
   // - memccpy - copying until hitting a special character.
 
   auto It = findFunctionInConfig(CustomPropagations, FData);
-  if (It != CustomPropagations.end()) {
-    const auto &Value = It->second;
-    return Value.second;
-  }
-
-  return TaintPropagationRule();
+  if (It != CustomPropagations.end())
+    return It->second.second;
+  return {};
 }
 
-void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
+void GenericTaintChecker::checkPreCall(const CallEvent &Call,
                                        CheckerContext &C) const {
-  if (addOverloadedOpPre(CE, C))
+  if (addOverloadedOpPre(Call, C))
     return;
 
-  Optional<FunctionData> FData = FunctionData::create(CE, C);
+  Optional<FunctionData> FData = FunctionData::create(Call, C);
   if (!FData)
     return;
 
   // Check for taintedness related errors first: system call, uncontrolled
   // format string, tainted buffer size.
-  if (checkPre(CE, *FData, C))
+  if (checkPre(Call, *FData, C))
     return;
 
-  // Marks the function's arguments and/or return value tainted if it present in
-  // the list.
-  if (addSourcesPre(CE, *FData, C))
+  // Marks the function's arguments and/or return value tainted if it
+  // present in the list.
+  if (addSourcesPre(Call, *FData, C))
     return;
 
-  addFiltersPre(CE, *FData, C);
+  addFiltersPre(Call, *FData, C);
 }
 
-void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
+void GenericTaintChecker::checkPostCall(const CallEvent &Call,
                                         CheckerContext &C) const {
   // Set the marked values as tainted. The return value only accessible from
   // checkPostStmt.
-  propagateFromPre(CE, C);
+  propagateFromPre(Call, C);
 }
 
 void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,
@@ -583,9 +585,9 @@
   printTaint(State, Out, NL, Sep);
 }
 
-bool GenericTaintChecker::addOverloadedOpPre(const CallExpr *CE,
-                                             CheckerContext &C) const {
-  const auto *OCE = dyn_cast<CXXOperatorCallExpr>(CE);
+bool GenericTaintChecker::addOverloadedOpPre(const CallEvent &Call,
+                                             CheckerContext &C) {
+  const auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call.getOriginExpr());
   if (OCE) {
     TaintPropagationRule Rule;
     switch (OCE->getOperator()) {
@@ -597,9 +599,9 @@
       break;
     default:
       return false;
-    };
+    }
 
-    ProgramStateRef State = Rule.process(CE, C);
+    ProgramStateRef State = Rule.process(Call, C);
     if (State) {
       C.addTransition(State);
       return true;
@@ -608,14 +610,14 @@
   return false;
 }
 
-bool GenericTaintChecker::addSourcesPre(const CallExpr *CE,
+bool GenericTaintChecker::addSourcesPre(const CallEvent &Call,
                                         const FunctionData &FData,
                                         CheckerContext &C) const {
   // First, try generating a propagation rule for this function.
   TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule(
       this->CustomPropagations, FData, C);
   if (!Rule.isNull()) {
-    ProgramStateRef State = Rule.process(CE, C);
+    ProgramStateRef State = Rule.process(Call, C);
     if (State) {
       C.addTransition(State);
       return true;
@@ -624,7 +626,7 @@
   return false;
 }
 
-bool GenericTaintChecker::addFiltersPre(const CallExpr *CE,
+bool GenericTaintChecker::addFiltersPre(const CallEvent &Call,
                                         const FunctionData &FData,
                                         CheckerContext &C) const {
   auto It = findFunctionInConfig(CustomFilters, FData);
@@ -635,11 +637,11 @@
   const auto &Value = It->second;
   const ArgVector &Args = Value.second;
   for (unsigned ArgNum : Args) {
-    if (ArgNum >= getNumArgs(CE))
+    if (ArgNum >= getNumArgs(Call))
       continue;
 
-    const Expr *Arg = getArg(CE, ArgNum);
-    Optional<SVal> V = getPointedToSVal(C, Arg);
+    const Expr *Arg = getArg(Call, ArgNum);
+    Optional<SVal> V = getPointeeOf(C, Arg);
     if (V)
       State = removeTaint(State, *V);
   }
@@ -651,8 +653,8 @@
   return false;
 }
 
-bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
-                                           CheckerContext &C) const {
+bool GenericTaintChecker::propagateFromPre(const CallEvent &Call,
+                                           CheckerContext &C) {
   ProgramStateRef State = C.getState();
 
   // Depending on what was tainted at pre-visit, we determined a set of
@@ -665,16 +667,16 @@
   for (unsigned ArgNum : TaintArgs) {
     // Special handling for the tainted return value.
     if (ArgNum == ReturnValueIndex) {
-      State = addTaint(State, CE, C.getLocationContext());
+      State = addTaint(State, Call.getReturnValue());
       continue;
     }
 
     // The arguments are pointer arguments. The data they are pointing at is
     // tainted after the call.
-    if (getNumArgs(CE) < (ArgNum + 1))
+    if (getNumArgs(Call) < (ArgNum + 1))
       return false;
-    const Expr *Arg = getArg(CE, ArgNum);
-    Optional<SVal> V = getPointedToSVal(C, Arg);
+    const Expr *Arg = getArg(Call, ArgNum);
+    Optional<SVal> V = getPointeeOf(C, Arg);
     if (V)
       State = addTaint(State, *V);
   }
@@ -689,27 +691,25 @@
   return false;
 }
 
-bool GenericTaintChecker::checkPre(const CallExpr *CE,
+bool GenericTaintChecker::checkPre(const CallEvent &Call,
                                    const FunctionData &FData,
                                    CheckerContext &C) const {
 
-  if (checkUncontrolledFormatString(CE, C))
-    return true;
+  if (checkUncontrolledFormatString(Call, C))
 
-  if (checkSystemCall(CE, FData.Name, C))
     return true;
 
-  if (checkTaintedBufferSize(CE, FData.FDecl, C))
+  if (checkSystemCall(Call, FData.Name, C))
     return true;
 
-  if (checkCustomSinks(CE, FData, C))
+  if (checkTaintedBufferSize(Call, C))
     return true;
 
-  return false;
+  return checkCustomSinks(Call, FData, C);
 }
 
-Optional<SVal> GenericTaintChecker::getPointedToSVal(CheckerContext &C,
-                                                     const Expr *Arg) {
+Optional<SVal> GenericTaintChecker::getPointeeOf(CheckerContext &C,
+                                                 const Expr *Arg) {
   ProgramStateRef State = C.getState();
   SVal AddrVal = C.getSVal(Arg->IgnoreParens());
   if (AddrVal.isUnknownOrUndef())
@@ -734,31 +734,32 @@
 }
 
 ProgramStateRef
-GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE,
+GenericTaintChecker::TaintPropagationRule::process(const CallEvent &Call,
                                                    CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
   bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
-    if (ArgNum >= getNumArgs(CE))
+    if (ArgNum >= getNumArgs(Call))
       continue;
 
-    if ((IsTainted = isTaintedOrPointsToTainted(getArg(CE, ArgNum), State, C)))
+    if ((IsTainted =
+             isTaintedOrPointsToTainted(getArg(Call, ArgNum), State, C)))
       break;
   }
 
   // Check for taint in variadic arguments.
   if (!IsTainted && VariadicType::Src == VarType) {
     // Check if any of the arguments is tainted
-    for (unsigned i = VariadicIndex; i < getNumArgs(CE); ++i) {
-      if ((IsTainted = isTaintedOrPointsToTainted(getArg(CE, i), State, C)))
+    for (unsigned i = VariadicIndex; i < getNumArgs(Call); ++i) {
+      if ((IsTainted = isTaintedOrPointsToTainted(getArg(Call, i), State, C)))
         break;
     }
   }
 
   if (PropagationFunc)
-    IsTainted = PropagationFunc(IsTainted, CE, C);
+    IsTainted = PropagationFunc(IsTainted, Call, C);
 
   if (!IsTainted)
     return State;
@@ -771,7 +772,7 @@
       continue;
     }
 
-    if (ArgNum >= getNumArgs(CE))
+    if (ArgNum >= getNumArgs(Call))
       continue;
 
     // Mark the given argument.
@@ -784,14 +785,15 @@
     //   If they are not pointing to const data, mark data as tainted.
     //   TODO: So far we are just going one level down; ideally we'd need to
     //         recurse here.
-    for (unsigned i = VariadicIndex; i < getNumArgs(CE); ++i) {
-      const Expr *Arg = getArg(CE, i);
+    for (unsigned i = VariadicIndex; i < getNumArgs(Call); ++i) {
+      const Expr *Arg = getArg(Call, i);
       // Process pointer argument.
       const Type *ArgTy = Arg->getType().getTypePtr();
       QualType PType = ArgTy->getPointeeType();
       if ((!PType.isNull() && !PType.isConstQualified()) ||
-          (ArgTy->isReferenceType() && !Arg->getType().isConstQualified()))
+          (ArgTy->isReferenceType() && !Arg->getType().isConstQualified())) {
         State = State->add<TaintArgsOnPostVisit>(i);
+      }
     }
   }
 
@@ -799,10 +801,9 @@
 }
 
 // If argument 0(protocol domain) is network, the return value should get taint.
-bool GenericTaintChecker::TaintPropagationRule::postSocket(bool /*IsTainted*/,
-                                                           const CallExpr *CE,
-                                                           CheckerContext &C) {
-  SourceLocation DomLoc = getArg(CE, 0)->getExprLoc();
+bool GenericTaintChecker::TaintPropagationRule::postSocket(
+    bool /*IsTainted*/, const CallEvent &Call, CheckerContext &C) {
+  SourceLocation DomLoc = getArg(Call, 0)->getExprLoc();
   StringRef DomName = C.getMacroNameOrSpelling(DomLoc);
   // White list the internal communication protocols.
   if (DomName.equals("AF_SYSTEM") || DomName.equals("AF_LOCAL") ||
@@ -812,8 +813,8 @@
   return true;
 }
 
-bool GenericTaintChecker::isStdstream(const Expr *E, CheckerContext &C) {
-  llvm::Regex R{".*std::basic_istream.*"};
+bool GenericTaintChecker::isStdIstream(const Expr *E, CheckerContext &C) {
+  const llvm::Regex R{".*std::basic_istream.*"};
   std::string Error;
   if (!R.isValid(Error))
     assert(false);
@@ -830,21 +831,20 @@
   const MemRegion *MemReg = Val.getAsRegion();
 
   // The region should be symbolic, we do not know it's value.
-  const SymbolicRegion *SymReg = dyn_cast_or_null<SymbolicRegion>(MemReg);
+  const auto *SymReg = dyn_cast_or_null<SymbolicRegion>(MemReg);
   if (!SymReg)
     return false;
 
   // Get it's symbol and find the declaration region it's pointing to.
-  const SymbolRegionValue *Sm =
-      dyn_cast<SymbolRegionValue>(SymReg->getSymbol());
+  const auto *Sm = dyn_cast<SymbolRegionValue>(SymReg->getSymbol());
   if (!Sm)
     return false;
-  const DeclRegion *DeclReg = dyn_cast_or_null<DeclRegion>(Sm->getRegion());
+  const auto *DeclReg = dyn_cast_or_null<DeclRegion>(Sm->getRegion());
   if (!DeclReg)
     return false;
 
-  // This region corresponds to a declaration, find out if it's a global/extern
-  // variable named stdin with the proper type.
+  // This region corresponds to a declaration, find out if it's a
+  // global/extern variable named stdin with the proper type.
   if (const auto *D = dyn_cast_or_null<VarDecl>(DeclReg->getDecl())) {
     D = D->getCanonicalDecl();
     if ((D->getName().find("stdin") != StringRef::npos) && D->isExternC()) {
@@ -857,23 +857,23 @@
   return false;
 }
 
-static bool getPrintfFormatArgumentNum(const CallExpr *CE,
+static bool getPrintfFormatArgumentNum(const CallEvent &Call,
                                        const CheckerContext &C,
                                        unsigned &ArgNum) {
   // Find if the function contains a format string argument.
-  // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf, vsprintf,
-  // vsnprintf, syslog, custom annotated functions.
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf,
+  // vsprintf, vsnprintf, syslog, custom annotated functions.
+  const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();
   if (!FDecl)
     return false;
   for (const auto *Format : FDecl->specific_attrs<FormatAttr>()) {
     ArgNum = Format->getFormatIdx() - 1;
-    if ((Format->getType()->getName() == "printf") && getNumArgs(CE) > ArgNum)
+    if ((Format->getType()->getName() == "printf") && getNumArgs(Call) > ArgNum)
       return true;
   }
 
   // Or if a function is named setproctitle (this is a heuristic).
-  if (C.getCalleeName(CE).find("setproctitle") != StringRef::npos) {
+  if (C.getCalleeName(FDecl).find("setproctitle") != StringRef::npos) {
     ArgNum = 0;
     return true;
   }
@@ -887,7 +887,7 @@
 
   // Check for taint.
   ProgramStateRef State = C.getState();
-  Optional<SVal> PointedToSVal = getPointedToSVal(C, E);
+  Optional<SVal> PointedToSVal = getPointeeOf(C, E);
   SVal TaintedSVal;
   if (PointedToSVal && isTainted(State, *PointedToSVal))
     TaintedSVal = *PointedToSVal;
@@ -909,23 +909,23 @@
 }
 
 bool GenericTaintChecker::checkUncontrolledFormatString(
-    const CallExpr *CE, CheckerContext &C) const {
+    const CallEvent &Call, CheckerContext &C) const {
   // Check if the function contains a format string argument.
   unsigned ArgNum = 0;
-  if (!getPrintfFormatArgumentNum(CE, C, ArgNum))
+  if (!getPrintfFormatArgumentNum(Call, C, ArgNum))
     return false;
 
   // If either the format string content or the pointer itself are tainted,
   // warn.
-  return generateReportIfTainted(getArg(CE, ArgNum),
+  return generateReportIfTainted(getArg(Call, ArgNum),
                                  MsgUncontrolledFormatString, C);
 }
 
-bool GenericTaintChecker::checkSystemCall(const CallExpr *CE, StringRef Name,
+bool GenericTaintChecker::checkSystemCall(const CallEvent &Call, StringRef Name,
                                           CheckerContext &C) const {
   // TODO: It might make sense to run this check on demand. In some cases,
-  // we should check if the environment has been cleansed here. We also might
-  // need to know if the user was reset before these calls(seteuid).
+  // we should check if the environment has been cleansed here. We also
+  // might need to know if the user was reset before these calls(seteuid).
   unsigned ArgNum = llvm::StringSwitch<unsigned>(Name)
                         .Case("system", 0)
                         .Case("popen", 0)
@@ -939,21 +939,22 @@
                         .Case("dlopen", 0)
                         .Default(InvalidArgIndex);
 
-  if (ArgNum == InvalidArgIndex || getNumArgs(CE) < (ArgNum + 1))
+  if (ArgNum == InvalidArgIndex || getNumArgs(Call) < (ArgNum + 1))
     return false;
 
-  return generateReportIfTainted(getArg(CE, ArgNum), MsgSanitizeSystemArgs, C);
+  return generateReportIfTainted(getArg(Call, ArgNum), MsgSanitizeSystemArgs,
+                                 C);
 }
 
 // TODO: Should this check be a part of the CString checker?
 // If yes, should taint be a global setting?
-bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE,
-                                                 const FunctionDecl *FDecl,
+bool GenericTaintChecker::checkTaintedBufferSize(const CallEvent &Call,
                                                  CheckerContext &C) const {
+  const auto *FDecl = Call.getDecl()->getAsFunction();
   // If the function has a buffer size argument, set ArgNum.
   unsigned ArgNum = InvalidArgIndex;
   unsigned BId = 0;
-  if ((BId = FDecl->getMemoryFunctionKind()))
+  if ((BId = FDecl->getMemoryFunctionKind())) {
     switch (BId) {
     case Builtin::BImemcpy:
     case Builtin::BImemmove:
@@ -965,26 +966,28 @@
       break;
     default:
       break;
-    };
+    }
+  }
 
   if (ArgNum == InvalidArgIndex) {
-    if (C.isCLibraryFunction(FDecl, "malloc") ||
-        C.isCLibraryFunction(FDecl, "calloc") ||
-        C.isCLibraryFunction(FDecl, "alloca"))
+    using CCtx = CheckerContext;
+    if (CCtx::isCLibraryFunction(FDecl, "malloc") ||
+        CCtx::isCLibraryFunction(FDecl, "calloc") ||
+        CCtx::isCLibraryFunction(FDecl, "alloca"))
       ArgNum = 0;
-    else if (C.isCLibraryFunction(FDecl, "memccpy"))
+    else if (CCtx::isCLibraryFunction(FDecl, "memccpy"))
       ArgNum = 3;
-    else if (C.isCLibraryFunction(FDecl, "realloc"))
+    else if (CCtx::isCLibraryFunction(FDecl, "realloc"))
       ArgNum = 1;
-    else if (C.isCLibraryFunction(FDecl, "bcopy"))
+    else if (CCtx::isCLibraryFunction(FDecl, "bcopy"))
       ArgNum = 2;
   }
 
-  return ArgNum != InvalidArgIndex && getNumArgs(CE) > ArgNum &&
-         generateReportIfTainted(getArg(CE, ArgNum), MsgTaintedBufferSize, C);
+  return ArgNum != InvalidArgIndex && getNumArgs(Call) > ArgNum &&
+         generateReportIfTainted(getArg(Call, ArgNum), MsgTaintedBufferSize, C);
 }
 
-bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE,
+bool GenericTaintChecker::checkCustomSinks(const CallEvent &Call,
                                            const FunctionData &FData,
                                            CheckerContext &C) const {
   auto It = findFunctionInConfig(CustomSinks, FData);
@@ -994,10 +997,10 @@
   const auto &Value = It->second;
   const GenericTaintChecker::ArgVector &Args = Value.second;
   for (unsigned ArgNum : Args) {
-    if (ArgNum >= getNumArgs(CE))
+    if (ArgNum >= getNumArgs(Call))
       continue;
 
-    if (generateReportIfTainted(getArg(CE, ArgNum), MsgCustomSink, C))
+    if (generateReportIfTainted(getArg(Call, ArgNum), MsgCustomSink, C))
       return true;
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to