martong updated this revision to Diff 267216. martong marked 2 inline comments as done. martong added a comment.
- Remove SValBuilder param Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77066/new/ https://reviews.llvm.org/D77066 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp clang/lib/StaticAnalyzer/Core/DynamicSize.cpp clang/test/Analysis/std-c-library-functions-arg-constraints.c
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c =================================================================== --- clang/test/Analysis/std-c-library-functions-arg-constraints.c +++ clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -122,3 +122,30 @@ // bugpath-warning{{Function argument constraint is not satisfied}} \ // bugpath-note{{Function argument constraint is not satisfied}} } + +int __buf_size_arg_constraint(const void *, size_t); +void test_buf_size_concrete() { + char buf[3]; // bugpath-note{{'buf' initialized here}} + __buf_size_arg_constraint(buf, 4); // \ + // report-warning{{Function argument constraint is not satisfied}} \ + // bugpath-warning{{Function argument constraint is not satisfied}} \ + // bugpath-note{{Function argument constraint is not satisfied}} +} +void test_buf_size_symbolic(int s) { + char buf[3]; + __buf_size_arg_constraint(buf, s); + clang_analyzer_eval(s <= 3); // \ + // report-warning{{TRUE}} \ + // bugpath-warning{{TRUE}} \ + // bugpath-note{{TRUE}} \ + // bugpath-note{{'s' is <= 3}} +} +void test_buf_size_symbolic_and_offset(int s) { + char buf[3]; + __buf_size_arg_constraint(buf + 1, s); + clang_analyzer_eval(s <= 2); // \ + // report-warning{{TRUE}} \ + // bugpath-warning{{TRUE}} \ + // bugpath-note{{TRUE}} \ + // bugpath-note{{'s' is <= 2}} +} Index: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/DynamicSize.cpp +++ clang/lib/StaticAnalyzer/Core/DynamicSize.cpp @@ -44,5 +44,28 @@ return DivisionV.castAs<DefinedOrUnknownSVal>(); } +SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV) { + SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder(); + const MemRegion *MRegion = BufV.getAsRegion(); + if (!MRegion) + return UnknownVal(); + RegionOffset Offset = MRegion->getAsOffset(); + if (Offset.hasSymbolicOffset()) + return UnknownVal(); + const MemRegion *BaseRegion = MRegion->getBaseRegion(); + if (!BaseRegion) + return UnknownVal(); + + NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex( + Offset.getOffset() / + MRegion->getMemRegionManager().getContext().getCharWidth()); + DefinedOrUnknownSVal ExtentInBytes = + getDynamicSize(State, BaseRegion, SvalBuilder); + + return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub, + ExtentInBytes, OffsetInBytes, + SvalBuilder.getArrayIndexType()); +} + } // namespace ento } // namespace clang Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -56,6 +56,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" using namespace clang; using namespace clang::ento; @@ -108,7 +109,8 @@ /// 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; + const Summary &Summary, + CheckerContext &C) const = 0; virtual ValueConstraintPtr negate() const { llvm_unreachable("Not implemented"); }; @@ -143,7 +145,8 @@ const Summary &Summary) const; public: ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, - const Summary &Summary) const override { + const Summary &Summary, + CheckerContext &C) const override { switch (Kind) { case OutOfRange: return applyAsOutOfRange(State, Call, Summary); @@ -178,7 +181,8 @@ ArgNo getOtherArgNo() const { return OtherArgN; } BinaryOperator::Opcode getOpcode() const { return Opcode; } ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, - const Summary &Summary) const override; + const Summary &Summary, + CheckerContext &C) const override; }; class NotNullConstraint : public ValueConstraint { @@ -188,7 +192,8 @@ public: ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, - const Summary &Summary) const override { + const Summary &Summary, + CheckerContext &C) const override { SVal V = getArgSVal(Call, getArgNo()); if (V.isUndef()) return State; @@ -207,6 +212,51 @@ } }; + // Represents a buffer argument with an additional size argument. + // E.g. the first two arguments here: + // ctime_s(char *buffer, rsize_t bufsz, const time_t *time); + class BufferSizeConstraint : public ValueConstraint { + // The argument which holds the size of the buffer. + ArgNo SizeArgN; + // The operator we use in apply. This is negated in negate(). + BinaryOperator::Opcode Op = BO_LE; + + public: + BufferSizeConstraint(ArgNo Buffer, ArgNo BufSize) + : ValueConstraint(Buffer), SizeArgN(BufSize) {} + + ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, + const Summary &Summary, + CheckerContext &C) const override { + // The buffer argument. + SVal BufV = getArgSVal(Call, getArgNo()); + // The size argument. + SVal SizeV = getArgSVal(Call, SizeArgN); + // The dynamic size of the buffer argument, got from the analyzer engine. + SVal BufDynSize = getDynamicSizeWithOffset(State, BufV); + + SValBuilder &SvalBuilder = C.getSValBuilder(); + SVal Feasible = SvalBuilder.evalBinOp(State, Op, 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); + } + }; + /// The complete list of constraints that defines a single branch. typedef std::vector<ValueConstraintPtr> ConstraintSet; @@ -416,8 +466,8 @@ } ProgramStateRef StdLibraryFunctionsChecker::ComparisonConstraint::apply( - ProgramStateRef State, const CallEvent &Call, - const Summary &Summary) const { + ProgramStateRef State, const CallEvent &Call, const Summary &Summary, + CheckerContext &C) const { ProgramStateManager &Mgr = State->getStateManager(); SValBuilder &SVB = Mgr.getSValBuilder(); @@ -448,8 +498,8 @@ ProgramStateRef NewState = State; for (const ValueConstraintPtr& VC : Summary.ArgConstraints) { - ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary); - ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary); + ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary, C); + ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary, C); // The argument constraint is not satisfied. if (FailureSt && !SuccessSt) { if (ExplodedNode *N = C.generateErrorNode(NewState)) @@ -482,7 +532,7 @@ for (const auto &VRS : Summary.CaseConstraints) { ProgramStateRef NewState = State; for (const auto &VR: VRS) { - NewState = VR->apply(NewState, Call, Summary); + NewState = VR->apply(NewState, Call, Summary, C); if (!NewState) break; } @@ -694,6 +744,9 @@ IntRangeVector Ranges) { return std::make_shared<RangeConstraint>(ArgN, Kind, Ranges); }; + auto BufferSize = [](ArgNo BufArgN, ArgNo SizeArgN) { + return std::make_shared<BufferSizeConstraint>(BufArgN, SizeArgN); + }; struct { auto operator()(RangeKind Kind, IntRangeVector Ranges) { return std::make_shared<RangeConstraint>(Ret, Kind, Ranges); @@ -929,6 +982,12 @@ RetType{IntTy}, EvalCallAsPure) .ArgConstraint(NotNull(ArgNo(0))) .ArgConstraint(NotNull(ArgNo(1)))); + addToFunctionSummaryMap( + "__buf_size_arg_constraint", + Summary(ArgTypes{ConstVoidPtrTy, SizeTy}, RetType{IntTy}, + EvalCallAsPure) + .ArgConstraint( + BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1)))); } } Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp @@ -63,28 +63,8 @@ SVal PlacementNewChecker::getExtentSizeOfPlace(const CXXNewExpr *NE, CheckerContext &C) const { - ProgramStateRef State = C.getState(); const Expr *Place = NE->getPlacementArg(0); - - const MemRegion *MRegion = C.getSVal(Place).getAsRegion(); - if (!MRegion) - return UnknownVal(); - RegionOffset Offset = MRegion->getAsOffset(); - if (Offset.hasSymbolicOffset()) - return UnknownVal(); - const MemRegion *BaseRegion = MRegion->getBaseRegion(); - if (!BaseRegion) - return UnknownVal(); - - SValBuilder &SvalBuilder = C.getSValBuilder(); - NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex( - Offset.getOffset() / C.getASTContext().getCharWidth()); - DefinedOrUnknownSVal ExtentInBytes = - getDynamicSize(State, BaseRegion, SvalBuilder); - - return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub, - ExtentInBytes, OffsetInBytes, - SvalBuilder.getArrayIndexType()); + return getDynamicSizeWithOffset(C.getState(), C.getSVal(Place)); } SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE, Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h @@ -32,6 +32,21 @@ SValBuilder &SVB, QualType ElementTy); +/// Get the dynamic size for a symbolic value that represents a buffer. If +/// there is an offsetting to the underlying buffer we consider that too. +/// Returns with an SVal that represents the size, this is Unknown if the +/// engine cannot deduce the size. +/// E.g. +/// char buf[3]; +/// (buf); // size is 3 +/// (buf + 1); // size is 2 +/// (buf + 3); // size is 0 +/// (buf + 4); // size is -1 +/// +/// char *bufptr; +/// (bufptr) // size is unknown +SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal &BufV); + } // namespace ento } // namespace clang
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits