On Mon, Mar 28, 2011 at 12:42 PM, Paolo Bonzini <bonz...@gnu.org> wrote:
> On 03/27/2011 06:27 AM, Ian Lance Taylor wrote:
>>
>> Nathan Boley<npbo...@gmail.com> writes:
>>
>>> In a much larger application, I was getting a weird segfault that an
>>> assignment to a temporary variable fixed. I distilled the example into
>>> the attached "test_case.c". When I run test_case.c under valgrind I
>>> get a memory read error, and it segfaults with electric fence, but I'm
>>> not actually able to get a true segfault. However, I am pretty sure
>>> that the same issue was causing the segfault in my application.
>>
>> There is nothing wrong if gcc is reading an 8-byte value at an 8-byte
>> aligned address.
>
> Note the struct is packed, so alignof = 1.
>
>> That said, I could not recreate the problem with your test case. I only
>> see 4-byte loads.
>
> I see it with this modified testcase:
>
> #include <stdio.h>
> #include <stdlib.h>
>
> /* GCC uses 4-byte + 2-byte loads and stack passing */
> typedef struct __attribute__((__packed__))
> {
> unsigned short chr;
> unsigned int loc;
> } GENOME_LOC_TYPE_1;
>
> /* GCC uses 8-byte loads and register passing even though sizeof = 6 */
> typedef struct __attribute__((__packed__))
> {
> unsigned chr :16;
> unsigned loc :32;
> } GENOME_LOC_TYPE_2;
>
> //#define GENOME_LOC_TYPE GENOME_LOC_TYPE_1
> #define GENOME_LOC_TYPE GENOME_LOC_TYPE_2
>
> static __attribute__((__noclone__,__noinline__))
> int f(GENOME_LOC_TYPE x)
> {
> return x.loc;
> }
>
> GENOME_LOC_TYPE h;
> GENOME_LOC_TYPE *g = &h;
>
> int
> main()
> {
> printf ("%d %d\n", sizeof (GENOME_LOC_TYPE),
> __alignof__(GENOME_LOC_TYPE));
> return f(*g);
> }
>
>
> Both definitions have a (sizeof = 6, alignof = 1) but GCC loads the second
> with an 8-byte load. It's really an ugly bug if I understood it correctly,
> because I would have expected the second struct to have sizeof = 8. The two
> final bytes are not padding, they are what's left of the unsigned int from
> which the bitfields are carved. If that were the correct fix for the bug,
> it would be a change to the ABI.
At expansion time we have the following for the call argument:
<mem_ref 0x7ffff7ff9118
type <record_type 0x7ffff5b295e8 GENOME_LOC_TYPE_2 packed type_0 BLK
size <integer_cst 0x7ffff5b256b8 constant 48>
unit size <integer_cst 0x7ffff5b25708 constant 6>
align 8 symtab 0 alias set -1 canonical type 0x7ffff5b29540
which looks ok to me. expand generates a MEM of BLKmode:
(mem/s:BLK (reg/f:DI 62) [0 MEM[(struct GENOME_LOC_TYPE_2 *)g.0_1]+0 S6 A8])
which is also ok. But then calls.c calls move_block_to_reg and messes
up via operand_subword_force (mem, 0, BLKmode) which simply makes
a DImode mem out of it.
Richard.
