[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

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

danielmarjamaki created this revision.
danielmarjamaki added reviewers: zaks.anna, dcoughlin.
danielmarjamaki added a subscriber: cfe-commits.
danielmarjamaki set the repository for this revision to rL LLVM.

Example code:

  void f1(int x) {
    int a[20] = {0};
    if (x==25) {}
    if (a[x] == 123) {}  // <- Warning
  }

If I don't enable alpha, only core, then Clang writes this misleading FP:

  undef.c:5:12: warning: The left operand of '==' is a garbage value

I say it's a FP because the message is wrong. If the message correctly said "array index out of bounds" and pointed out a[x] directly, then it would be TP. This message goes away if alpha is enabled and I believe that is by intention.

Since there is a array-index-out-of-bounds check in alpha I am guessing that the UndefinedBinaryOperatorResult should not report "array index out of bounds". Therefore I remove this warning from this check.

This patch is a experimental work in progress. I would like to know if you think I should modifiy the UndefinedBinaryOperatorResult check or if I should do something in the ExprEngine? Maybe array index out of bounds should not lead to Undef SVal?

With this patch, all the existing tests succeed.


Repository:
  rL LLVM

https://reviews.llvm.org/D28278

Files:
  lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp


Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -61,7 +61,7 @@
     SmallString<256> sbuf;
     llvm::raw_svector_ostream OS(sbuf);
     const Expr *Ex = nullptr;
-    bool isLeft = true;
+    bool isLeft;
 
     if (state->getSVal(B->getLHS(), LCtx).isUndef()) {
       Ex = B->getLHS()->IgnoreParenCasts();
@@ -73,6 +73,24 @@
     }
 
     if (Ex) {
+      if (isa<ArraySubscriptExpr>(Ex)) {
+        SVal Loc = state->getSVal(Ex,LCtx);
+        if (Loc.isValid()) {
+          const MemRegion *MR = Loc.castAs<loc::MemRegionVal>().getRegion();
+          if (const ElementRegion *ER = dyn_cast<ElementRegion>(MR)) {
+            DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
+            DefinedOrUnknownSVal NumElements
+              = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion(),
+                ER->getValueType());
+            ProgramStateRef StInBound = state->assumeInBound(Idx, NumElements, true);
+            ProgramStateRef StOutBound = state->assumeInBound(Idx, NumElements, false);
+            if (StOutBound && !StInBound) {
+              return;
+            }
+          }
+        }
+      }
+
       OS << "The " << (isLeft ? "left" : "right")
          << " operand of '"
          << BinaryOperator::getOpcodeStr(B->getOpcode())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28278.83010.patch
Type: text/x-patch
Size: 1497 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170104/fd0057ae/attachment-0001.bin>