[PATCH] D88477: [analyzer] Overwrite cast type in getBinding only if that was null originally

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

A load from a region of type `T` should always yield a value of type `T` because that's what a load is.

I'd rather have it as an assertion: if the region is typed, the type shouldn't be specified. If such assertion is hard to satisfy, we could allow the same canonical type to be specified. But having a conflict between two sources of type information and resolving it by arbitrarily choosing one of them sounds scary and unreliable. This patch doesn't eliminate the source of the conflict, it simply changes the way we flip the coin to resolve it.

Normally then loading through a casted pointer the conflict is resolved by representing the casted pointer differently, so that the pointer appeared to be a `TypedValueRegion` of the correct type. Namely, it is achieved by wrapping it into an `ElementRegion` with index 0 and the correct type. Why didn't the same occur in this case? Do I understand correctly that `L` is just `&b`, not `&ElementRegion{b, 0 S32b, unsigned char **}`?

---

This is the current status quo and how the patch should probably go within it. In the long term I'd prefer to undo this entire concept of "region having a type". Most of the time memory regions in C are "kinda" typed and type punning is indeed largely problematic due to the Strict Aliasing rule (and object lifetime rules in C++) but there are always ways to bypass those rules and our idea of regions having types fails us every single time this happens. Types are at best a mutable property attached to a region (i.e., like a dynamic type map), definitely not part of its identity. Every access should be typed separately and explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477