Re: [Tinycc-devel] Weird bitfield size handling, discrepancy with gcc

[Michael Matz]

Hi,

On Tue, 18 Oct 2016, David Mertens wrote:

> According to Christian, we have at least one major compiler (VC++) whose
> behavior matches tcc's current behavior and another (GCC) whose behavior
> differs.

Yes, I've implemented the GCC way privately already (necessary for e.g. 
linux kernel).  Need to clean this up, sigh.

> While it would be nice to just pick one implementation and go with it, I 
> am personally much more concerned with binary compatibility with one's 
> major compiler of choice. 

Exactly.  Actually also GCC itself has different implementations.  E.g. 
when configured for windows (or with -mms-bitfields) it implements the 
microsoft way, otherwise it defaults to the old PCC layout, which GCC uses 
on most UNIXen and other operating systems.  There're also minor 
variations for the latter e.g. on ppc64, but those wouldn't concern us.

> Here is a patch that makes the bit field size logic match gcc. I can 
> confirm that the original alignment bug is gone (though other alignment 
> bugs may yet be around). I am not 100% sure if this is correct, but I 
> throw it out there for folks to mull over. Note that indentation may not 
> be consistent; the indentation pattern in the existing code was weird. 
> Finally, it might be more efficient to use (bit_size & 7) rather than 
> (bit_size % 8), or optimizing compilers might do that for us anyway; I 
> leave that to the gurus.

>                          } else {
> +                           size = bit_size / 8;
> +                            if (bit_size % 8) size++; /* round up */
> +                            lbit_pos = bit_pos;
>                              /* we do not have enough room ?
>                                 did the type change?
>                                 is it a union? */
>                              if ((bit_pos + bit_size) > bsize ||
> -                                bt != prevbt || a == TOK_UNION)
> -                                bit_pos = 0;
> -                            lbit_pos = bit_pos;
> +                                bt != prevbt || a == TOK_UNION) {
> +                                lbit_pos = bit_pos = 0;

For instance, this is wrong for PCC layout.  MS layout indeed switches 
storage containers when the base type changes, i.e. here:

struct S {
  char  c:1;
  short s:1;
  char foo;
};

PCC layout has no such provisions.  But the base type _does_ influence 
alignment of the containing struct nevertheless.  Except if the bitfield 
has :0 size, then it _does_ force the next field to be placed into a new 
storage container.  It's all quite terrible and twisted :-/


Ciao,
Michael.