[C++ Patch] PR 54501

[C++ Patch] PR 54501

[Paolo Carlini]

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

Hi,

for this kind of code, using the GNU zero-size array extension:

   int a[][0] = {0};

we end up looping forever in the reshape_init_array_1 loop because it 
calls reshape_init_r -> reshape_init_array -> reshape_init_array_1 which 
returns early a CONSTRUCTOR with no error. Having considered various 
solutions, I'm proposing to break the endless loop in 
reshape_init_array, where we have information about the type of the 
zero-size array and we can easily provide an accurate error message.

Tested x86_64-linux.

Thanks,
Paolo.

//////////////////////



[-- Attachment #2: CL_54501 --]
[-- Type: text/plain, Size: 315 bytes --]

/cp
2012-10-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54501
	* decl.c (reshape_init_array): Check for zero-size arrays.

/testsuite
2012-10-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54501
	* g++.dg/init/array30.C: New.
	* g++.dg/init/array0.C: Adjust.
	* g++.dg/parse/pr43765.C: Likewise.

[-- Attachment #3: patch_54501 --]
[-- Type: text/plain, Size: 1743 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 192527)
+++ cp/decl.c	(working copy)
@@ -5068,6 +5068,13 @@ reshape_init_array (tree type, reshape_iter *d, ts
   if (TYPE_DOMAIN (type))
     max_index = array_type_nelts (type);
 
+  if (max_index && integer_all_onesp (max_index))
+    {
+      if (complain & tf_error)
+	error ("initializers provided for zero-size array of type %qT", type);
+      return error_mark_node;
+    }
+
   return reshape_init_array_1 (TREE_TYPE (type), max_index, d, complain);
 }
 
Index: testsuite/g++.dg/init/array0.C
===================================================================
--- testsuite/g++.dg/init/array0.C	(revision 192527)
+++ testsuite/g++.dg/init/array0.C	(working copy)
@@ -8,5 +8,5 @@ void foo()
     unsigned char dir;
     int data[0];
   } yanito;
-  static const yanito horse = { 1,  { 2,  3 }  }; // { dg-error "too many" }
+  static const yanito horse = { 1,  { 2,  3 }  }; // { dg-error "zero-size" }
 }
Index: testsuite/g++.dg/init/array30.C
===================================================================
--- testsuite/g++.dg/init/array30.C	(revision 0)
+++ testsuite/g++.dg/init/array30.C	(working copy)
@@ -0,0 +1,7 @@
+// PR c++/54501
+// { dg-options "" }
+
+int main()
+{
+  int a[][0] = {0};  // { dg-error "zero-size" }
+}
Index: testsuite/g++.dg/parse/pr43765.C
===================================================================
--- testsuite/g++.dg/parse/pr43765.C	(revision 192527)
+++ testsuite/g++.dg/parse/pr43765.C	(working copy)
@@ -11,4 +11,4 @@ SomeType vals[] =
     {
         { values : temp, },
         0
-    };          // { dg-error "invalid" }
+    };          // { dg-error "zero-size" }

Re: [C++ Patch] PR 54501

[Paolo Carlini]

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

... oh well, I just realized that zero-size VECTORs don't make much 
sense and are early rejected, thus I can improve my earlier patch.

Now I'm happier: essentially I'm only *moving* code around ;)

Thanks,
Paolo.

//////////////////////

[-- Attachment #2: CL_54501_2 --]
[-- Type: text/plain, Size: 376 bytes --]

/cp
2012-10-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54501
	* decl.c (reshape_init_array): Check for zero-size arrays.
	(reshape_init_array_1): Don't handle zero-size arrays here.

/testsuite
2012-10-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54501
	* g++.dg/init/array30.C: New.
	* g++.dg/init/array0.C: Adjust.
	* g++.dg/parse/pr43765.C: Likewise.

[-- Attachment #3: patch_54501_2 --]
[-- Type: text/plain, Size: 2161 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 192527)
+++ cp/decl.c	(working copy)
@@ -5022,10 +5022,6 @@ reshape_init_array_1 (tree elt_type, tree max_inde
 
   if (sized_array_p)
     {
-      /* Minus 1 is used for zero sized arrays.  */
-      if (integer_all_onesp (max_index))
-	return new_init;
-
       if (host_integerp (max_index, 1))
 	max_index_cst = tree_low_cst (max_index, 1);
       /* sizetype is sign extended, not zero extended.  */
@@ -5068,6 +5064,14 @@ reshape_init_array (tree type, reshape_iter *d, ts
   if (TYPE_DOMAIN (type))
     max_index = array_type_nelts (type);
 
+  /* Minus 1 is used for zero sized arrays.  */
+  if (max_index && integer_all_onesp (max_index))
+    {
+      if (complain & tf_error)
+	error ("initializers provided for zero-size array of type %qT", type);
+      return error_mark_node;
+    }
+
   return reshape_init_array_1 (TREE_TYPE (type), max_index, d, complain);
 }
 
Index: testsuite/g++.dg/parse/pr43765.C
===================================================================
--- testsuite/g++.dg/parse/pr43765.C	(revision 192527)
+++ testsuite/g++.dg/parse/pr43765.C	(working copy)
@@ -11,4 +11,4 @@ SomeType vals[] =
     {
         { values : temp, },
         0
-    };          // { dg-error "invalid" }
+    };          // { dg-error "zero-size" }
Index: testsuite/g++.dg/init/array30.C
===================================================================
--- testsuite/g++.dg/init/array30.C	(revision 0)
+++ testsuite/g++.dg/init/array30.C	(working copy)
@@ -0,0 +1,7 @@
+// PR c++/54501
+// { dg-options "" }
+
+int main()
+{
+  int a[][0] = {0};  // { dg-error "zero-size" }
+}
Index: testsuite/g++.dg/init/array0.C
===================================================================
--- testsuite/g++.dg/init/array0.C	(revision 192527)
+++ testsuite/g++.dg/init/array0.C	(working copy)
@@ -8,5 +8,5 @@ void foo()
     unsigned char dir;
     int data[0];
   } yanito;
-  static const yanito horse = { 1,  { 2,  3 }  }; // { dg-error "too many" }
+  static const yanito horse = { 1,  { 2,  3 }  }; // { dg-error "zero-size" }
 }

Re: [C++ Patch] PR 54501

[Jason Merrill]

Hmm, I thought I fixed a very similar bug recently.

I'm concerned that this change will cause problems with brace-elision 
situations.  But then again, can we have a zero-length array followed by 
anything else?

Jason

Re: [C++ Patch] PR 54501

[Paolo Carlini]

Hi,

On 10/18/2012 03:17 AM, Jason Merrill wrote:
> Hmm, I thought I fixed a very similar bug recently.
>
> I'm concerned that this change will cause problems with brace-elision 
> situations.  But then again, can we have a zero-length array followed 
> by anything else?
If I understand correctly your hesitations, I don't think there are 
exceptions to the general rule that if the size of the array is zero 
there can be no initializers. This morning I investigated in some detail 
this special case (from a testcase I recently added, pr43765.C):

struct SomeType
{
   const char *values[];
};

it's in fact related because we parse it as:

struct SomeType
{
   const char *values[0];
};

(per grokdeclarator around line 10246). Given the way we parse it (not 
completely uncontroversial, IMHO, but that would be a separate issue), 
we do the right thing, we accept:

SomeType s = { };

and we reject:

SomeType s = { 1 };

Only the wording of the error changes (from "too many initializers" to 
"initializers provided") (*)

In any case, I can't imagine a different, safer, way to handle the issue 
we are facing. Do you have anything specific to suggest?

Thanks,
Paolo.

(*) Moreover we likewise accept:

SomeType s = { {  } };

and likewise reject:

SomeType s = { { { } } };

Re: [C++ Patch] PR 54501

[Jason Merrill]

On 10/18/2012 01:15 AM, Paolo Carlini wrote:
> If I understand correctly your hesitations, I don't think there are
> exceptions to the general rule that if the size of the array is zero
> there can be no initializers.

I'm thinking of a testcase like this, which is currently accepted:

struct A
{
   int i[0];
   int j;
};

struct A a = { 1 };

Here, since i has no elements, we should skip over it and apply the one 
initializer to j.

Jason

Re: [C++ Patch] PR 54501

[Paolo Carlini]

On 10/18/2012 06:02 PM, Jason Merrill wrote:
> On 10/18/2012 01:15 AM, Paolo Carlini wrote:
>> If I understand correctly your hesitations, I don't think there are
>> exceptions to the general rule that if the size of the array is zero
>> there can be no initializers.
>
> I'm thinking of a testcase like this, which is currently accepted:
>
> struct A
> {
>   int i[0];
>   int j;
> };
>
> struct A a = { 1 };
>
> Here, since i has no elements, we should skip over it and apply the 
> one initializer to j.
Ah, Ok, I was missing the "skipping" thing ;)

Let me see how we can handle this... it seems tricky.

Paolo.

Re: [C++ Patch] PR 54501

[Paolo Carlini]

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

Hi again,

On 10/18/2012 06:02 PM, Jason Merrill wrote:
> On 10/18/2012 01:15 AM, Paolo Carlini wrote:
>> If I understand correctly your hesitations, I don't think there are
>> exceptions to the general rule that if the size of the array is zero
>> there can be no initializers.
>
> I'm thinking of a testcase like this, which is currently accepted:
>
> struct A
> {
>   int i[0];
>   int j;
> };
>
> struct A a = { 1 };
>
> Here, since i has no elements, we should skip over it and apply the 
> one initializer to j.
Thus, I looked into what you did lately for c++/54441, and the below is 
what I regtested so far: I'm directly recognizing the situations leading 
to infinite loops. Note, I don't exclude that formally the whole 
iteration could be streamlined a bit, but like this it seems still quite 
understandable.

Tested x86_64-linux as usual..

Thanks,
Paolo.

///////////////////////////

[-- Attachment #2: CL_54501_3 --]
[-- Type: text/plain, Size: 277 bytes --]

/cp
2012-10-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54501
	* decl.c (reshape_init_array_1): Avoid infinite loops.

/testsuite
2012-10-18  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54501
	* g++.dg/init/array30.C: New.
	* g++.dg/init/array31.C: Likewise.

[-- Attachment #3: patch_54501_3 --]
[-- Type: text/plain, Size: 1451 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 192569)
+++ cp/decl.c	(working copy)
@@ -5040,6 +5040,7 @@ reshape_init_array_1 (tree elt_type, tree max_inde
        ++index)
     {
       tree elt_init;
+      constructor_elt *old_cur = d->cur;
 
       check_array_designated_initializer (d->cur, index);
       elt_init = reshape_init_r (elt_type, d, /*first_initializer_p=*/false,
@@ -5050,6 +5051,10 @@ reshape_init_array_1 (tree elt_type, tree max_inde
 			      size_int (index), elt_init);
       if (!TREE_CONSTANT (elt_init))
 	TREE_CONSTANT (new_init) = false;
+
+      /* This can happen with an invalid initializer (c++/54501).  */
+      if (d->cur == old_cur && !sized_array_p)
+	break;
     }
 
   return new_init;
Index: testsuite/g++.dg/init/array30.C
===================================================================
--- testsuite/g++.dg/init/array30.C	(revision 0)
+++ testsuite/g++.dg/init/array30.C	(working copy)
@@ -0,0 +1,7 @@
+// PR c++/54501
+// { dg-options "" }
+
+int main()
+{
+  int a[][0] = {0};  // { dg-error "too many" }
+}
Index: testsuite/g++.dg/init/array31.C
===================================================================
--- testsuite/g++.dg/init/array31.C	(revision 0)
+++ testsuite/g++.dg/init/array31.C	(working copy)
@@ -0,0 +1,10 @@
+// PR c++/54501
+// { dg-options "" }
+
+struct A
+{
+  int i[0];
+  int j;
+};
+
+struct A a = { 1 };

Re: [C++ Patch] PR 54501

[Jason Merrill]

OK.

Jason