[analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

[ASDenysPetrov]

Summary

Add support of multi-dimensional arrays in RegionStoreManager::getBindingForElement. Handle nested ElementRegion's getting offsets and checking for being in bounds. Get values from the nested initialization lists using obtained offsets.

Example:

const int arr[3][2] = {{1, 2}, {3, 4}};
arr[0][0];  // 1
arr[0][1];  // 2
arr[0][2];  // UB
arr[1][0];  // 3
arr[1][1];  // 4
arr[1][-1]; // UB
arr[2][0];  // 0
arr[2][1];  // 0
arr[-2][0]; // UB

[martong]

Could you please rebase on top of D111542? That would make the changes direct and clear and the review would be easier.

[ASDenysPetrov]

In D111654#3060822, @martong wrote:

Could you please rebase on top of D111542? That would make the changes direct and clear and the review would be easier.

OK. I'll rebase.

[ASDenysPetrov]

Improved. Rebased on top of D107339.
@martong kind ping.

[steakhal]

This is an important step towards better handling of global initializer expressions.
I'm looking forward to it. Although, I have concerns to address.

clang/lib/AST/Type.cpp

141–143

These comments should be in the header file instead.

149

I suspect this not gonna look through typedefs.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1627–1628

Just spell out uint64_t here. We don't win much by using auto.

1632–1643

I dislike this part of code. It has side effects all over the place.
Please use immediately called lambdas to describe your intentions in a more pure manner.

Besides that, when I see ElementRegions, I'm always scared. They sometimes represent reinterpret casts.
Since you have dealt with so many casts and region store stuff in the past, I'm assuming you have thought about this here.
Although, I'm still scared slightly, and I'd like to have evidence of this thought process. Likely some assertions or at least some comments? Could you @ASDenysPetrov elaborate on this?

1642

I would rather take it by value. They are designed so.
A mutable reference is always a bad sign anyway.
BTW for a brief moment I thought reverse() returns a temporary, but it doesn't, so we don't dangle at least.

1645–1650

Please, restructure this. Probably negating the condition and returning on that path would do the job.

1648

Please use the isNegative(). And don't have sideffects in conditions. advance the iterator at the end of the if.
I also think you can eliminate the I variable.
That way the getExtValue() would be guarded by the negativity check. I know that your code would behave the same, but I would find that phrasing slightly easier to follow.
This way you could even spare the 2 comment lines.

1652

This is probably a rare case when a continue is preferred instead of the else statement.

clang/test/Analysis/initialization.cpp

15–21

Sorry If I was misunderstood in the previous patches.
I think, for this instance, the key is that arr is const, so this TU is supposed to provide this linker symbol, thus any other definitions would violate the ODR.
So, the FIXME is actually accurate, and we should report 0 here.

56

Does this work if the = {} is not present?

93

Uh, this would be a regression.
Accessing Unknown is not a bug, unlike accessing Undefined - which is a clear indication that we must have had UB in the calculation previously, to get this. So, this is slightly similar to llvm poison on that sense.

106

typo

[martong]

clang/include/clang/AST/Type.h

2953

Could this be a free function (not a member function)? That way we could put it as an implementation detail of RegionStore.cpp and we could spare the timely review from the AST code owners.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

443–445

Could you please add docs to this function? Especially to ConcreteOffsets param.

1627–1628

Perhaps this hunk could be in an individual implementation function?

1630–1643

I think this could be implemented in it's own separate function, couldn't it?

1645

Please address this TODO.

clang/test/Analysis/initialization.c

76

Very good!

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1632–1643

Please use immediately called lambdas.

I'll do.

They sometimes represent reinterpret casts. ... I'm assuming you have thought about this here.

Yes. This is my another revision in the stack. You are welcome: D110927

1645

I found the tests:

• Analysis/MemRegion.cpp
• Analysis/ctor.mm
• Analysis/mpichecker.cpp
• Analysis/string.c

They cover this check.

[ASDenysPetrov]

clang/test/Analysis/initialization.cpp

15–21

Right. FALSE expected. And it is fixed with the patch. But this case duplicates some more detailed cases I've added below. So I decide to remove it.

56

The compiler (AST) doesn't pass you through without an initializer by emitting a warning. But still there is a case without initializer at the end of the file. Yes, it does work.

93

This is a big problem with casts (in SValBuilder::evalCast). I've investigated it. Relates to D89055. IMO the solution will take a separate patch stack. I'm going to fix this next.

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1627–1628

There are two returns inside the loop. That would not such easy to do a separate function for this.

[ASDenysPetrov]

@martong @steakhal
Thank you for your remarks. Updated according to them (at least the best I could do for now).

[martong]

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1627–1628

There are two returns inside the loop. That would not such easy to do a separate function for this.

I think you could return with an Optional<SVal> and then in the call site you could check if the optional is set.
Optional<SVal> areOffsetsOutOfBounds(...) ?

Then in the caller:

if (Optional<SVal> CheckResult = areOffsetsOutOfBounds(...))
  return *CheckResult;

[steakhal]

Please, try to focus on marking inline comments done if you accomplished them.
It takes some time to reevaluate them one-by-one on each update.
Aside from that, I'd like to apply these and play with them but I'm frequently having conflicts applying your patches.

Regarding the patch, I think it's in a pretty good shape, given that you fix the regression I pointed in an inline comment.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1637–1638

Can this be empty? If so, the subsequent .front() is UB.
Put here an assertion if you are sure that it cannot be empty.

clang/test/Analysis/initialization.cpp

15–21

Awesome, thanks!

56

Okay, thanks!

93

I think until you do that we should stick to the previous behavior.
Please address this regression, so report UNKNOWN for these cases or the correct answer.

[ASDenysPetrov]

I'll do an update soon taking into account all the suggestions.

Please, try to focus on marking inline comments done if you accomplished them.

In the next update.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1627–1628

Great idea.

[ASDenysPetrov]

@steakhal @martong Updated according to your comments.
I think I'm done with this patch.

[ASDenysPetrov]

• Added support of aliased types (work with canonical array type).
• Added tests aliased types.
• Added tests with initialization values in parentheses.

[ASDenysPetrov]

Changed test to fix a buildbot failure.

[steakhal]

I'm still worried about the fact that you assume that there is a correspondence between ElementRegions and InitListExprs.
I cannot see why this assumption holds, since reinterpret casts might introduce ElementRegions which could mess with this assumption.
Aside from that, I'm okay with the general idea of doing this kind of stuff.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1627–1628

I think you can hide this within the getConstantArrayExtents(). That is the only function touching this variable anyway.
That way you could also spare the comments about CAT requiring it to be canonical.

1652–1653

If I'm right then this is called the base of the region more often than not.
It would be probably a better choice than using MR.

1654–1656

IMO this is not a concern in real-life code.

1657–1658

I would prefer returning a pair instead of an output parameter.

[ASDenysPetrov]

@steakhal Thank you for your suggestion. I'll make corresponding changes.

I'm still worried about the fact that you assume that there is a correspondence between ElementRegions and InitListExprs.
I cannot see why this assumption holds, since reinterpret casts might introduce ElementRegions which could mess with this assumption.

Currently in practice ElementRegions for array element access expression look like InitListExprs structure. But you are right about mess with different casts. That's why I have FIXME's in glob_ptr_index2 and glob_invalid_index6.
Right now I'm working on fixing this stuff. I'm gonna refactor StoreManager::castRegion in a part of ElementRegions to distinguish between array indirections and casts.

[steakhal]

In D111654#3109585, @ASDenysPetrov wrote:

@steakhal Thank you for your suggestion. I'll make corresponding changes.

I'm still worried about the fact that you assume that there is a correspondence between ElementRegions and InitListExprs.
I cannot see why this assumption holds, since reinterpret casts might introduce ElementRegions which could mess with this assumption.

Currently in practice ElementRegions for array element access expression look like InitListExprs structure. But you are right about mess with different casts. That's why I have FIXME's in glob_ptr_index2 and glob_invalid_index6.
Right now I'm working on fixing this stuff. I'm gonna refactor StoreManager::castRegion in a part of ElementRegions to distinguish between array indirections and casts.

Awesome! Let's get the minor nits addressed and land it.
Thank you for working on this.

[ASDenysPetrov]

Updated according to @steakhal's comments.

[steakhal]

Feel free to commit your change given that you fix my final nits.
Yeah, I'm soo bad in review. I should have proposed these previously. Sorry.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

1631

The std::tie() pattern is probably more readable.

1632

You don't need to document this.
Aside from that consider marking these implementation detail functions static if they are not yet in an anonymous namespace - I haven't checked.

1637

I think we will get it :D

[ASDenysPetrov]

Updated according to the nits.

[ASDenysPetrov]

@steakhal

Thanks for your effort!

Yeah, I'm soo bad in review. I should have proposed these previously. Sorry.

No, you're not! I'm telling you. Keep it up! :)

clang/include/clang/AST/Type.h

class ConstantArrayType final
   }
 
   unsigned numTrailingObjects(OverloadToken<const Expr*>) const {
     return ConstantArrayTypeBits.HasStoredSizeExpr;
   }
 
 public:
   const llvm::APInt &getSize() const { return Size; }
+  SmallVector<uint64_t, 2> getAllExtents() const;
   const Expr *getSizeExpr() const {
     return ConstantArrayTypeBits.HasStoredSizeExpr
                ? *getTrailingObjects<const Expr *>()
                : nullptr;
   }
   bool isSugared() const { return false; }
   QualType desugar() const { return QualType(this, 0); }
 

clang/lib/AST/Type.cpp

: Type(tc, can,
                (tc == DependentSizedArray
                     ? TypeDependence::DependentInstantiation
                     : TypeDependence::None)),
       ElementType(et) {
   ArrayTypeBits.IndexTypeQuals = tq;
   ArrayTypeBits.SizeModifier = sm;
 }
 
+/// Return an array with extents of the declared array type.
+///
+/// E.g. for `const int x[1][2][3];` returns {1,2,3}.
+SmallVector<uint64_t, 2> ConstantArrayType::getAllExtents() const {
+  SmallVector<uint64_t, 2> Extents;
+  const ConstantArrayType *CAT = this;
+  do {
+    Extents.push_back(CAT->getSize().getZExtValue());
+  } while ((CAT = dyn_cast<ConstantArrayType>(CAT->getElementType())));
+  return Extents;
+}
+
 unsigned ConstantArrayType::getNumAddressingBits(const ASTContext &Context,
                                                  QualType ElementType,
                                                const llvm::APInt &NumElements) {
   uint64_t ElementSize = Context.getTypeSizeInChars(ElementType).getQuantity();
 
   // Fast path the common cases so we can avoid the conservative computation
   // below, which in common cases allocates "large" APSInt values, which are
   // slow.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

public:
   bool scanReachableSymbols(Store S, const MemRegion *R,
                             ScanReachableSymbols &Callbacks) override;
 
   RegionBindingsRef removeSubRegionBindings(RegionBindingsConstRef B,
                                             const SubRegion *R);
 
 public: // Part of public interface to class.
 
   StoreRef Bind(Store store, Loc LV, SVal V) override {
     return StoreRef(bind(getRegionBindings(store), LV, V).asStore(), *this);
   }
 
   RegionBindingsRef bind(RegionBindingsConstRef B, Loc LV, SVal V);
 
   // BindDefaultInitial is only used to initialize a region with
   // a default value.
   StoreRef BindDefaultInitial(Store store, const MemRegion *R,
                               SVal V) override {
     RegionBindingsRef B = getRegionBindings(store);
if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
 
     if (Result.second)
       Result.second = MRMgr.getCXXBaseObjectRegionWithSuper(BaseReg,
                                                             Result.second);
   }
 
   return Result;
 }
 
 SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
                                               const ElementRegion* R) {
   // Check if the region has a binding.
   if (const Optional<SVal> &V = B.getDirectBinding(R))
     return *V;
 
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
   if (const StringRegion *StrR = dyn_cast<StringRegion>(superR)) {
     // FIXME: Handle loads from strings where the literal is treated as
     // an integer, e.g., *((unsigned int*)"hello")
     QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
     if (!Ctx.hasSameUnqualifiedType(T, R->getElementType()))
       return UnknownVal();
 
     const StringLiteral *Str = StrR->getStringLiteral();
     SVal Idx = R->getIndex();
     if (Optional<nonloc::ConcreteInt> CI = Idx.getAs<nonloc::ConcreteInt>()) {
       int64_t i = CI->getValue().getSExtValue();
       // Abort on string underrun.  This can be possible by arbitrary
       // clients of getBindingForElement().
       if (i < 0)
         return UndefinedVal();
       int64_t length = Str->getLength();
       // Technically, only i == length is guaranteed to be null.
       // However, such overflows should be caught before reaching this point;
       // the only time such an access would be made is if a string literal was
       // used to initialize a larger array.
       char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
       return svalBuilder.makeIntVal(c, T);
     }
-  } else if (const VarRegion *VR = dyn_cast<VarRegion>(superR)) {
-    // Check if the containing array has an initialized value that we can trust.
-    // We can trust a const value or a value of a global initializer in main().
-    const VarDecl *VD = VR->getDecl();
+  } else if (isa<ElementRegion>(superR) || isa<VarRegion>(superR)) {
+    // Assume we are treating a n-dimensional array.
+    // Get offsets from the expression, like `arr[4][2][1];`
+    // `SValOffsets` should be {1, 2, 4};
+    const VarRegion *VR = dyn_cast<VarRegion>(superR);
+    SmallVector<SVal, 2> SValOffsets;
+    SValOffsets.push_back(R->getIndex());
+    if (const ElementRegion *ER = dyn_cast<ElementRegion>(superR)) {
+      const ElementRegion *LastER = nullptr;
+      do {
+        SValOffsets.push_back(ER->getIndex());
+        LastER = ER;
+        ER = dyn_cast<ElementRegion>(ER->getSuperRegion());
+      } while (ER);
+      VR = dyn_cast<VarRegion>(LastER->getSuperRegion());
+    }
+
+    if (VR) {
+      const VarDecl *VD = VR->getDecl()->getMostRecentDecl();
       if (VD->getType().isConstQualified() ||
           R->getElementType().isConstQualified() ||
           (B.isMainAnalysis() && VD->hasGlobalStorage())) {
-      if (const Expr *Init = VD->getAnyInitializer()) {
-        if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
-          // The array index has to be known.
-          if (auto CI = R->getIndex().getAs<nonloc::ConcreteInt>()) {
-            // If it is not an array, return Undef.
-            QualType T = VD->getType();
-            const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(T);
-            if (!CAT)
-              return UndefinedVal();
+        if (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(T)) {
+          // The number of offsets should equal to the numbers of extents,
+          // otherwise wrong type punning occured. For instance:
+          //  int arr[1][2][3];
+          //  auto ptr = (int(*)[42])arr;
+          //  auto x = ptr[4][2]; // UB
+          SmallVector<uint64_t, 2> Extents = CAT->getAllExtents();
+          if (SValOffsets.size() != Extents.size())
 
-            // Support one-dimensional array.
-            // C++20 [expr.add] 7.6.6.4 (excerpt):
-            //   If P points to an array element i of an array object x with n
-            //   elements, where i < 0 or i > n, the behavior is undefined.
-            //   Dereferencing is not allowed on the "one past the last
-            //   element", when i == n.
-            // Example:
-            //   const int arr[4] = {1, 2};
-            //   const int *ptr = arr;
-            //   int x0 = ptr[0]; // 1
-            //   int x1 = ptr[1]; // 2
-            //   int x2 = ptr[2]; // 0
-            //   int x3 = ptr[3]; // 0
-            //   int x4 = ptr[4]; // UB
-            // TODO: Support multidimensional array.
-            if (!isa<ConstantArrayType>(CAT->getElementType())) {
-              // One-dimensional array.
-              const llvm::APSInt &Idx = CI->getValue();
-              const auto I = static_cast<uint64_t>(Idx.getExtValue());
-              // Use `getZExtValue` because array extent can not be negative.
-              const uint64_t Extent = CAT->getSize().getZExtValue();
-              // Check for `Idx < 0`, NOT for `I < 0`, because `Idx` CAN be
-              // negative, but `I` can NOT.
-              if (Idx < 0 || I >= Extent)
+          // Check offsets for being out of bounds.
+          //  const int arr[3][2] = {{1, 2}, {3, 4}};
+          //  arr[0][0];  // 1
+          //  arr[0][1];  // 2
+          //  arr[0][2];  // UB
+          //  arr[1][0];  // 3
+          //  arr[1][1];  // 4
+          //  arr[1][-1]; // UB
+          //  arr[2][0];  // 0
+          //  arr[2][1];  // 0
+          //  arr[-2][0]; // UB
+          SmallVector<uint64_t, 2> ConcreteOffsets;
+          ConcreteOffsets.resize(SValOffsets.size());
+          auto ExtentIt = Extents.begin();
+          auto OffsetIt = ConcreteOffsets.begin();
+          // Reversion `SValOffsets` to make it consistent with `Extents`.
+          for (SVal &V : llvm::reverse(SValOffsets)) {
+            if (auto CI = V.getAs<nonloc::ConcreteInt>()) {
+              const llvm::APSInt &Offset = CI->getValue();
+              const auto I = static_cast<uint64_t>(Offset.getExtValue());
+              // Check for `Offset < 0`, NOT for `I < 0`, because `Offset` CAN
+              // be negative, but `I` can NOT (because it's an uint64_t).
+              if (Offset < 0 || I >= *(ExtentIt++))
                 return UndefinedVal();
+              // Store index in a reversive order.
+              *(OffsetIt++) = I;
+            } else
+              // Symbolic index presented. Return Unknown value.
+              // FIXME: We also need to take ElementRegions with symbolic
+              // indexes into account.
+              return UnknownVal();
+          }
 
+          // Here we are sure that all the indexes are in bounds.
+          if (const Expr *Init = VD->getAnyInitializer()) {
+            if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
+              for (auto I : ConcreteOffsets) {
                 // C++20 [expr.add] 9.4.17.5 (excerpt):
                 //   i-th array element is value-initialized for each k < i ≤ n,
-              //   where k is an expression-list size and n is an array extent.
+                //   where k is an expression-list size and n is an array
+                //   extent.
                 if (I >= InitList->getNumInits())
                   return svalBuilder.makeZeroVal(R->getElementType());
 
+                const Expr *E = InitList->getInit(I);
+                // Go to the nested initializer list.
+                if (const auto *IL = dyn_cast<InitListExpr>(E))
+                  InitList = IL;
                 // Return a constant value, if it is presented.
                 // FIXME: Support other SVals.
-              const Expr *E = InitList->getInit(I);
-              if (Optional<SVal> V = svalBuilder.getConstantVal(E))
+                else if (Optional<SVal> V = svalBuilder.getConstantVal(E))
                   return *V;
               }
             }
           }
         }
       }
     }
+  }
 
   // Check for loads from a code text region.  For such loads, just give up.
   if (isa<CodeTextRegion>(superR))
     return UnknownVal();
 
   // Handle the case where we are indexing into a larger scalar object.
   // For example, this handles:
   //   int x = ...

clang/test/Analysis/initialization.c

 }
 
 void glob_invalid_index2() {
   const int *ptr = glob_arr1;
   int x = 42;
   int res = ptr[x]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 const int glob_arr2[3][3] = {[0][0] = 1, [1][1] = 5, [2][0] = 7};
 void glob_arr_index3() {
-  // FIXME: These all should be TRUE.
-  clang_analyzer_eval(glob_arr2[0][0] == 1); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[0][1] == 0); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[0][2] == 0); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[1][0] == 0); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[1][1] == 5); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[1][2] == 0); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[2][0] == 7); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[2][1] == 0); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(glob_arr2[2][2] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr2[0][0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[0][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[0][2] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[1][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[1][1] == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[1][2] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[2][0] == 7); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[2][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr2[2][2] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void negative_index() {
   int x = 2, y = -2;
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(glob_arr2[x][y] == 5); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr2[x][y] == 5); // expected-warning{{UNDEFINED}}
   x = 3;
   y = -3;
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(glob_arr2[x][y] == 7); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr2[x][y] == 7); // expected-warning{{UNDEFINED}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index3() {
   int x = -1, y = -1;
-  // FIXME: Should warn {{garbage or undefined}}.
-  int res = glob_arr2[x][y]; // no-warning
+  int res = glob_arr2[x][y]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index4() {
   int x = 3, y = 2;
-  // FIXME: Should warn {{garbage or undefined}}.
-  int res = glob_arr2[x][y]; // no-warning
+  int res = glob_arr2[x][y]; // expected-warning{{garbage or undefined}}
 }

clang/test/Analysis/initialization.cpp

 // RUN: %clang_cc1 -std=c++14 -triple i386-apple-darwin10 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=core.uninitialized.Assign,core.builtin,debug.ExprInspection,core.uninitialized.UndefReturn -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}}
+  clang_analyzer_eval(arr[i][0]); // expected-warning{{FALSE}}
 }
 
 int const glob_arr1[3] = {};
 void glob_array_index1() {
   clang_analyzer_eval(glob_arr1[0] == 0); // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_arr1[1] == 0); // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_arr1[2] == 0); // expected-warning{{TRUE}}
 }
 
 void glob_invalid_index1() {

 }
 
 const float glob_arr3[] = {
     0.0000, 0.0235, 0.0470, 0.0706, 0.0941, 0.1176};
 float no_warn_garbage_value() {
   return glob_arr3[0]; // no-warning (garbage or undefined)
 }
 
-// TODO: Support multidimensional array.
 int const glob_arr4[4][2] = {};
 void glob_array_index2() {
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr4[1][0] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr4[1][1] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr4[0][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr4[1][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr4[1][1] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index3() {
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = glob_arr4[1][idx]; // no-warning
+  auto x = glob_arr4[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index4() {
   const int *ptr = glob_arr4[1];
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // no-warning
+  auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 int const glob_arr5[4][2] = {{1}, 3, 4, 5};
 void glob_array_index3() {
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[0][0] == 1); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[0][1] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[1][0] == 3); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[1][1] == 4); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[2][0] == 5); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[2][1] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[3][0] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[3][1] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr5[0][0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[0][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[1][0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[1][1] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[2][0] == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[2][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[3][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[3][1] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void glob_ptr_index2() {
+  // FIXME: Fix cast.
   int const *ptr = glob_arr5[1];
   // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{UNDEFINED}}
   // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[2] == 0); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNDEFINED}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index5() {
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = glob_arr5[1][idx]; // no-warning
+  auto x = glob_arr5[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index6() {
   int const *ptr = &glob_arr5[1][0];
   int idx = 42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // // no-warning
+  auto x = ptr[idx]; // // expected-warning{{garbage or undefined}}
 }