[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

Sat Oct 17 05:22:41 PDT 2020

NoQ added a comment.

Aha, great, sounds like all casting can be made more correct, not just casting on store retrieve! Maybe it's worth celebrating through extra unittests on `evalCast()`. Thank you for looking into this.

The new code should obviously be restricted into `evalCastFromLoc()` because if it's a region it's a `Loc`.

You didn't need to move `castRegion()`: `StoreManager` is available in `SValBuilder` through `this->StateMgr`. I don't have a strong preference on where `castRegion()` should live; the original idea was that `StoreManager` should be allowed to have an opinion on this matter because different store managers would handle that differently; however, as of now there's only one store manager and there doesn't seem to be an interest in introducing more of them whereas the abstraction has already leaked all over the place anyway.

I still have questions about the extra if-statements that you've added. Shouldn't we do `castRegion()` unconditionally, given that we're trying to cast a region and that function presumably does exactly that? I just want to avoid these situations:

F13337846: photo_2020-10-16_14-19-19.jpg <https://reviews.llvm.org/F13337846>

In D89055#2320363 <https://reviews.llvm.org/D89055#2320363>, @NoQ wrote:

> `dispatchCast()` was supposed to be the ultimate method to do so

Ugh, sorry, no, that's `evalCast()`. Like `evalBinOp()` etc. My bad. Can we also use `evalCast()`?

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

https://reviews.llvm.org/D89055