[analyzer] Fix region cast between the same types with different qualifiers.

[ASDenysPetrov]

Summary

Specifically, this fixes the case when we get an access to array element through the pointer to element. This covers several FIXME's. in D111654.
Example:

const int arr[4][2];
const int *ptr = arr[1]; // Fixes this.

The issue is that arr[1] is int* (&Element{Element{glob_arr5,1 S64b,int[2]},0 S64b,int}), and ptr is const int*. We don't take qualifiers into account. Consequently, we doesn't match the types as the same ones.

[steakhal]

Very solid patch!

clang/lib/StaticAnalyzer/Core/Store.cpp

98–99

You only use CanonPointeeTy.getLocalUnqualifiedType() all the places. Spell it once and reuse it everywhere.

105

For me at least, a name like IsSameRegionType() would imply a function with two parameters.

115

BTW do you know what isBoundable() is? I'm just curious.

182–183

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/Store.cpp

98–99

Make sense!

105

I'll carry CanonPointeeTy out.

115

I've looked for information about it. It's poor, I'd say there is nothing of it. I'm curious as well.

182–183

+1

[ASDenysPetrov]

Updated according to comments.

[steakhal]

Please clang-format the patch to reflow the touched comment. Given that's done, you are good to go.
Also, wait a couple days before committing to give chance to the rest of the reviewers to have a look.

[ASDenysPetrov]

@steakhal Thank you!

Please clang-format the patch to reflow the touched comment. Given that's done, you are good to go.

The comment is OK. clang-format doesn't complain on it.

Also, wait a couple days before committing to give chance to the rest of the reviewers to have a look.

Sure.

clang/lib/StaticAnalyzer/Core/Store.cpp

if (CastToTy->isBlockPointerType()) {
     // We don't know what to make of it.  Return a NULL region, which
     // will be interpreted as UnknownVal.
     return None;
   }
 
   // Now assume we are casting from pointer to pointer. Other cases should
   // already be handled.
   QualType PointeeTy = CastToTy->getPointeeType();
   QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+  CanonPointeeTy = CanonPointeeTy.getLocalUnqualifiedType();
 
   // Handle casts to void*.  We just pass the region through.
-  if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
+  if (CanonPointeeTy == Ctx.VoidTy)
     return R;
 
-  // Handle casts from compatible types.
-  if (R->isBoundable())
+  const auto IsSameRegionType = [&Ctx](const MemRegion *R, QualType OtherTy) {
     if (const auto *TR = dyn_cast<TypedValueRegion>(R)) {
       QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-      if (CanonPointeeTy == ObjTy)
-        return R;
+      if (OtherTy == ObjTy.getLocalUnqualifiedType())
+        return true;
     }
+    return false;
+  };
+
+  // Handle casts from compatible types.
+  if (R->isBoundable() && IsSameRegionType(R, CanonPointeeTy))
+    return R;
 
   // Process region cast according to the kind of the region being cast.
   switch (R->getKind()) {
     case MemRegion::CXXThisRegionKind:
     case MemRegion::CodeSpaceRegionKind:
     case MemRegion::StackLocalsSpaceRegionKind:
     case MemRegion::StackArgumentsSpaceRegionKind:
     case MemRegion::HeapSpaceRegionKind:
case MemRegion::ElementRegionKind: {
 
       // If we cannot compute a raw offset, throw up our hands and return
       // a NULL MemRegion*.
       if (!baseR)
         return None;
 
       CharUnits off = rawOff.getOffset();
 
       if (off.isZero()) {
-        // Edge case: we are at 0 bytes off the beginning of baseR.  We
-        // check to see if type we are casting to is the same as the base
-        // region.  If so, just return the base region.
-        if (const auto *TR = dyn_cast<TypedValueRegion>(baseR)) {
-          QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-          QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
-          if (CanonPointeeTy == ObjTy)
-            return baseR;
-        }
-
+        // Edge case: we are at 0 bytes off the beginning of baseR. We check to
+        // see if the type we are casting to is the same as the type of the base
+        // region. If so, just return the base region.
+        if (IsSameRegionType(baseR, CanonPointeeTy))
         // Otherwise, create a new ElementRegion at offset 0.
         return MakeElementRegion(cast<SubRegion>(baseR), PointeeTy);
       }
 
       // We have a non-zero offset from the base region.  We want to determine
       // if the offset can be evenly divided by sizeof(PointeeTy).  If so,
       // we create an ElementRegion whose index is that value.  Otherwise, we
       // create two ElementRegions, one that reflects a raw offset and the other

clang/test/Analysis/initialization.cpp

 void glob_invalid_index3() {
   int idx = -42;
   auto x = glob_arr4[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
 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}}
 }
 
 int const glob_arr5[4][2] = {{1}, 3, 4, 5};
 void glob_array_index3() {
   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}}
 }
 
 void glob_ptr_index2() {
   int const *ptr = glob_arr5[1];
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{UNKNOWN}}
-  // 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[0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNDEFINED}}
 }
 
 void glob_invalid_index5() {
   int idx = -42;
   auto x = glob_arr5[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
 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}}
 }
 
 extern const int glob_arr_no_init[10];
 void glob_array_index4() {
   clang_analyzer_eval(glob_arr_no_init[2]); // expected-warning{{UNKNOWN}}
 }
 
 struct S2 {