[PATCH] D60742: [analyzer] RegionStore: Enable loading default bindings from variables.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet, mgorny.
Herald added a project: clang.

D44934 <https://reviews.llvm.org/D44934> added modeling for `memset()` that, for the case of setting memory to 0, boils down to default-binding a concrete 0 to the memory region.

Surprisingly, in some cases RegionStore fails to load default bindings from a region. That is, it assumes that the region either has a direct binding (i.e., initialized via assignment), or it is uninitialized. In particular, this applies to local variables regions of integer/pointer/float types.

It seems that a lot of code (eg., `invalidateRegions()`) carefully works around this problem by adding either a direct binding or a default binding, depending on what `RegionStore` expects. This essentially duplicates `RegionStore`'s already-convoluted logic on this subject on the caller side.

Our options are:

1. Duplicate this logic again in `CStringChecker`.
2. Let `RegionStore` automatically do the direct binding for such plain integers.
3. Add support for loading default bindings from variables.

Option 1 is a failure to improve checker API in an aspect in which it's already very bad, so i didn't go for it. In options 2 and 3 the difference is entirely within `RegionStore`. I believe that option 2 is slightly superior because it produces a more "normalized" data structure and, additionally, because stores are more rare than loads (unless they are "dead stores"^^), so it should be slightly more performant. But it is also harder to implement, as it requires gathering the logic of when exactly do we need a direct binding in one place, so i went for option 3 in order to address the regression and simplify that logic. I did not yet try to see how other callers of `bindDefault***()` can be simplified.


Repository:
  rC Clang

https://reviews.llvm.org/D60742

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/string.c
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/StoreTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60742.195274.patch
Type: text/x-patch
Size: 5678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190416/2a8fdf69/attachment-0001.bin>