[PATCH] D56632: [analyzer] Track region liveness only through base regions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D56632#1359576 <https://reviews.llvm.org/D56632#1359576>, @xazax.hun wrote:

> If you find out the reason why we need `markElementIndicesLive`, documenting that would also be useful. But it is also independent of this change. 
>  Maybe something like we could learn new information regarding the indices after they are dead?
> Like:
>
>   void f(int i, char *c) {
>       char e = c[i];
>       if (strlen(c) == 5) {
>            // The value of `i` is no longer used, could be dead
>            // but we did learn something new. Assuming no UB, `i <= 5` (if null terminated).
>            // So maybe having symbols for indices around for representing the info above is useful?
>           use(c);
>       }
>   }
>


Yep, that was pretty much the original motivation behind adding this functionality in D12726 <https://reviews.llvm.org/D12726>.

A more ridiculous example:

  struct rlimit rlim;
  getrlimit(RLIMIT_NOFILE, &lim); // Max file descriptor on the system.
  int *arr = calloc(rlim.rlim_cur, sizeof(int)); // An expensive but fast map from FDs to ints.
  arr[open("foo.txt", O_RDONLY)] = 1; // Remember that this descriptor is open.

After that even though the file descriptor is otherwise dead, as long as `arr` is alive and its contents are more or less preserved, you can close the file as follows:

  for (int i = 0; i < rlim.lim_cur; ++i)
    if (arr[i] == 1)
      close(i);

Therefore, we kinda should not diagnose a file descriptor leak here.

> One fundamental question is, do we have one property here or two?
>  Maybe the liveness analysis we use for leaks (and other purposes?),
>  and the garbage collection of symbols are inherently two different kind of things that are only slightly related?

This constantly bothers me, but surprisingly, i don't see any reasonable counter-examples to them being the same thing.

One of the brightest examples i have is that if the parent region of a `LazyCompoundVal` is an `ElementRegion` with symbolic index, constraints on its index ideally need to be kept alive in order to access the data within the `LazyCompoundVal` as accurately as possible, but it's not really accessible from within the program because the parent region of a `LazyCompoundVal` is entirely immaterial. However, this is merely a weird implementation detail of our `LazyCompoundVal`s: we could have implemented "eager" compound values instead, and in that case it wouldn't have been a problem anymore. Note that it is not a sufficient solution to simply make `LazyCompoundVal` capture constraints together with the store, because constraints might have improved since then (and then dropped, and only then we're trying to load the value).

So i believe that as long as our state is not over-saturated with information (i.e., it looks kinda like a normalized database), then the amount of information we need to track is going to be exactly as much as the programmer is able to extract from memory in run-time.


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

https://reviews.llvm.org/D56632