ffi/doc: bugfixes to bitfield alignment.

Three issues issues. The documentation is wrong about the
concept of a "leading bitfield" whose alignment matters.
The alignment of all named bitfields matters. Secondly,
the alignment of unnamed bitfields doesn't matter.
Thirdly, there is a flaw in the treatment of bitfields
in unions, from both alignment and sizing.

* ffi.c (make_ffi_type_struct): A bitfield member contributes
to alignment regardless of its offset within an allocation
unit: no bits_alloc == 0 check should be applied. Secondly, a
bitfield member that is unnamed has an alignment of 1,
otherwise the alignment of its declared type.
(make_ffi_type_union): The size and alignment of a bitfield
member is not simply that of its type.  The size of a bitfield
is the number of bytes required to store all of the bits. The
alignment is 1 if it is unnamed, otherwise that of its type.
The sizing of bitfields now means that alignment can add
padding to a union. For instance, a uint32_t x : 17 bitfield
has size 3, but alignment 4, requiring one byte of alignment
padding in the union so that its size is 4. This means that we
must now do the padding alignment calculation for unions
to adjust the size, just like we do for structures.

2 files changed, 24 insertions, 22 deletions

diff --git a/ffi.c b/ffi.c
index d8522d89..821c7685 100644
--- a/ffi.c
+++ b/ffi.c
@@ -3293,6 +3293,7 @@ static val make_ffi_type_struct(val syntax, val lisp_type,
       ucnum unit_offs = offs & align_mask;
       ucnum bits_alloc = 8 * (offs - unit_offs) + bit_offs;
       ucnum room = bits_type - bits_alloc;
+      ucnum align = if3(slot, mtft->align, 1);
 
       if (bits == 0) {
         if (offs != unit_offs || bit_offs > 0)
@@ -3307,11 +3308,6 @@ static val make_ffi_type_struct(val syntax, val lisp_type,
         bit_offs = bits_alloc = 0;
       }
 
-      if (bits_alloc == 0) {
-        if (most_align < (ucnum) mtft->align)
-          most_align = mtft->align;
-      }
-
       memb[i].offs = offs;
 
 #if HAVE_LITTLE_ENDIAN
@@ -3326,6 +3322,9 @@ static val make_ffi_type_struct(val syntax, val lisp_type,
       bit_offs += bits;
       offs += bit_offs / 8;
       bit_offs %= 8;
+
+      if (most_align < align)
+        most_align = align;
     } else {
       ucnum align = mtft->align;
       ucnum almask = align - 1;
@@ -3451,14 +3450,10 @@ static val make_ffi_type_union(val syntax, val use_existing, val self)
     setcheck(obj, slot);
     setcheck(obj, type);
 
-    if (most_align < (ucnum) mtft->align)
-      most_align = mtft->align;
-
-    if (biggest_size < (ucnum) mtft->size)
-      biggest_size = mtft->size;
-
     if (mtft->bitfield) {
       ucnum bits = mtft->nelem;
+      ucnum size = (bits + 7) / 8;
+      ucnum align = if3(slot, mtft->align, 1);
 
       if (bits == 0) {
         nmemb--, i--;
@@ -3474,6 +3469,16 @@ static val make_ffi_type_union(val syntax, val use_existing, val self)
         mtft->mask = UINT_MAX;
       else
         mtft->mask = ((1U << bits) - 1) << mtft->shift;
+
+      if (most_align < align)
+        most_align = align;
+      if (biggest_size < size)
+        biggest_size = size;
+    } else {
+      if (most_align < (ucnum) mtft->align)
+        most_align = mtft->align;
+      if (biggest_size < (ucnum) mtft->size)
+        biggest_size = mtft->size;
     }
   }
 
@@ -3486,7 +3491,7 @@ static val make_ffi_type_union(val syntax, val use_existing, val self)
 
   tft->nelem = i;
 
-  tft->size = biggest_size;
+  tft->size = (biggest_size + most_align - 1) & ~(most_align - 1);
   tft->align = most_align;
 
 #if HAVE_LIBFFI
diff --git a/txr.1 b/txr.1
index 4497be5c..dea21da5 100644
--- a/txr.1
+++ b/txr.1
@@ -69387,16 +69387,6 @@ bitfield, according to the bitfield's width, then the bitfield is
 placed into that unit. Otherwise, the bitfield is placed into the
 next available unit.
 
-Whenever a bitfield is placed at the start of new unit of its size,
-it is called the
-.IR "leading bitfield" .
-The alignment of the leading bitfield is taken into account for
-the purposes of determining the most strictly aligned member of
-the structure. This alignment property of the leading bitfield
-can be altered with the
-.code align
-type operator, applied either to the bitfield type, or to its base type.
-
 After a bitfield is placed, the values of
 .I O
 and
@@ -69432,6 +69422,13 @@ is incremented by one to cover that byte.
 As the last allocation step, the size of the structure is then padded up to a
 size which is a multiple of the alignment of the most strictly aligned member.
 
+A named bitfield contributes to the alignment of the structure, according to
+its type, the same way as a non-bitfield member of the same type.
+An unnamed bitfield doesn't contribute alignment, or else may be regarded as
+having the weakest possible alignment, which is byte alignment.
+If all of the members of a structure are unnamed bitfield members of any type,
+it exhibits byte alignment.
+
 The description isn't complete without a treatment of byte and bit order.
 Bitfield allocation follows an imaginary "bit endianness" whose direction
 follows the machine's byte order: most significant bits are allocated first on