[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+    ProgramStateRef State;
----------------
xazax.hun wrote:
> NoQ wrote:
> > WDYT about re-using `ExprEngine::escapeValue()` by changing it to accept `ArrayRef<SVal>` instead of a single `SVal`?
> It is not entirely the same. Here we only collect symbols from non-stack regions. There (as far as I understand) we collect all symbols. I could add a flag but at that point I wonder if it is worth the change.
Umm, wait, right, so what are you doing in this callback to begin with? The code says "gather all symbols that are encountered as immediate values stored in traversed regions". Why not simply "gather all symbols that are traversed from parameter regions"?


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:648
+          continue;
+        State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner);
+      }
----------------
I guess technically, for our own sanity, it's worth it to-rescan the symbols for every node in `dstPostCall`. But i'll be very surprised if they are //actually// going to yield different results for every predecessor node.


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

https://reviews.llvm.org/D71224