[PATCH] D106681: [analyzer][NFCI] Move a block from `getBindingForElement` to separate functions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D106681#3075337 <https://reviews.llvm.org/D106681#3075337>, @steakhal wrote:

> I just wanted you to think about these cases.
> BTW this should work for **L1646**:
>
>   extern const int arr[]; // Incomplete array type
>   void top() {
>     (void)arr[42];
>   }

Yes this will work for **L1646**. But I've already added similar tests for this line: `glob_array_index4` and `struct_arr_index1` functions in //initialization.cpp//. I mean it doesn't matter whether it is an incomplete or complete type.



================
Comment at: clang/test/Analysis/initialization.c:103
+void glob_arr_index4() {
+  clang_analyzer_eval(glob_arr_no_init[2]); // expected-warning{{UNKNOWN}}
+}
----------------
steakhal wrote:
> I'm pretty sure we should not get `Unknown` for simply loading from a global variable.
> That would imply that loading two times subsequently we could not prove that the value remained the same.
> But that should remain the same, thus compare equal unless some other code modifier that memory region.
> 
> That could happen for two reasons:
> 1) There is a racecondition, and another thread modified that location. But that would be UB, so that could not happen.
> 2) The global variable is //volatile//, so the hardware might changed its content- but this global is not volatile so this does not apply.
> 
> That being said, this load should have resulted in a //fresh// conjured symbolic value instead of //unknown//.
> Could you please check if it did result in //unknown// before your patch, or you did introduce this behavior?
I'm not sure I caught your thoughts.
But I think the things is much simplier:
`clang_analyzer_eval` can only produce `UNKNOWN` or `TRUE` or `FALSE`. If we know the constraint of `glob_arr_no_init[2]` we return `TRUE` or `FALSE`, and `UNKNOWN` otherwise.
But in fact I should use `clang_analyzer_dump` here instead of `clang_analyzer_eval`. This is actually my fault. I'll update this.


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

https://reviews.llvm.org/D106681