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

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

Reply via email to