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

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

steakhal added a comment.

I'm getting lost :D

In D88477#2304230 <https://reviews.llvm.org/D88477#2304230>, @NoQ wrote:

> And I believe that this part is already incorrect. Like, regardless of how we do the dereference (the implicit lvalue-to-rvalue cast), or *whether* we do it at all (nobody guarantees we'll ever do that!), the part of the static analyzer that computes the lvalue `**b` has to work correctly. As of now it computes an lvalue of incorrect type (currently it's `unsigned char` but it has to be `char *`).

Are you implying that when we evaluate the assignment to the lvalue (line `#1`), we should have cast the stored value to the static type before binding in the `RegionStore`?

Doesn't it contradict with your previous statement:

In D77062#2298748 <https://reviews.llvm.org/D77062#2298748>, @NoQ wrote:

> The contract of RegionStore with respect to type punning is that it stores the value //as written//, even if its type doesn't match the storage type, but then it casts the value to the correct type upon reading by invoking `CastRetrievedVal()` on it. That's where the fix should probably be.



---

Or after evaluating the first dereference - but before evaluating the second - should we cast the lvalue to `char**` and only then do the LValueToRValue conversion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88477