[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D31868#904814, @MTC wrote:

> > One of the possible improvements for future work here would be to actually bind the second argument value to the buffer instead of just invalidating it. Like, after `memset(buf, 0, sizeof(buf))` the analyzer should know that all values in the `buf` array are `0`. In the analyzer we have the notion of *default bindings* to handle that (see documentation in docs/analyzer/RegionStore.txt for more details).
>
> `BindDefault()` is the only function that can make the default binding, is it? If so, `evalMemset()` uses `bindDefault()`, the binding may not take effect. Because the current `BindDefault()` logic is that if the memory area has been initialized, then the default binding will no longer be done, see https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Core/RegionStore.cpp#L429. Before `evalMemset()`, `MallocMemAux()` in MallocChecker.cpp may have already made the default binding. Am I right?


Seems so, and it also looks super counter-intuitive. We definitely do need to overwrite default bindings from time to time. When `RegionStore` itself wants to overwrite a default binding, it does `removeBindings()` first (see `invalidateGlobalRegion()` as an example). I think `ProgramState::bindDefault()` should behave this way as well, not sure about impact of such change.


Repository:
  rL LLVM

https://reviews.llvm.org/D31868