[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok, got it! Yeah, this sounds like a valid way of supporting non-base-region-based worklist items, i'm seeing no problems with it and i don't immediately see a solution that'd be better than gradually supporting non-base-regions in more and more places.

Here's the slight modification of the original test case (D57230#1389766 <https://reviews.llvm.org/D57230#1389766>) that my problem finally boiled down to:

  typedef __typeof(sizeof(int)) size_t;
  void *malloc(size_t);
  
  struct S {
    int x; // Make sure ptr has a non-zero offset within baseR.
    int *ptr;
  };
  
  struct T {
    struct S s; // This is baseR.
  };
  
  void escape(struct S *);
  
  void foo() {
    struct T t; // This is realBaseR.
    t.s.ptr = malloc(sizeof(int));
    escape(&t.s);
  }

This happens because in this patch you're pin-pointing the value by trying to look it up via an exact binding key (up to default/direct), however it may also reside at a non-zero offset within `baseR`. In my case the offset was also symbolic - which kept me confused for a while because i was thinking that symbolic offsets are the root of all evil. I think you may be able to generalize the patch to cover this scenario by replacing the exact offset lookup with a `collectSubRegionBindings()` (it might be cute to make some sort of `iterSubRegionBindings()` that takes a lambda, and/or combine it with `iterBindings()`).

I'm still worried about D57230#1434161 <https://reviews.llvm.org/D57230#1434161> though. It kinda sounds like a good idea to suppress store invalidation but not escaping, but it's a very annoying thing to implement because our invalidation traits are very inflexible. In order to express complex requirements like "please don't invalidate this region, except this sub-region", the whole data structure has to be changed. This is slightly similar to this problem we have in:

  void escape(int *x, const int *y);
  // ...
  escape(&a, &a);

where the value of `a` would not be invalidated because `y` is a const pointer, even though it is passed through a non-const pointer `x`. @xazax.hun, do you have enough time/inspiration to dig that deeply into this stuff?


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

https://reviews.llvm.org/D58121