[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

In https://reviews.llvm.org/D44934#1088771, @NoQ wrote:

> Hmm, ok, it seems that i've just changed the API in https://reviews.llvm.org/D46368, and i should have thought about this use case. Well, at least i have some background understanding of these problems now. Sorry for not keeping eye on this problem.
>
> In https://reviews.llvm.org/D44934#1051002, @NoQ wrote:
>
> > Why do you need separate code for null and non-null character? The function's semantics doesn't seem to care.
>
>
> I guess i can answer myself here:
>
>   int32_t x;
>   memset(&x, 1, sizeof(int32_t));
>   clang_analyzer_eval(x == 0x1010101); // should be TRUE
>
>
> I really doubt that we support this case.
>
> So, yeah, zero character is indeed special.


Thank you, Artem! I did not consider this common situation.  This patch does not really support this situation, in this patch the value of `x` will be 1, it's not correct!

> And since zero character is special, i guess we could use the new `bindDefaultZero()` API, and perform a simple invalidation in the non-zero character case.

Agree with you. The core problem here is that there is no perfect way to `bind default` for non-zero value, not the string length stuff. So **invalidate the destination buffer** but still **set string length** for non-zero character is OK. Right?

> @MTC, would this be enough for your use case? Or is it really important for you to support the non-zero character case? Cause my example is fairly artificial, and probably i'm worrying too much about it. If nobody really codes that way, i'm fine with supporting the non-zero character case via a simple default binding. In this case we should merge `bindDefaultZero()` with your `overwriteRegion()` (i'd prefer your function name, but please keep the empty class check).

Based on my limited programming experience, I think `memset` with non-zero character is common. However given that we can't handle non-zero character binding problems very well, we should now support only zero character. After all, IMHO,  correctness of semantic is also important  for the analyzer.

I will update this patch to only handle zero character case. In the future, as I am more familiar with the `RegionStore`, I will solve the problem of non-zero character binding.


Repository:
  rC Clang

https://reviews.llvm.org/D44934