[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

For testing this I would recommend using a separate test file.
That being said, you should avoid globals even in tests when you can. The distance between its declaration and use just makes it harder to comprehend and reason about.
You could have a parameter, and take its address to accomplish your reinterpret casts and type puns.

BTW your patch crashes on the test suite:
`initialization.cpp:242`:

  void glob_array_typedef1() {
    clang_analyzer_eval(glob_arr8[0][0] == 1); // <-- crashes here
    // ...
  }

  clang: ../../clang/lib/AST/ASTContext.cpp:10230: clang::QualType clang::ASTContext::getCorrespondingUnsignedType(clang::QualType) const: Assertion `(T->hasSignedIntegerRepresentation() || T->isSignedFixedPointType()) && "Unexpected type"' failed.
  Called by clang::ASTContext::getCorrespondingUnsignedType() from RegionStoreManager::canAccessStoredValue()



================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1661
+/// E.g. for `const int[1][2][3];` returns `int`.
+QualType getConstantArrayRootElement(const ConstantArrayType *CAT) {
+  assert(CAT && "ConstantArrayType should not be null");
----------------
Maybe //unwrapConstantArrayTypes()//?


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1790
+  // paper P1839R0 be considered by the committee.
+  if ((Index != 0))
+    return false;
----------------
Even though you are technically correct, how can you know that the pointer you try to dereference actually points to the beginning of the object?
Consider something like this:

```lang=C++
void callback(void *payload) {
  // lets skip the first char object...
  int *num = (int*)((char*)payload + 1);
  clang_analyzer_dump(num); // Element{Element{SymRegion{payload}, 1, char}, 0, int}
}
```


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1794-1795
+  // - is char, uchar, std::byte
+  if ((ThroughT == Ctx.CharTy) || (ThroughT == Ctx.UnsignedCharTy) ||
+      ThroughT->isStdByteType())
+    return true;
----------------



================
Comment at: clang/test/Analysis/initialization.cpp:295-299
+void glob_cast_opposite_sign1() {
+  auto *ptr = (unsigned int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
----------------
I think it's not correct.

`glob_arr2` refers to an object of dynamic type `int[2]`.
And the pointer decayed from it is `int *`, which has //similar type// to `unsigned *` what you are using to access the memory.
Since they are //similar//, this operation should work for all the valid indices, thus `ptr[0]`, `ptr[1]`, `ptr[2]`, `ptr[3]` should all be valid.




================
Comment at: clang/test/Analysis/initialization.cpp:311-314
+void glob_cast_invalid3() {
+  auto *ptr = (char32_t *)glob_arr2;
+  auto x = ptr[0]; // expected-warning{{garbage or undefined}}
+}
----------------
Please also try `char8_t`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110927/new/

https://reviews.llvm.org/D110927