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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
