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

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

danielmarjamaki created this revision.

This is a work in progress patch. I try to detect such error:

  void foo(int X) {
      int Buf[X];
      Buf[X] = 0;
  }

The patch successfully detects this bug. However it writes FP when you try to take the address.

I would like to know if you think my approach to add a checkPreStmt is good. I would need to add logic to handle *(Buf+X) also.

This checker doesn't really logically belong in the ArrayBounds checker but from a user perspective it seems to belong there. I can move it to a new checker if that sounds best.


Repository:
  rL LLVM

https://reviews.llvm.org/D30489

Files:
  lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp


Index: lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -24,17 +24,20 @@
 
 namespace {
 class ArrayBoundChecker :
-    public Checker<check::Location> {
+    public Checker<check::Location, check::PreStmt<ArraySubscriptExpr>> {
   mutable std::unique_ptr<BuiltinBug> BT;
+  mutable std::unique_ptr<BuiltinBug> BT_VLA;
 
 public:
   void checkLocation(SVal l, bool isLoad, const Stmt* S,
                      CheckerContext &C) const;
+  void checkPreStmt(const ArraySubscriptExpr *A, CheckerContext &C) const;
 };
 }
 
 void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS,
                                       CheckerContext &C) const {
+
   // Check for out of bound array element access.
   const MemRegion *R = l.getAsRegion();
   if (!R)
@@ -88,6 +91,39 @@
   C.addTransition(StInBound);
 }
 
+void ArrayBoundChecker::checkPreStmt(const ArraySubscriptExpr *A, CheckerContext &C) const
+{
+  const Expr *Base = A->getBase()->IgnoreImpCasts();
+
+  ASTContext &Ctx = C.getASTContext();
+  const VariableArrayType *VLA = Ctx.getAsVariableArrayType(Base->getType());
+  if (!VLA)
+    return;
+
+  ProgramStateRef State = C.getState();
+  SVal sizeV = State->getSVal(VLA->getSizeExpr(), C.getLocationContext());
+  SVal idxV = State->getSVal(A->getIdx(), C.getLocationContext());
+
+  // Is idx greater than size?
+  SValBuilder &Bldr = C.getSValBuilder();
+  SVal GE = Bldr.evalBinOp(State, BO_GE, idxV, sizeV, Bldr.getConditionType());
+  if (!GE.isConstant(1))
+    return;
+
+  ExplodedNode *N = C.generateErrorNode(State);
+  if (!N)
+    return;
+
+  if (!BT_VLA)
+    BT_VLA.reset(new BuiltinBug(
+      this, "Out-of-bound VLA access",
+      "Out-of-bounds VLA access (symbolically this index is greater than the size)"));
+  // Generate a report for this bug.
+  auto report = llvm::make_unique<BugReport>(*BT_VLA, BT_VLA->getDescription(), N);
+  report->addRange(A->getSourceRange());
+  C.emitReport(std::move(report));
+}
+
 void ento::registerArrayBoundChecker(CheckerManager &mgr) {
   mgr.registerChecker<ArrayBoundChecker>();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30489.90128.patch
Type: text/x-patch
Size: 2256 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170301/c0aa93ba/attachment-0001.bin>