danielmarjamaki updated this revision to Diff 119590. danielmarjamaki added a comment. Herald added a subscriber: szepet.
As suggested, use a ProgramState trait to detect VLA overflows. I did not yet manage to get a SubRegion from the DeclStmt that matches the location SubRegion. Therefore I am using VariableArrayType in the trait for now. Repository: rL LLVM https://reviews.llvm.org/D30489 Files: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp test/Analysis/out-of-bounds.c
Index: test/Analysis/out-of-bounds.c =================================================================== --- test/Analysis/out-of-bounds.c +++ test/Analysis/out-of-bounds.c @@ -174,3 +174,7 @@ clang_analyzer_eval(x <= 99); // expected-warning{{TRUE}} } +void vla(int X) { + char buf[X]; + buf[X] = 0; // expected-warning {{Out of bound memory access (access exceeds upper limit of memory block)}} +} Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -27,17 +27,18 @@ using namespace ento; namespace { -class ArrayBoundCheckerV2 : - public Checker<check::Location> { +class ArrayBoundCheckerV2 + : public Checker<check::Location, check::PreStmt<DeclStmt>> { mutable std::unique_ptr<BuiltinBug> BT; enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted }; void reportOOB(CheckerContext &C, ProgramStateRef errorState, OOB_Kind kind) const; public: - void checkLocation(SVal l, bool isLoad, const Stmt*S, + void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const; + void checkLocation(SVal l, bool isLoad, const Stmt *S, CheckerContext &C) const; }; @@ -64,7 +65,10 @@ void dump() const; void dumpToStream(raw_ostream &os) const; }; -} +} // namespace + +REGISTER_MAP_WITH_PROGRAMSTATE(VariableLengthArrayExtent, + const VariableArrayType *, NonLoc); static SVal computeExtentBegin(SValBuilder &svalBuilder, const MemRegion *region) { @@ -111,8 +115,46 @@ return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent); } +void ArrayBoundCheckerV2::checkPreStmt(const DeclStmt *DS, + CheckerContext &checkerContext) const { + const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl()); + if (!VD) + return; + + ASTContext &Ctx = checkerContext.getASTContext(); + const VariableArrayType *VLA = Ctx.getAsVariableArrayType(VD->getType()); + if (!VLA) + return; + + SVal VLASize = checkerContext.getSVal(VLA->getSizeExpr()); + + Optional<NonLoc> Sz = VLASize.getAs<NonLoc>(); + if (!Sz) + return; + + ProgramStateRef state = checkerContext.getState(); + checkerContext.addTransition( + state->set<VariableLengthArrayExtent>(VLA, Sz.getValue())); +} + +static const VariableArrayType *getVLAFromExtent(DefinedOrUnknownSVal extentVal, + ASTContext &Ctx) { + if (extentVal.getSubKind() != nonloc::SymbolValKind) + return nullptr; + + SymbolRef SR = extentVal.castAs<nonloc::SymbolVal>().getSymbol(); + const SymbolExtent *SE = dyn_cast<SymbolExtent>(SR); + const MemRegion *SEMR = SE->getRegion(); + if (SEMR->getKind() != MemRegion::VarRegionKind) + return nullptr; + + const VarRegion *VR = cast<VarRegion>(SEMR); + QualType T = VR->getDecl()->getType(); + return Ctx.getAsVariableArrayType(T); +} + void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad, - const Stmt* LoadS, + const Stmt *LoadS, CheckerContext &checkerContext) const { // NOTE: Instead of using ProgramState::assumeInBound(), we are prototyping @@ -175,13 +217,21 @@ } do { + // CHECK UPPER BOUND: Is byteOffset >= extent(baseRegion)? If so, // we are doing a load/store after the last valid offset. DefinedOrUnknownSVal extentVal = - rawOffset.getRegion()->getExtent(svalBuilder); + rawOffset.getRegion()->getExtent(svalBuilder); if (!extentVal.getAs<NonLoc>()) break; + if (const VariableArrayType *VLA = + getVLAFromExtent(extentVal, checkerContext.getASTContext())) { + const NonLoc *V = state->get<VariableLengthArrayExtent>(VLA); + if (V) + extentVal = *V; + } + if (extentVal.getAs<nonloc::ConcreteInt>()) { std::pair<NonLoc, nonloc::ConcreteInt> simplifiedOffsets = getSimplifiedOffsets(rawOffset.getByteOffset(),
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits