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

[David Mertens]

On Tue, Oct 18, 2016 at 11:37 AM, Michael Matz <address@hidden> wrote:

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.

Should we add this as a compiler flag to tcc, perhaps?

 

> 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 :-/

I question the wisdom of targeting the binary layout of pcc. It's a neat compiler, with an aim similar to tcc, but developer time and resources are limited. I'd vote for just MSVC and GCC compatibility as a start. (Presumably we get clang for free with that.) If anybody later comes along with inclination, they can extend it to handle PCC layout.

David

--

 "Debugging is twice as hard as writing the code in the first place.
  Therefore, if you write the code as cleverly as possible, you are,
  by definition, not smart enough to debug it." -- Brian Kernighan