[arm][RFC] PR target/88469 fix incorrect argument passing with 64-bit bitfields

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Unfortunately another PCS bug has come to light with the layout of
structs whose alignment is dominated by a 64-bit bitfield element.  Such
fields in the type list appear to have alignment 1, but in reality, for
the purposes of alignment of the underlying structure, the alignment is
derived from the underlying bitfield's type.  We've been getting this
wrong since support for over-aligned record types was added several
releases back.  Worse still, the existing code may generate unaligned
memory accesses that may fault on some versions of the architecture.

I'd appreciate a comment from someone with a bit more knowledge of
record layout - the code in stor-layout.c looks hideously complex when
it comes to bit-field layout; but it's quite possible that all of that
is irrelevant on Arm.

PR target/88469
	* config/arm/arm.c (arm_needs_doubleword_align): Return 2 if a
	record's alignment is dominated by a bitfield with 64-bit
	aligned base type.
	(arm_function_arg): Emit a warning if the alignment has changed
	since earlier GCC releases.
	(arm_function_arg_boundary): Likewise.
	(arm_setup_incoming_varargs): Likewise.
	* testsuite/gcc.target/arm/aapcs/bitfield1.c: New test.

I'm not committing this yet, as I'll await comments as to whether folks
think this is sufficient.

R.

[-- Attachment #2: aapcs.patch --]
[-- Type: text/x-patch, Size: 3656 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 40f0574e32e..5f5269c71c9 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6589,7 +6589,9 @@ arm_init_cumulative_args (CUMULATIVE_ARGS *pcum, tree fntype,
     }
 }
 
-/* Return 1 if double word alignment is required for argument passing.
+/* Return 2 if double word alignment is required for argument passing,
+   but wasn't required before the fix for PR88469.
+   Return 1 if double word alignment is required for argument passing.
    Return -1 if double word alignment used to be required for argument
    passing before PR77728 ABI fix, but is not required anymore.
    Return 0 if double word alignment is not required and wasn't requried
@@ -6609,7 +6611,8 @@ arm_needs_doubleword_align (machine_mode mode, const_tree type)
     return TYPE_ALIGN (TREE_TYPE (type)) > PARM_BOUNDARY;
 
   int ret = 0;
-  /* Record/aggregate types: Use greatest member alignment of any member.  */ 
+  int ret2 = 0;
+  /* Record/aggregate types: Use greatest member alignment of any member.  */
   for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
     if (DECL_ALIGN (field) > PARM_BOUNDARY)
       {
@@ -6621,6 +6624,13 @@ arm_needs_doubleword_align (machine_mode mode, const_tree type)
 	     Make sure we can warn about that with -Wpsabi.  */
 	  ret = -1;
       }
+    else if (TREE_CODE (field) == FIELD_DECL
+	     && DECL_BIT_FIELD (field)
+	     && TYPE_ALIGN (DECL_BIT_FIELD_TYPE (field)) > PARM_BOUNDARY)
+      ret2 = 1;
+
+  if (ret2)
+    return 2;
 
   return ret;
 }
@@ -6686,7 +6696,12 @@ arm_function_arg (cumulative_args_t pcum_v, machine_mode mode,
 	inform (input_location, "parameter passing for argument of type "
 		"%qT changed in GCC 7.1", type);
       else if (res > 0)
-	pcum->nregs++;
+	{
+	  pcum->nregs++;
+	  if (res > 1 && warn_psabi)
+	    inform (input_location, "parameter passing for argument of type "
+		    "%qT changed in GCC 9.1", type);
+	}
     }
 
   /* Only allow splitting an arg between regs and memory if all preceding
@@ -6713,6 +6728,9 @@ arm_function_arg_boundary (machine_mode mode, const_tree type)
   if (res < 0 && warn_psabi)
     inform (input_location, "parameter passing for argument of type %qT "
 	    "changed in GCC 7.1", type);
+  if (res > 1 && warn_psabi)
+    inform (input_location, "parameter passing for argument of type "
+	    "%qT changed in GCC 9.1", type);
 
   return res > 0 ? DOUBLEWORD_ALIGNMENT : PARM_BOUNDARY;
 }
@@ -26953,7 +26971,13 @@ arm_setup_incoming_varargs (cumulative_args_t pcum_v,
 	    inform (input_location, "parameter passing for argument of "
 		    "type %qT changed in GCC 7.1", type);
 	  else if (res > 0)
-	    nregs++;
+	    {
+	      nregs++;
+	      if (res > 1 && warn_psabi)
+		inform (input_location,
+			"parameter passing for argument of type "
+			"%qT changed in GCC 9.1", type);
+	    }
 	}
     }
   else
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/bitfield1.c b/gcc/testsuite/gcc.target/arm/aapcs/bitfield1.c
new file mode 100644
index 00000000000..7d8b7065484
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/bitfield1.c
@@ -0,0 +1,23 @@
+/* Test AAPCS layout (alignment).  */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "bitfield1.c"
+
+struct bf
+{
+  unsigned long long a: 61;
+  unsigned b: 3;
+} v = {1, 1};
+
+#include "abitest.h"
+#else
+  ARG (int, 7, R0)
+  /* Attribute suggests R2, but we should use only natural alignment:  */
+  ARG (int, 9, R1)
+  ARG (int, 11, R2)
+  LAST_ARG (struct bf, v, STACK)
+#endif