[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
Related show

Commit Message

Alexandre Oliva Feb. 28, 2018, 12:08 p.m.
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


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Comments

Jason Merrill Feb. 28, 2018, 6:57 p.m. | #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. | #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" }


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill March 2, 2018, 7:01 p.m. | #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. | #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, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Alexandre Oliva March 6, 2018, 6:14 a.m. | #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" }


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Jason Merrill March 6, 2018, 3:28 p.m. | #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

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" }