[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

[Ding Fei via Phabricator via cfe-commits]

danix800 added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:92
+
+  return ElementCount.castAs<DefinedOrUnknownSVal>();
+}
----------------
donat.nagy wrote:
> Are you sure that this cannot cause crashes? (E.g. did you check that there is no corner case when `getElementExtent` returns 0?)
> 
> I can accept this cast, especially if you have a clear proof that it's valid, but I'd prefer a more defensive solution that turns `UndefinedVal` into `UnknownVal` either here or preferably in the `assert()` function family that consumes the results from functions like this. 
Wow~ Yeah! This really is a bug with zero-sized types, the original `getDynamicElementCount` suffers from this too!
Verified on 13.0.1 & main.

```
void clang_analyzer_dumpElementCount(const void *);

int a[1][0];

void var_simple_ref() {
  clang_analyzer_dumpElementCount(a); // expected-warning {{1 S64b}}
}
```

Dividing by zero gives undefined, casts failed:
```
clang: /home/danis/Sources/llvm-project-main/llvm/include/llvm/Support/Casting.h:566: decltype(auto) llvm::cast(const From &) [To = clang::ento::DefinedOrUnknownSVal, From = clang::ento::SVal]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
```

`getDynamicElementCountWithOffset` is the extended offset version of `getDynamicElementCount`, same issue.

Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707