[analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

[martong]

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.

[balazske]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

114

Is it possible to have the validate on the signature (not function decl)? Because the function decl (where the rule is applied) should match the signature already. For example, if the signature contains an irrelevant type it is not good to have a rule that expects a void* at that place. After these rules are validated with the signature, they should be applicable on a (any) function that matches the signature.

344

So this is a check to be used at debug build only to check for programming errors?
It could be better to separate this task from setting the FD itself. Or this function can be called setFD and contain a validation if in debug mode (or always). Probably it is better to do the validation always to filter out unexpected found functions (if some unusual API is encountered).

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

114

void* would not be a problem. However, there are more intricate pointer types like FILE *. To get that type from the ASTContext we'd have to do another lookup for "FILE" which would complicate things. So, rather we mark that as Irrelevant in the signature. That's what we do in case of fread for instance. And after lookup we will have the concrete type in our hands, and that is the moment where we can do all the validation.

Perhaps we could alternatively have different kind of Irrelevant types like IrrelevantPtr, etc. But we'd forget to enumerate all of them.

344

The goal here is to avoid inadvertently adding a summary to a wrong function. This is all because of irrelevant types. The summary as described in the form of a signature and with cases and argument constraints has several presumptions on the types. And the type may be not concrete (irrelevant) so we cannot check the presumptions. We can validate those presumptions once we have the full and complete type.

It could be better to separate this task from setting the FD itself.

Yes, you are right, I am going to separate that.

Probably it is better to do the validation always to filter out unexpected found functions

Yes, I agree. I am not sure about the assertions though. Right now the programmer writes the summaries (e.g. me). But once in the future, a library author could write summaries in JSON format. Perhaps a warning diagnostic would be more appropriate that time. Or we could have a checker option that logs out if a function was added to the summary map.

[Szelethus]

The idea is noble with the addition of validate functions, assert in debug builds and just move on in release. However, I'd expect it to be integrated into the signature matching function.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

250–252

It might be worth putting a TODO here to not forget the constness methods :^)

732

This looks a bit odd, we're checking whether the function matches, and than we validate right after? Shouldn't we just not match the FD if it isn't valid?

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

250–252

Two types do not match if one of them is a const and the other is non-const. Is this what you're referring for?

732

Yeah, ok, I moved the validation back into matchesSignature.

[martong]

• Better encapsulate the data in a 'Summary'

[balazske]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

131

Function names should be verb phrases. Maybe checkSanity is good or validateInternal, validateMore (this function relates to validate, not something different from it as "sanity").

validate can be called isValid or checkValidity, so we can not think (from the name) that the function changes something on the object. (checkValidity is good, then sanityCheck can be renamed to checkSpecificValidity or like this.)

305–306

I would extend the documentation with information about that the signature and constraints together contain information about what functions are accepted by the summary. The signature can use "wildcards" (the irrelevant types) and the constraints may specify additional rules for that type (that comes from the kind of constraint).

349

Again isValid or checkValidity is a better name, or validateByConstraints, matchesConstraints. Additionally matchesSignature checks in its current form not only for signature, so its name is not correct too.

732

I think these are not related, there should be signature match and validness check, but it is natural to check first the signature, then the validness.

[martong]

• Rebase to master

[martong]

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

131

Thanks for the naming suggestions, they are definitely better than my naming!

305–306

OK, I added some more docs to the class.

349

Ok, renamed to validateByConstraints.

732

@Szelethus, @balazske : I agree with both of you so I renamed matchesSignature to matches, which is a shorthand to the the signature match and then the validity check (and added a comment):

// Returns true if the summary should be applied to the given function.
bool matches(const FunctionDecl *FD) const {
  return Sign.matches(FD) && validateByConstraints(FD);
}

[martong]

• Rename: validate -> checkValidity, sanityCheck -> checkSpecificValidity
• Rename: Summary::checkValidity -> validateByConstraints
• Add more docs to the Summary class
• Rename matchesSignature -> matches

[balazske]

The code looks basically good to me, some documentations can be improved.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

65–66

This text above is not the documentation of Summary (is it attached to Summary by doxygen?). Probably not /// is needed, only //. And it is probably out of date now, at least I can not understand immediately what is it about (what typedef's are these, what kind of "nesting types").

114

We check here that a function is a suitable candidate to be used with the constraint? (We select a function for the constraint, not a constraint for a function.) Maybe a better description would help to clarify this.

575

It may be better to see type of VRS instead of auto (and know what the VRS abbrevation means, why not CC for case constraint and not VC for ValueConstraint). Same for VR below.

732

Suggestion: Maybe we can have one function for matches and setFunctionDecl (set it if matches). We do not want to change the function decl later anyway, and not to something that does not match.

[martong]

• Add comments, remove auto from 'for' loops, matches -> matchesAndSet

[MaskRay]

arc adds many unneeded tags from Phabricator. You can drop Reviewers: Subscribers: Tags: and the text Summary: with the following script:

arcfilter () {
  arc amend
  git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people (https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You should keep the tag. (I started to use --date=now because some people find author date != committer date annoying. The committer date is usually what people care.))

[martong]

In D77658#2065174, @MaskRay wrote:

arc adds many unneeded tags from Phabricator. You can drop Reviewers: Subscribers: Tags: and the text Summary: with the following script:

arcfilter () {
  arc amend
  git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people (https://lists.llvm.org/pipermail/llvm-dev/2020-January/137889.html). You should keep the tag. (I started to use --date=now because some people find author date != committer date annoying. The committer date is usually what people care.))

Thanks for pointing this out and for the script! I'll try to omit unnecessary tags from arc in the future.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

65–66

Ok, I removed this outdated comment.

114

We check here, 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 arguments list).

Added a comment in the code about that.

575

Indeed, I agree. Removed auto and refreshed the outdated variable names.

732

Alright, I renamed matches to matchesAndSet and we set the FD in this function now. Also setFunctionDecl is removed.

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 
 using namespace clang;
 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.
   enum InvalidationKind { NoEvalCall, EvalCallAsPure };
 
   // The universal integral type to use in value range descriptions.
   // Unsigned to make sure overflows are well-defined.
   typedef uint64_t RangeInt;
public:
     virtual ~ValueConstraint() {}
     /// Apply the effects of the constraint on the given program state. If null
     /// is returned then the constraint is not feasible.
     virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
                                   const Summary &Summary) const = 0;
     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?
   enum RangeKind { OutOfRange, WithinRange };
 
   /// Encapsulates a single range on a single symbol within a branch.
   class RangeConstraint : public ValueConstraint {
     RangeKind Kind;      // Kind of range definition.
ValueConstraintPtr negate() const override {
         Tmp.Kind = WithinRange;
         break;
       case WithinRange:
         Tmp.Kind = OutOfRange;
         break;
       }
       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 {
     BinaryOperator::Opcode Opcode;
     ArgNo OtherArgN;
 
   public:
     ComparisonConstraint(ArgNo ArgN, BinaryOperator::Opcode Opcode,
ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
       return State->assume(L, CannotBeNull);
     }
 
     ValueConstraintPtr negate() const override {
       NotNullConstraint Tmp(*this);
       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;
+    }
   };
 
   /// The complete list of constraints that defines a single branch.
   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
   ///     is a tree). So, a branch is a series of assumptions. In other words,
   ///     branches represent split states and additional assumptions on top of
   ///     the splitting assumption.
   ///     For example, consider the branches in `isalpha(x)`
   ///       Branch 1)
   ///         x is in range ['A', 'Z'] or in ['a', 'z']
   ///         then the return value is not 0. (I.e. out-of-range [0, 0])
   ///       Branch 2)
   ///         x is out-of-range ['A', 'Z'] and out-of-range ['a', 'z']
   ///         then the return value is 0.
   ///   * 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));
       return *this;
     }
     Summary &ArgConstraint(ValueConstraintPtr VC) {
       ArgConstraints.push_back(VC);
       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);
+    }
+
+    // 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;
     }
 
-    /// 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;
+  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
   // lazily, and it doesn't change after initialization.
   using FunctionSummaryMapType = llvm::DenseMap<const FunctionDecl *, Summary>;
   mutable FunctionSummaryMapType FunctionSummaryMap;
 
   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);
   }
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;

 ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsOutOfRange(
     ProgramStateRef State, const CallEvent &Call,
     const Summary &Summary) const {
 
   ProgramStateManager &Mgr = State->getStateManager();
   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>()) {
     const IntRangeVector &R = getRanges();
     size_t E = R.size();
     for (size_t I = 0; I != E; ++I) {
       const llvm::APSInt &Min = BVF.getValue(R[I].first, T);
       const llvm::APSInt &Max = BVF.getValue(R[I].second, T);

 ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsWithinRange(
     ProgramStateRef State, const CallEvent &Call,
     const Summary &Summary) const {
 
   ProgramStateManager &Mgr = State->getStateManager();
   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".
   // We cut off [T_MIN, min(R) - 1] and [max(R) + 1, T_MAX] if necessary,
   // and then cut away all holes in R one by one.
   //
   // E.g. consider a range list R as [A, B] and [C, D]
   // -------+--------+------------------+------------+----------->

 
 ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply(
     ProgramStateRef State, const CallEvent &Call,
     const Summary &Summary) const {
 
   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)
                        .getAs<DefinedOrUnknownSVal>())
     State = State->assume(*CompV, true);
   return State;
 }
 
 void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
                                               CheckerContext &C) const {
   Optional<Summary> FoundSummary = findFunctionSummary(Call, C);
   if (!FoundSummary)
     return;
 
   const Summary &Summary = *FoundSummary;
   ProgramStateRef State = C.getState();
 
   ProgramStateRef NewState = State;
-  for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
+  for (const ValueConstraintPtr &VC : Summary.getArgConstraints()) {
     ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary);
     ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary);
     // The argument constraint is not satisfied.
     if (FailureSt && !SuccessSt) {
       if (ExplodedNode *N = C.generateErrorNode(NewState))
         reportBug(Call, N, C);
       break;
     } else {
void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
   if (!FoundSummary)
     return;
 
   // Now apply the constraints.
   const Summary &Summary = *FoundSummary;
   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);
+    for (const ValueConstraintPtr &Constraint : Case) {
+      NewState = Constraint->apply(NewState, Call, Summary);
       if (!NewState)
         break;
     }
 
     if (NewState && NewState != State)
       C.addTransition(NewState);
   }
 }
 
 bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
                                           CheckerContext &C) const {
   Optional<Summary> FoundSummary = findFunctionSummary(Call, C);
   if (!FoundSummary)
     return false;
 
   const Summary &Summary = *FoundSummary;
-  switch (Summary.InvalidationKd) {
+  switch (Summary.getInvalidationKd()) {
   case EvalCallAsPure: {
     ProgramStateRef State = C.getState();
     const LocationContext *LC = C.getLocationContext();
     const auto *CE = cast_or_null<CallExpr>(Call.getOriginExpr());
     SVal V = C.getSValBuilder().conjureSymbolVal(
         CE, LC, CE->getType().getCanonicalType(), C.blockCount());
     State = State->BindExpr(CE, LC, V);
     C.addTransition(State);
     return true;
   }
   case NoEvalCall:
     // Summary tells us to avoid performing eval::Call. The function is possibly
     // evaluated by another checker, or evaluated conservatively.
     return false;
   }
   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())
+  // 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;
   }
 
   return true;
 }
 
 Optional<StdLibraryFunctionsChecker::Summary>
 StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
void StdLibraryFunctionsChecker::initFunctionSummaries(
 
   // These types are useful for writing specifications quickly,
   // New specifications should probably introduce more types.
   // Some types are hard to obtain from the AST, eg. "ssize_t".
   // In such cases it should be possible to provide multiple variants
   // 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;
   const QualType SizeTy = ACtx.getSizeType();
   const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
   const QualType VoidPtrRestrictTy =
       ACtx.getRestrictType(VoidPtrTy); // void *restrict
   const QualType ConstVoidPtrTy =
struct AddToFunctionSummaryMap {
     AddToFunctionSummaryMap(const ASTContext &ACtx, FunctionSummaryMapType &FSM,
                             bool DisplayLoadedSummaries)
         : ACtx(ACtx), Map(FSM), DisplayLoadedSummaries(DisplayLoadedSummaries) {
     }
 
     // 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;
             if (DisplayLoadedSummaries) {
               llvm::errs() << "Loaded summary for: ";
               FD->print(llvm::errs());
               llvm::errs() << "\n";
             }