diff mbox series

correct format of flexible array members in diagnostics (PR c/92326)

Message ID e3454753-51d6-e901-3d15-159bdd2194d2@gmail.com
State New
Headers show
Series correct format of flexible array members in diagnostics (PR c/92326) | expand

Commit Message

Martin Sebor April 13, 2020, 6:39 p.m. UTC 
GCC 10 has changed the formatting of zero-length arrays in diagnostics
to include their bound, but it also inadvertently added the zero bound
to flexible array members which are confusingly represented differently
between the C and C++ front ends.

The attached patch corrects the problem so both zero-length arrays and
flexible array members are formatted consistently by both front ends
(i.e., as T[0] and T[]).

Tested on x86_64-linux.

Martin

Comments

Li, Pan2 via Gcc-patches April 13, 2020, 7:25 p.m. UTC | #1 
On Mon, 2020-04-13 at 12:39 -0600, Martin Sebor via Gcc-patches wrote:
> GCC 10 has changed the formatting of zero-length arrays in diagnostics
> to include their bound, but it also inadvertently added the zero bound
> to flexible array members which are confusingly represented differently
> between the C and C++ front ends.
> 
> The attached patch corrects the problem so both zero-length arrays and
> flexible array members are formatted consistently by both front ends
> (i.e., as T[0] and T[]).
> 
> Tested on x86_64-linux.
This is fine.

Though I don't see that it actually addressed the qemu or grub2 issues raised by
Martin L.  ISTM those really should be a separate bug independent of the wrong-
bound in the diagnostic.  It looks like Martin L's claim is that for qemu & grub2
we've got new false positives from the warning.  Thoughts?

Jeff
Martin Sebor April 13, 2020, 7:41 p.m. UTC | #2 
On 4/13/20 1:25 PM, Jeff Law wrote:
> On Mon, 2020-04-13 at 12:39 -0600, Martin Sebor via Gcc-patches wrote:
>> GCC 10 has changed the formatting of zero-length arrays in diagnostics
>> to include their bound, but it also inadvertently added the zero bound
>> to flexible array members which are confusingly represented differently
>> between the C and C++ front ends.
>>
>> The attached patch corrects the problem so both zero-length arrays and
>> flexible array members are formatted consistently by both front ends
>> (i.e., as T[0] and T[]).
>>
>> Tested on x86_64-linux.
> This is fine.
> 
> Though I don't see that it actually addressed the qemu or grub2 issues raised by
> Martin L.  ISTM those really should be a separate bug independent of the wrong-
> bound in the diagnostic.  It looks like Martin L's claim is that for qemu & grub2
> we've got new false positives from the warning.  Thoughts?

I don't see how this could cause false positives, unless something
is set up to filter out the specific text of the warning and this
change makes the filtering fail.

I had asked Martin (CC'd) about it but didn't understand his answer
in c#5 on the bug.  Martin?

M.
Martin Liška April 14, 2020, 7:05 a.m. UTC | #3 
On 4/13/20 9:41 PM, Martin Sebor wrote:
> On 4/13/20 1:25 PM, Jeff Law wrote:
>> On Mon, 2020-04-13 at 12:39 -0600, Martin Sebor via Gcc-patches wrote:
>>> GCC 10 has changed the formatting of zero-length arrays in diagnostics
>>> to include their bound, but it also inadvertently added the zero bound
>>> to flexible array members which are confusingly represented differently
>>> between the C and C++ front ends.
>>>
>>> The attached patch corrects the problem so both zero-length arrays and
>>> flexible array members are formatted consistently by both front ends
>>> (i.e., as T[0] and T[]).
>>>
>>> Tested on x86_64-linux.
>> This is fine.
>>
>> Though I don't see that it actually addressed the qemu or grub2 issues raised by
>> Martin L.  ISTM those really should be a separate bug independent of the wrong-
>> bound in the diagnostic.  It looks like Martin L's claim is that for qemu & grub2
>> we've got new false positives from the warning.  Thoughts?
> 
> I don't see how this could cause false positives, unless something
> is set up to filter out the specific text of the warning and this
> change makes the filtering fail.
> 
> I had asked Martin (CC'd) about it but didn't understand his answer
> in c#5 on the bug.  Martin?

Hi.

Official SUSE guideline recommends not to use -Werror for package builds. However, there
are package like grub or qemu that use -Werror. That's why these packages hit the error
in a way that I noticed. Both these packages decided to fix the warnings.

Martin

> 
> M.
diff mbox series

Patch

PR c/92326 - wrong bound in zero-length array diagnostics

gcc/c-family/ChangeLog:

	PR c/92326
	* c-pretty-print.c (c_pretty_printer::direct_abstract_declarator): Avoid
	printing array bound for flexible array members.

gcc/testsuite/ChangeLog:

	PR c/92326
	* c-c++-common/Warray-bounds-8.c: New test.
	* gcc.dg/Warray-bounds-46.c: Adjust expected format of flexible array
	memebrs in diagnostics.
	* gcc.dg/Warray-bounds-49.c: Same.

diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c
index 0e4303f1a49..eccb63096fd 100644
--- a/gcc/c-family/c-pretty-print.c
+++ b/gcc/c-family/c-pretty-print.c
@@ -593,7 +593,9 @@  c_pretty_printer::direct_abstract_declarator (tree t)
 		expression (fold_build2 (PLUS_EXPR, type, maxval,
 					 build_int_cst (type, 1)));
 	    }
-	  else
+	  else if (TYPE_SIZE (t))
+	    /* Print zero for zero-length arrays but not for flexible
+	       array members whose TYPE_SIZE is null.  */
 	    pp_string (this, "0");
 	}
       pp_c_right_bracket (this);
diff --git a/gcc/testsuite/c-c++-common/Warray-bounds-8.c b/gcc/testsuite/c-c++-common/Warray-bounds-8.c
new file mode 100644
index 00000000000..64202ddd5df
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-bounds-8.c
@@ -0,0 +1,22 @@ 
+/* PR c/92326 - wrong bound in zero-length array diagnostics
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+extern int a0[0];
+extern int ax[];
+
+void warn_global_array (void)
+{
+  a0[0] = 0;        // { dg-warning "array bounds of 'int *\\\[0]'" }
+  ax[-1] = 0;       // { dg-warning "array bounds of 'int *\\\[]'" }
+}
+
+
+struct S0 { int n, a0[0]; } s0;
+struct Sx { int n, ax[]; } sx = { 0 };
+
+void warn_member_array (void)
+{
+  s0.a0[0] = 0;     // { dg-warning "array bounds of 'int *\\\[0]'" }
+  sx.ax[0] = 0;     // { dg-warning "array bounds of 'int *\\\[]'" }
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-46.c b/gcc/testsuite/gcc.dg/Warray-bounds-46.c
index 74e78cbdbe8..9078c6f5998 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-46.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-46.c
@@ -95,13 +95,13 @@  void strcpy_global_array (void)
   /* GMA2 is external but because it's an array its definition in another
      translation unit may not provide an initializer for the flexible array
      member.  Verify that a warning is issued for access to it.  */
-  T (gma2[0].ax, 1);      // { dg-warning "'strcpy' offset \\\[157, 158] from the object at 'gma2' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 157" }
-  T (gma2[0].ax, 7);      // { dg-warning "'strcpy' offset \\\[157, 164] from the object at 'gma2' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 157" }
+  T (gma2[0].ax, 1);      // { dg-warning "'strcpy' offset \\\[157, 158] from the object at 'gma2' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 157" }
+  T (gma2[0].ax, 7);      // { dg-warning "'strcpy' offset \\\[157, 164] from the object at 'gma2' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 157" }
 
-  /* IGMA_ is internal and provides on definition for the flexible array
-     member.  Verify that a warnin is issued for out-of-bounds accesses
+  /* IGMA2_ is internal and provides no definition for the flexible array
+     member.  Verify that a warning is issued for out-of-bounds accesses
      to it.  */
-  T (igma2_[0].ax, 1);    // { dg-warning "'strcpy' offset \\\[157, 158] from the object at 'igma2_' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 157" }
+  T (igma2_[0].ax, 1);    // { dg-warning "'strcpy' offset \\\[157, 158] from the object at 'igma2_' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 157" }
 
   T (igma_3.ax, 0);
   T (igma_3.ax, 1);
@@ -134,7 +134,7 @@  void strcpy_local (void)
   T (lma.a17, 16);
   T (lma.a17, 17);        // { dg-warning "'strcpy' offset 157 from the object at 'lma' is out of the bounds of referenced subobject 'a17' with type 'char\\\[17]' at offset 140" }
 
-  T (lma.ax, 0);          // { dg-warning "'strcpy' offset 157 from the object at 'lma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 157" }
+  T (lma.ax, 0);          // { dg-warning "'strcpy' offset 157 from the object at 'lma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 157" }
 }
 
 
@@ -191,11 +191,11 @@  void strcpy_ref (struct MA17 *pma)
      array.  The warning assumes that PMA doesn't point to the last element
      of the array which could in theory have nonzero elements without
      overlapping other objects.  */
-  T (pma[1].ax, 0);       // { dg-warning "'strcpy' offset 314 from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 314" }
-  T ((pma + 1)->ax, 1);   // { dg-warning "'strcpy' offset \\\[314, 315] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 314" }
-  T ((pma + 1)[1].ax, 2); // { dg-warning "'strcpy' offset \\\[471, 473] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 471" }
-  T ((*(pma + 2)).ax, 2); // { dg-warning "'strcpy' offset \\\[471, 473] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 471" }
-  T (pma[3].ax, 9);       // { dg-warning "'strcpy' offset \\\[628, 637] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[0]' at offset 628" }
+  T (pma[1].ax, 0);       // { dg-warning "'strcpy' offset 314 from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 314" }
+  T ((pma + 1)->ax, 1);   // { dg-warning "'strcpy' offset \\\[314, 315] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 314" }
+  T ((pma + 1)[1].ax, 2); // { dg-warning "'strcpy' offset \\\[471, 473] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 471" }
+  T ((*(pma + 2)).ax, 2); // { dg-warning "'strcpy' offset \\\[471, 473] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 471" }
+  T (pma[3].ax, 9);       // { dg-warning "'strcpy' offset \\\[628, 637] from the object at 'pma' is out of the bounds of referenced subobject 'ax' with type 'char\\\[]' at offset 628" }
 
   T (pma[-1].a1, 0);
   T (pma[-1].a1, 1);      // { dg-warning "'strcpy' offset -152 from the object at 'pma' is out of the bounds of referenced subobject 'a1' with type 'char\\\[1]' at offset -153" }
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-49.c b/gcc/testsuite/gcc.dg/Warray-bounds-49.c
index 7a847acf4d2..f271dd526b8 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-49.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-49.c
@@ -17,7 +17,7 @@  void test_a0 (void)
   // The first three elements fit in the tail padding.
   a0.a2[0] = 0; a0.a2[1] = 1; a0.a2[2] = 2;
 
-  a0.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[0]'" }
+  a0.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -27,7 +27,7 @@  void test_a1 (void)
 {
   a1.a2[0] = 0; a1.a2[1] = 1; a1.a2[2] = 2;
 
-  a1.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[0]'" }
+  a1.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -37,7 +37,7 @@  void test_a2 (void)
 {
   a2.a2[0] = 0; a2.a2[1] = 1; a2.a2[2] = 2;
 
-  a2.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[0]'" }
+  a2.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -47,7 +47,7 @@  void test_a3 (void)
 {
   a3.a2[0] = 0; a3.a2[1] = 1; a3.a2[2] = 2;
 
-  a3.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[0]'" }
+  a3.a2[3] = 3;     // { dg-warning "array subscript 3 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -57,7 +57,7 @@  void test_a4 (void)
 {
   a4.a2[0] = 0; a4.a2[1] = 1; a4.a2[2] = 2; a4.a2[3] = 3;
 
-  a4.a2[4] = 4;     // { dg-warning "array subscript 4 is above array bounds of 'short int\\\[0]'" }
+  a4.a2[4] = 4;     // { dg-warning "array subscript 4 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -67,7 +67,7 @@  void test_a5 (void)
 {
   a5.a2[0] = 0; a5.a2[1] = 1; a5.a2[2] = 2; a5.a2[3] = 3; a5.a2[4] = 4;
 
-  a5.a2[5] = 5;     // { dg-warning "array subscript 5 is above array bounds of 'short int\\\[0]'" }
+  a5.a2[5] = 5;     // { dg-warning "array subscript 5 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -78,7 +78,7 @@  void test_a6 (void)
   a6.a2[0] = 0; a6.a2[1] = 1; a6.a2[2] = 2; a6.a2[3] = 3; a6.a2[4] = 4;
   a6.a2[5] = 5;
 
-  a6.a2[6] = 6;     // { dg-warning "array subscript 6 is above array bounds of 'short int\\\[0]'" }
+  a6.a2[6] = 6;     // { dg-warning "array subscript 6 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -89,7 +89,7 @@  void test_a7 (void)
   a7.a2[0] = 0; a7.a2[1] = 1; a7.a2[2] = 2; a7.a2[3] = 3; a7.a2[4] = 4;
   a7.a2[5] = 5; a7.a2[5] = 5; a7.a2[6] = 6;
 
-  a7.a2[7] = 7;     // { dg-warning "array subscript 7 is above array bounds of 'short int\\\[0]'" }
+  a7.a2[7] = 7;     // { dg-warning "array subscript 7 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -100,7 +100,7 @@  void test_a8 (void)
   a8.a2[0] = 0; a8.a2[1] = 1; a8.a2[2] = 2; a8.a2[3] = 3; a8.a2[4] = 4;
   a8.a2[5] = 5; a8.a2[5] = 5; a8.a2[6] = 6; a8.a2[7] = 7;
 
-  a8.a2[8] = 8;     // { dg-warning "array subscript 8 is above array bounds of 'short int\\\[0]'" }
+  a8.a2[8] = 8;     // { dg-warning "array subscript 8 is above array bounds of 'short int\\\[]'" }
 }
 
 
@@ -111,5 +111,5 @@  void test_a9 (void)
   a8.a2[0] = 8; a8.a2[1] = 7; a8.a2[2] = 6; a8.a2[3] = 5; a8.a2[4] = 4;
   a8.a2[5] = 3; a8.a2[5] = 2; a8.a2[6] = 1; a8.a2[7] = 0;
 
-  a8.a2[9] = 8;     // { dg-warning "array subscript 9 is above array bounds of 'short int\\\[0]'" }
+  a8.a2[9] = 8;     // { dg-warning "array subscript 9 is above array bounds of 'short int\\\[]'" }
 }