[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Daniel Marjamäki via Phabricator via cfe-commits]

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