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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:696
   // And make the result node.
   Bldr.generateNode(Call.getProgramPoint(), State, Pred);
 }
----------------
NoQ wrote:
> xazax.hun wrote:
> > After some offline conversation it is very likely that we want to move the `runCheckersForPointerEscape` here. 
> > 
> > The main question is, how should we get all the data?
> > 
> > We should know about:
> > * What regions are output params.
> > * What regions are considered escaped.
> > * What regions have traits that prevents escaping.
> > 
> > Is there anything else?
> > What regions are output params.
> 
> That should be obvious from the AST. Like, parameters of non-const pointer/reference types.
> 
> > What regions are considered escaped.
> 
> Output parameter regions (as `TopLevelInvalidated`) and whatever's reachable from them.
> 
> > What regions have traits that prevents escaping.
> 
> Currently the only trait that affects escaping (as opposed to invalidation) is `TK_SuppressEscape` and it is never applied to out-parameters.
> After some offline conversation it is very likely that we want to move the runCheckersForPointerEscape here.

Nope, we do not want to move it here. We want the pointerEscape to happen AFTER postCall callbacks.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:650
+
+    // TODO: the PSK is a lie.
+    State = getCheckerManager().runCheckersForPointerEscape(
----------------
How much do we care about the escape kind? For each symbol we need to check if it was directly passed to the callee. It is not too bad I guess, but I was wondering.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:1151
+  // TODO: it is a path sensitive property if a call was inlined.
+  return false;
 }
----------------
Hmm. Should I introduce a very short lived trait or do we have a better way to deal with stuff like this? A short lived map? 


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

https://reviews.llvm.org/D71224