Hi,
This patch addresses an issue where the compiler gets stuck in an
infinite mutually-recursive loop between store_fixed_bit_field and
store_split_bit_field. This affects versions back at least as far as
4.6 (or so). We observed this happening on PowerPC E500, but other
targets may be affected too. (The symptom is the same as PR 55438, but
the cause is different.)
A small testcase is as follows (compile with a toolchain targeting
"powerpc-linux-gnuspe" and configured with "--with-cpu=8548",
currently requiring minor hacks to work around e.g. libsanitizer
breakage):
#include <stdlib.h>
typedef struct {
char pad;
int arr[0];
} __attribute__((packed)) str;
str *
foo (int* src)
{
str *s = malloc (sizeof (str) + sizeof (int));
s->arr[0] = 0x12345678;
return s;
}
$ powerpc-linux-gnuspe-gcc -O2 -c min.c
(Segfault)
The problem is as follows: in stor-layout.c:compute_record_mode, the
record (struct) "str" is considered to have a single element (the "char
pad"), since only the size is checked and not the elements themselves:
as an optimisation the record as a whole is given the mode of the first
element, since that fits nicely into a machine word and then (the idea
is that) the record can be held in a register. In this case, the mode
given will be QImode.
Now, E500 cores cannot handle misaligned data accesses, at least for
some subset of instructions (STRICT_ALIGNMENT is true on such cores), so
accessing elements of the array "arr" in the packed structure will
typically use read-modify-write operations.
The function expmed.c:store_fixed_bit_field uses get_best_mode to try
to find a suitable mode for that read-modify-write operation: the
mode passed into get_best_mode is taken from op0 (inside the "if (MEM_P
(op0))" clause). Because the record type we are accessing has
QImode, this looks something like:
(mem:QI (reg:SI ...))
Now stor-layout.c:bit_field_iterator::next_mode will reject any mode
which is smaller than the size of the access we want to do (32 bits, or
24 bits after store_split_bit_field has been called once), skipping over
QImode and HImode. The SImode value returned is then rejected in
get_best_mode because it is bigger than largest_mode, which is QImode
(from before), so it returns VOIDmode.
That means that store_split_bit_field is called (from
store_fixed_bit_field), but now the damage has been done: we still have
a MEM for op0, so the "else" clause "word = op0" is executed, and we
recurse back into store_fixed_bit_field at the end of the function, and
we're back where we started -- this leads to infinite recursion between
those two functions, which eventually blows up the stack and crashes
the compiler.
Anyway: the short story is that a record that finishes with a
zero-length array should never be given the mode of its
"only" (non-zero-sized) element to start with. The attached patch stops
that from happening. (A flexible trailing array member, "int arr[];" is
handled correctly -- left as BLKmode -- due to the existing "DECL_SIZE
(field) == 0" check.)
Tested (gcc/g++/libstdc++) with an E500 cross-compiler as configured
above. The newly-added test fails without the patch, and passes with. OK
to apply, or any comments?
Thanks,
Julian
ChangeLog
gcc/
* stor-layout.c (compute_record_mode): Handle zero-sized array
members.
gcc/testsuite/
* gcc.dg/packed-struct-mode-1.c: New.Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c (revision 204674)
+++ gcc/stor-layout.c (working copy)
@@ -1601,6 +1601,7 @@ compute_record_mode (tree type)
&& integer_zerop (TYPE_SIZE (TREE_TYPE (field)))))
|| ! host_integerp (bit_position (field), 1)
|| DECL_SIZE (field) == 0
+ || integer_zerop (DECL_SIZE (field))
|| ! host_integerp (DECL_SIZE (field), 1))
return;
Index: gcc/testsuite/gcc.dg/packed-struct-mode-1.c
===================================================================
--- gcc/testsuite/gcc.dg/packed-struct-mode-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/packed-struct-mode-1.c (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -O2" } */
+
+#include <stdlib.h>
+
+typedef struct {
+ char pad;
+ int arr[0];
+} __attribute__((packed)) str;
+
+str *
+foo (int* src)
+{
+ str *s = malloc (sizeof (str) + sizeof (int));
+ s->arr[0] = 0x12345678;
+ return s;
+}
