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

[Martin Sebor <msebor at gmail dot com>]

On 01/21/2016 04:32 PM, Martin Sebor wrote:
> 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.

In a discussion with Jason in IRC I agreed to submit a patch
changing the representation of flexible array members in the C++
front end to use a null domain rather than a domain with a null
upper bound.  Attached is a patch making the requested change.
It fixes the following bugs:

c++/69251 - [6 Regression] ICE in unify_array_domain on a flexible
  array member
  (the bug in the Subject)
c++/69253 - [6 Regression] ICE in cxx_incomplete_type_diagnostic
  initializing a flexible array member with empty string
  with the original patch here:
  https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01325.htm
and
c++/69290 - [6 Regression] ICE on invalid initialization
  of a flexible array member
  with the original patch here:
  https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01685.html
as well as
c++/69277 - [6 Regression] ICE mangling a flexible array member
  with its final patch posted here
  https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01233.html

The downside of this approach is that it prevents everything but
the front end from distinguishing flexible array members from
arrays of unspecified or unknown bounds.  The immediate impact
is that prevents us from maintaining ABI compatibility with GCC
5 (with -fabi-version=9) and from diagnosing the mangling change.
This means should we decide to adopt this approach, the final
version of the patch for c++/69277 mentioned above that's still
pending approval will need to be tweaked to have the ABI checks
removed.

I successfully tested the new patch on x86_64.

Martin

PR c++/69251 - [6 Regression] ICE in unify_array_domain on a flexible array
	member
PR c++/69253 - [6 Regression] ICE in cxx_incomplete_type_diagnostic
	initializing a flexible array member with empty string
PR c++/69290 - [6 Regression] ICE on invalid initialization of a flexible
	array member

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

	PR c++/69253
	PR c++/69251
	PR c++/69290
	* g++.dg/ext/flexarray-subst.C: New test.
	* g++.dg/ext/flexary11.C: New test.
	* g++.dg/ext/flexary12.C: New test.
	* g++.dg/ext/flexary13.C: New test.
	* g++.dg/ext/flexary14.C: New test.
	* g++.dg/other/dump-ada-spec-2.C: Adjust.

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

	PR c++/69253
	PR c++/69251
	PR c++/69290
	* decl.c (compute_array_index_type): Return null for flexible array
	members.
	* tree.c (array_of_runtime_bound_p): Handle gracefully array types
	with null TYPE_MAX_VALUE.
	(build_ctor_subob_ref): Loosen debug checking to handle flexible
	array members.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index ceeef60..beb7c58 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8638,8 +8638,9 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain)
   tree itype;
   tree osize = size;
 
+  /* Flexible array members have no domain.  */
   if (size == NULL_TREE)
-    return build_index_type (NULL_TREE);
+    return NULL_TREE;
 
   if (error_operand_p (size))
     return error_mark_node;
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index e2123ac..779652c 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -937,9 +937,10 @@ array_of_runtime_bound_p (tree t)
   tree dom = TYPE_DOMAIN (t);
   if (!dom)
     return false;
-  tree max = TYPE_MAX_VALUE (dom);
-  return (!potential_rvalue_constant_expression (max)
-	  || (!value_dependent_expression_p (max) && !TREE_CONSTANT (max)));
+  if (tree max = TYPE_MAX_VALUE (dom))
+    return (!potential_rvalue_constant_expression (max)
+	    || (!value_dependent_expression_p (max) && !TREE_CONSTANT (max)));
+  return false;
 }
 
 /* Return a reference type node referring to TO_TYPE.  If RVAL is
@@ -2556,8 +2557,21 @@ build_ctor_subob_ref (tree index, tree type, tree obj)
     obj = build_class_member_access_expr (obj, index, NULL_TREE,
 					  /*reference*/false, tf_none);
   if (obj)
-    gcc_assert (same_type_ignoring_top_level_qualifiers_p (type,
-							   TREE_TYPE (obj)));
+    {
+      tree objtype = TREE_TYPE (obj);
+      if (TREE_CODE (objtype) == ARRAY_TYPE
+	  && (!TYPE_DOMAIN (objtype)
+	      || !TYPE_MAX_VALUE (TYPE_DOMAIN (objtype))))
+	{
+	  /* When the destination object refers to a flexible array member
+	     verify that it matches the type of the source object except
+	     for its domain.  */
+	  gcc_assert (comptypes (type, objtype, COMPARE_REDECLARATION));
+	}
+      else
+	gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, objtype));
+    }
+
   return obj;
 }
 
diff --git a/gcc/testsuite/g++.dg/ext/flexarray-subst.C b/gcc/testsuite/g++.dg/ext/flexarray-subst.C
new file mode 100644
index 0000000..f644636
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexarray-subst.C
@@ -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);
+}
diff --git a/gcc/testsuite/g++.dg/ext/flexary11.C b/gcc/testsuite/g++.dg/ext/flexary11.C
new file mode 100644
index 0000000..5bf774f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary11.C
@@ -0,0 +1,19 @@
+// PR c++/69253 - [6 Regression] g++ ICE at -O0 on x86_64-linux-gnu
+//                in "cxx_incomplete_type_diagnostic"
+// { dg-do compile }
+
+struct A {
+  int n;
+  char a [];
+};
+
+void f ()
+{
+  // Compound literals and flexible array members are G++ extensions
+  // accepted for compatibility with C and GCC.
+
+  // The following use of a flexible array member in a compound literal
+  // is invalid in C and rejected by GCC in C mode and so it's also
+  // rejected in C++ mode.
+  (struct A){ 1, "" };   // { dg-error "forbids compound-literals|initialization of a flexible array member|invalid use of a flexible array member" }
+}
diff --git a/gcc/testsuite/g++.dg/ext/flexary12.C b/gcc/testsuite/g++.dg/ext/flexary12.C
new file mode 100644
index 0000000..3d8c805
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary12.C
@@ -0,0 +1,63 @@
+// PR c++/69290 - [6 Regression] g++ ICE on invalid initialization
+//     of a flexible array member
+// { dg-do compile }
+
+// Suppress pedantic errors about initialization of a flexible array member.
+// { dg-options "-Wno-pedantic" }
+
+struct A {
+  int a [];  // { dg-error "flexible array member .A::a. in an otherwise empty .struct A." }
+};
+
+void f1 ()
+{
+  // This is the meat of the test from c++/69290:
+  struct A a
+    = { "c" };   // { dg-error "invalid conversion from .const char\\*. to .int." }
+
+  (void)&a;
+}
+
+
+// Exercise other forms of invalid initialization besides the one in the bug.
+struct B {
+  int n;
+  int a [];
+};
+
+void f2 ()
+{
+  struct B b1
+    = { 0, "c" };   // { dg-error "invalid conversion from .const char\\*. to .int." }
+
+  (void)&b1;
+
+  const char s[] = "c";
+  struct B b2
+    = { 0, s };   // { dg-error "invalid conversion from .const char\\*. to .int." }
+
+  (void)&b2;
+}
+
+struct D {
+  int a [];  // { dg-error "flexible array member .D::a. in an otherwise empty .struct D." }
+  D ();
+};
+
+D::D ():
+  a ("c")   // { dg-error "incompatible types in assignment of .const char \\\[2\\\]. to .int \\\[\\\]." }
+{ }
+
+
+template <class T>
+struct C {
+  T a [];  // { dg-error "flexible array member" }
+};
+
+void f3 ()
+{
+  struct C<double> cd
+    = { "c" };   // { dg-error "cannot convert .const char\\*. to .double." }
+
+  (void)&cd;
+}
diff --git a/gcc/testsuite/g++.dg/ext/flexary13.C b/gcc/testsuite/g++.dg/ext/flexary13.C
new file mode 100644
index 0000000..462ed65
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary13.C
@@ -0,0 +1,64 @@
+// { dg-do compile }
+// { dg-options -Wno-pedantic }
+
+#define STR(s) #s
+#define ASSERT(exp) \
+  ((exp) ? (void)0 : (void)(__builtin_printf ("%s:%i: assertion %s failed\n", \
+                     __FILE__, __LINE__, STR(exp)), \
+                      __builtin_abort ()))
+
+struct Ax { int n, a[]; };
+struct AAx { int i; Ax ax; };
+
+int i = 12345678;
+
+int main ()
+{
+  {
+    Ax s = { 0 };
+    ASSERT (s.n == 0);
+  }
+  {
+    Ax s =
+      { 0, { } };   // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.n == 0);
+  }
+  {
+    Ax s =
+      { 1, { 2 } };   // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.n == 1 && s.a [0] == 2);
+  }
+  {
+    Ax s =
+      { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4);
+  }
+  {
+    Ax s =
+      { 123, i };   // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.n == 123 && s.a [0] == i);
+  }
+  {
+    Ax s =
+      { 456, { i } }; // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.n == 456 && s.a [0] == i);
+  }
+  {
+    int j = i + 1, k = j + 1;
+    Ax s =
+      { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k);
+  }
+
+  {
+    AAx s =
+      { 1, { 2 } };   // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.i == 1 && s.ax.n == 2);
+  }
+
+  {
+    AAx s =
+      { 1, { 2, { 3 } } };   // dg-warning "initialization of a flexible array member" }
+    ASSERT (s.i == 1 && s.ax.n == 2 && s.ax.a [0] == 3);
+  }
+}
diff --git a/gcc/testsuite/g++.dg/ext/flexary14.C b/gcc/testsuite/g++.dg/ext/flexary14.C
new file mode 100644
index 0000000..7365357
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary14.C
@@ -0,0 +1,17 @@
+// PR c++/69349 - template substitution error for flexible array members
+// { dg-do compile }
+
+template <class>
+struct A;
+
+template <class T>
+struct A<T[]> { typedef int X; };
+
+template <class T> int foo (T&, typename A<T>::X = 0) { return 0; }
+
+struct B { int n, a[]; };
+
+void bar (B *b)
+{
+    foo (b->a);
+}
diff --git a/gcc/testsuite/g++.dg/other/dump-ada-spec-2.C b/gcc/testsuite/g++.dg/other/dump-ada-spec-2.C
index d1af7e0..608b5be 100644
--- a/gcc/testsuite/g++.dg/other/dump-ada-spec-2.C
+++ b/gcc/testsuite/g++.dg/other/dump-ada-spec-2.C
@@ -7,5 +7,5 @@ struct S
   __extension__ unsigned char data[];
 };
 
-/* { dg-final { scan-ada-spec "array \\(0 .. 0\\)" } } */
+/* { dg-final { scan-ada-spec "array \\(size_t\\)" } } */
 /* { dg-final { cleanup-ada-spec } } */