After discussions with Jakub on IRC, I've committed this patch along
with a couple of other tweaks to the testsuite. New ChangeLog below.
This issue has existed since GCC-6 but has only come to light following
the release of gcc-8 where code was added to the compiler sources that
exposed the bug. I'm therefore reasonably confident that this idiom
won't be a widely hit problem. I'm therefore looking to mitigate it in
the GCC-7/8 sources rather than try to back-port an ABI changing bug.
I'll post a patch for that shortly.
R.
On 17/12/2018 16:04, Richard Earnshaw (lists) wrote:
> 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.
>
gcc:
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.
gcc/testsuite:
* gcc.target/arm/aapcs/bitfield1.c: New test.
* gcc.target/arm/aapcs/overalign_rec1.c: New test.
* gcc.target/arm/aapcs/overalign_rec2.c: New test.
* gcc.target/arm/aapcs/overalign_rec3.c: New test.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 73cb8df9af1..c6fbda25e96 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6598,7 +6598,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
@@ -6618,7 +6620,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)
{
@@ -6630,6 +6633,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;
}
@@ -6695,7 +6705,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
@@ -6722,6 +6737,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;
}
@@ -26999,7 +27017,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..cac786eec37
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/bitfield1.c
@@ -0,0 +1,24 @@
+/* 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)
+ ARG (int, 9, R1)
+ ARG (int, 11, R2)
+ /* Alignment of the bitfield type should affect alignment of the overall
+ type, so R3 not used. */
+ LAST_ARG (struct bf, v, STACK)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec1.c b/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec1.c
new file mode 100644
index 00000000000..1d33da42bdf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec1.c
@@ -0,0 +1,27 @@
+/* Test AAPCS layout (alignment). */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "overalign_rec1.c"
+
+typedef struct __attribute__((aligned(8)))
+{
+ int a;
+ int b;
+} overaligned;
+
+overaligned v = {1, 3};
+overaligned w = {33, 99};
+
+#include "abitest.h"
+#else
+ ARG (int, 7, R0)
+ /* Overalignment is ignored for the purposes of parameter passing. */
+ ARG (overaligned, v, R1)
+ ARG (int, 11, R3)
+ ARG (int, 9, STACK)
+ LAST_ARG (overaligned, w, STACK+4)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec2.c b/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec2.c
new file mode 100644
index 00000000000..b19fa70c591
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec2.c
@@ -0,0 +1,25 @@
+/* Test AAPCS layout (alignment). */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "overalign_rec2.c"
+
+typedef struct
+{
+ int __attribute__((aligned(8))) a;
+ int b;
+} overaligned;
+
+overaligned v = {1, 3};
+overaligned w = {33, 99};
+
+#include "abitest.h"
+#else
+ ARG (int, 7, R0)
+ ARG (overaligned, v, R2)
+ ARG (int, 9, STACK)
+ LAST_ARG (overaligned, w, STACK+8)
+#endif
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec3.c b/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec3.c
new file mode 100644
index 00000000000..b1c793e04e6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/overalign_rec3.c
@@ -0,0 +1,28 @@
+/* Test AAPCS layout (alignment). */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O" } */
+
+#ifndef IN_FRAMEWORK
+#define TESTFILE "overalign_rec3.c"
+
+typedef struct
+{
+ int __attribute__((aligned(16))) a;
+ int b;
+} overaligned;
+
+overaligned v = {1, 3};
+overaligned w = {33, 99};
+
+#include "abitest.h"
+#else
+ ARG (int, 7, R0)
+ /* Objects with alignment > 8 are passed with alignment 8. */
+ ARG (overaligned, v, R2)
+ ARG (int, 9, STACK+8)
+ ARG (int, 10, STACK+12)
+ ARG (int, 11, STACK+16)
+ LAST_ARG (overaligned, w, STACK+24)
+#endif
