[PATCH] D51397: [analyzer] Remove the "postponed" hack, deal with derived symbols using an extra map

[Artem Dergachev via Phabricator via llvm-commits]

NoQ added inline comments.
Herald added subscribers: dkrupp, donat.nagy.


================
Comment at: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp:2539-2543
+    if (Loc::isLocType(SD->getType()) && !SymReaper.isLive(SD)) {
+      const SymbolicRegion *SR = RM.getRegionManager().getSymbolicRegion(SD);
+
+      if (B.contains(SR))
+        AddToWorkList(SR);
----------------
Argh, this isn't enough.

In a nutshell, this code says "Uhm, is this symbol `$x` (say, `reg_$N<x>`) now live and it is a pointer? Ok, `RegionStore`, re-check the symbolic region `*$x` (aka `SymRegion{reg_$N<x>}`)." Which is good. By "re-check" we mean "add it to the `RegionStore`'s worklist", which would cause re-exploration of *bindings* within it. However, not every value stored in a region is a binding within the region! For instance, all values within `**$x` (aka `SymRegion{reg_$M<SymRegion{reg_$N<x>}}` are also kept alive, which we will fail to mark live when, say, `*x` has no bindings at all - `SymbolRegionValue` is still presumed to be there.

It would have been fine if it was just sub-regions, but in fact an infinite tree of *base* regions (namely, symbolic regions of non-assigned/invalidated pointer-type sub-regions) also become live every time a region becomes live, while only a finite amount of base regions become reachable when the traversal method implemented here gets applied.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D51397