Address the remaining comments

Author: Szelethus

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

@@ -439,9 +439,6 @@ static const TypedValueRegion *getOriginRegion(const ElementRegion *ER) {
       return nullptr;
     Ret = sym2->getOriginRegion();
   }
-  if (const auto *Elem = MR->getAs<ElementRegion>()) {
-    Ret = Elem->getBaseRegion();
-  }
   return dyn_cast_or_null<TypedValueRegion>(Ret);
 }
 
@@ -460,10 +457,7 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C,
     return nullptr;
 
   const MemRegion *R = Element.getAsRegion();
-  if (!R)
-    return State;
-
-  const auto *ER = dyn_cast<ElementRegion>(R);
+  const auto *ER = dyn_cast_or_null<ElementRegion>(R);
   if (!ER)
     return State;
 
@@ -552,11 +546,11 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C,
     }
     llvm::SmallString<258> Buf;
     llvm::raw_svector_ostream OS(Buf);
-    OS << "The last element of the ";
-    printIdxWithOrdinalSuffix(OS, Buffer.ArgumentIndex + 1);
-    OS << " argument to access (the ";
+    OS << "The last (";
     printIdxWithOrdinalSuffix(OS, IdxInt->getExtValue() + 1);
-    OS << ") is undefined";
+    OS << ") element to access in the ";
+    printIdxWithOrdinalSuffix(OS, Buffer.ArgumentIndex + 1);
+    OS << " argument is undefined";
     emitUninitializedReadBug(C, State, Buffer.Expression, OS.str());
     return nullptr;
   }

clang/test/Analysis/bstring_UninitRead.c

@@ -23,7 +23,7 @@ void memcpy_array_fully_uninit(char *dst) {
 void memcpy_array_partially_uninit(char *dst) {
   char buf[10];
   buf[0] = 'i';
-  memcpy(dst, buf, 10); // expected-warning{{The last element of the 2nd argument to access (the 10th) is undefined}}
+  memcpy(dst, buf, 10); // expected-warning{{The last (10th) element to access in the 2nd argument is undefined}}
                         // expected-note@-1{{Other elements might also be undefined}}
   (void)buf;
 }
@@ -38,8 +38,23 @@ void memcpy_array_only_init_portion(char *dst) {
 void memcpy_array_partially_init_error(char *dst) {
   char buf[10];
   buf[0] = 'i';
-  memcpy(dst, buf, 2); // expected-warning{{The last element of the 2nd argument to access (the 2nd) is undefined}}
-                        // expected-note@-1{{Other elements might also be undefined}}
+  memcpy(dst, buf, 2); // expected-warning{{The last (2nd) element to access in the 2nd argument is undefined}}
+                      // expected-note@-1{{Other elements might also be undefined}}
+  (void)buf;
+}
+
+// The interesting case here is that the portion we're copying is initialized,
+// but not the whole matrix. We need to be careful to extract buf[1], and not
+// buf when trying to peel region layers off from the source argument.
+void memcpy_array_from_matrix(char *dst) {
+  char buf[2][2];
+  buf[1][0] = 'i';
+  buf[1][1] = 'j';
+  // FIXME: This is a FP -- we mistakenly retrieve the first element of buf,
+  // instead of the first element of buf[1]. getLValueElement simply peels off
+  // another ElementRegion layer, when in this case it really shouldn't.
+  memcpy(dst, buf[1], 2); // expected-warning{{The first element of the 2nd argument is undefined}}
+                          // expected-note@-1{{Other elements might also be undefined}}
   (void)buf;
 }
 
@@ -96,8 +111,8 @@ void mempcpy_struct_fully_uninit() {
   mempcpy(&s2, &s1, sizeof(struct st));
 }
 
-// Creduced crash.
-
+// Creduced crash. In this case, an symbolicregion is wrapped in an
+// elementregion for the src argument.
 void *ga_copy_strings_from_0;
 void *memmove();
 void alloc();