diff mbox series

[PR,c++/84593] ice on braced init with uninit ref field

Message ID orbmg9pb95.fsf@lxoliva.fsfla.org
State New
Headers show
Series [PR,c++/84593] ice on braced init with uninit ref field | expand

Commit Message

Alexandre Oliva Feb. 28, 2018, 12:08 p.m. UTC 
Don't allow the initializer expr to be NULL in a ctor initializer
list, make it error_marker_node instead.

Regstrapped on x86_64- and i686-linux-gnu.  Ok to install?

for  gcc/cp/ChangeLog

	PR c++/84593
	* typeck2.c (process_init_constructor_record): Set NULL next
	to error_marker_node.

for  gcc/testsuite/ChangeLog

	PR c++/84593
	* g++.dg/cpp1y/pr84593.C: New.
---
 gcc/cp/typeck2.c                     |    2 ++
 gcc/testsuite/g++.dg/cpp1y/pr84593.C |    8 ++++++++
 2 files changed, 10 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr84593.C

Comments

Jason Merrill Feb. 28, 2018, 6:57 p.m. UTC | #1 
On Wed, Feb 28, 2018 at 7:08 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> Don't allow the initializer expr to be NULL in a ctor initializer
> list, make it error_marker_node instead.

I don't want error_mark_nodes in a CONSTRUCTOR, either.  When there
isn't an NSDMI to worry about, we zero-initialize the reference, and
it seems reasonable to continue doing that, by fixing
build_zero_init_1 to return something non-null for a reference.

Jason
Alexandre Oliva March 2, 2018, 7:57 a.m. UTC | #2 
On Feb 28, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Wed, Feb 28, 2018 at 7:08 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> Don't allow the initializer expr to be NULL in a ctor initializer
>> list, make it error_marker_node instead.

> I don't want error_mark_nodes in a CONSTRUCTOR, either.  When there
> isn't an NSDMI to worry about, we zero-initialize the reference, and
> it seems reasonable to continue doing that, by fixing
> build_zero_init_1 to return something non-null for a reference.

Like this?  Regstrapped on i686- and x86_64-linux-gnu.

[PR c++/84593] ice on braced init with uninit ref field

If an initializer expr is to be NULL in a ctor initializer list, we
ICE in picflag_from_initializer and elsewhere.

If we're missing an initializer for a reference field, we report the
error, but then build a zero initializer to avoid the ICE.

for  gcc/cp/ChangeLog

	PR c++/84593
	* init.c (build_zero_init_1): Zero-initialize references.

for  gcc/testsuite/ChangeLog

	PR c++/84593
	* g++.dg/cpp1y/pr84593.C: New.
---
 gcc/cp/init.c                        |    5 ++++-
 gcc/testsuite/g++.dg/cpp1y/pr84593.C |    8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr84593.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index d0d14abdc9fa..ed28e9a46dbc 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -284,7 +284,10 @@ build_zero_init_1 (tree type, tree nelts, bool static_storage_p,
   else if (VECTOR_TYPE_P (type))
     init = build_zero_cst (type);
   else
-    gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
+    {
+      gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
+      init = fold (convert (type, integer_zero_node));
+    }
 
   /* In all cases, the initializer is a constant.  */
   if (init)
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr84593.C b/gcc/testsuite/g++.dg/cpp1y/pr84593.C
new file mode 100644
index 000000000000..8aa869f19193
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr84593.C
@@ -0,0 +1,8 @@
+// PR c++/84593
+// { dg-do compile { target c++14 } }
+
+struct a {
+  int x;
+  int c = 0;
+  int &b;
+} c = {}; // { dg-error "uninitialized reference" }
Jason Merrill March 2, 2018, 7:01 p.m. UTC | #3 
On Fri, Mar 2, 2018 at 2:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Feb 28, 2018, Jason Merrill <jason@redhat.com> wrote:
>
>> On Wed, Feb 28, 2018 at 7:08 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> Don't allow the initializer expr to be NULL in a ctor initializer
>>> list, make it error_marker_node instead.
>
>> I don't want error_mark_nodes in a CONSTRUCTOR, either.  When there
>> isn't an NSDMI to worry about, we zero-initialize the reference, and
>> it seems reasonable to continue doing that, by fixing
>> build_zero_init_1 to return something non-null for a reference.
>
> Like this?  Regstrapped on i686- and x86_64-linux-gnu.
>
> [PR c++/84593] ice on braced init with uninit ref field
>
> If an initializer expr is to be NULL in a ctor initializer list, we
> ICE in picflag_from_initializer and elsewhere.
>
> If we're missing an initializer for a reference field, we report the
> error, but then build a zero initializer to avoid the ICE.
>
> for  gcc/cp/ChangeLog
>
>         PR c++/84593
>         * init.c (build_zero_init_1): Zero-initialize references.
>
> for  gcc/testsuite/ChangeLog
>
>         PR c++/84593
>         * g++.dg/cpp1y/pr84593.C: New.
> ---
>  gcc/cp/init.c                        |    5 ++++-
>  gcc/testsuite/g++.dg/cpp1y/pr84593.C |    8 ++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr84593.C
>
> diff --git a/gcc/cp/init.c b/gcc/cp/init.c
> index d0d14abdc9fa..ed28e9a46dbc 100644
> --- a/gcc/cp/init.c
> +++ b/gcc/cp/init.c
> @@ -284,7 +284,10 @@ build_zero_init_1 (tree type, tree nelts, bool static_storage_p,
>    else if (VECTOR_TYPE_P (type))
>      init = build_zero_cst (type);
>    else
> -    gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
> +    {
> +      gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
> +      init = fold (convert (type, integer_zero_node));

Maybe build_zero_cst?

OK either way.

Jason
Alexandre Oliva March 6, 2018, 5:42 a.m. UTC | #4 
On Mar  2, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Fri, Mar 2, 2018 at 2:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> +      gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
>> +      init = fold (convert (type, integer_zero_node));

> Maybe build_zero_cst?

Sure.


I wonder, is there any reason to not change any of these to use
build_zero_cst, too?

  else if (TYPE_PTR_OR_PTRMEM_P (type))
    init = fold (convert (type, nullptr_node));
  else if (SCALAR_TYPE_P (type))
    init = fold (convert (type, integer_zero_node));

I suppose pointers to members might need an explicit conversion, which
build_zero_cst might fallback to if it doesn't consider them aggregate
types.  As for scalar types, are there any C++-specific scalar types
that build_zero_cst wouldn't know how to deal with?  Anyway, it's
probably not the time to change these, if it doesn't fix a regression.
Just curious.
Alexandre Oliva March 6, 2018, 6:14 a.m. UTC | #5 
On Mar  2, 2018, Jason Merrill <jason@redhat.com> wrote:

>> +      gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
>> +      init = fold (convert (type, integer_zero_node));

> Maybe build_zero_cst?

> OK either way.

Here's what I'm installing:

[PR c++/84593] ice on braced init with uninit ref field

If an initializer expr is to be NULL in a ctor initializer list, we
ICE in picflag_from_initializer and elsewhere.

If we're missing an initializer for a reference field, we report the
error, but then build a zero initializer to avoid the ICE.

for  gcc/cp/ChangeLog

	PR c++/84593
	* init.c (build_zero_init_1): Zero-initialize references.

for  gcc/testsuite/ChangeLog

	PR c++/84593
	* g++.dg/cpp1y/pr84593.C: New.
---
 gcc/cp/init.c                        |    5 ++++-
 gcc/testsuite/g++.dg/cpp1y/pr84593.C |    8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr84593.C

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index d0d14abdc9fa..15cee17c780c 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -284,7 +284,10 @@ build_zero_init_1 (tree type, tree nelts, bool static_storage_p,
   else if (VECTOR_TYPE_P (type))
     init = build_zero_cst (type);
   else
-    gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
+    {
+      gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
+      init = build_zero_cst (type);
+    }
 
   /* In all cases, the initializer is a constant.  */
   if (init)
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr84593.C b/gcc/testsuite/g++.dg/cpp1y/pr84593.C
new file mode 100644
index 000000000000..8aa869f19193
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr84593.C
@@ -0,0 +1,8 @@
+// PR c++/84593
+// { dg-do compile { target c++14 } }
+
+struct a {
+  int x;
+  int c = 0;
+  int &b;
+} c = {}; // { dg-error "uninitialized reference" }
Jason Merrill March 6, 2018, 3:28 p.m. UTC | #6 
On Tue, Mar 6, 2018 at 12:42 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Mar  2, 2018, Jason Merrill <jason@redhat.com> wrote:
>
>> On Fri, Mar 2, 2018 at 2:57 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> +      gcc_assert (TREE_CODE (type) == REFERENCE_TYPE);
>>> +      init = fold (convert (type, integer_zero_node));
>
>> Maybe build_zero_cst?
>
> Sure.
>
>
> I wonder, is there any reason to not change any of these to use
> build_zero_cst, too?
>
>   else if (TYPE_PTR_OR_PTRMEM_P (type))
>     init = fold (convert (type, nullptr_node));
>   else if (SCALAR_TYPE_P (type))
>     init = fold (convert (type, integer_zero_node));
>
> I suppose pointers to members might need an explicit conversion, which
> build_zero_cst might fallback to if it doesn't consider them aggregate
> types.  As for scalar types, are there any C++-specific scalar types
> that build_zero_cst wouldn't know how to deal with?  Anyway, it's
> probably not the time to change these, if it doesn't fix a regression.
> Just curious.

Indeed, build_zero_cst is wrong for pointers to members, but it should
be right for other scalars, including regular pointers.

Jason
diff mbox series

Patch

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 153b46cca775..8c2aa3ed55a5 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1529,6 +1529,8 @@  process_init_constructor_record (tree type, tree init, int nested,
       /* If this is a bitfield, now convert to the lowered type.  */
       if (type != TREE_TYPE (field))
 	next = cp_convert_and_check (TREE_TYPE (field), next, complain);
+      if (!next)
+	next = error_mark_node;
       flags |= picflag_from_initializer (next);
       CONSTRUCTOR_APPEND_ELT (v, field, next);
     }
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr84593.C b/gcc/testsuite/g++.dg/cpp1y/pr84593.C
new file mode 100644
index 000000000000..8aa869f19193
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr84593.C
@@ -0,0 +1,8 @@ 
+// PR c++/84593
+// { dg-do compile { target c++14 } }
+
+struct a {
+  int x;
+  int c = 0;
+  int &b;
+} c = {}; // { dg-error "uninitialized reference" }