[PATCH] D81254: [analyzer] Produce symbolic values for C-array elements

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D81254#2218196 <https://reviews.llvm.org/D81254#2218196>, @ASDenysPetrov wrote:

>> We know where it points to (aka. the pointer's value is conjured), but we don't know what the value if there.
>
> Absolutely right. I looked at this patch after a while and don't currently see a proper solution.
>
>> I feel like this problem should be handled by some sort of invalidation mechanism.
>
> Definitely it should be some criteria/mark/flag about the region invalidation. Maybe someone more confident could prompt us how to.

How about using the mistical `SymbolDerived`?
The clang-analyzer-guide <https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf> says:

> Another atomic symbol, closely related to SymbolRegionValue, is **SymbolDerived**. **It represents a value of a region after another symbol was written into a direct or indirect super-region.** SymbolDerived contains a reference to both the parent symbol and the parent region. This symbol is mostly a technical hack. Usually SymbolDerived appears after invalidation: the whole structure of a certain type gets smashed with a single SymbolConjured, and then values of its fields become represented with the help of SymbolDerived of that conjured symbol and the region of the field. In any case, SymbolDerived is similar to SymbolRegionValue, just refers to a value after a certain event during analysis rather than at start of analysis.

Could we use that here @NoQ?

---

In D81254#2218613 <https://reviews.llvm.org/D81254#2218613>, @ASDenysPetrov wrote:

> Should this revision be //closed //or //rejected //somehow?

I'm not sure about that.
I think it still has some value. Especially these two test-cases:

  int index_sym(const int *a, int index) {
    int var;
    int ret = 0;
    if (a[index] < 2)
      var = 1;
  
    // Since we did not write through a pointer, we can be sure that the values bound to 'a' are still valid.
    // (We should also take strict-aliasing into account if -fno-strict-aliasing is not enabled.)
    //
    // We should take this branch if it was taken previously.
    if (a[index] < 2) {
      // FIXME: We should not warn here.
      ret = var; // expected-warning{{Assigned value is garbage or undefined [core.uninitialized.Assign]}}
    }
    return ret;
  }
  
  int index_sym_invalidated(const int *a, int index, int index2) {
    int var;
    int ret = 0;
    if (a[index] < 2)
      var = 1;
  
    a[index2] = 42; // Store a value to //somewhere// in 'a'.
    // A store happens through a pointer which //may// alias with the region 'a'.
    // So this store might overwrite the value of 'a[index]'
    // Thus, we should invalidate all the bindings to the region 'a'.
    // Except the binding to 'a[index2]' which is known to be 42.
    if (a[index] < 2) {
      // This warning is reasonable.
      ret = var; // expected-warning{{Assigned value is garbage or undefined [core.uninitialized.Assign]}}
    }
    return ret;
  }


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

https://reviews.llvm.org/D81254