[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 148809.
r.stahl added a comment.

Added FIXME tests.

I know my example didn't exercise your scenario, but it was the only one I 
could think of where returning zero would have been straight up wrong. Note 
that I default initialized `a` to 3 there, so `sarr` is not just 
zero-initialized.

I do not have access yet, but applied today!


https://reviews.llvm.org/D46823

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c
  test/Analysis/initialization.cpp


Index: test/Analysis/initialization.cpp
===================================================================
--- test/Analysis/initialization.cpp
+++ test/Analysis/initialization.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int a = 3;
+};
+S const sarr[2] = {};
+void definit() {
+  int i = 1;
+  // FIXME: Should recognize that it is 3.
+  clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
+}
+
+int const arr[2][2] = {};
+void arr2init() {
+  int i = 1;
+  // FIXME: Should recognize that it is 0.
+  clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/initialization.c
===================================================================
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+  i = 1;
+  clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+  i = -1;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
           // The array index has to be known.
           if (auto CI = R->getIndex().getAs<nonloc::ConcreteInt>()) {
             int64_t i = CI->getValue().getSExtValue();
-            // Return unknown value if index is out of bounds.
-            if (i < 0 || i >= InitList->getNumInits())
-              return UnknownVal();
+            // If it is known that the index is out of bounds, we can return
+            // an undefined value.
+            if (i < 0)
+              return UndefinedVal();
+
+            if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+              if (CAT->getSize().sle(i))
+                return UndefinedVal();
+
+            // If there is a list, but no init, it must be zero.
+            if (i >= InitList->getNumInits())
+              return svalBuilder.makeZeroVal(R->getElementType());
 
             if (const Expr *ElemInit = InitList->getInit(i))
               if (Optional<SVal> V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
     // Either the record variable or the field has to be const qualified.
     if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
       if (const Expr *Init = VD->getInit())
-        if (const auto *InitList = dyn_cast<InitListExpr>(Init))
-          if (Index < InitList->getNumInits())
+        if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
+          if (Index < InitList->getNumInits()) {
             if (const Expr *FieldInit = InitList->getInit(Index))
               if (Optional<SVal> V = svalBuilder.getConstantVal(FieldInit))
                 return *V;
+          } else {
+            return svalBuilder.makeZeroVal(Ty);
+          }
+        }
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits