martong updated this revision to Diff 292550.
martong added a comment.
- apply -> assume, Add isApplicable
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87785/new/
https://reviews.llvm.org/D87785
Files:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions.c
Index: clang/test/Analysis/std-c-library-functions.c
===================================================================
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -126,6 +126,25 @@
(void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
}
+static void test_fread_buffer_size_constraint_no_crash(FILE *f, size_t n) {
+ size_t rlen; /* how much to read */
+ size_t nr; /* number of chars actually read */
+ char b[8192];
+ rlen = 8192;
+ do {
+ char *p = b;
+ if (rlen > n)
+ rlen = n; /* cannot read more than asked */
+ nr = fread(p, sizeof(char), rlen, f);
+ n -= nr; /* still have to read `n' chars */
+ } while (n > 0 && nr == rlen); /* until end of count or eof */
+}
+// Test that we do not crash here.
+void test_fread_buffer_size_constraint_no_crash_caller(FILE *f) {
+ /* read MAX_SIZE_T chars */
+ test_fread_buffer_size_constraint_no_crash(f, ~((size_t)0));
+}
+
ssize_t getline(char **, size_t *, FILE *);
void test_getline(FILE *fp) {
char *line = 0;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -104,14 +104,18 @@
public:
ValueConstraint(ArgNo ArgN) : ArgN(ArgN) {}
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,
- CheckerContext &C) const = 0;
- virtual ValueConstraintPtr negate() const {
- llvm_unreachable("Not implemented");
- };
+
+ /// Returns true if we can do any assumption on the constraint, i.e. if it
+ /// is meaningful to call an Engine `assume` function.
+ virtual bool isApplicable(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const = 0;
+
+ /// Returns the program states for the cases when the constraint is
+ /// satisfiable and non satisfiable.
+ virtual std::pair<ProgramStateRef, ProgramStateRef>
+ assume(ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const = 0;
// 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
@@ -169,29 +173,25 @@
const Summary &Summary) const;
public:
- ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary,
- CheckerContext &C) const override {
- switch (Kind) {
- case OutOfRange:
- return applyAsOutOfRange(State, Call, Summary);
- case WithinRange:
- return applyAsWithinRange(State, Call, Summary);
- }
- llvm_unreachable("Unknown range kind!");
+ bool isApplicable(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override {
+ SVal V = getArgSVal(Call, getArgNo());
+ return V.getAs<NonLoc>().hasValue();
}
- ValueConstraintPtr negate() const override {
- RangeConstraint Tmp(*this);
+ std::pair<ProgramStateRef, ProgramStateRef>
+ assume(ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const override {
switch (Kind) {
case OutOfRange:
- Tmp.Kind = WithinRange;
- break;
+ return {applyAsOutOfRange(State, Call, Summary),
+ applyAsWithinRange(State, Call, Summary)};
case WithinRange:
- Tmp.Kind = OutOfRange;
- break;
+ return {applyAsWithinRange(State, Call, Summary),
+ applyAsOutOfRange(State, Call, Summary)};
}
- return std::make_shared<RangeConstraint>(Tmp);
+ llvm_unreachable("Unknown range kind!");
}
bool checkSpecificValidity(const FunctionDecl *FD) const override {
@@ -214,36 +214,37 @@
: ValueConstraint(ArgN), Opcode(Opcode), OtherArgN(OtherArgN) {}
ArgNo getOtherArgNo() const { return OtherArgN; }
BinaryOperator::Opcode getOpcode() const { return Opcode; }
- ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary,
- CheckerContext &C) const override;
+
+ bool isApplicable(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override {
+ SVal V = getArgSVal(Call, getArgNo());
+ SVal OtherV = getArgSVal(Call, getOtherArgNo());
+ return !(V.isUndef() || OtherV.isUndef());
+ }
+
+ std::pair<ProgramStateRef, ProgramStateRef>
+ assume(ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const override;
};
class NotNullConstraint : public ValueConstraint {
using ValueConstraint::ValueConstraint;
- // This variable has a role when we negate the constraint.
- bool CannotBeNull = true;
public:
StringRef getName() const override { return "NonNull"; }
- ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary,
- CheckerContext &C) const override {
- SVal V = getArgSVal(Call, getArgNo());
- if (V.isUndef())
- return State;
-
- DefinedOrUnknownSVal L = V.castAs<DefinedOrUnknownSVal>();
- if (!L.getAs<Loc>())
- return State;
- return State->assume(L, CannotBeNull);
+ bool isApplicable(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override {
+ SVal V = getArgSVal(Call, getArgNo());
+ return V.getAs<Loc>().hasValue();
}
- ValueConstraintPtr negate() const override {
- NotNullConstraint Tmp(*this);
- Tmp.CannotBeNull = !this->CannotBeNull;
- return std::make_shared<NotNullConstraint>(Tmp);
+ std::pair<ProgramStateRef, ProgramStateRef>
+ assume(ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const override {
+ return State->assume(getArgSVal(Call, getArgNo()).castAs<Loc>());
}
bool checkSpecificValidity(const FunctionDecl *FD) const override {
@@ -273,8 +274,6 @@
// `fread` like functions where the size is computed as a multiplication of
// two arguments.
llvm::Optional<ArgNo> SizeMultiplierArgN;
- // The operator we use in apply. This is negated in negate().
- BinaryOperator::Opcode Op = BO_LE;
public:
StringRef getName() const override { return "BufferSize"; }
@@ -286,9 +285,29 @@
: ValueConstraint(Buffer), SizeArgN(BufSize),
SizeMultiplierArgN(BufSizeMultiplier) {}
- ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
- const Summary &Summary,
- CheckerContext &C) const override {
+ bool isApplicable(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override {
+ SVal BufV = getArgSVal(Call, getArgNo());
+ if (BufV.isUndef())
+ return false;
+ if (SizeArgN) {
+ // The size argument.
+ SVal SizeV = getArgSVal(Call, *SizeArgN);
+ if (SizeV.isUndef())
+ return false;
+ if (SizeMultiplierArgN) {
+ SVal SizeMulV = getArgSVal(Call, *SizeMultiplierArgN);
+ if (SizeMulV.isUndef())
+ return false;
+ }
+ }
+ return true;
+ }
+
+ std::pair<ProgramStateRef, ProgramStateRef>
+ assume(ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const override {
SValBuilder &SvalBuilder = C.getSValBuilder();
// The buffer argument.
SVal BufV = getArgSVal(Call, getArgNo());
@@ -316,24 +335,9 @@
// The dynamic size of the buffer argument, got from the analyzer engine.
SVal BufDynSize = getDynamicSizeWithOffset(State, BufV);
- SVal Feasible = SvalBuilder.evalBinOp(State, Op, SizeV, BufDynSize,
+ SVal Feasible = SvalBuilder.evalBinOp(State, BO_LE, SizeV, BufDynSize,
SvalBuilder.getContext().BoolTy);
- if (auto F = Feasible.getAs<DefinedOrUnknownSVal>())
- return State->assume(*F, true);
-
- // We can get here only if the size argument or the dynamic size is
- // undefined. But the dynamic size should never be undefined, only
- // unknown. So, here, the size of the argument is undefined, i.e. we
- // cannot apply the constraint. Actually, other checkers like
- // CallAndMessage should catch this situation earlier, because we call a
- // function with an uninitialized argument.
- llvm_unreachable("Size argument or the dynamic size is Undefined");
- }
-
- ValueConstraintPtr negate() const override {
- BufferSizeConstraint Tmp(*this);
- Tmp.Op = BinaryOperator::negateComparisonOp(Op);
- return std::make_shared<BufferSizeConstraint>(Tmp);
+ return State->assume(Feasible.castAs<DefinedOrUnknownSVal>());
}
bool checkSpecificValidity(const FunctionDecl *FD) const override {
@@ -597,17 +601,16 @@
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);
- assert(Min <= Max);
- State = CM.assumeInclusiveRange(State, *N, Min, Max, false);
- if (!State)
- break;
- }
+ auto N = V.castAs<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);
+ assert(Min <= Max);
+ State = CM.assumeInclusiveRange(State, N, Min, Max, false);
+ if (!State)
+ break;
}
return State;
@@ -635,44 +638,44 @@
// A B C D
// Then we assume that the value is not in [-inf, A - 1],
// then not in [D + 1, +inf], then not in [B + 1, C - 1]
- if (auto N = V.getAs<NonLoc>()) {
- const IntRangeVector &R = getRanges();
- size_t E = R.size();
-
- const llvm::APSInt &MinusInf = BVF.getMinValue(T);
- const llvm::APSInt &PlusInf = BVF.getMaxValue(T);
+ auto N = V.castAs<NonLoc>();
+ const IntRangeVector &R = getRanges();
+ size_t E = R.size();
+
+ const llvm::APSInt &MinusInf = BVF.getMinValue(T);
+ const llvm::APSInt &PlusInf = BVF.getMaxValue(T);
+
+ const llvm::APSInt &Left = BVF.getValue(R[0].first - 1ULL, T);
+ if (Left != PlusInf) {
+ assert(MinusInf <= Left);
+ State = CM.assumeInclusiveRange(State, N, MinusInf, Left, false);
+ if (!State)
+ return nullptr;
+ }
- const llvm::APSInt &Left = BVF.getValue(R[0].first - 1ULL, T);
- if (Left != PlusInf) {
- assert(MinusInf <= Left);
- State = CM.assumeInclusiveRange(State, *N, MinusInf, Left, false);
- if (!State)
- return nullptr;
- }
+ const llvm::APSInt &Right = BVF.getValue(R[E - 1].second + 1ULL, T);
+ if (Right != MinusInf) {
+ assert(Right <= PlusInf);
+ State = CM.assumeInclusiveRange(State, N, Right, PlusInf, false);
+ if (!State)
+ return nullptr;
+ }
- const llvm::APSInt &Right = BVF.getValue(R[E - 1].second + 1ULL, T);
- if (Right != MinusInf) {
- assert(Right <= PlusInf);
- State = CM.assumeInclusiveRange(State, *N, Right, PlusInf, false);
+ for (size_t I = 1; I != E; ++I) {
+ const llvm::APSInt &Min = BVF.getValue(R[I - 1].second + 1ULL, T);
+ const llvm::APSInt &Max = BVF.getValue(R[I].first - 1ULL, T);
+ if (Min <= Max) {
+ State = CM.assumeInclusiveRange(State, N, Min, Max, false);
if (!State)
return nullptr;
}
-
- for (size_t I = 1; I != E; ++I) {
- const llvm::APSInt &Min = BVF.getValue(R[I - 1].second + 1ULL, T);
- const llvm::APSInt &Max = BVF.getValue(R[I].first - 1ULL, T);
- if (Min <= Max) {
- State = CM.assumeInclusiveRange(State, *N, Min, Max, false);
- if (!State)
- return nullptr;
- }
- }
}
return State;
}
-ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply(
+std::pair<ProgramStateRef, ProgramStateRef>
+StdLibraryFunctionsChecker::ComparisonConstraint::assume(
ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
CheckerContext &C) const {
@@ -688,10 +691,9 @@
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;
+ auto CompV =
+ SVB.evalBinOp(State, Op, V, OtherV, CondT).castAs<DefinedOrUnknownSVal>();
+ return State->assume(CompV);
}
void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call,
@@ -705,9 +707,11 @@
ProgramStateRef NewState = State;
for (const ValueConstraintPtr &Constraint : Summary.getArgConstraints()) {
- ProgramStateRef SuccessSt = Constraint->apply(NewState, Call, Summary, C);
- ProgramStateRef FailureSt =
- Constraint->negate()->apply(NewState, Call, Summary, C);
+ if (!Constraint->isApplicable(NewState, Call, Summary, C))
+ continue;
+ ProgramStateRef SuccessSt, FailureSt;
+ std::tie(SuccessSt, FailureSt) =
+ Constraint->assume(NewState, Call, Summary, C);
// The argument constraint is not satisfied.
if (FailureSt && !SuccessSt) {
if (ExplodedNode *N = C.generateErrorNode(NewState))
@@ -740,7 +744,8 @@
for (const ConstraintSet &Case : Summary.getCaseConstraints()) {
ProgramStateRef NewState = State;
for (const ValueConstraintPtr &Constraint : Case) {
- NewState = Constraint->apply(NewState, Call, Summary, C);
+ std::tie(NewState, std::ignore) =
+ Constraint->assume(NewState, Call, Summary, C);
if (!NewState)
break;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
