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,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 + 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().hasValue(); } - ValueConstraintPtr negate() const override { - RangeConstraint Tmp(*this); + std::pair + 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(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 + 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(); - if (!L.getAs()) - 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().hasValue(); } - ValueConstraintPtr negate() const override { - NotNullConstraint Tmp(*this); - Tmp.CannotBeNull = !this->CannotBeNull; - return std::make_shared(Tmp); + std::pair + assume(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, + CheckerContext &C) const override { + return State->assume(getArgSVal(Call, getArgNo()).castAs()); } 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 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 + 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()) - 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(Tmp); + return State->assume(Feasible.castAs()); } 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()) { - 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(); + 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()) { - 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(); + 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 +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()) - State = State->assume(*CompV, true); - return State; + auto CompV = + SVB.evalBinOp(State, Op, V, OtherV, CondT).castAs(); + 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; } 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;