diff mbox

fix #69251 - [6 Regression] ICE in unify_array_domain on a flexible array member

Message ID 569D9FAB.90705@gmail.com
State New
Headers show

Commit Message

Martin Sebor Jan. 19, 2016, 2:30 a.m. UTC
The attached is a minimal patch to avoid the ICE.  The patch doesn't
fix the type substitution of flexible array members as that seems
more involved and is, strictly speaking, outside the scope of this
bug.

Type substitution of flexible array is wrong in 5.3.0 (which treats
flexible array members the same as zero-length arrays). It's also
wrong in 6.0 but for a different reason (one having to do with their
domain, unlike the domain of arrays of unspecified bound which have
no domain, having no upper bound.  Fixing that will require more
time and surgery than just fixing the ICE and might also be more
intrusive than is appropriate at this stage.

Jason, please let me know whether or not you would like to see
the substitution failure fixed before the upcoming release as well.

Martin

Comments

Jason Merrill Jan. 21, 2016, 11:19 p.m. UTC | #1
Can we reconsider the representation of flexible arrays?  Following the 
example of the C front-end is causing a lot of trouble, and using a null 
TYPE_DOMAIN seems more intuitive.

Jason
Martin Sebor Jan. 21, 2016, 11:32 p.m. UTC | #2
On 01/21/2016 04:19 PM, Jason Merrill wrote:
> Can we reconsider the representation of flexible arrays?  Following the
> example of the C front-end is causing a lot of trouble, and using a null
> TYPE_DOMAIN seems more intuitive.

I remember running into at least one ICE in the middle end with
the alternate representation (null TYPE_DOMAIN).  At this late
stage I would worry about the fallout from that. It seems that
outside of 69251 and 69277 the problems are mostly triggered by
ill-formed code that wasn't being tested and I'm hoping that
the problems in the well-formed cases have been reported (and
with the patches I've sent fixed).

At the same time, based on some debugging I had to do for 69251
(ICE in unify_array_domain on a flexible array member) it seems
that it might make handling them in template easier.

Martin
diff mbox

Patch

gcc/testsuite/ChangeLog:
2016-01-18  Martin Sebor  <msebor@redhat.com>

	PR target/69318
	* g++.dg/ext/flexarray-subst.C: New test.

gcc/cp/ChangeLog:
2016-01-18  Martin Sebor  <msebor@redhat.com>

	PR target/69318
	* pt.c (unify): Handle flexible array members somewhat more gracefully.

Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 232526)
+++ gcc/cp/pt.c	(working copy)
@@ -19657,12 +19657,17 @@  unify (tree tparms, tree targs, tree par
     case ARRAY_TYPE:
       if (TREE_CODE (arg) != ARRAY_TYPE)
 	return unify_type_mismatch (explain_p, parm, arg);
+
+      /* Flexible array members have no upper bound.  Have them match
+         template parameters of unspecified bounds (which have a null
+         domain).  */
       if ((TYPE_DOMAIN (parm) == NULL_TREE)
-	  != (TYPE_DOMAIN (arg) == NULL_TREE))
-	return unify_type_mismatch (explain_p, parm, arg);
+          != (TYPE_DOMAIN (arg) == NULL_TREE
+              || TYPE_MAX_VALUE (TYPE_DOMAIN (arg)) == NULL_TREE))
+        return unify_type_mismatch (explain_p, parm, arg);
       RECUR_AND_CHECK_FAILURE (tparms, targs, TREE_TYPE (parm), TREE_TYPE (arg),
 			       strict & UNIFY_ALLOW_MORE_CV_QUAL, explain_p);
-      if (TYPE_DOMAIN (parm) != NULL_TREE)
+      if (TYPE_DOMAIN (parm))
 	return unify_array_domain (tparms, targs, TYPE_DOMAIN (parm),
 				   TYPE_DOMAIN (arg), explain_p);
       return unify_success (explain_p);
Index: gcc/testsuite/g++.dg/ext/flexarray-subst.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexarray-subst.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/flexarray-subst.C	(working copy)
@@ -0,0 +1,33 @@ 
+// PR c++/69251 - [6 Regression] ICE (segmentation fault) in unify_array_domain
+// on i686-linux-gnu
+// { dg-do compile }
+
+struct A { int n; char a[]; };
+
+template <class>
+struct B;
+
+// The following definition shouldn't be needed but is provided to prevent
+// the test from failing with an error due to PR c++/69349 - template
+// substitution error for flexible array members.  (This doesn't compromise
+// the validity of this test since all it tests for is the absennce of
+// the ICE.)
+template <class>
+struct B { typedef int X; };
+
+template <class T>
+struct B<T[]> { typedef int X; };
+
+template <class T>
+struct C { typedef typename B<T>::X X; };
+
+template <class T>
+int foo (T&, typename C<T>::X = 0)
+{
+  return 0;
+}
+
+void bar (A *a)
+{
+  foo (a->a);
+}