martong updated this revision to Diff 261855.
martong marked 2 inline comments as done.
martong added a comment.
- Better encapsulate the data in a 'Summary'
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77658/new/
https://reviews.llvm.org/D77658
Files:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -66,7 +66,7 @@
/// 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.
@@ -112,10 +112,23 @@
virtual ValueConstraintPtr negate() const {
llvm_unreachable("Not implemented");
};
+
+ /// Do sanity check on the constraint.
+ bool validate(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 sanityCheck(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 sanityCheck(const FunctionDecl *FD) const { return true; }
};
/// Given a range, should the argument stay inside or outside this range?
@@ -165,6 +178,14 @@
}
return std::make_shared<RangeConstraint>(Tmp);
}
+
+ bool sanityCheck(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 {
@@ -205,12 +226,61 @@
Tmp.CannotBeNull = !this->CannotBeNull;
return std::make_shared<NotNullConstraint>(Tmp);
}
+
+ bool sanityCheck(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
@@ -232,15 +302,19 @@
/// * 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;
+ 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));
@@ -251,24 +325,33 @@
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");
- }
+ void setFunctionDecl(const FunctionDecl *FD) { this->FD = FD; }
+
+ 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);
+ }
+
+ bool matchesSignature(const FunctionDecl *FD) const {
+ return Sign.matches(FD) && validate(FD);
}
- /// 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 validate(const FunctionDecl *FD) const {
+ for (const auto &Case : CaseConstraints)
+ for (const ValueConstraintPtr &Constraint : Case)
+ if (!Constraint->validate(FD))
+ return false;
+ for (const ValueConstraintPtr &Constraint : ArgConstraints)
+ if (!Constraint->validate(FD))
+ return false;
+ return true;
+ }
};
// The map of all functions supported by the checker. It is initialized
@@ -278,11 +361,6 @@
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);
}
@@ -337,7 +415,7 @@
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>()) {
@@ -364,7 +442,7 @@
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".
@@ -420,13 +498,13 @@
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)
@@ -445,7 +523,7 @@
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.
@@ -477,7 +555,7 @@
ProgramStateRef State = C.getState();
// Apply case/branch specifications.
- for (const auto &VRS : Summary.CaseConstraints) {
+ for (const auto &VRS : Summary.getCaseConstraints()) {
ProgramStateRef NewState = State;
for (const auto &VR: VRS) {
NewState = VR->apply(NewState, Call, Summary);
@@ -497,7 +575,7 @@
return false;
const Summary &Summary = *FoundSummary;
- switch (Summary.InvalidationKd) {
+ switch (Summary.getInvalidationKd()) {
case EvalCallAsPure: {
ProgramStateRef State = C.getState();
const LocationContext *LC = C.getLocationContext();
@@ -516,27 +594,23 @@
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;
}
@@ -597,8 +671,6 @@
// 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;
@@ -644,7 +716,7 @@
// 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)
@@ -652,6 +724,7 @@
for (Decl *D : LookupRes) {
if (auto *FD = dyn_cast<FunctionDecl>(D)) {
if (S.matchesSignature(FD)) {
+ S.setFunctionDecl(FD);
auto Res = Map.insert({FD->getCanonicalDecl(), S});
assert(Res.second && "Function already has a summary set!");
(void)Res;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
