[PATCH] D21536: [CFLAA] Include externally-visible memory aliasing information in function summaries

[Jia Chen via llvm-commits]

grievejia added inline comments.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:792
@@ +791,3 @@
+  // RetParamRelations
+  for (const auto &mapping : InterfaceMap) {
+    auto &Interfaces = mapping.second;
----------------
grievejia wrote:
> george.burgess.iv wrote:
> > george.burgess.iv wrote:
> > > Nit: Mapping
> > It looks like there are cases where this won't add the correct StratifiedAttrs to sets. Consider:
> > 
> > ```
> > int **g;
> > void foo(int **a) {
> >   *a = *g;
> > }
> > 
> > void bar() {
> >   int *p, *p2;
> >   *g = p2;
> > 
> >   int **a = &p;
> >   foo(a);
> >   // p and p2 now alias
> > }
> > ```
> > 
> > Because there's only arg and no return values when analyzing `foo`, `Interfaces.size()` will never be > 1, so we'll end up with no `RetParamRelations`. We need to somehow apply the attributes from the set containing `a` in `foo` to the set containing `a` in `bar`, though.
> That's the reason why there are still two xfail test cases, and that's the reason why I mentioned in the description section that more work need to be done here :)
> 
> I think almost all StratifiedAttr in the callee's graph is useless to the caller, except for AttrUnknown. The summary should also include a list of InterfaceValues that must be tagged with AttrUnknown. However, we currently tag all nodes below formal parameters "AttrUnknown", so doing what I suggested before would unnecessarily tag too many "AttrUnknown"s in the caller. I think we may need to introduce another StratifiedAttr here just to distinguish between "things below parameters, which are known by the caller" and "things that are truly unknown, such as globals or inttoptr". 
> 
> Anyway, whatever we do, that's going to be the story for the next patch :)
After a second thought, maybe AttrEscaped is useful to the caller as well...


http://reviews.llvm.org/D21536