Re: Possible Bug

[Richard Guenther <richard.guenther@gmail.com>]

On Mon, Mar 28, 2011 at 12:42 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 03/27/2011 06:27 AM, Ian Lance Taylor wrote:
>>
>> Nathan Boley<npboley@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.