[analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

Summary:
Once we found a matching FunctionDecl for the given summary then we
validate the given constraints against that FunctionDecl. E.g. we
validate that a NotNull constraint is applied only on arguments that
have pointer types.

This is needed because when we matched the signature of the summary we
were working with incomplete function types, i.e. some intricate type
could have been marked as `Irrelevant` in the signature.

Reviewers: NoQ, Szelethus, balazske

Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, steakhal, ASDenysPetrov, cfe-commits

Tags: #clang

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

Author: Gabor Marton

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

@@ -64,10 +64,8 @@ using namespace clang::ento;
 namespace {
 class StdLibraryFunctionsChecker
     : public Checker<check::PreCall, check::PostCall, eval::Call> {
-  /// Below is a series of typedefs necessary to define function specs.
-  /// We avoid nesting types here because each additional qualifier
-  /// would need to be repeated in every function spec.
-  struct Summary;
+
+  class Summary;
 
   /// Specify how much the analyzer engine should entrust modeling this function
   /// to us. If he doesn't, he performs additional invalidations.
@@ -114,10 +112,27 @@ class StdLibraryFunctionsChecker
     virtual ValueConstraintPtr negate() const {
       llvm_unreachable("Not implemented");
     };
+
+    // Check whether the constraint is malformed or not. It is malformed if the
+    // specified argument has a mismatch with the given FunctionDecl (e.g. the
+    // arg number is out-of-range of the function's argument list).
+    bool checkValidity(const FunctionDecl *FD) const {
+      const bool ValidArg = ArgN == Ret || ArgN < FD->getNumParams();
+      assert(ValidArg && "Arg out of range!");
+      if (!ValidArg)
+        return false;
+      // Subclasses may further refine the validation.
+      return checkSpecificValidity(FD);
+    }
     ArgNo getArgNo() const { return ArgN; }
 
   protected:
     ArgNo ArgN; // Argument to which we apply the constraint.
+
+    /// Do polymorphic sanity check on the constraint.
+    virtual bool checkSpecificValidity(const FunctionDecl *FD) const {
+      return true;
+    }
   };
 
   /// Given a range, should the argument stay inside or outside this range?
@@ -168,6 +183,14 @@ class StdLibraryFunctionsChecker
       }
       return std::make_shared<RangeConstraint>(Tmp);
     }
+
+    bool checkSpecificValidity(const FunctionDecl *FD) const override {
+      const bool ValidArg =
+          getArgType(FD, ArgN)->isIntegralType(FD->getASTContext());
+      assert(ValidArg &&
+             "This constraint should be applied on an integral type");
+      return ValidArg;
+    }
   };
 
   class ComparisonConstraint : public ValueConstraint {
@@ -210,6 +233,13 @@ class StdLibraryFunctionsChecker
       Tmp.CannotBeNull = !this->CannotBeNull;
       return std::make_shared<NotNullConstraint>(Tmp);
     }
+
+    bool checkSpecificValidity(const FunctionDecl *FD) const override {
+      const bool ValidArg = getArgType(FD, ArgN)->isPointerType();
+      assert(ValidArg &&
+             "This constraint should be applied only on a pointer type");
+      return ValidArg;
+    }
   };
 
   // Represents a buffer argument with an additional size argument.
@@ -278,11 +308,52 @@ class StdLibraryFunctionsChecker
   typedef std::vector<ValueConstraintPtr> ConstraintSet;
 
   using ArgTypes = std::vector<QualType>;
+
+  // A placeholder type, we use it whenever we do not care about the concrete
+  // type in a Signature.
+  const QualType Irrelevant{};
+  bool static isIrrelevant(QualType T) { return T.isNull(); }
+
+  // The signature of a function we want to describe with a summary. This is a
+  // concessive signature, meaning there may be irrelevant types in the
+  // signature which we do not check against a function with concrete types.
+  struct Signature {
+    const ArgTypes ArgTys;
+    const QualType RetTy;
+    Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
+      assertRetTypeSuitableForSignature(RetTy);
+      for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
+        QualType ArgTy = ArgTys[I];
+        assertArgTypeSuitableForSignature(ArgTy);
+      }
+    }
+    bool matches(const FunctionDecl *FD) const;
+
+  private:
+    static void assertArgTypeSuitableForSignature(QualType T) {
+      assert((T.isNull() || !T->isVoidType()) &&
+             "We should have no void types in the spec");
+      assert((T.isNull() || T.isCanonical()) &&
+             "We should only have canonical types in the spec");
+    }
+    static void assertRetTypeSuitableForSignature(QualType T) {
+      assert((T.isNull() || T.isCanonical()) &&
+             "We should only have canonical types in the spec");
+    }
+  };
+
+  static QualType getArgType(const FunctionDecl *FD, ArgNo ArgN) {
+    assert(FD && "Function must be set");
+    QualType T = (ArgN == Ret)
+                     ? FD->getReturnType().getCanonicalType()
+                     : FD->getParamDecl(ArgN)->getType().getCanonicalType();
+    return T;
+  }
+
   using Cases = std::vector<ConstraintSet>;
 
-  /// Includes information about
-  ///   * function prototype (which is necessary to
-  ///     ensure we're modeling the right function and casting values properly),
+  /// A summary includes information about
+  ///   * function prototype (signature)
   ///   * approach to invalidation,
   ///   * a list of branches - a list of list of ranges -
   ///     A branch represents a path in the exploded graph of a function (which
@@ -299,15 +370,28 @@ class StdLibraryFunctionsChecker
   ///   * a list of argument constraints, that must be true on every branch.
   ///     If these constraints are not satisfied that means a fatal error
   ///     usually resulting in undefined behaviour.
-  struct Summary {
-    const ArgTypes ArgTys;
-    const QualType RetTy;
+  ///
+  /// Application of a summary:
+  ///   The signature and argument constraints together contain information
+  ///   about which functions are handled by the summary. The signature can use
+  ///   "wildcards", i.e. Irrelevant types. Irrelevant type of a parameter in
+  ///   a signature means that type is not compared to the type of the parameter
+  ///   in the found FunctionDecl. Argument constraints may specify additional
+  ///   rules for the given parameter's type, those rules are checked once the
+  ///   signature is matched.
+  class Summary {
+    const Signature Sign;
     const InvalidationKind InvalidationKd;
     Cases CaseConstraints;
     ConstraintSet ArgConstraints;
 
+    // The function to which the summary applies. This is set after lookup and
+    // match to the signature.
+    const FunctionDecl *FD = nullptr;
+
+  public:
     Summary(ArgTypes ArgTys, QualType RetTy, InvalidationKind InvalidationKd)
-        : ArgTys(ArgTys), RetTy(RetTy), InvalidationKd(InvalidationKd) {}
+        : Sign(ArgTys, RetTy), InvalidationKd(InvalidationKd) {}
 
     Summary &Case(ConstraintSet&& CS) {
       CaseConstraints.push_back(std::move(CS));
@@ -318,24 +402,38 @@ class StdLibraryFunctionsChecker
       return *this;
     }
 
-  private:
-    static void assertTypeSuitableForSummary(QualType T) {
-      assert(!T->isVoidType() &&
-             "We should have had no significant void types in the spec");
-      assert(T.isCanonical() &&
-             "We should only have canonical types in the spec");
-    }
+    InvalidationKind getInvalidationKd() const { return InvalidationKd; }
+    const Cases &getCaseConstraints() const { return CaseConstraints; }
+    const ConstraintSet &getArgConstraints() const { return ArgConstraints; }
 
-  public:
     QualType getArgType(ArgNo ArgN) const {
-      QualType T = (ArgN == Ret) ? RetTy : ArgTys[ArgN];
-      assertTypeSuitableForSummary(T);
-      return T;
+      return StdLibraryFunctionsChecker::getArgType(FD, ArgN);
     }
 
-    /// Try our best to figure out if the summary's signature matches
-    /// *the* library function to which this specification applies.
-    bool matchesSignature(const FunctionDecl *FD) const;
+    // Returns true if the summary should be applied to the given function.
+    // And if yes then store the function declaration.
+    bool matchesAndSet(const FunctionDecl *FD) {
+      bool Result = Sign.matches(FD) && validateByConstraints(FD);
+      if (Result) {
+        assert(!this->FD && "FD must not be set more than once");
+        this->FD = FD;
+      }
+      return Result;
+    }
+
+  private:
+    // Once we know the exact type of the function then do sanity check on all
+    // the given constraints.
+    bool validateByConstraints(const FunctionDecl *FD) const {
+      for (const ConstraintSet &Case : CaseConstraints)
+        for (const ValueConstraintPtr &Constraint : Case)
+          if (!Constraint->checkValidity(FD))
+            return false;
+      for (const ValueConstraintPtr &Constraint : ArgConstraints)
+        if (!Constraint->checkValidity(FD))
+          return false;
+      return true;
+    }
   };
 
   // The map of all functions supported by the checker. It is initialized
@@ -345,11 +443,6 @@ class StdLibraryFunctionsChecker
 
   mutable std::unique_ptr<BugType> BT_InvalidArg;
 
-  // Auxiliary functions to support ArgNo within all structures
-  // in a unified manner.
-  static QualType getArgType(const Summary &Summary, ArgNo ArgN) {
-    return Summary.getArgType(ArgN);
-  }
   static SVal getArgSVal(const CallEvent &Call, ArgNo ArgN) {
     return ArgN == Ret ? Call.getReturnValue() : Call.getArgSVal(ArgN);
   }
@@ -406,7 +499,7 @@ ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsOutOfRange(
   SValBuilder &SVB = Mgr.getSValBuilder();
   BasicValueFactory &BVF = SVB.getBasicValueFactory();
   ConstraintManager &CM = Mgr.getConstraintManager();
-  QualType T = getArgType(Summary, getArgNo());
+  QualType T = Summary.getArgType(getArgNo());
   SVal V = getArgSVal(Call, getArgNo());
 
   if (auto N = V.getAs<NonLoc>()) {
@@ -433,7 +526,7 @@ ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsWithinRange(
   SValBuilder &SVB = Mgr.getSValBuilder();
   BasicValueFactory &BVF = SVB.getBasicValueFactory();
   ConstraintManager &CM = Mgr.getConstraintManager();
-  QualType T = getArgType(Summary, getArgNo());
+  QualType T = Summary.getArgType(getArgNo());
   SVal V = getArgSVal(Call, getArgNo());
 
   // "WithinRange R" is treated as "outside [T_MIN, T_MAX] \ R".
@@ -489,13 +582,13 @@ ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply(
   ProgramStateManager &Mgr = State->getStateManager();
   SValBuilder &SVB = Mgr.getSValBuilder();
   QualType CondT = SVB.getConditionType();
-  QualType T = getArgType(Summary, getArgNo());
+  QualType T = Summary.getArgType(getArgNo());
   SVal V = getArgSVal(Call, getArgNo());
 
   BinaryOperator::Opcode Op = getOpcode();
   ArgNo OtherArg = getOtherArgNo();
   SVal OtherV = getArgSVal(Call, OtherArg);
-  QualType OtherT = getArgType(Summary, OtherArg);
+  QualType OtherT = Summary.getArgType(OtherArg);
   // Note: we avoid integral promotion for comparison.
   OtherV = SVB.evalCast(OtherV, T, OtherT);
   if (auto CompV = SVB.evalBinOp(State, Op, V, OtherV, CondT)
@@ -514,9 +607,10 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
   ProgramStateRef State = C.getState();
 
   ProgramStateRef NewState = State;
-  for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
-    ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary, C);
-    ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary, C);
+  for (const ValueConstraintPtr &Constraint : Summary.getArgConstraints()) {
+    ProgramStateRef SuccessSt = Constraint->apply(NewState, Call, Summary, C);
+    ProgramStateRef FailureSt =
+        Constraint->negate()->apply(NewState, Call, Summary, C);
     // The argument constraint is not satisfied.
     if (FailureSt && !SuccessSt) {
       if (ExplodedNode *N = C.generateErrorNode(NewState))
@@ -546,10 +640,10 @@ void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
   ProgramStateRef State = C.getState();
 
   // Apply case/branch specifications.
-  for (const auto &VRS : Summary.CaseConstraints) {
+  for (const ConstraintSet &Case : Summary.getCaseConstraints()) {
     ProgramStateRef NewState = State;
-    for (const auto &VR: VRS) {
-      NewState = VR->apply(NewState, Call, Summary, C);
+    for (const ValueConstraintPtr &Constraint : Case) {
+      NewState = Constraint->apply(NewState, Call, Summary, C);
       if (!NewState)
         break;
     }
@@ -566,7 +660,7 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
     return false;
 
   const Summary &Summary = *FoundSummary;
-  switch (Summary.InvalidationKd) {
+  switch (Summary.getInvalidationKd()) {
   case EvalCallAsPure: {
     ProgramStateRef State = C.getState();
     const LocationContext *LC = C.getLocationContext();
@@ -585,27 +679,23 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
   llvm_unreachable("Unknown invalidation kind!");
 }
 
-bool StdLibraryFunctionsChecker::Summary::matchesSignature(
+bool StdLibraryFunctionsChecker::Signature::matches(
     const FunctionDecl *FD) const {
   // Check number of arguments:
   if (FD->param_size() != ArgTys.size())
     return false;
 
-  // Check return type if relevant:
-  if (!RetTy.isNull() && RetTy != FD->getReturnType().getCanonicalType())
-    return false;
+  // Check return type.
+  if (!isIrrelevant(RetTy))
+    if (RetTy != FD->getReturnType().getCanonicalType())
+      return false;
 
-  // Check argument types when relevant:
+  // Check argument types.
   for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-    QualType FormalT = ArgTys[I];
-    // Null type marks irrelevant arguments.
-    if (FormalT.isNull())
+    QualType ArgTy = ArgTys[I];
+    if (isIrrelevant(ArgTy))
       continue;
-
-    assertTypeSuitableForSummary(FormalT);
-
-    QualType ActualT = FD->getParamDecl(I)->getType().getCanonicalType();
-    if (ActualT != FormalT)
+    if (ArgTy != FD->getParamDecl(I)->getType().getCanonicalType())
       return false;
   }
 
@@ -651,8 +741,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   // of function summary for common cases (eg. ssize_t could be int or long
   // or long long, so three summary variants would be enough).
   // Of course, function variants are also useful for C++ overloads.
-  const QualType
-      Irrelevant{}; // A placeholder, whenever we do not care about the type.
   const QualType IntTy = ACtx.IntTy;
   const QualType LongTy = ACtx.LongTy;
   const QualType LongLongTy = ACtx.LongLongTy;
@@ -702,14 +790,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
     // Add a summary to a FunctionDecl found by lookup. The lookup is performed
     // by the given Name, and in the global scope. The summary will be attached
     // to the found FunctionDecl only if the signatures match.
-    void operator()(StringRef Name, const Summary &S) {
+    void operator()(StringRef Name, Summary S) {
       IdentifierInfo &II = ACtx.Idents.get(Name);
       auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
       if (LookupRes.size() == 0)
         return;
       for (Decl *D : LookupRes) {
         if (auto *FD = dyn_cast<FunctionDecl>(D)) {
-          if (S.matchesSignature(FD)) {
+          if (S.matchesAndSet(FD)) {
             auto Res = Map.insert({FD->getCanonicalDecl(), S});
             assert(Res.second && "Function already has a summary set!");
             (void)Res;