diff mbox series

c++: Fix crash in gimplifier with paren init of aggregates [PR94155]

Message ID 20200330202823.428754-1-polacek@redhat.com
State New
Headers show
Series c++: Fix crash in gimplifier with paren init of aggregates [PR94155] | expand

Commit Message

Li, Pan2 via Gcc-patches March 30, 2020, 8:28 p.m. UTC 
Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
expect null indexes for a constructor:

      /* ??? Here's to hoping the front end fills in all of the indices,
         so we don't have to figure out what's missing ourselves.  */
      gcc_assert (purpose);

The indexes weren't filled because we never called reshape_init: for
a constructor that represents parenthesized initialization of an
aggregate we don't allow brace elision or designated initializers.  So
fill in the indexes manually, here we have an array, and we can simply
assign indexes starting from 0.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

	PR c++/94155 - crash in gimplifier with paren init of aggregates.
	* decl.c (check_initializer): Fill in constructor indexes.

	* g++.dg/cpp2a/paren-init22.C: New test.
---
 gcc/cp/decl.c                             |  6 ++++++
 gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
 2 files changed, 21 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C


base-commit: 48e331d63827a0500670d685c0fe7d609e0a807a

Comments

Li, Pan2 via Gcc-patches April 3, 2020, 7:01 p.m. UTC | #1 
On 3/30/20 4:28 PM, Marek Polacek wrote:
> Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> expect null indexes for a constructor:
> 
>        /* ??? Here's to hoping the front end fills in all of the indices,
>           so we don't have to figure out what's missing ourselves.  */
>        gcc_assert (purpose);
> 
> The indexes weren't filled because we never called reshape_init: for
> a constructor that represents parenthesized initialization of an
> aggregate we don't allow brace elision or designated initializers.  So
> fill in the indexes manually, here we have an array, and we can simply
> assign indexes starting from 0.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Shouldn't digest_init fill in the indexes?  In 
process_init_constructor_array I see

       if (!ce->index)
         ce->index = size_int (i);

> 	PR c++/94155 - crash in gimplifier with paren init of aggregates.
> 	* decl.c (check_initializer): Fill in constructor indexes.
> 
> 	* g++.dg/cpp2a/paren-init22.C: New test.
> ---
>   gcc/cp/decl.c                             |  6 ++++++
>   gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
>   2 files changed, 21 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> 
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 69a238997b4..80dd2d8b931 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6754,6 +6754,12 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
>   	      init = build_constructor_from_list (init_list_type_node, init);
>   	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
>   	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
> +	      /* The gimplifier expects that the front end fills in all of the
> +		 indices.  Normally, reshape_init_array fills these in, but we
> +		 don't call reshape_init because that does nothing when it gets
> +		 CONSTRUCTOR_IS_PAREN_INIT.  */
> +	      for (unsigned int i = 0; i < CONSTRUCTOR_NELTS (init); i++)
> +		CONSTRUCTOR_ELT (init, i)->index = size_int (i);
>   	    }
>   	}
>         else if (TREE_CODE (init) == TREE_LIST
> diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init22.C b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> new file mode 100644
> index 00000000000..1b2959e7731
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> @@ -0,0 +1,15 @@
> +// PR c++/94155 - crash in gimplifier with paren init of aggregates.
> +// { dg-do compile { target c++2a } }
> +
> +struct S { int i, j; };
> +
> +struct A {
> +  S s;
> +  constexpr A(S e) : s(e) {}
> +};
> +
> +void
> +f()
> +{
> +  A g[1]({{1, 1}});
> +}
> 
> base-commit: 48e331d63827a0500670d685c0fe7d609e0a807a
>
Li, Pan2 via Gcc-patches April 4, 2020, 1:08 a.m. UTC | #2 
On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches wrote:
> On 3/30/20 4:28 PM, Marek Polacek wrote:
> > Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> > expect null indexes for a constructor:
> > 
> >        /* ??? Here's to hoping the front end fills in all of the indices,
> >           so we don't have to figure out what's missing ourselves.  */
> >        gcc_assert (purpose);
> > 
> > The indexes weren't filled because we never called reshape_init: for
> > a constructor that represents parenthesized initialization of an
> > aggregate we don't allow brace elision or designated initializers.  So
> > fill in the indexes manually, here we have an array, and we can simply
> > assign indexes starting from 0.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> Shouldn't digest_init fill in the indexes?  In
> process_init_constructor_array I see
> 
>       if (!ce->index)
>         ce->index = size_int (i);

Yes, that works too.  Thus:

Bootstrapped/regtested on x86_64-linux, ok for trunk?

-- >8 --
Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
expect null indexes for a constructor:

      /* ??? Here's to hoping the front end fills in all of the indices,
         so we don't have to figure out what's missing ourselves.  */
      gcc_assert (purpose);

The indexes weren't filled because we never called reshape_init: for
a constructor that represents parenthesized initialization of an
aggregate we don't allow brace elision or designated initializers.  So
call digest_init to fill in the indexes.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

	PR c++/94155 - crash in gimplifier with paren init of aggregates.
	* decl.c (check_initializer): Call digest_init.

	* g++.dg/cpp2a/paren-init22.C: New test.
---
 gcc/cp/decl.c                             |  5 +++++
 gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 69a238997b4..63e7bda09f5 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
 	      init = build_constructor_from_list (init_list_type_node, init);
 	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
 	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
+	      /* The gimplifier expects that the front end fills in all of the
+		 indices.  Normally, reshape_init_array fills these in, but we
+		 don't call reshape_init because that does nothing when it gets
+		 CONSTRUCTOR_IS_PAREN_INIT.  */
+	      init = digest_init (type, init, tf_warning_or_error);
 	    }
 	}
       else if (TREE_CODE (init) == TREE_LIST
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init22.C b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
new file mode 100644
index 00000000000..1b2959e7731
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
@@ -0,0 +1,15 @@
+// PR c++/94155 - crash in gimplifier with paren init of aggregates.
+// { dg-do compile { target c++2a } }
+
+struct S { int i, j; };
+
+struct A {
+  S s;
+  constexpr A(S e) : s(e) {}
+};
+
+void
+f()
+{
+  A g[1]({{1, 1}});
+}

base-commit: 0c809f727cd2a6c70c307d9dd53d26dc84bf292a
Li, Pan2 via Gcc-patches April 4, 2020, 2:39 a.m. UTC | #3 
On 4/3/20 9:08 PM, Marek Polacek wrote:
> On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches wrote:
>> On 3/30/20 4:28 PM, Marek Polacek wrote:
>>> Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
>>> expect null indexes for a constructor:
>>>
>>>         /* ??? Here's to hoping the front end fills in all of the indices,
>>>            so we don't have to figure out what's missing ourselves.  */
>>>         gcc_assert (purpose);
>>>
>>> The indexes weren't filled because we never called reshape_init: for
>>> a constructor that represents parenthesized initialization of an
>>> aggregate we don't allow brace elision or designated initializers.  So
>>> fill in the indexes manually, here we have an array, and we can simply
>>> assign indexes starting from 0.
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>
>> Shouldn't digest_init fill in the indexes?  In
>> process_init_constructor_array I see
>>
>>        if (!ce->index)
>>          ce->index = size_int (i);
> 
> Yes, that works too.  Thus:
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> -- >8 --
> Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> expect null indexes for a constructor:
> 
>        /* ??? Here's to hoping the front end fills in all of the indices,
>           so we don't have to figure out what's missing ourselves.  */
>        gcc_assert (purpose);
> 
> The indexes weren't filled because we never called reshape_init: for
> a constructor that represents parenthesized initialization of an
> aggregate we don't allow brace elision or designated initializers.  So
> call digest_init to fill in the indexes.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 	PR c++/94155 - crash in gimplifier with paren init of aggregates.
> 	* decl.c (check_initializer): Call digest_init.
> 
> 	* g++.dg/cpp2a/paren-init22.C: New test.
> ---
>   gcc/cp/decl.c                             |  5 +++++
>   gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
>   2 files changed, 20 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> 
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 69a238997b4..63e7bda09f5 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
>   	      init = build_constructor_from_list (init_list_type_node, init);
>   	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
>   	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
> +	      /* The gimplifier expects that the front end fills in all of the
> +		 indices.  Normally, reshape_init_array fills these in, but we
> +		 don't call reshape_init because that does nothing when it gets
> +		 CONSTRUCTOR_IS_PAREN_INIT.  */
> +	      init = digest_init (type, init, tf_warning_or_error);

But why weren't we already calling digest_init in store_init_value?  Was 
the CONSTRUCTOR making it all the way to gimplification still having 
init_list_type_node?

>   	    }
>   	}
>         else if (TREE_CODE (init) == TREE_LIST
> diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init22.C b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> new file mode 100644
> index 00000000000..1b2959e7731
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> @@ -0,0 +1,15 @@
> +// PR c++/94155 - crash in gimplifier with paren init of aggregates.
> +// { dg-do compile { target c++2a } }
> +
> +struct S { int i, j; };
> +
> +struct A {
> +  S s;
> +  constexpr A(S e) : s(e) {}
> +};
> +
> +void
> +f()
> +{
> +  A g[1]({{1, 1}});
> +}
> 
> base-commit: 0c809f727cd2a6c70c307d9dd53d26dc84bf292a
>
Li, Pan2 via Gcc-patches April 4, 2020, 5:56 p.m. UTC | #4 
On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via Gcc-patches wrote:
> On 4/3/20 9:08 PM, Marek Polacek wrote:
> > On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches wrote:
> > > On 3/30/20 4:28 PM, Marek Polacek wrote:
> > > > Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> > > > expect null indexes for a constructor:
> > > > 
> > > >         /* ??? Here's to hoping the front end fills in all of the indices,
> > > >            so we don't have to figure out what's missing ourselves.  */
> > > >         gcc_assert (purpose);
> > > > 
> > > > The indexes weren't filled because we never called reshape_init: for
> > > > a constructor that represents parenthesized initialization of an
> > > > aggregate we don't allow brace elision or designated initializers.  So
> > > > fill in the indexes manually, here we have an array, and we can simply
> > > > assign indexes starting from 0.
> > > > 
> > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > 
> > > Shouldn't digest_init fill in the indexes?  In
> > > process_init_constructor_array I see
> > > 
> > >        if (!ce->index)
> > >          ce->index = size_int (i);
> > 
> > Yes, that works too.  Thus:
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > -- >8 --
> > Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> > expect null indexes for a constructor:
> > 
> >        /* ??? Here's to hoping the front end fills in all of the indices,
> >           so we don't have to figure out what's missing ourselves.  */
> >        gcc_assert (purpose);
> > 
> > The indexes weren't filled because we never called reshape_init: for
> > a constructor that represents parenthesized initialization of an
> > aggregate we don't allow brace elision or designated initializers.  So
> > call digest_init to fill in the indexes.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > 	PR c++/94155 - crash in gimplifier with paren init of aggregates.
> > 	* decl.c (check_initializer): Call digest_init.
> > 
> > 	* g++.dg/cpp2a/paren-init22.C: New test.
> > ---
> >   gcc/cp/decl.c                             |  5 +++++
> >   gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
> >   2 files changed, 20 insertions(+)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> > 
> > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > index 69a238997b4..63e7bda09f5 100644
> > --- a/gcc/cp/decl.c
> > +++ b/gcc/cp/decl.c
> > @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
> >   	      init = build_constructor_from_list (init_list_type_node, init);
> >   	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
> >   	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
> > +	      /* The gimplifier expects that the front end fills in all of the
> > +		 indices.  Normally, reshape_init_array fills these in, but we
> > +		 don't call reshape_init because that does nothing when it gets
> > +		 CONSTRUCTOR_IS_PAREN_INIT.  */
> > +	      init = digest_init (type, init, tf_warning_or_error);
> 
> But why weren't we already calling digest_init in store_init_value?  Was the
> CONSTRUCTOR making it all the way to gimplification still having
> init_list_type_node?

It's because we set LOOKUP_ALREADY_DIGESTED a few lines below:
 6813               /* Don't call digest_init; it's unnecessary and will complain
 6814                  about aggregate initialization of non-aggregate classes.  */
 6815               flags |= LOOKUP_ALREADY_DIGESTED;
and so store_init_value doesn't digest.  Given the comment I'd be nervous about
not setting that flag here.

Marek
Li, Pan2 via Gcc-patches April 6, 2020, 2:47 p.m. UTC | #5 
On 4/4/20 1:56 PM, Marek Polacek wrote:
> On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via Gcc-patches wrote:
>> On 4/3/20 9:08 PM, Marek Polacek wrote:
>>> On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches wrote:
>>>> On 3/30/20 4:28 PM, Marek Polacek wrote:
>>>>> Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
>>>>> expect null indexes for a constructor:
>>>>>
>>>>>          /* ??? Here's to hoping the front end fills in all of the indices,
>>>>>             so we don't have to figure out what's missing ourselves.  */
>>>>>          gcc_assert (purpose);
>>>>>
>>>>> The indexes weren't filled because we never called reshape_init: for
>>>>> a constructor that represents parenthesized initialization of an
>>>>> aggregate we don't allow brace elision or designated initializers.  So
>>>>> fill in the indexes manually, here we have an array, and we can simply
>>>>> assign indexes starting from 0.
>>>>>
>>>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>>
>>>> Shouldn't digest_init fill in the indexes?  In
>>>> process_init_constructor_array I see
>>>>
>>>>         if (!ce->index)
>>>>           ce->index = size_int (i);
>>>
>>> Yes, that works too.  Thus:
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>
>>> -- >8 --
>>> Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
>>> expect null indexes for a constructor:
>>>
>>>         /* ??? Here's to hoping the front end fills in all of the indices,
>>>            so we don't have to figure out what's missing ourselves.  */
>>>         gcc_assert (purpose);
>>>
>>> The indexes weren't filled because we never called reshape_init: for
>>> a constructor that represents parenthesized initialization of an
>>> aggregate we don't allow brace elision or designated initializers.  So
>>> call digest_init to fill in the indexes.
>>>
>>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>>
>>> 	PR c++/94155 - crash in gimplifier with paren init of aggregates.
>>> 	* decl.c (check_initializer): Call digest_init.
>>>
>>> 	* g++.dg/cpp2a/paren-init22.C: New test.
>>> ---
>>>    gcc/cp/decl.c                             |  5 +++++
>>>    gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
>>>    2 files changed, 20 insertions(+)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
>>>
>>> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
>>> index 69a238997b4..63e7bda09f5 100644
>>> --- a/gcc/cp/decl.c
>>> +++ b/gcc/cp/decl.c
>>> @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
>>>    	      init = build_constructor_from_list (init_list_type_node, init);
>>>    	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
>>>    	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
>>> +	      /* The gimplifier expects that the front end fills in all of the
>>> +		 indices.  Normally, reshape_init_array fills these in, but we
>>> +		 don't call reshape_init because that does nothing when it gets
>>> +		 CONSTRUCTOR_IS_PAREN_INIT.  */
>>> +	      init = digest_init (type, init, tf_warning_or_error);
>>
>> But why weren't we already calling digest_init in store_init_value?  Was the
>> CONSTRUCTOR making it all the way to gimplification still having
>> init_list_type_node?
> 
> It's because we set LOOKUP_ALREADY_DIGESTED a few lines below:
>   6813               /* Don't call digest_init; it's unnecessary and will complain
>   6814                  about aggregate initialization of non-aggregate classes.  */
>   6815               flags |= LOOKUP_ALREADY_DIGESTED;
> and so store_init_value doesn't digest.  Given the comment I'd be nervous about
> not setting that flag here.

OK, then why isn't it called by build_aggr_init?  How is the CONSTRUCTOR 
getting a type without this being fixed up?

...

Ah, because build_vec_init builds up a new CONSTRUCTOR and gives it a 
type without setting the indexes like process_init_constructor_array does:

Jason
Li, Pan2 via Gcc-patches April 6, 2020, 3:57 p.m. UTC | #6 
On Mon, Apr 06, 2020 at 10:47:49AM -0400, Jason Merrill via Gcc-patches wrote:
> On 4/4/20 1:56 PM, Marek Polacek wrote:
> > On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via Gcc-patches wrote:
> > > On 4/3/20 9:08 PM, Marek Polacek wrote:
> > > > On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via Gcc-patches wrote:
> > > > > On 3/30/20 4:28 PM, Marek Polacek wrote:
> > > > > > Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> > > > > > expect null indexes for a constructor:
> > > > > > 
> > > > > >          /* ??? Here's to hoping the front end fills in all of the indices,
> > > > > >             so we don't have to figure out what's missing ourselves.  */
> > > > > >          gcc_assert (purpose);
> > > > > > 
> > > > > > The indexes weren't filled because we never called reshape_init: for
> > > > > > a constructor that represents parenthesized initialization of an
> > > > > > aggregate we don't allow brace elision or designated initializers.  So
> > > > > > fill in the indexes manually, here we have an array, and we can simply
> > > > > > assign indexes starting from 0.
> > > > > > 
> > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > > 
> > > > > Shouldn't digest_init fill in the indexes?  In
> > > > > process_init_constructor_array I see
> > > > > 
> > > > >         if (!ce->index)
> > > > >           ce->index = size_int (i);
> > > > 
> > > > Yes, that works too.  Thus:
> > > > 
> > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > 
> > > > -- >8 --
> > > > Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> > > > expect null indexes for a constructor:
> > > > 
> > > >         /* ??? Here's to hoping the front end fills in all of the indices,
> > > >            so we don't have to figure out what's missing ourselves.  */
> > > >         gcc_assert (purpose);
> > > > 
> > > > The indexes weren't filled because we never called reshape_init: for
> > > > a constructor that represents parenthesized initialization of an
> > > > aggregate we don't allow brace elision or designated initializers.  So
> > > > call digest_init to fill in the indexes.
> > > > 
> > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > 
> > > > 	PR c++/94155 - crash in gimplifier with paren init of aggregates.
> > > > 	* decl.c (check_initializer): Call digest_init.
> > > > 
> > > > 	* g++.dg/cpp2a/paren-init22.C: New test.
> > > > ---
> > > >    gcc/cp/decl.c                             |  5 +++++
> > > >    gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
> > > >    2 files changed, 20 insertions(+)
> > > >    create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> > > > 
> > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > > > index 69a238997b4..63e7bda09f5 100644
> > > > --- a/gcc/cp/decl.c
> > > > +++ b/gcc/cp/decl.c
> > > > @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
> > > >    	      init = build_constructor_from_list (init_list_type_node, init);
> > > >    	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
> > > >    	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
> > > > +	      /* The gimplifier expects that the front end fills in all of the
> > > > +		 indices.  Normally, reshape_init_array fills these in, but we
> > > > +		 don't call reshape_init because that does nothing when it gets
> > > > +		 CONSTRUCTOR_IS_PAREN_INIT.  */
> > > > +	      init = digest_init (type, init, tf_warning_or_error);
> > > 
> > > But why weren't we already calling digest_init in store_init_value?  Was the
> > > CONSTRUCTOR making it all the way to gimplification still having
> > > init_list_type_node?
> > 
> > It's because we set LOOKUP_ALREADY_DIGESTED a few lines below:
> >   6813               /* Don't call digest_init; it's unnecessary and will complain
> >   6814                  about aggregate initialization of non-aggregate classes.  */
> >   6815               flags |= LOOKUP_ALREADY_DIGESTED;
> > and so store_init_value doesn't digest.  Given the comment I'd be nervous about
> > not setting that flag here.
> 
> OK, then why isn't it called by build_aggr_init?  How is the CONSTRUCTOR
> getting a type without this being fixed up?
> 
> ...
> 
> Ah, because build_vec_init builds up a new CONSTRUCTOR and gives it a type
> without setting the indexes like process_init_constructor_array does:
> 
> Jason

> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index 27623cf4db1..ea95a3bc910 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree init,
>  	    errors = true;
>  	  if (try_const)
>  	    {
> +	      if (!field)
> +		field = size_int (idx);
>  	      tree e = maybe_constant_init (one_init);
>  	      if (reduced_constant_expression_p (e))
>  		{

That works, thanks for figuring that out.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
expect null indexes for a constructor:

      /* ??? Here's to hoping the front end fills in all of the indices,
         so we don't have to figure out what's missing ourselves.  */
      gcc_assert (purpose);

The indexes weren't filled because we never called reshape_init: for
a constructor that represents parenthesized initialization of an
aggregate we don't allow brace elision or designated initializers.

	PR c++/94155 - crash in gimplifier with paren init of aggregates.
	* init.c (build_vec_init): Fill in indexes.

	* g++.dg/cpp2a/paren-init22.C: New test.
---
 gcc/cp/init.c                             |  2 ++
 gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 27623cf4db1..ea95a3bc910 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree init,
 	    errors = true;
 	  if (try_const)
 	    {
+	      if (!field)
+		field = size_int (idx);
 	      tree e = maybe_constant_init (one_init);
 	      if (reduced_constant_expression_p (e))
 		{
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init22.C b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
new file mode 100644
index 00000000000..1b2959e7731
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
@@ -0,0 +1,15 @@
+// PR c++/94155 - crash in gimplifier with paren init of aggregates.
+// { dg-do compile { target c++2a } }
+
+struct S { int i, j; };
+
+struct A {
+  S s;
+  constexpr A(S e) : s(e) {}
+};
+
+void
+f()
+{
+  A g[1]({{1, 1}});
+}

base-commit: c72a1b6f8b26de37d1a922a8af143af009747498
Li, Pan2 via Gcc-patches April 6, 2020, 4:35 p.m. UTC | #7 
Ok.

On Mon, Apr 6, 2020, 11:57 AM Marek Polacek <polacek@redhat.com> wrote:

> On Mon, Apr 06, 2020 at 10:47:49AM -0400, Jason Merrill via Gcc-patches
> wrote:
> > On 4/4/20 1:56 PM, Marek Polacek wrote:
> > > On Fri, Apr 03, 2020 at 10:39:49PM -0400, Jason Merrill via
> Gcc-patches wrote:
> > > > On 4/3/20 9:08 PM, Marek Polacek wrote:
> > > > > On Fri, Apr 03, 2020 at 03:01:37PM -0400, Jason Merrill via
> Gcc-patches wrote:
> > > > > > On 3/30/20 4:28 PM, Marek Polacek wrote:
> > > > > > > Here we crash in the gimplifier because
> gimplify_init_ctor_eval doesn't
> > > > > > > expect null indexes for a constructor:
> > > > > > >
> > > > > > >          /* ??? Here's to hoping the front end fills in all of
> the indices,
> > > > > > >             so we don't have to figure out what's missing
> ourselves.  */
> > > > > > >          gcc_assert (purpose);
> > > > > > >
> > > > > > > The indexes weren't filled because we never called
> reshape_init: for
> > > > > > > a constructor that represents parenthesized initialization of
> an
> > > > > > > aggregate we don't allow brace elision or designated
> initializers.  So
> > > > > > > fill in the indexes manually, here we have an array, and we
> can simply
> > > > > > > assign indexes starting from 0.
> > > > > > >
> > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > > >
> > > > > > Shouldn't digest_init fill in the indexes?  In
> > > > > > process_init_constructor_array I see
> > > > > >
> > > > > >         if (!ce->index)
> > > > > >           ce->index = size_int (i);
> > > > >
> > > > > Yes, that works too.  Thus:
> > > > >
> > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > >
> > > > > -- >8 --
> > > > > Here we crash in the gimplifier because gimplify_init_ctor_eval
> doesn't
> > > > > expect null indexes for a constructor:
> > > > >
> > > > >         /* ??? Here's to hoping the front end fills in all of the
> indices,
> > > > >            so we don't have to figure out what's missing
> ourselves.  */
> > > > >         gcc_assert (purpose);
> > > > >
> > > > > The indexes weren't filled because we never called reshape_init:
> for
> > > > > a constructor that represents parenthesized initialization of an
> > > > > aggregate we don't allow brace elision or designated
> initializers.  So
> > > > > call digest_init to fill in the indexes.
> > > > >
> > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > > > >
> > > > >         PR c++/94155 - crash in gimplifier with paren init of
> aggregates.
> > > > >         * decl.c (check_initializer): Call digest_init.
> > > > >
> > > > >         * g++.dg/cpp2a/paren-init22.C: New test.
> > > > > ---
> > > > >    gcc/cp/decl.c                             |  5 +++++
> > > > >    gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
> > > > >    2 files changed, 20 insertions(+)
> > > > >    create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> > > > >
> > > > > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > > > > index 69a238997b4..63e7bda09f5 100644
> > > > > --- a/gcc/cp/decl.c
> > > > > +++ b/gcc/cp/decl.c
> > > > > @@ -6754,6 +6754,11 @@ check_initializer (tree decl, tree init,
> int flags, vec<tree, va_gc> **cleanups)
> > > > >               init = build_constructor_from_list
> (init_list_type_node, init);
> > > > >               CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
> > > > >               CONSTRUCTOR_IS_PAREN_INIT (init) = true;
> > > > > +             /* The gimplifier expects that the front end fills
> in all of the
> > > > > +                indices.  Normally, reshape_init_array fills
> these in, but we
> > > > > +                don't call reshape_init because that does nothing
> when it gets
> > > > > +                CONSTRUCTOR_IS_PAREN_INIT.  */
> > > > > +             init = digest_init (type, init, tf_warning_or_error);
> > > >
> > > > But why weren't we already calling digest_init in store_init_value?
> Was the
> > > > CONSTRUCTOR making it all the way to gimplification still having
> > > > init_list_type_node?
> > >
> > > It's because we set LOOKUP_ALREADY_DIGESTED a few lines below:
> > >   6813               /* Don't call digest_init; it's unnecessary and
> will complain
> > >   6814                  about aggregate initialization of
> non-aggregate classes.  */
> > >   6815               flags |= LOOKUP_ALREADY_DIGESTED;
> > > and so store_init_value doesn't digest.  Given the comment I'd be
> nervous about
> > > not setting that flag here.
> >
> > OK, then why isn't it called by build_aggr_init?  How is the CONSTRUCTOR
> > getting a type without this being fixed up?
> >
> > ...
> >
> > Ah, because build_vec_init builds up a new CONSTRUCTOR and gives it a
> type
> > without setting the indexes like process_init_constructor_array does:
> >
> > Jason
>
> > diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> > index 27623cf4db1..ea95a3bc910 100644
> > --- a/gcc/cp/init.c
> > +++ b/gcc/cp/init.c
> > @@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree
> init,
> >           errors = true;
> >         if (try_const)
> >           {
> > +           if (!field)
> > +             field = size_int (idx);
> >             tree e = maybe_constant_init (one_init);
> >             if (reduced_constant_expression_p (e))
> >               {
>
> That works, thanks for figuring that out.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >8 --
> Here we crash in the gimplifier because gimplify_init_ctor_eval doesn't
> expect null indexes for a constructor:
>
>       /* ??? Here's to hoping the front end fills in all of the indices,
>          so we don't have to figure out what's missing ourselves.  */
>       gcc_assert (purpose);
>
> The indexes weren't filled because we never called reshape_init: for
> a constructor that represents parenthesized initialization of an
> aggregate we don't allow brace elision or designated initializers.
>
>         PR c++/94155 - crash in gimplifier with paren init of aggregates.
>         * init.c (build_vec_init): Fill in indexes.
>
>         * g++.dg/cpp2a/paren-init22.C: New test.
> ---
>  gcc/cp/init.c                             |  2 ++
>  gcc/testsuite/g++.dg/cpp2a/paren-init22.C | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init22.C
>
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index 27623cf4db1..ea95a3bc910 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -4438,6 +4438,8 @@ build_vec_init (tree base, tree maxindex, tree init,
>             errors = true;
>           if (try_const)
>             {
> +             if (!field)
> +               field = size_int (idx);
>               tree e = maybe_constant_init (one_init);
>               if (reduced_constant_expression_p (e))
>                 {
> diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> new file mode 100644
> index 00000000000..1b2959e7731
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
> @@ -0,0 +1,15 @@
> +// PR c++/94155 - crash in gimplifier with paren init of aggregates.
> +// { dg-do compile { target c++2a } }
> +
> +struct S { int i, j; };
> +
> +struct A {
> +  S s;
> +  constexpr A(S e) : s(e) {}
> +};
> +
> +void
> +f()
> +{
> +  A g[1]({{1, 1}});
> +}
>
> base-commit: c72a1b6f8b26de37d1a922a8af143af009747498
> --
> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
>
>
diff mbox series

Patch

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 69a238997b4..80dd2d8b931 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6754,6 +6754,12 @@  check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
 	      init = build_constructor_from_list (init_list_type_node, init);
 	      CONSTRUCTOR_IS_DIRECT_INIT (init) = true;
 	      CONSTRUCTOR_IS_PAREN_INIT (init) = true;
+	      /* The gimplifier expects that the front end fills in all of the
+		 indices.  Normally, reshape_init_array fills these in, but we
+		 don't call reshape_init because that does nothing when it gets
+		 CONSTRUCTOR_IS_PAREN_INIT.  */
+	      for (unsigned int i = 0; i < CONSTRUCTOR_NELTS (init); i++)
+		CONSTRUCTOR_ELT (init, i)->index = size_int (i);
 	    }
 	}
       else if (TREE_CODE (init) == TREE_LIST
diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init22.C b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
new file mode 100644
index 00000000000..1b2959e7731
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/paren-init22.C
@@ -0,0 +1,15 @@ 
+// PR c++/94155 - crash in gimplifier with paren init of aggregates.
+// { dg-do compile { target c++2a } }
+
+struct S { int i, j; };
+
+struct A {
+  S s;
+  constexpr A(S e) : s(e) {}
+};
+
+void
+f()
+{
+  A g[1]({{1, 1}});
+}