diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -104,14 +104,13 @@ 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"); - }; + + /// Apply the effects of the constraint on the given program state. Returns + /// the program states for the cases when the constraint is satisfiable and + /// non satisfiable. + virtual std::pair + apply(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,31 +168,20 @@ const Summary &Summary) const; public: - ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, - const Summary &Summary, - CheckerContext &C) const override { + std::pair + apply(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, + CheckerContext &C) const override { switch (Kind) { case OutOfRange: - return applyAsOutOfRange(State, Call, Summary); + return {applyAsOutOfRange(State, Call, Summary), + applyAsWithinRange(State, Call, Summary)}; case WithinRange: - return applyAsWithinRange(State, Call, Summary); + return {applyAsWithinRange(State, Call, Summary), + applyAsOutOfRange(State, Call, Summary)}; } llvm_unreachable("Unknown range kind!"); } - ValueConstraintPtr negate() const override { - RangeConstraint Tmp(*this); - switch (Kind) { - case OutOfRange: - Tmp.Kind = WithinRange; - break; - case WithinRange: - Tmp.Kind = OutOfRange; - break; - } - return std::make_shared(Tmp); - } - bool checkSpecificValidity(const FunctionDecl *FD) const override { const bool ValidArg = getArgType(FD, ArgN)->isIntegralType(FD->getASTContext()); @@ -214,36 +202,28 @@ : 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; + std::pair + apply(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 { + std::pair + apply(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, + CheckerContext &C) const override { SVal V = getArgSVal(Call, getArgNo()); if (V.isUndef()) - return State; + return {State, State}; DefinedOrUnknownSVal L = V.castAs(); if (!L.getAs()) - return State; - - return State->assume(L, CannotBeNull); - } + return {State, State}; - ValueConstraintPtr negate() const override { - NotNullConstraint Tmp(*this); - Tmp.CannotBeNull = !this->CannotBeNull; - return std::make_shared(Tmp); + return State->assume(L); } bool checkSpecificValidity(const FunctionDecl *FD) const override { @@ -273,8 +253,6 @@ // `fread` like functions where the size is computed as a multiplication of // two arguments. llvm::Optional 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 +264,9 @@ : ValueConstraint(Buffer), SizeArgN(BufSize), SizeMultiplierArgN(BufSizeMultiplier) {} - ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, - const Summary &Summary, - CheckerContext &C) const override { + std::pair + apply(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,10 +294,10 @@ // 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()) - return State->assume(*F, true); + return State->assume(*F); // We can get here only if the size argument or the dynamic size is // undefined. But the dynamic size should never be undefined, only @@ -330,12 +308,6 @@ 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(Tmp); - } - bool checkSpecificValidity(const FunctionDecl *FD) const override { const bool ValidArg = getArgType(FD, ArgN)->isPointerType(); assert(ValidArg && @@ -672,7 +644,8 @@ return State; } -ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply( +std::pair +StdLibraryFunctionsChecker::ComparisonConstraint::apply( ProgramStateRef State, const CallEvent &Call, const Summary &Summary, CheckerContext &C) const { @@ -690,8 +663,8 @@ OtherV = SVB.evalCast(OtherV, T, OtherT); if (auto CompV = SVB.evalBinOp(State, Op, V, OtherV, CondT) .getAs()) - State = State->assume(*CompV, true); - return State; + return State->assume(*CompV); + return {State, State}; } void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call, @@ -705,9 +678,9 @@ 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); + ProgramStateRef SuccessSt, FailureSt; + std::tie(SuccessSt, FailureSt) = + Constraint->apply(NewState, Call, Summary, C); // The argument constraint is not satisfied. if (FailureSt && !SuccessSt) { if (ExplodedNode *N = C.generateErrorNode(NewState)) @@ -740,7 +713,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->apply(NewState, Call, Summary, C); if (!NewState) break; } diff --git a/clang/test/Analysis/std-c-library-functions.c b/clang/test/Analysis/std-c-library-functions.c --- a/clang/test/Analysis/std-c-library-functions.c +++ b/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;