aaron.ballman added inline comments.

================
Comment at: clang/test/SemaCXX/array-bounds.cpp:240
 
-    ((char*)foo)[sizeof(foo)] = '\0';  // expected-warning {{array index 32768 
is past the end of the array (which contains 32768 elements)}}
+    ((char*)foo)[sizeof(foo)] = '\0';  // expected-warning {{array index 32768 
is past the end of the array (which contains 4096 elements)}}
 
----------------
void wrote:
> serge-sans-paille wrote:
> > kees wrote:
> > > serge-sans-paille wrote:
> > > > I find this new message quite confusing, because the array index is 
> > > > given in terms of char elements, while the array size is given in terms 
> > > > of double elements. I actually find the original message more 
> > > > consistent to that respect.
> > > Perhaps show both element count and byte offset?
> > > 
> > > `array index $count is $(bytes - end_of_array_in_bytes) past the end of 
> > > the array (which contains $end_of_array_in_bytes bytes of 
> > > $array_max_index elements)`
> > if we have no cast, I find the version without the patch  just fine.
> > 
> > If we have a cast, I suggest we just express the result in bytes, something 
> > along those lines (inspired by your suggestion):
> > 
> >     array index $count is $(bytes - end_of_array_in_bytes) bytes past the 
> > end of the allocated memory for that array (which contains 
> > $end_of_array_in_bytes bytes)
> I so far have this. It looks not too bad even for the non-cast. Thoughts?
> 
> ```
> $ cat ab.c
> void test_pr10771() {
>     double foo[4096];  // expected-note {{array 'foo' declared here}}
> 
>     ((char*)foo)[sizeof(foo)] = '\0';  // expected-warning {{array index 
> 32768 is past the end of the array (which contains 4096 elements of type 
> 'double[4096])}}
>     foo[4098] = '\0';  // expected-warning {{array index 32768 is past the 
> end of the array (which contains 4096 elements of type 'double[4096])}}
> }
> $ clang++ -x c++ -o /dev/null -S -std=c++14 ~/llvm/ab.c
> /home/morbo/llvm/ab.c:4:13: warning: array index 32768 is 0 bytes past the 
> end of the array (that has type 'double[4096]') [-Warray-bounds]
>     ((char*)foo)[sizeof(foo)] = '\0';  // expected-warning {{array index 
> 32768 is past the end of the array (which contains 4096 elements of type 
> 'double[4096])}}
>             ^    ~~~~~~~~~~~
> /home/morbo/llvm/ab.c:2:5: note: array 'foo' declared here
>     double foo[4096];  // expected-note {{array 'foo' declared here}}
>     ^
> /home/morbo/llvm/ab.c:5:5: warning: array index 4098 is 16 bytes past the end 
> of the array (that has type 'double[4096]') [-Warray-bounds]
>     foo[4098] = '\0';  // expected-warning {{array index 32768 is past the 
> end of the array (which contains 4096 elements of type 'double[4096])}}
>     ^   ~~~~
> /home/morbo/llvm/ab.c:2:5: note: array 'foo' declared here
>     double foo[4096];  // expected-note {{array 'foo' declared here}}
>     ^
> 2 warnings generated.
> ```
The part I struggle with is the "is 0 bytes past the end of the array" -- that 
reads as if it's not past the end of the array at all because it's saying 
"You've accessed right up to the end of the array; congratulations on your very 
careful use!".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135920

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

Reply via email to