On Thu, 22 Mar 2012, Eric Botcazou wrote:
> > This is what I have applied after bootstrapping and testing on
> > x86_64-unknown-linux-gnu.
> Thanks. Last identified problem: the miscompilation with variant record.
> raised PROGRAM_ERROR : pack17.adb:36 explicit raise
> FAIL: gnat.dg/pack17.adb execution test
> Things start to go wrong at line 28:
> 24 procedure Fill (R : out Record_With_QImode_Variants) is
> 25 begin
> 26 R.C_Filler := (True, False, True, False, True, False, True);
> 27 R.C_Map := (True, False, True);
> 28 R.T_Int := 17;
> 29 end;
> R is a packed record with variant part and -gnatR1 gives its layout:
> for Record_With_Qimode_Variants'Object_Size use 24;
> for Record_With_Qimode_Variants'Value_Size use 19;
> for Record_With_Qimode_Variants'Alignment use 1;
> for Record_With_Qimode_Variants use record
> D at 0 range 0 .. 0;
> C_Filler at 0 range 1 .. 7;
> C_Map at 1 range 0 .. 2;
> F_Bit at 1 range 3 .. 3;
> F_Filler at 1 range 4 .. 10;
> T_Int at 1 range 3 .. 10;
> Now, since T_Int is within a variant, the assignment is translated into:
> r_1(D)->d___XVN.O.t_int = 17;
> d___XVN is QUAL_UNION_TYPE and O a RECORD_TYPE (fields of QUAL_UNION_TYPE are
> always of RECORD_TYPE, that's why the overloading of qualifier is indeed OK).
> The compiler now generates:
> movl 8(%ebp), %eax
> movb $17, 1(%eax)
> which is of course wrong given the layout above.
> It seems to me that the new code implicitly assumes that the first field in a
> bitfield group starts on a byte boundary, which doesn't hold in Ada.
Yes, I have noticed that from what you can see in the code. So the
issue is that get_bit_range tells us that it's ok to touch bits
outside of the field - but that's obviously required. We may not
change bits that are not covered by the FIELD_DECL which now
somehow happens? That sounds like a latent bug in the bitfield
expander to me - it should never _change_ bits outside of bitpos/bitsize
as returned by get_inner_reference.
I guess I'll have to debug this if you don't beat me on it.