[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

Bill Wendling via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 18 00:07:41 PDT 2022


void added a comment.

In D135920#3863648 <https://reviews.llvm.org/D135920#3863648>, @shafik wrote:

> The current approach of mixing bytes and indices in the same diagnostic is too confusing.

"Current" approach?

> I think we see some acceptable messages for out of bounds access by looking at three different implementations diagnostic for OOB access in a constant expression context:
>
>   int main() {
>       constexpr int arr[1]{};
>       constexpr int x = arr[3];
>   }
>
> We have clang which produces:
>
>   <source>:3:23: note: cannot refer to element 3 of array of 1 element in a constant expression
>       constexpr int x = arr[3];
>                         ^
>
> gcc produces:
>
>   <source>:3:28: error: array subscript value '3' is outside the bounds of array 'arr' of type 'const int [1]'
>       3 |     constexpr int x = arr[3];
>         |                       ~~~~~^
>
> and MSVC produces:
>
>   <source>(3): note: failure was caused by out of range index 3; allowed range is 0 <= index < 1
>
> I think any approach similar to this would be acceptable.

The issue is when we get strange mixtures of byte indexes with non-byte arrays. The situations we're dealing with is this one:

  void test() {
    double foo[4096];
  
    ((char*)foo)[sizeof(foo)] = '\0';
  }

What's generated by ToT is a diagnostic saying that the index (in bytes) is past the array size (also in bytes):

  array index 32768 is past the end of the array (which contains 32768 elements)

So there's a disconnect between the original size of the array (which is in doubles) and the array size being reported (which is in bytes). It seems that no matter what it's not going to be easy to give anything but "mixed" information (bytes and indices) to the user to ensure that we've fully described the issue at hand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135920



More information about the cfe-commits mailing list