public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
@ 2021-02-17  3:34 Martin Sebor
  2021-02-17 13:56 ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2021-02-17  3:34 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

-Warray-bounds makes a couple of assumptions about arrays that hold
neither for VLAs of zero-length arrays nor for zero-length arrays
of VLAs.  The attached patch removes these assumptions and simplifies
the code that deals with them in the process.

This resolves a P2 GCC 9-11 regression so I'm looking for approval
to commit this fix to both release branches as well as the trunk.

Tested on x86_64-linux.

Martin

[-- Attachment #2: gcc-99121.diff --]
[-- Type: text/x-patch, Size: 6405 bytes --]

PR tree-optimization/99121 - ICE in -Warray-bounds on a VLA of zero-length array

gcc/ChangeLog:

	PR tree-optimization/99121
	* gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
	Avoid assuming array element size is constant.  Handle zero-length
	arrays of VLAs.

gcc/testsuite/ChangeLog:

	PR tree-optimization/99121
	* c-c++-common/Warray-bounds-9.c: New test.
	* gcc.dg/Warray-bounds-71.c: New test.

diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index 2576556f76b..25b91f6e215 100644
--- a/gcc/gimple-array-bounds.cc
+++ b/gcc/gimple-array-bounds.cc
@@ -594,34 +594,32 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
 	      || (DECL_EXTERNAL (arg) && array_at_struct_end_p (ref))))
 	return false;
 
-      /* FIXME: Should this be 1 for Fortran?  */
       arrbounds[0] = 0;
 
       if (TREE_CODE (reftype) == ARRAY_TYPE)
 	{
-	  /* Set to the size of the array element (and adjust below).  */
-	  eltsize = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (reftype)));
-	  /* Use log2 of size to convert the array byte size in to its
-	     upper bound in elements.  */
-	  const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
-	  if (tree dom = TYPE_DOMAIN (reftype))
+	  tree nelts = array_type_nelts (reftype);
+	  if (integer_all_onesp (nelts))
+	    /* Zero length array.  */
+	    eltsize = 0;
+	  else
 	    {
-	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
-	      if (TREE_CODE (arg) == COMPONENT_REF)
-		{
-		  offset_int size = maxobjsize;
-		  if (tree fldsize = component_ref_size (arg))
-		    size = wi::to_offset (fldsize);
-		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
-		}
-	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
-		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
-	      else
-		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
-				+ 1) * eltsize;
+	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
+	      if (TREE_CODE (esz) == INTEGER_CST)
+		/* Array element is not a VLA.  */
+		eltsize = wi::to_offset (esz);
 	    }
+
+	  if (!array_at_struct_end_p (arg)
+	      && TREE_CODE (nelts) == INTEGER_CST)
+	    arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
 	  else
-	    arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
+	    {
+	      /* Use log2 of size to convert the array byte size in to its
+		 upper bound in elements.  */
+	      const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
+	      arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
+	    }
 
 	  /* Determine a tighter bound of the non-array element type.  */
 	  tree eltype = TREE_TYPE (reftype);
diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-9.c b/gcc/testsuite/c-c++-common/Warray-bounds-9.c
new file mode 100644
index 00000000000..48ad16828f2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-bounds-9.c
@@ -0,0 +1,55 @@
+/* PR tree-optimization/99121 - ICE in -Warray-bounds on a multidimensional
+   VLA
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+void sink (void*, ...);
+#define T(a, x) sink (a, x)
+
+
+NOIPA void a_0_n (int n)
+{
+  int a[0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0 (int n)
+{
+  int a[n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+NOIPA void f_2_n_0 (int n)
+{
+  int a[2][n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void f_n_n_0 (int n)
+{
+  int a[n][n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-71.c b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
new file mode 100644
index 00000000000..c2af9bef78c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
@@ -0,0 +1,53 @@
+/* PR tree-optimization/99121 - ICE in -Warray-bounds on a multidimensional
+   VLA
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-strict-aliasing -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+void sink (void*, ...);
+#define T(a, x) sink (a, x)
+
+
+NOIPA void ma_0_n (int n)
+{
+  struct {
+    int a[0][n];
+  } s;
+
+  sink (&s);
+
+  T (&s, ((int *) s.a)[0]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[0]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[0]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[1]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[1]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[1]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[n]);   // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((char *) s.a)[n]);  // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((float *) s.a)[n]); // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+}
+
+
+NOIPA void ma_n_0 (int n)
+{
+  struct {
+    int a[n][0];
+  } s;
+
+  sink (&s);
+
+  T (&s, ((int *) s.a)[0]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[0]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[0]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[1]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[1]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[1]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[n]);   // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((char *) s.a)[n]);  // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((float *) s.a)[n]); // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-17  3:34 [PATCH] handle VLA of zero length arrays and vice versa (PR 99121) Martin Sebor
@ 2021-02-17 13:56 ` Jakub Jelinek
  2021-02-17 20:27   ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2021-02-17 13:56 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Tue, Feb 16, 2021 at 08:34:41PM -0700, Martin Sebor via Gcc-patches wrote:
> +	  if (integer_all_onesp (nelts))
> +	    /* Zero length array.  */
> +	    eltsize = 0;
> +	  else
>  	    {
> -	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
> -	      if (TREE_CODE (arg) == COMPONENT_REF)
> -		{
> -		  offset_int size = maxobjsize;
> -		  if (tree fldsize = component_ref_size (arg))
> -		    size = wi::to_offset (fldsize);
> -		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
> -		}
> -	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
> -		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
> -	      else
> -		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
> -				+ 1) * eltsize;
> +	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
> +	      if (TREE_CODE (esz) == INTEGER_CST)
> +		/* Array element is not a VLA.  */
> +		eltsize = wi::to_offset (esz);
>  	    }
> +
> +	  if (!array_at_struct_end_p (arg)
> +	      && TREE_CODE (nelts) == INTEGER_CST)
> +	    arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
>  	  else
> -	    arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
> +	    {
> +	      /* Use log2 of size to convert the array byte size in to its
> +		 upper bound in elements.  */
> +	      const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
> +	      arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);

So, what will this do for zero length arrays at the end of struct?
eltsize will be 0 and wi::floor_log2 (eltsize) is -1, and shifting by -1,
while maybe not UB in wide_int computations, is certainly just weird.
Why do you use eltsize = 0 for the [0] arrays?  They can still have their
element size and if array_at_struct_end_p and the element type is not a variable
length type, using the actual eltsize seems better.
Only when !array_at_struct_end_p we should ensure arrbounds[1] will be 0
and indeed that (wi::to_offset (nelts) + 1) * eltsize would likely not do
that because wi::to_offset is -1ULL or so, not -1.

Also, I'm not sure I understand the right shift by floor_log2 of eltsize,
why can't you simply divide maxobjsize by eltsize (if eltsize is not 0).

	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-17 13:56 ` Jakub Jelinek
@ 2021-02-17 20:27   ` Martin Sebor
  2021-02-17 20:47     ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2021-02-17 20:27 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2648 bytes --]

On 2/17/21 6:56 AM, Jakub Jelinek wrote:
> On Tue, Feb 16, 2021 at 08:34:41PM -0700, Martin Sebor via Gcc-patches wrote:
>> +	  if (integer_all_onesp (nelts))
>> +	    /* Zero length array.  */
>> +	    eltsize = 0;
>> +	  else
>>   	    {
>> -	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
>> -	      if (TREE_CODE (arg) == COMPONENT_REF)
>> -		{
>> -		  offset_int size = maxobjsize;
>> -		  if (tree fldsize = component_ref_size (arg))
>> -		    size = wi::to_offset (fldsize);
>> -		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
>> -		}
>> -	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
>> -		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
>> -	      else
>> -		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
>> -				+ 1) * eltsize;
>> +	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
>> +	      if (TREE_CODE (esz) == INTEGER_CST)
>> +		/* Array element is not a VLA.  */
>> +		eltsize = wi::to_offset (esz);
>>   	    }
>> +
>> +	  if (!array_at_struct_end_p (arg)
>> +	      && TREE_CODE (nelts) == INTEGER_CST)
>> +	    arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
>>   	  else
>> -	    arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
>> +	    {
>> +	      /* Use log2 of size to convert the array byte size in to its
>> +		 upper bound in elements.  */
>> +	      const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
>> +	      arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
> 
> So, what will this do for zero length arrays at the end of struct?
> eltsize will be 0 and wi::floor_log2 (eltsize) is -1, and shifting by -1,
> while maybe not UB in wide_int computations, is certainly just weird.
> Why do you use eltsize = 0 for the [0] arrays?  They can still have their
> element size and if array_at_struct_end_p and the element type is not a variable
> length type, using the actual eltsize seems better.
> Only when !array_at_struct_end_p we should ensure arrbounds[1] will be 0
> and indeed that (wi::to_offset (nelts) + 1) * eltsize would likely not do
> that because wi::to_offset is -1ULL or so, not -1.

The code is only entered for references to declared objects so
the array_at_struct_end_p() test is unnecessary.  I've removed
it in the attached revision.

> 
> Also, I'm not sure I understand the right shift by floor_log2 of eltsize,
> why can't you simply divide maxobjsize by eltsize (if eltsize is not 0).

I'm pretty sure that's because wide_int doesn't have division and
I assumed offset_int didn't either when I originally wrote the code.
I've changed it to use division.

Attached is a revised patch.

Martin

[-- Attachment #2: gcc-99121.diff --]
[-- Type: text/x-patch, Size: 8540 bytes --]

PR tree-optimization/99121 - ICE in -Warray-bounds on a VLA of zero-length array

gcc/ChangeLog:

	PR tree-optimization/99121
	* gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
	Avoid assuming array element size is constant.  Handle zero-length
	arrays of VLAs.

gcc/testsuite/ChangeLog:

	PR tree-optimization/99121
	* c-c++-common/Warray-bounds-9.c: New test.
	* gcc.dg/Warray-bounds-71.c: New test.

diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index 2576556f76b..046b78d463e 100644
--- a/gcc/gimple-array-bounds.cc
+++ b/gcc/gimple-array-bounds.cc
@@ -594,34 +594,31 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
 	      || (DECL_EXTERNAL (arg) && array_at_struct_end_p (ref))))
 	return false;
 
-      /* FIXME: Should this be 1 for Fortran?  */
       arrbounds[0] = 0;
 
       if (TREE_CODE (reftype) == ARRAY_TYPE)
 	{
-	  /* Set to the size of the array element (and adjust below).  */
-	  eltsize = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (reftype)));
-	  /* Use log2 of size to convert the array byte size in to its
-	     upper bound in elements.  */
-	  const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
-	  if (tree dom = TYPE_DOMAIN (reftype))
+	  tree nelts = array_type_nelts (reftype);
+	  if (integer_all_onesp (nelts))
+	    /* Zero length array.  */
+	    arrbounds[1] = 0;
+	  else
 	    {
-	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
-	      if (TREE_CODE (arg) == COMPONENT_REF)
-		{
-		  offset_int size = maxobjsize;
-		  if (tree fldsize = component_ref_size (arg))
-		    size = wi::to_offset (fldsize);
-		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
-		}
-	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
-		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
+	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
+	      if (TREE_CODE (esz) == INTEGER_CST)
+		/* Array element is either not a VLA or it's a VLA with
+		   zero size (such as int A[n][n][0];).  */
+		eltsize = wi::to_offset (esz);
 	      else
-		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
-				+ 1) * eltsize;
+		return false;
+
+	      if (TREE_CODE (nelts) == INTEGER_CST)
+		arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
+	      else if (eltsize == 0)
+		arrbounds[1] = 0;
+	      else
+		arrbounds[1] = maxobjsize / eltsize;
 	    }
-	  else
-	    arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
 
 	  /* Determine a tighter bound of the non-array element type.  */
 	  tree eltype = TREE_TYPE (reftype);
diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-9.c b/gcc/testsuite/c-c++-common/Warray-bounds-9.c
new file mode 100644
index 00000000000..8ff592b363c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-bounds-9.c
@@ -0,0 +1,144 @@
+/* PR tree-optimization/99121 - ICE in -Warray-bounds on a multidimensional
+   VLA
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+void sink (void*, ...);
+#define T(a, x) sink (a, x)
+
+
+NOIPA void a_0_n (int n)
+{
+  int a[0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0 (int n)
+{
+  int a[n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+NOIPA void a_1_n_0 (int n)
+{
+  int a[1][n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_1_0_n (int n)
+{
+  int a[1][0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_1_n (int n)
+{
+  int a[0][1][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_n_1 (int n)
+{
+  int a[0][n][1];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0_n (int n)
+{
+  int a[n][0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_n_0 (int n)
+{
+  int a[n][n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_n_n (int n)
+{
+  int a[0][n][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_0_n (int n)
+{
+  int a[0][0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0_0 (int n)
+{
+  int a[n][0][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_n_n (int n)
+{
+  int a[n][n][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[-1]);     // { dg-warning "\\\[-Warray-bounds" "pr99140" { xfail *-*-* } }
+  T (a, ((int *) a)[0]);
+  T (a, ((char *) a)[1]);
+  T (a, ((float *) a)[n]);
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-71.c b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
new file mode 100644
index 00000000000..c2af9bef78c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
@@ -0,0 +1,53 @@
+/* PR tree-optimization/99121 - ICE in -Warray-bounds on a multidimensional
+   VLA
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-strict-aliasing -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+void sink (void*, ...);
+#define T(a, x) sink (a, x)
+
+
+NOIPA void ma_0_n (int n)
+{
+  struct {
+    int a[0][n];
+  } s;
+
+  sink (&s);
+
+  T (&s, ((int *) s.a)[0]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[0]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[0]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[1]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[1]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[1]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[n]);   // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((char *) s.a)[n]);  // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((float *) s.a)[n]); // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+}
+
+
+NOIPA void ma_n_0 (int n)
+{
+  struct {
+    int a[n][0];
+  } s;
+
+  sink (&s);
+
+  T (&s, ((int *) s.a)[0]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[0]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[0]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[1]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[1]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[1]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[n]);   // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((char *) s.a)[n]);  // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((float *) s.a)[n]); // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-17 20:27   ` Martin Sebor
@ 2021-02-17 20:47     ` Jakub Jelinek
  2021-02-17 21:11       ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2021-02-17 20:47 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Wed, Feb 17, 2021 at 01:27:55PM -0700, Martin Sebor wrote:

Not in this patch, but I've looked at what maxobjsize is and wonder why
the roundtrip tree -> HOST_WIDE_INT -> offset_int:
  const offset_int maxobjsize = tree_to_shwi (max_object_size ());
Can't it be
  const offset_int maxobjsize = wi::to_offset (max_object_size ());
?

> I'm pretty sure that's because wide_int doesn't have division and
> I assumed offset_int didn't either when I originally wrote the code.
> I've changed it to use division.

wide_int does have division, otherwise offset_int wouldn't have it either.
One needs to choose if one wants signed or unsigned division, operator /
does signed, one can use wi::{,s,u}div_{trunc,ceil,round} etc.
As maxobjsize shouldn't have MSB set, it probably doesn't matter if one
uses signed or unsigned division.
> +	  tree nelts = array_type_nelts (reftype);
> +	  if (integer_all_onesp (nelts))
> +	    /* Zero length array.  */
> +	    arrbounds[1] = 0;

Ok then.

> +	  else
>  	    {
> -	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
> -	      if (TREE_CODE (arg) == COMPONENT_REF)
> -		{
> -		  offset_int size = maxobjsize;
> -		  if (tree fldsize = component_ref_size (arg))
> -		    size = wi::to_offset (fldsize);
> -		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
> -		}
> -	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
> -		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
> +	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
> +	      if (TREE_CODE (esz) == INTEGER_CST)
> +		/* Array element is either not a VLA or it's a VLA with
> +		   zero size (such as int A[n][n][0];).  */
> +		eltsize = wi::to_offset (esz);
>  	      else
> -		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
> -				+ 1) * eltsize;
> +		return false;
> +
> +	      if (TREE_CODE (nelts) == INTEGER_CST)
> +		arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
> +	      else if (eltsize == 0)
> +		arrbounds[1] = 0;

Doesn't arrbounds[1] == 0 mean there will be warnings for any accesses?
For eltsize == 0 I think you shouldn't warn when nelts isn't known,
instead of always warning, arr[100000000] will have the same address as
arr[0] ...

Otherwise LGTM.

	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-17 20:47     ` Jakub Jelinek
@ 2021-02-17 21:11       ` Martin Sebor
  2021-02-18  9:30         ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2021-02-17 21:11 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 2/17/21 1:47 PM, Jakub Jelinek wrote:
> On Wed, Feb 17, 2021 at 01:27:55PM -0700, Martin Sebor wrote:
> 
> Not in this patch, but I've looked at what maxobjsize is and wonder why
> the roundtrip tree -> HOST_WIDE_INT -> offset_int:
>    const offset_int maxobjsize = tree_to_shwi (max_object_size ());
> Can't it be
>    const offset_int maxobjsize = wi::to_offset (max_object_size ());
> ?

Yes, that's what it is elsewhere so this should do the same.  I've
changed it.

> 
>> I'm pretty sure that's because wide_int doesn't have division and
>> I assumed offset_int didn't either when I originally wrote the code.
>> I've changed it to use division.
> 
> wide_int does have division, otherwise offset_int wouldn't have it either.
> One needs to choose if one wants signed or unsigned division, operator /
> does signed, one can use wi::{,s,u}div_{trunc,ceil,round} etc.
> As maxobjsize shouldn't have MSB set, it probably doesn't matter if one
> uses signed or unsigned division.
>> +	  tree nelts = array_type_nelts (reftype);
>> +	  if (integer_all_onesp (nelts))
>> +	    /* Zero length array.  */
>> +	    arrbounds[1] = 0;
> 
> Ok then.
> 
>> +	  else
>>   	    {
>> -	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
>> -	      if (TREE_CODE (arg) == COMPONENT_REF)
>> -		{
>> -		  offset_int size = maxobjsize;
>> -		  if (tree fldsize = component_ref_size (arg))
>> -		    size = wi::to_offset (fldsize);
>> -		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
>> -		}
>> -	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
>> -		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
>> +	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
>> +	      if (TREE_CODE (esz) == INTEGER_CST)
>> +		/* Array element is either not a VLA or it's a VLA with
>> +		   zero size (such as int A[n][n][0];).  */
>> +		eltsize = wi::to_offset (esz);
>>   	      else
>> -		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
>> -				+ 1) * eltsize;
>> +		return false;
>> +
>> +	      if (TREE_CODE (nelts) == INTEGER_CST)
>> +		arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
>> +	      else if (eltsize == 0)
>> +		arrbounds[1] = 0;
> 
> Doesn't arrbounds[1] == 0 mean there will be warnings for any accesses?
> For eltsize == 0 I think you shouldn't warn when nelts isn't known,
> instead of always warning, arr[100000000] will have the same address as
> arr[0] ...

This branch is entered for VLAs of zero-length arrays where we want
to warn, like this:

void f (void*);

void g (int n)
{
   int a[n][0];
   ((int*)a)[0] = 0;
   f (a);
}

Martin

> 
> Otherwise LGTM.
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-17 21:11       ` Martin Sebor
@ 2021-02-18  9:30         ` Jakub Jelinek
  2021-02-18 16:24           ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2021-02-18  9:30 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Wed, Feb 17, 2021 at 02:11:43PM -0700, Martin Sebor wrote:
> On 2/17/21 1:47 PM, Jakub Jelinek wrote:
> > On Wed, Feb 17, 2021 at 01:27:55PM -0700, Martin Sebor wrote:
> > 
> > Not in this patch, but I've looked at what maxobjsize is and wonder why
> > the roundtrip tree -> HOST_WIDE_INT -> offset_int:
> >    const offset_int maxobjsize = tree_to_shwi (max_object_size ());
> > Can't it be
> >    const offset_int maxobjsize = wi::to_offset (max_object_size ());
> > ?
> 
> Yes, that's what it is elsewhere so this should do the same.  I've
> changed it.

Ok.

> > Doesn't arrbounds[1] == 0 mean there will be warnings for any accesses?
> > For eltsize == 0 I think you shouldn't warn when nelts isn't known,
> > instead of always warning, arr[100000000] will have the same address as
> > arr[0] ...
> 
> This branch is entered for VLAs of zero-length arrays where we want
> to warn, like this:
> 
> void f (void*);
> 
> void g (int n)
> {
>   int a[n][0];
>   ((int*)a)[0] = 0;
>   f (a);
> }

For this you do want to warn, but not the way you warn with the patch:
xxx.c: In function ‘g’:
xxx.c:6:12: warning: array subscript 0 is outside array bounds of ‘int[<Uec60>][0]’ [-Warray-bounds]
    6 |   ((int*)a)[0] = 0;
      |   ~~~~~~~~~^~~
xxx.c:5:7: note: while referencing ‘a’
    5 |   int a[n][0];
      |       ^

The message doesn't make it clear which of the two subscripts is out of
bounds, yes, for [0] it would be outside of bounds, but for the VLA index
no index < n would be outside of bounds.

Consider a different (GNU C, in C++ struct S has non-zero size) testcase:
void f (void*);

void g (int n)
{
  struct S {} a[n];
  ((int*)a)[0] = 0;
  f (a);
}
yyy.c:6:12: warning: array subscript 0 is outside array bounds of ‘struct S[<Ucc60>]’ [-Warray-bounds]
    6 |   ((int*)a)[0] = 0;
      |   ~~~~~~~~~^~~
yyy.c:5:15: note: while referencing ‘a’
    5 |   struct S {} a[n];
      |               ^
I bet that means you are really complaining about the VLA bound rather than
the [0] bound even in the first case, because the wording is otherwise the
same.  And for g (154) the array subscript 0 is certainly not a problem,
so the warning would need to be worded differently in that case.

	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-18  9:30         ` Jakub Jelinek
@ 2021-02-18 16:24           ` Martin Sebor
  2021-02-18 18:00             ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2021-02-18 16:24 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 2/18/21 2:30 AM, Jakub Jelinek wrote:
> On Wed, Feb 17, 2021 at 02:11:43PM -0700, Martin Sebor wrote:
>> On 2/17/21 1:47 PM, Jakub Jelinek wrote:
>>> On Wed, Feb 17, 2021 at 01:27:55PM -0700, Martin Sebor wrote:
>>>
>>> Not in this patch, but I've looked at what maxobjsize is and wonder why
>>> the roundtrip tree -> HOST_WIDE_INT -> offset_int:
>>>     const offset_int maxobjsize = tree_to_shwi (max_object_size ());
>>> Can't it be
>>>     const offset_int maxobjsize = wi::to_offset (max_object_size ());
>>> ?
>>
>> Yes, that's what it is elsewhere so this should do the same.  I've
>> changed it.
> 
> Ok.
> 
>>> Doesn't arrbounds[1] == 0 mean there will be warnings for any accesses?
>>> For eltsize == 0 I think you shouldn't warn when nelts isn't known,
>>> instead of always warning, arr[100000000] will have the same address as
>>> arr[0] ...
>>
>> This branch is entered for VLAs of zero-length arrays where we want
>> to warn, like this:
>>
>> void f (void*);
>>
>> void g (int n)
>> {
>>    int a[n][0];
>>    ((int*)a)[0] = 0;
>>    f (a);
>> }
> 
> For this you do want to warn, but not the way you warn with the patch:
> xxx.c: In function ‘g’:
> xxx.c:6:12: warning: array subscript 0 is outside array bounds of ‘int[<Uec60>][0]’ [-Warray-bounds]
>      6 |   ((int*)a)[0] = 0;
>        |   ~~~~~~~~~^~~
> xxx.c:5:7: note: while referencing ‘a’
>      5 |   int a[n][0];
>        |       ^
> 
> The message doesn't make it clear which of the two subscripts is out of
> bounds, yes, for [0] it would be outside of bounds, but for the VLA index
> no index < n would be outside of bounds.

There's only one subscript in '((int*)a)[0] = 0;' and in the vrp1 IL
and that's the one the warning mentions.  The size of the VLA is zero
so it doesn't matter what the index is, all accesses to it are out of
bounds.  I see nothing wrong here.

> 
> Consider a different (GNU C, in C++ struct S has non-zero size) testcase:
> void f (void*);
> 
> void g (int n)
> {
>    struct S {} a[n];
>    ((int*)a)[0] = 0;
>    f (a);
> }
> yyy.c:6:12: warning: array subscript 0 is outside array bounds of ‘struct S[<Ucc60>]’ [-Warray-bounds]
>      6 |   ((int*)a)[0] = 0;
>        |   ~~~~~~~~~^~~
> yyy.c:5:15: note: while referencing ‘a’
>      5 |   struct S {} a[n];
>        |               ^
> I bet that means you are really complaining about the VLA bound rather than
> the [0] bound even in the first case, because the wording is otherwise the
> same.  And for g (154) the array subscript 0 is certainly not a problem,
> so the warning would need to be worded differently in that case.

I'm not sure I follow what you're saying here either.  The vrp1 dump
has this:

void g (int n)
{
   struct S a[0:D.1951];

   <bb 2> [local count: 1073741824]:
   MEM[(int *)&a] = 0;
   f (&a);
   a ={v} {CLOBBER};
   return;

}

The size of the VLA is zero regardless of its bound and accessing
it is invalid so the warning is expected.

VLAs of zero-lengthg arrays are without a doubt rare, pathological
cases.  We could special case the warning for them and print
a different message but  I see very little value in complicating
the code just for them.  Do you consider this special casing
a requirement for approving the fix for the ICE?

Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-18 16:24           ` Martin Sebor
@ 2021-02-18 18:00             ` Jakub Jelinek
  2021-02-18 18:03               ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2021-02-18 18:00 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Thu, Feb 18, 2021 at 09:24:28AM -0700, Martin Sebor via Gcc-patches wrote:
> > Consider a different (GNU C, in C++ struct S has non-zero size) testcase:
> > void f (void*);
> > 
> > void g (int n)
> > {
> >    struct S {} a[n];
> >    ((int*)a)[0] = 0;
> >    f (a);
> > }
> > yyy.c:6:12: warning: array subscript 0 is outside array bounds of ‘struct S[<Ucc60>]’ [-Warray-bounds]
> >      6 |   ((int*)a)[0] = 0;
> >        |   ~~~~~~~~~^~~
> > yyy.c:5:15: note: while referencing ‘a’
> >      5 |   struct S {} a[n];
> >        |               ^
> > I bet that means you are really complaining about the VLA bound rather than
> > the [0] bound even in the first case, because the wording is otherwise the
> > same.  And for g (154) the array subscript 0 is certainly not a problem,
> > so the warning would need to be worded differently in that case.
> 
> I'm not sure I follow what you're saying here either.  The vrp1 dump
> has this:
> 
> void g (int n)
> {
>   struct S a[0:D.1951];
> 
>   <bb 2> [local count: 1073741824]:
>   MEM[(int *)&a] = 0;
>   f (&a);
>   a ={v} {CLOBBER};
>   return;
> 
> }
> 
> The size of the VLA is zero regardless of its bound and accessing
> it is invalid so the warning is expected.

Yes, some warning, but not the one you are giving, that is nonsensical.
Array subscript 0 is not outside of array bounds of struct S[n], a[anything]
will still be zero sized and will not be problematic.

> VLAs of zero-lengthg arrays are without a doubt rare, pathological
> cases.  We could special case the warning for them and print
> a different message but  I see very little value in complicating
> the code just for them.  Do you consider this special casing
> a requirement for approving the fix for the ICE?

Yes.

	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-18 18:00             ` Jakub Jelinek
@ 2021-02-18 18:03               ` Jakub Jelinek
  2021-02-18 20:55                 ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2021-02-18 18:03 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Thu, Feb 18, 2021 at 07:00:52PM +0100, Jakub Jelinek wrote:
> > The size of the VLA is zero regardless of its bound and accessing
> > it is invalid so the warning is expected.
> 
> Yes, some warning, but not the one you are giving, that is nonsensical.
> Array subscript 0 is not outside of array bounds of struct S[n], a[anything]
> will still be zero sized and will not be problematic.

Scalar objects with zero size will always have that zero size,
similarly arrays thereof (constant or variable sized).
So the warning should be simply if eltsize == 0,
check if the access is before or after the object and complain
that a memory access is done before or after a zero sized object %qD.

	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-18 18:03               ` Jakub Jelinek
@ 2021-02-18 20:55                 ` Martin Sebor
  2021-03-09  2:37                   ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2021-02-18 20:55 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 2/18/21 11:03 AM, Jakub Jelinek wrote:
> On Thu, Feb 18, 2021 at 07:00:52PM +0100, Jakub Jelinek wrote:
>>> The size of the VLA is zero regardless of its bound and accessing
>>> it is invalid so the warning is expected.
>>
>> Yes, some warning, but not the one you are giving, that is nonsensical.
>> Array subscript 0 is not outside of array bounds of struct S[n], a[anything]
>> will still be zero sized and will not be problematic.

The warning is designed for ordinary arrays of nonzero size.  There's
no point in putting an effort into coming up with a special warning
just for those because they serve no purpose in these contexts (as
complete objects).

> 
> Scalar objects with zero size will always have that zero size,
> similarly arrays thereof (constant or variable sized).
> So the warning should be simply if eltsize == 0,
> check if the access is before or after the object and complain
> that a memory access is done before or after a zero sized object %qD.
> 
> 	Jakub

No, I don't think making this exception would be helpful.  Zero length
arrays are a non-standard extension meant to be used as struct members,
before flexible array members were added to C.  In other contexts, they
are almost certainly unintended and so likely bugs.  There's no valid
use case for such arrays, and diagnosing accesses to them helps find
such bugs.

That said, I also don't think a fix for the ICE should be held up
because we disagree on this vanishingly unimportant corner case.
The ICE effectively prevents using such arrays (VLAs) and since no
bug reports have been raised for it since it was introduced in GCC
9 it's unlikely that any code relies on it.  (I suspect the bug
itself was the result of fuzzing.)

Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-02-18 20:55                 ` Martin Sebor
@ 2021-03-09  2:37                   ` Martin Sebor
  2021-03-12 13:27                     ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Sebor @ 2021-03-09  2:37 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]

Attached is a revised patch with three changes:

1) use wi::to_offset (max_object_size ()) instead of tree_to_shwi()
    as requested,
2) avoid warning for accesses to elements of arrays of empty types
    (PR 99475 that I noticed while testing the original patch),
3) include the size of the destination even for declared objects to
    make warnings for nonempty accesses to elements of arrays of empty
    structs slightly clearer.

Accesses to zero-length arrays continue to be diagnosed (except for
trailing arrays of unknown objects), as are nonempty accesses to empty
types.

The warning message for (3) remains unchanged, i.e., for the following:

   struct S { } a[3];

   void g (int n)
   {
     ((int*)a)[0] = 0;
   }

it's:

   warning: array subscript 0 is outside array bounds of ‘struct S[3]’ 
[-Warray-bounds]

This could be improved by mentioning the type of the access when
it's not the same as that of the accessed object as is already done
for partially out of bounds accesses:

   warning: subscript int[0] is outside array bounds of ‘struct S[3]’ 
[-Warray-bounds]

but making this change at the same time feels like feature creep and
out of scope for stage 4.

Retested on x86_64-linux.

On 2/18/21 1:55 PM, Martin Sebor wrote:
> On 2/18/21 11:03 AM, Jakub Jelinek wrote:
>> On Thu, Feb 18, 2021 at 07:00:52PM +0100, Jakub Jelinek wrote:
>>>> The size of the VLA is zero regardless of its bound and accessing
>>>> it is invalid so the warning is expected.
>>>
>>> Yes, some warning, but not the one you are giving, that is nonsensical.
>>> Array subscript 0 is not outside of array bounds of struct S[n], 
>>> a[anything]
>>> will still be zero sized and will not be problematic.
> 
> The warning is designed for ordinary arrays of nonzero size.  There's
> no point in putting an effort into coming up with a special warning
> just for those because they serve no purpose in these contexts (as
> complete objects).
> 
>>
>> Scalar objects with zero size will always have that zero size,
>> similarly arrays thereof (constant or variable sized).
>> So the warning should be simply if eltsize == 0,
>> check if the access is before or after the object and complain
>> that a memory access is done before or after a zero sized object %qD.
>>
>>     Jakub
> 
> No, I don't think making this exception would be helpful.  Zero length
> arrays are a non-standard extension meant to be used as struct members,
> before flexible array members were added to C.  In other contexts, they
> are almost certainly unintended and so likely bugs.  There's no valid
> use case for such arrays, and diagnosing accesses to them helps find
> such bugs.
> 
> That said, I also don't think a fix for the ICE should be held up
> because we disagree on this vanishingly unimportant corner case.
> The ICE effectively prevents using such arrays (VLAs) and since no
> bug reports have been raised for it since it was introduced in GCC
> 9 it's unlikely that any code relies on it.  (I suspect the bug
> itself was the result of fuzzing.)
> 
> Martin


[-- Attachment #2: gcc-99121.diff --]
[-- Type: text/x-patch, Size: 22770 bytes --]

PR tree-optimization/99121 - ICE in -Warray-bounds on a VLA of zero-length array
PR tree-optimization/99475 - bogus -Warray-bounds accessing an array element of empty structs

gcc/ChangeLog:

	PR tree-optimization/99121
	PR tree-optimization/99475
	* gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
	Avoid assuming array element size is constant.  Handle zero-length
	arrays of VLAs.

gcc/testsuite/ChangeLog:

	PR tree-optimization/99121
	PR tree-optimization/99475
	* c-c++-common/Warray-bounds-10.c: New test.
	* c-c++-common/Warray-bounds-9.c: New test.
	* gcc.dg/Warray-bounds-71.c: New test.
	* gcc.dg/Warray-bounds-72.c: New test.
	* gcc.dg/Warray-bounds-73.c: New test.

diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index 54f32051199..bd6caee7d5a 100644
--- a/gcc/gimple-array-bounds.cc
+++ b/gcc/gimple-array-bounds.cc
@@ -419,7 +419,7 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
   tree cstoff = TREE_OPERAND (ref, 1);
   tree varoff = NULL_TREE;
 
-  const offset_int maxobjsize = tree_to_shwi (max_object_size ());
+  const offset_int maxobjsize = wi::to_offset (max_object_size ());
 
   /* The zero-based array or string constant bounds in bytes.  Initially
      set to [-MAXOBJSIZE - 1, MAXOBJSIZE]  until a tighter bound is
@@ -553,7 +553,12 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
 	offrange[1] = arrbounds[1];
     }
 
+  /* Type of the access.  */
+  tree axstype = TREE_TYPE (ref);
+  /* TYpe of the referenced object if it can be determined or array
+     of unsigned char for allocated objects.  */
   tree reftype = NULL_TREE;
+  /* The byte size of the referenced array element.  */
   offset_int eltsize = -1;
   if (arrbounds[0] >= 0)
     {
@@ -602,34 +607,44 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
 	      || (DECL_EXTERNAL (arg) && array_at_struct_end_p (ref))))
 	return false;
 
-      /* FIXME: Should this be 1 for Fortran?  */
       arrbounds[0] = 0;
 
       if (TREE_CODE (reftype) == ARRAY_TYPE)
 	{
-	  /* Set to the size of the array element (and adjust below).  */
-	  eltsize = wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (reftype)));
-	  /* Use log2 of size to convert the array byte size in to its
-	     upper bound in elements.  */
-	  const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
-	  if (tree dom = TYPE_DOMAIN (reftype))
+	  tree nelts = array_type_nelts (reftype);
+	  if (integer_all_onesp (nelts))
+	    /* Zero length array.  */
+	    arrbounds[1] = 0;
+	  else
 	    {
-	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
-	      if (TREE_CODE (arg) == COMPONENT_REF)
+	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
+	      if (TREE_CODE (esz) != INTEGER_CST)
+		return false;
+
+	      eltsize = wi::to_offset (esz);
+	      if (eltsize == 0)
 		{
-		  offset_int size = maxobjsize;
-		  if (tree fldsize = component_ref_size (arg))
-		    size = wi::to_offset (fldsize);
-		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
+		  /* Array element is either not a VLA or it's a VLA with
+		     zero size (such as int[n][n][0]).  Check to see if
+		     the most derived element type is an empty type such
+		     as a struct with zero size and if so, treat it as if
+		     it had size of 1 to avoid warning on empty accesses
+		     to elements of such arrays that GCC eliminates.  */
+		  tree elt = strip_array_types (reftype);
+		  if (TYPE_EMPTY_P (elt) && TYPE_EMPTY_P (axstype))
+		    {
+		      nelts = integer_zero_node;
+		      eltsize = 1;
+		    }
 		}
-	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
-		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
+
+	      if (TREE_CODE (nelts) == INTEGER_CST)
+		arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
+	      else if (eltsize == 0)
+		arrbounds[1] = 0;
 	      else
-		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
-				+ 1) * eltsize;
+		arrbounds[1] = maxobjsize / eltsize;
 	    }
-	  else
-	    arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
 
 	  /* Determine a tighter bound of the non-array element type.  */
 	  tree eltype = TREE_TYPE (reftype);
@@ -667,16 +682,15 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
   const bool lboob = (arrbounds[0] == arrbounds[1]
 		      || offrange[0] >= ubound
 		      || offrange[1] < arrbounds[0]);
-  /* Set if only the upper bound of the subscript is out of bounds.
-     This can happen when using a bigger type to index into an array
-     of a smaller type, as is common with unsigned char.  */
-  tree axstype = TREE_TYPE (ref);
   offset_int axssize = 0;
   if (TREE_CODE (axstype) != UNION_TYPE)
     if (tree access_size = TYPE_SIZE_UNIT (axstype))
       if (TREE_CODE (access_size) == INTEGER_CST)
 	axssize = wi::to_offset (access_size);
 
+  /* Set if only the upper bound of the subscript is out of bounds.
+     This can happen when using a bigger type to index into an array
+     of a smaller type, as is common with unsigned char.  */
   const bool uboob = !lboob && offrange[0] + axssize > ubound;
   if (lboob || uboob)
     {
@@ -731,7 +745,8 @@ array_bounds_checker::check_mem_ref (location_t location, tree ref,
   if (warned)
     {
       if (DECL_P (arg))
-	inform (DECL_SOURCE_LOCATION (arg), "while referencing %qD", arg);
+	inform (DECL_SOURCE_LOCATION (arg), "while referencing %qD "
+		"of size %wu", arg, minbound.to_uhwi ());
       else if (alloc_stmt)
 	{
 	  location_t loc = gimple_location (alloc_stmt);
diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-10.c b/gcc/testsuite/c-c++-common/Warray-bounds-10.c
new file mode 100644
index 00000000000..cfe9a383410
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-bounds-10.c
@@ -0,0 +1,114 @@
+/* PR tree-optimization/99475 - bogus -Warray-bounds accessing an array
+   element of empty structs
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+struct S
+{
+#if SOME_CONFIG_MACRO
+  /* Suppose the contents are empty in the development configuration
+     but non-empty in others.  Out of bounds accesses to elements of
+     the arrays below should be diagnosed in all configurations,
+     including when S is empty, even if they are folded away.  */
+  int member;
+#endif
+};
+
+extern struct S sa3[3];
+extern struct S sa2_3[2][3];
+extern struct S sa3_4_5[3][4][5];
+
+void sink (void*);
+
+
+void access_sa3 (struct S s)
+{
+  sa3[0] = s;
+  sa3[1] = s;
+  sa3[2] = s;
+  sa3[3] = s;       // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void access_sa3_ptr (struct S s)
+{
+  struct S *p = &sa3[0];
+
+  p[0] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[3] = s;         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void access_sa2_3_ptr (struct S s)
+{
+  struct S *p = &sa2_3[0][0];
+
+  p[0] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[6] = s;         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void access_sa3_4_5_ptr (struct S s, int i)
+{
+  struct S *p = &sa3_4_5[0][0][0];
+
+  p[0] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[60] = s;        // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+
+void access_vla3 (struct S s, unsigned n)
+{
+  struct S vla3[3 < n ? 3 : n];
+
+  vla3[0] = s;
+  vla3[1] = s;
+  vla3[2] = s;
+  vla3[3] = s;       // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla3);
+}
+
+void access_vla3_ptr (struct S s, unsigned n)
+{
+  struct S vla3[3 < n ? 3 : n];
+  struct S *p = &vla3[0];
+
+  p[0] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[3] = s;         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla3);
+}
+
+void access_vla2_3_ptr (struct S s, unsigned n)
+{
+  struct S vla2_3[2 < n ? 2 : n][3 < n ? 3 : n];
+  struct S *p = &vla2_3[0][0];
+
+  p[0] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[6] = s;         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla2_3);
+}
+
+void access_vla3_4_5_ptr (struct S s, unsigned n)
+{
+  struct S vla3_4_5[3 < n ? 3 : n][4 < n ? 4 : n][5 < n ? 5 : n];
+  struct S *p = &vla3_4_5[0][0][0];
+
+  p[0] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = s;         // { dg-bogus "\\\[-Warray-bounds" }
+  p[60] = s;        // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla3_4_5);
+}
+
+// { dg-prune-output "empty struct has size 0 in C" }
diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-9.c b/gcc/testsuite/c-c++-common/Warray-bounds-9.c
new file mode 100644
index 00000000000..8ff592b363c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-bounds-9.c
@@ -0,0 +1,144 @@
+/* PR tree-optimization/99121 - ICE in -Warray-bounds on a multidimensional
+   VLA
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+void sink (void*, ...);
+#define T(a, x) sink (a, x)
+
+
+NOIPA void a_0_n (int n)
+{
+  int a[0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0 (int n)
+{
+  int a[n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+NOIPA void a_1_n_0 (int n)
+{
+  int a[1][n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_1_0_n (int n)
+{
+  int a[1][0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_1_n (int n)
+{
+  int a[0][1][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_n_1 (int n)
+{
+  int a[0][n][1];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0_n (int n)
+{
+  int a[n][0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_n_0 (int n)
+{
+  int a[n][n][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_n_n (int n)
+{
+  int a[0][n][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_0_0_n (int n)
+{
+  int a[0][0][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_0_0 (int n)
+{
+  int a[n][0][0];
+
+  sink (a);
+
+  T (a, ((int *) a)[0]);      // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((char *) a)[1]);     // { dg-warning "\\\[-Warray-bounds" }
+  T (a, ((float *) a)[n]);    // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void a_n_n_n (int n)
+{
+  int a[n][n][n];
+
+  sink (a);
+
+  T (a, ((int *) a)[-1]);     // { dg-warning "\\\[-Warray-bounds" "pr99140" { xfail *-*-* } }
+  T (a, ((int *) a)[0]);
+  T (a, ((char *) a)[1]);
+  T (a, ((float *) a)[n]);
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-71.c b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
new file mode 100644
index 00000000000..c2af9bef78c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-71.c
@@ -0,0 +1,53 @@
+/* PR tree-optimization/99121 - ICE in -Warray-bounds on a multidimensional
+   VLA
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-strict-aliasing -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+void sink (void*, ...);
+#define T(a, x) sink (a, x)
+
+
+NOIPA void ma_0_n (int n)
+{
+  struct {
+    int a[0][n];
+  } s;
+
+  sink (&s);
+
+  T (&s, ((int *) s.a)[0]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[0]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[0]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[1]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[1]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[1]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[n]);   // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((char *) s.a)[n]);  // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((float *) s.a)[n]); // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+}
+
+
+NOIPA void ma_n_0 (int n)
+{
+  struct {
+    int a[n][0];
+  } s;
+
+  sink (&s);
+
+  T (&s, ((int *) s.a)[0]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[0]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[0]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[1]);   // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((char *) s.a)[1]);  // { dg-warning "\\\[-Warray-bounds" }
+  T (&s, ((float *) s.a)[1]); // { dg-warning "\\\[-Warray-bounds" }
+
+  T (&s, ((int *) s.a)[n]);   // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((char *) s.a)[n]);  // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+  T (&s, ((float *) s.a)[n]); // { dg-warning "\\\[-Warray-bounds" "pr99129" { xfail *-*-* } }
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-72.c b/gcc/testsuite/gcc.dg/Warray-bounds-72.c
new file mode 100644
index 00000000000..b44ac9d3aa2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-72.c
@@ -0,0 +1,112 @@
+/* PR tree-optimization/99475 - bogus -Warray-bounds accessing an array
+   element of empty structs
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+struct S
+{
+#if SOME_CONFIG_MACRO
+  /* Suppose the contents are empty in the development configuration
+     but non-empty in others.  Out of bounds accesses to elements of
+     the arrays below should be diagnosed in all configurations,
+     including when S is empty, even if they are folded away.  */
+  int member;
+#endif
+};
+
+extern struct S sa3[3];
+extern struct S sa2_3[2][3];
+extern struct S sa3_4_5[3][4][5];
+
+void sink (void*);
+
+
+void access_sa3 (void)
+{
+  sa3[0] = (struct S){ };
+  sa3[1] = (struct S){ };
+  sa3[2] = (struct S){ };
+  sa3[3] = (struct S){ };       // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void access_sa3_ptr (void)
+{
+  struct S *p = &sa3[0];
+
+  p[0] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[3] = (struct S){ };         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void access_sa2_3_ptr (void)
+{
+  struct S *p = &sa2_3[0][0];
+
+  p[0] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[6] = (struct S){ };         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void access_sa3_4_5_ptr (struct S s, int i)
+{
+  struct S *p = &sa3_4_5[0][0][0];
+
+  p[0] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[60] = (struct S){ };        // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+
+void access_vla3 (struct S s, unsigned n)
+{
+  struct S vla3[3 < n ? 3 : n];
+
+  vla3[0] = (struct S){ };
+  vla3[1] = (struct S){ };
+  vla3[2] = (struct S){ };
+  vla3[3] = (struct S){ };       // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla3);
+}
+
+void access_vla3_ptr (struct S s, unsigned n)
+{
+  struct S vla3[3 < n ? 3 : n];
+  struct S *p = &vla3[0];
+
+  p[0] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[3] = (struct S){ };         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla3);
+}
+
+void access_vla2_3_ptr (struct S s, unsigned n)
+{
+  struct S vla2_3[2 < n ? 2 : n][3 < n ? 3 : n];
+  struct S *p = &vla2_3[0][0];
+
+  p[0] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[6] = (struct S){ };         // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla2_3);
+}
+
+void access_vla3_4_5_ptr (struct S s, unsigned n)
+{
+  struct S vla3_4_5[3 < n ? 3 : n][4 < n ? 4 : n][5 < n ? 5 : n];
+  struct S *p = &vla3_4_5[0][0][0];
+
+  p[0] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[1] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[2] = (struct S){ };         // { dg-bogus "\\\[-Warray-bounds" }
+  p[60] = (struct S){ };        // { dg-warning "\\\[-Warray-bounds" "pr?????" { xfail *-*-* } }
+
+  sink (vla3_4_5);
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-73.c b/gcc/testsuite/gcc.dg/Warray-bounds-73.c
new file mode 100644
index 00000000000..73c335fd8a6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-73.c
@@ -0,0 +1,109 @@
+/* PR tree-optimization/99475 - bogus -Warray-bounds accessing an array
+   element of empty structs
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-strict-aliasing" } */
+
+typedef _Bool bool;
+
+#define NOIPA __attribute__ ((noipa))
+
+struct S { };
+
+extern struct S sa3[3];
+extern struct S sa2_3[2][3];
+extern struct S sa3_4_5[3][4][5];
+
+void sink (void*);
+
+
+NOIPA void access_sa3 (void)
+{
+  ((bool*)sa3)[0] = __LINE__;     // { dg-warning "\\\[-Warray-bounds" }
+  ((bool*)sa3)[1] = __LINE__;     // { dg-warning "\\\[-Warray-bounds" }
+  ((bool*)sa3)[2] = __LINE__;     // { dg-warning "\\\[-Warray-bounds" }
+  ((bool*)sa3)[3] = __LINE__;     // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void access_sa3_ptr (void)
+{
+  bool *p = (bool*)&sa3[0];
+
+  p[0] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[1] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[2] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[3] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void access_sa2_3_ptr (void)
+{
+  bool *p = (bool*)&sa2_3[0][0];
+
+  p[0] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[1] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[2] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[6] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+}
+
+NOIPA void access_sa3_4_5_ptr (struct S s, int i)
+{
+  bool *p = (bool*)&sa3_4_5[0][0][0];
+
+  p[0] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[1] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[2] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[60] = __LINE__;       // { dg-warning "\\\[-Warray-bounds" }
+}
+
+
+NOIPA void access_vla3 (struct S s, unsigned n)
+{
+  struct S vla3[3 < n ? 3 : n];
+
+  ((bool*)vla3)[0] = __LINE__;    // { dg-warning "\\\[-Warray-bounds" }
+  ((bool*)vla3)[1] = __LINE__;    // { dg-warning "\\\[-Warray-bounds" }
+  ((bool*)vla3)[2] = __LINE__;    // { dg-warning "\\\[-Warray-bounds" }
+  ((bool*)vla3)[3] = __LINE__;    // { dg-warning "\\\[-Warray-bounds" }
+
+  sink (vla3);
+}
+
+NOIPA void access_vla3_ptr (struct S s, unsigned n)
+{
+  struct S vla3[3 < n ? 3 : n];
+  bool *p = (bool*)&vla3[0];
+
+  p[0] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[1] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[2] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[3] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+
+  sink (vla3);
+}
+
+NOIPA void access_vla2_3_ptr (struct S s, unsigned n)
+{
+  struct S vla2_3[2 < n ? 2 : n][3 < n ? 3 : n];
+  bool *p = (bool*)&vla2_3[0][0];
+
+  p[0] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[1] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[2] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[6] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+
+  sink (vla2_3);
+}
+
+NOIPA void access_vla3_4_5_ptr (struct S s, unsigned n)
+{
+  struct S vla3_4_5[3 < n ? 3 : n][4 < n ? 4 : n][5 < n ? 5 : n];
+  bool *p = (bool*)&vla3_4_5[0][0][0];
+
+  p[0] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[1] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[2] = __LINE__;        // { dg-warning "\\\[-Warray-bounds" }
+  p[60] = __LINE__;       // { dg-warning "\\\[-Warray-bounds" }
+
+  sink (vla3_4_5);
+}
+
+// { dg-prune-output "empty struct has size 0 in C" }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-03-09  2:37                   ` Martin Sebor
@ 2021-03-12 13:27                     ` Jakub Jelinek
  2021-03-13 21:46                       ` Martin Sebor
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2021-03-12 13:27 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

On Mon, Mar 08, 2021 at 07:37:46PM -0700, Martin Sebor via Gcc-patches wrote:
> Accesses to zero-length arrays continue to be diagnosed (except for
> trailing arrays of unknown objects), as are nonempty accesses to empty
> types.
> 
> The warning message for (3) remains unchanged, i.e., for the following:
> 
>   struct S { } a[3];
> 
>   void g (int n)
>   {
>     ((int*)a)[0] = 0;
>   }
> 
> it's:
> 
>   warning: array subscript 0 is outside array bounds of ‘struct S[3]’
> [-Warray-bounds]

As I tried to explain several times, this is completely unacceptable to me.
We want to warn, I agree with that, but we don't care that we emit completely
nonsensical warning?
The user will be just confused by that, will (rightly) think it is a bug in
the compiler and might not fix the actual bug.
Array subscript 0 is not outside of those array bounds.

If you don't want a shortcut that for arrays with zero sized elements
and for non-array objects with zero size uses a different code path
that would just warn
"access outside of zero sized object %qD"
(which is what I'd prefer, any non-zero sized access to object of zero size
unless it is flexible array member or decl with flexible array member
is undefined, whatever offset you use)
and want to reuse the current code, at least please change reftype
to build_printable_array_type (TREE_TYPE (ref), 0);
so that it prints
warning: array subscript 0 is outside array bounds of ‘int[0]’ [-Warray-bounds]
You'll need to redo the:
          || !COMPLETE_TYPE_P (reftype)
          || TREE_CODE (TYPE_SIZE_UNIT (reftype)) != INTEGER_CST)
        return false;
check on the new reftype.
It should be done not just where you currently do:
                      nelts = integer_zero_node;
                      eltsize = 1;
but also somewhere in:
          eltsize = 1;
          tree size = TYPE_SIZE_UNIT (reftype);
          if (VAR_P (arg))
            if (tree initsize = DECL_SIZE_UNIT (arg))
              if (tree_int_cst_lt (size, initsize))
                size = initsize;

          arrbounds[1] = wi::to_offset (size);
below that for the case where integer_zerop (size) && TYPE_EMPTY_P (reftype).
There should be also:

  struct S { } a;

  void g (int n)
  {
    ((int*)&a)[0] = 0;
  }

testcase that covers that.

BTW, what is the reason behind the POINTER_TYPE_P check in:
      /* The type of the object being referred to.  It can be an array,
         string literal, or a non-array type when the MEM_REF represents
         a reference/subscript via a pointer to an object that is not
         an element of an array.  Incomplete types are excluded as well
         because their size is not known.  */
      reftype = TREE_TYPE (arg);
      if (POINTER_TYPE_P (reftype)
?
It disables the second warning on:
  __UINTPTR_TYPE__ a;
  void *b;

  void g (int n)
  {
    ((int*)&a)[4] = 0;
    ((int*)&b)[4] = 0;
  }

I really don't see what is different on vars with pointer type vs. vars with
non-pointer type of the same size for this warning.

	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)
  2021-03-12 13:27                     ` Jakub Jelinek
@ 2021-03-13 21:46                       ` Martin Sebor
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Sebor @ 2021-03-13 21:46 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 3/12/21 6:27 AM, Jakub Jelinek wrote:
> On Mon, Mar 08, 2021 at 07:37:46PM -0700, Martin Sebor via Gcc-patches wrote:
>> Accesses to zero-length arrays continue to be diagnosed (except for
>> trailing arrays of unknown objects), as are nonempty accesses to empty
>> types.
>>
>> The warning message for (3) remains unchanged, i.e., for the following:
>>
>>    struct S { } a[3];
>>
>>    void g (int n)
>>    {
>>      ((int*)a)[0] = 0;
>>    }
>>
>> it's:
>>
>>    warning: array subscript 0 is outside array bounds of ‘struct S[3]’
>> [-Warray-bounds]
> 
> As I tried to explain several times, this is completely unacceptable to me.
> We want to warn, I agree with that, but we don't care that we emit completely
> nonsensical warning?
> The user will be just confused by that, will (rightly) think it is a bug in
> the compiler and might not fix the actual bug.
> Array subscript 0 is not outside of those array bounds.

You're being unreasonable.  As I explained, I'm open to improving
the text of the warning but consider tweaking it outside the scope
of this bug.

> 
> If you don't want a shortcut that for arrays with zero sized elements
> and for non-array objects with zero size uses a different code path
> that would just warn
> "access outside of zero sized object %qD"

You're quibbling over a minute difference in the phrasing of a warning
for an obscure case that will probably never even come up.  But if it
did, the text is secondary to issuing it since it detects a bug.  So
again, I'm open to rewording the warning for GCC 12 but I'm not willing
to start rewriting the code in stage 4 just because you don't like how
the warning is phrased.

Martin

> (which is what I'd prefer, any non-zero sized access to object of zero size
> unless it is flexible array member or decl with flexible array member
> is undefined, whatever offset you use)
> and want to reuse the current code, at least please change reftype
> to build_printable_array_type (TREE_TYPE (ref), 0);
> so that it prints
> warning: array subscript 0 is outside array bounds of ‘int[0]’ [-Warray-bounds]
> You'll need to redo the:
>            || !COMPLETE_TYPE_P (reftype)
>            || TREE_CODE (TYPE_SIZE_UNIT (reftype)) != INTEGER_CST)
>          return false;
> check on the new reftype.
> It should be done not just where you currently do:
>                        nelts = integer_zero_node;
>                        eltsize = 1;
> but also somewhere in:
>            eltsize = 1;
>            tree size = TYPE_SIZE_UNIT (reftype);
>            if (VAR_P (arg))
>              if (tree initsize = DECL_SIZE_UNIT (arg))
>                if (tree_int_cst_lt (size, initsize))
>                  size = initsize;
> 
>            arrbounds[1] = wi::to_offset (size);
> below that for the case where integer_zerop (size) && TYPE_EMPTY_P (reftype).
> There should be also:
> 
>    struct S { } a;
> 
>    void g (int n)
>    {
>      ((int*)&a)[0] = 0;
>    }
> 
> testcase that covers that.
> 
> BTW, what is the reason behind the POINTER_TYPE_P check in:
>        /* The type of the object being referred to.  It can be an array,
>           string literal, or a non-array type when the MEM_REF represents
>           a reference/subscript via a pointer to an object that is not
>           an element of an array.  Incomplete types are excluded as well
>           because their size is not known.  */
>        reftype = TREE_TYPE (arg);
>        if (POINTER_TYPE_P (reftype)
> ?
> It disables the second warning on:
>    __UINTPTR_TYPE__ a;
>    void *b;
> 
>    void g (int n)
>    {
>      ((int*)&a)[4] = 0;
>      ((int*)&b)[4] = 0;
>    }
> 
> I really don't see what is different on vars with pointer type vs. vars with
> non-pointer type of the same size for this warning.
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-03-13 21:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-17  3:34 [PATCH] handle VLA of zero length arrays and vice versa (PR 99121) Martin Sebor
2021-02-17 13:56 ` Jakub Jelinek
2021-02-17 20:27   ` Martin Sebor
2021-02-17 20:47     ` Jakub Jelinek
2021-02-17 21:11       ` Martin Sebor
2021-02-18  9:30         ` Jakub Jelinek
2021-02-18 16:24           ` Martin Sebor
2021-02-18 18:00             ` Jakub Jelinek
2021-02-18 18:03               ` Jakub Jelinek
2021-02-18 20:55                 ` Martin Sebor
2021-03-09  2:37                   ` Martin Sebor
2021-03-12 13:27                     ` Jakub Jelinek
2021-03-13 21:46                       ` Martin Sebor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).