[PATCH] D130306: [clang][dataflow] Analyze calls to in-TU functions

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:220
+  assert(Body != nullptr);
+  initGlobalVars(*Body, Env);
+
----------------
samestep wrote:
> ymandel wrote:
> > samestep wrote:
> > > ymandel wrote:
> > > > I wonder how this will work between caller and callee. Do we need separate global var state in the frame? If so, maybe mention that as well in the FIXME above.
> > > Could you clarify what you mean? Perhaps I just don't understand exactly what is meant by "global vars" here.
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp#L131-L135
> > 
> > ```
> > /// Initializes global storage values that are declared or referenced from
> > /// sub-statements of `S`.
> > // FIXME: Add support for resetting globals after function calls to enable
> > // the implementation of sound analyses.
> > ```
> > Since this already mentions a need to reset after function calls, seemed relevant here.
> Hmm, OK. I pretty much just pattern-matched from the `Environment` constructor right above this method implementation. Would it be better for me to instead just remove this `initGlobalVars` call for now and replace it with a `FIXME` saying that we'll probably need to call this but it's unclear how exactly to do so?
SGTM. We can note this constraint on models' reference implementations. Specifically, that they cannot reference globals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130306