steakhal added inline comments.

================
Comment at: clang/test/Analysis/initialization.c:103
+void glob_arr_index4() {
+  clang_analyzer_eval(glob_arr_no_init[2]); // expected-warning{{UNKNOWN}}
+}
----------------
martong wrote:
> ASDenysPetrov wrote:
> > 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.
> > Could you please check if it did result in unknown before your patch, or 
> > you did introduce this behavior?
> 
> I've just checked it, it was `Unknown` before this patch as well. 
> And actually, that is wrong because the array has static storage duration and 
> as such, we know that it is initialized with zeros according to the C 
> standard. But, that should be addressed in a follow-up patch (if someone has 
> the capacity).
> https://stackoverflow.com/questions/32708161/value-of-uninitialized-elements-in-array-of-c-language/32708288
Oh true. I was actually tricked by the `initialization.cpp:38`, where I 
actually caught this. And in that case, you use `dump()` yet you get `Unknown` 
as a result. But the issue remains the same.


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

https://reviews.llvm.org/D106681

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to