Re: [PATCH] fix #69277 - [6 Regression] ICE mangling a flexible array member

[Martin Sebor <msebor at gmail dot com>]

On 01/14/2016 07:52 PM, Jeff Law wrote:
> On 01/14/2016 05:02 PM, Martin Sebor wrote:
> > On 01/14/2016 03:51 PM, Jakub Jelinek wrote:
> > > On Thu, Jan 14, 2016 at 03:46:26PM -0700, Martin Sebor wrote:
> > > > c++/69277 reports an ICE when mangling a template specialization
> > > > involving flexible array member.  Debugging the problem revealed
> > > > that GCC (prior to the ICE), due to treating flexible array
> > > > members the same as zero-length arrays, produced the wrong mangling
> > > > for the former.
> > > >
> > > > The attached patch fixes the ICE and also corrects the mangling
> > > > (which now also matches Clang's).
> > >
> > > But then, shouldn't the decision whether to mangle it the old way or
> > > the new
> > > way depend on -fabi-version= ?
> >
> > Perhaps it should, I don't know.  Jason didn't mention anything
> > when we briefly discussed the mangling change on IRC.  I can
> > certainly modify the patch if it's necessary or a good idea.
> > Let me know.
> Jason would have the final call here, but I suspect we'll need to do
> something.
>
> While it is a bugfix, we have traditionally queued them up to avoid
> regular ABI breakages.  It's also the case that we've traditionally
> added a warning for this kind of thing (controlled by Wabi) and also
> noted something in the manual (I think there's a section dedicated to
> this kind of thing).

In anticipation of needing to do something I put together the attached
patch that rolls this change into version 10, letting version 9 and
prior roll it back.  I also mention it in the manual.  What the patch
doesn't do is add a warning.

I did notice with the mangling change we already issue a note for one
ABI change about flexible arrays:

note: the ABI of passing struct with a flexible array member has changed 
in GCC 4.4

so maybe this is an opportunity to update it.

Martin

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

	PR c++/69277
	* gcc/testsuite/g++.dg/abi/flexarray-abi-v9.C: New test.
	* g++.dg/ext/flexarray-mangle-2.C: New test.
	* g++.dg/ext/flexarray-mangle.C: New test.

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

	PR c++/69277
	* mangle.c (write_array_type): Mangle flexible array members
	the same as arrays with an unspecified size.

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

	PR c++/69277
	* invoke.texi (-fabi-version=): Update version 10.

Index: gcc/cp/mangle.c
===================================================================
--- gcc/cp/mangle.c	(revision 232296)
+++ gcc/cp/mangle.c	(working copy)
@@ -3280,8 +3280,10 @@ write_template_template_arg (const tree
 		  ::= A <expression> _ </element/ type>
 
      "Array types encode the dimension (number of elements) and the
-     element type. For variable length arrays, the dimension (but not
-     the '_' separator) is omitted."  */
+     element type.  For variable length arrays, the dimension (but not
+     the '_' separator) is omitted."
+     Note that for flexible array members, like for other arrays of
+     unspecified size, the dimension is also omitted.  */
 
 static void
 write_array_type (const tree type)
@@ -3290,29 +3292,37 @@ write_array_type (const tree type)
   if (TYPE_DOMAIN (type))
     {
       tree index_type;
-      tree max;
 
       index_type = TYPE_DOMAIN (type);
-      /* The INDEX_TYPE gives the upper and lower bounds of the
-	 array.  */
-      max = TYPE_MAX_VALUE (index_type);
-      if (TREE_CODE (max) == INTEGER_CST)
+      /* The INDEX_TYPE gives the upper and lower bounds of the array.
+	 It's null for flexible array members which have no upper bound
+	 (this is a change from GCC 5 and prior where such members were
+	 incorrectly mangled as zero-length arrays).  */
+      if (tree max = TYPE_MAX_VALUE (index_type))
 	{
-	  /* The ABI specifies that we should mangle the number of
-	     elements in the array, not the largest allowed index.  */
-	  offset_int wmax = wi::to_offset (max) + 1;
-	  /* Truncate the result - this will mangle [0, SIZE_INT_MAX]
-	     number of elements as zero.  */
-	  wmax = wi::zext (wmax, TYPE_PRECISION (TREE_TYPE (max)));
-	  gcc_assert (wi::fits_uhwi_p (wmax));
-	  write_unsigned_number (wmax.to_uhwi ());
+	  if (TREE_CODE (max) == INTEGER_CST)
+	    {
+	      /* The ABI specifies that we should mangle the number of
+		 elements in the array, not the largest allowed index.  */
+	      offset_int wmax = wi::to_offset (max) + 1;
+	      /* Truncate the result - this will mangle [0, SIZE_INT_MAX]
+		 number of elements as zero.  */
+	      wmax = wi::zext (wmax, TYPE_PRECISION (TREE_TYPE (max)));
+	      gcc_assert (wi::fits_uhwi_p (wmax));
+	      write_unsigned_number (wmax.to_uhwi ());
+	    }
+	  else
+	    {
+	      max = TREE_OPERAND (max, 0);
+	      write_expression (max);
+	    }
 	}
-      else
+      else if (!abi_version_at_least (10))
 	{
-	  max = TREE_OPERAND (max, 0);
-	  write_expression (max);
+	  /* In ABI versions prior to 10 mangle flexible array members
+	     the same as zero-length arrays for compatibility.  */
+	  write_unsigned_number (0);
 	}
-
     }
   write_char ('_');
   write_type (TREE_TYPE (type));
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 232296)
+++ gcc/doc/invoke.texi	(working copy)
@@ -2173,7 +2173,8 @@ Version 9, which first appeared in G++ 5
 
 Version 10, which first appeared in G++ 6.1, adds mangling of
 attributes that affect type identity, such as ia32 calling convention
-attributes (e.g. @samp{stdcall}).
+attributes (e.g. @samp{stdcall}).  This version also corrects the mangling
+of flexible array members.
 
 See also @option{-Wabi}.
 
Index: gcc/testsuite/g++.dg/ext/flexarray-mangle-2.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexarray-mangle-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/flexarray-mangle-2.C	(working copy)
@@ -0,0 +1,48 @@
+// PR c++/69277 - [6 Regression] ICE mangling a flexible array member
+// { dg-do compile { target c++11 } }
+
+struct A {
+  int n;
+  char a [];
+};
+
+// Declare but do not define function templates.
+template <class T>
+void foo ();
+
+template <typename T>
+void fooref (T&);
+
+// Rvalue references are a C++ 11 feature.
+template <typename T>
+void foorefref (T&&);
+
+void bar (A a)
+{
+  // Decltype is also a C++ 11 feature.
+  // Verify that decltype gets the right type and that foo is
+  // mangled correctly.
+  foo<decltype (a.a)>();
+
+  // Verify that function templates taking a reference and an rvalue
+  // references (as in PR c++/69277) are also mangled correctly.
+  fooref (a.a);
+  foorefref (a.a);
+}
+
+// In G++ versions prior to 6 (ABI version 9 and prior), flexible array
+// members were incorrectly mangled as arrays of zero elements.  Verify
+// that flexible array members are mangled correctly as arrays of
+// an unspecified number of elements by default (ABI version 10 and
+// newer).  See g++.dg/abi/flexarray-abi-v9.C for the compatibility
+// test.
+
+// void foo<char []>():
+// { dg-final { scan-assembler _Z3fooIA_cEvv } }
+
+// The following is derived from PR c++/69277:
+// void fooref<char []>(char (&) [])
+// { dg-final { scan-assembler _Z6foorefIA_cEvRT_ } }
+
+// void foorefref<char (&) []>(char (&) [])
+// { dg-final { scan-assembler _Z9foorefrefIRA_cEvOT_ } }
Index: gcc/testsuite/g++.dg/ext/flexarray-mangle.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexarray-mangle.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/flexarray-mangle.C	(working copy)
@@ -0,0 +1,26 @@
+// PR c++/69277 - [6 Regression] ICE mangling a flexible array member
+// { dg-do compile }
+
+struct A {
+  int n;
+  char a [];
+};
+
+// Declare but do not define function templates.
+template <typename T>
+void fooref (T&);
+
+void bar (A a)
+{
+  fooref (a.a);
+}
+
+// In G++ versions prior to 6 (ABI version 9 and prior), flexible array
+// members were incorrectly mangled as arrays of zero elements.  Verify
+// that flexible array members are mangled correctly as arrays of
+// an unspecified number of elements by default (ABI version 10 and
+// newer).  See g++.dg/abi/flexarray-abi-v9.C for the compatibility
+// test.
+
+// void fooref<char []>(char (&) [])
+// { dg-final { scan-assembler _Z6foorefIA_cEvRT_ } }
Index: gcc/testsuite/g++.dg/abi/flexarray-abi-v9.C
===================================================================
--- gcc/testsuite/g++.dg/abi/flexarray-abi-v9.C	(revision 0)
+++ gcc/testsuite/g++.dg/abi/flexarray-abi-v9.C	(working copy)
@@ -0,0 +1,49 @@
+// Test to verify ABI version 9 (and prior) compatibility mangling
+// of a flexible array member.  See also PR c++/69277.
+// { dg-do compile { target c++11 } }
+// { dg-options "-fabi-version=9" }
+
+struct A {
+  int n;
+  char a [];
+};
+
+// Declare but do not define function templates.
+template <class T>
+void foo ();
+
+template <typename T>
+void fooref (T&);
+
+// Rvalue references are a C++ 11 feature.
+template <typename T>
+void foorefref (T&&);
+
+void bar (A a)
+{
+  // Decltype is also a C++ 11 feature.
+  // Verify that decltype gets the right type and that foo is
+  // mangled correctly.
+  foo<decltype (a.a)>();
+
+  // Verify that function templates taking a reference and an rvalue
+  // references (as in PR c++/69277) are also mangled correctly.
+  fooref (a.a);
+  foorefref (a.a);
+}
+
+// In G++ versions prior to 6 (ABI version 10), flexible array members
+// were incorrectly mangled as arrays of zero elements.  Verify that
+// they are still mangled that way with -fabi-version=9 (i.e., unless
+// -fabi-version=10 is used.  See g++.dg/ext/flexarray-mangle-3.C
+// for that test.)
+
+// void foo<char [0]>():
+// { dg-final { scan-assembler _Z3fooIA0_cEvv } }
+
+// The following is derived from PR c++/69277:
+// void fooref<char [0]>(char (&) [0])
+// { dg-final { scan-assembler _Z6foorefIA0_cEvRT_ } }
+
+// void foorefref<char (&) [0]>(char (&) [0])
+// { dg-final { scan-assembler _Z9foorefrefIRA0_cEvOT_ } }