Packing of union fields and bool_bf

Packing of union fields and bool_bf

[Eli Zaretskii]

While working on bug#33017, I found out something unexpected: using
bool_bf in union members bloats the union.

A case in point is this part of 'struct glyph' (I removed the comments
for brevity):

  /* A union of sub-structures for different glyph types.  */
  union
  {
    unsigned ch;
    struct
    {
      bool_bf automatic : 1;
      unsigned id    : 31;
    } cmp;
    int img_id;
#ifdef HAVE_XWIDGETS
    struct xwidget *xwidget;
#endif
    struct
    {
      unsigned height  : 16;
      unsigned ascent  : 16;
    }
    stretch;
    struct
    {
      unsigned method : 2;
      bool_bf for_no_font : 1;
      unsigned len : 4;
      unsigned ch : 25;
    } glyphless;
    unsigned val;
  } u;

Here's how this is compiled with 32-bit MinGW GCC 7.3.0, according to
GDB:

  (gdb) ptype /o desired_glyph->u
  /* offset    |  size */  type = union {
  /*                 4 */    unsigned int ch;
  /*                 8 */    struct {
  /*    0: 7   |     1 */        bool_bf automatic : 1;
  /* XXX  7-bit hole  */
  /* XXX  3-byte hole */
  /*    4: 1   |     4 */        unsigned int id : 31;
  /* XXX  1-bit padding  */

				 /* total size (bytes):    8 */
			     } cmp;
  /*                 4 */    int img_id;
  /*                 4 */    struct {
  /*    0:16   |     4 */        unsigned int height : 16;
  /*    0: 0   |     4 */        unsigned int ascent : 16;

				 /* total size (bytes):    4 */
			     } stretch;
  /*                12 */    struct {
  /*    0:30   |     4 */        unsigned int method : 2;
  /* XXX  6-bit hole  */
  /* XXX  3-byte hole */
  /*    4: 7   |     1 */        bool_bf for_no_font : 1;
  /* XXX  7-bit hole  */
  /* XXX  3-byte hole */
  /*    8:28   |     4 */        unsigned int len : 4;
  /*    8: 3   |     4 */        unsigned int ch : 25;
  /* XXX  3-bit padding  */

				 /* total size (bytes):   12 */
			     } glyphless;
  /*                 4 */    unsigned int val;

			     /* total size (bytes):   12 */
			   }

Note the difference between handling 'unsigned int' fields and
'bool_bf' fields: the former are nicely packed, whereas the latter are
not.  Which causes the size of union to be 12 bytes, instead of the
expected 4.  The 'bool' type is 1-byte wide on this platform.

Is this expected?  Should we continue using bool_bf in these cases?

Re: Packing of union fields and bool_bf

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> While working on bug#33017, I found out something unexpected: using
Eli> bool_bf in union members bloats the union.
...
Eli> Note the difference between handling 'unsigned int' fields and
Eli> 'bool_bf' fields: the former are nicely packed, whereas the latter are
Eli> not.  Which causes the size of union to be 12 bytes, instead of the
Eli> expected 4.  The 'bool' type is 1-byte wide on this platform.

Eli> Is this expected?  Should we continue using bool_bf in these cases?

I think the MS ABI has special rules about packing bit fields.
From https://docs.microsoft.com/en-us/cpp/c-language/padding-and-alignment-of-structure-members?view=vs-2017:

    Adjacent bit fields are packed into the same 1-, 2-, or 4-byte
    allocation unit if the integral types are the same size and if the next
    bit field fits into the current allocation unit without crossing the
    boundary imposed by the common alignment requirements of the bit fields.

So in this case, I think if the underlying type of bool_bf is "unsigned int",
then it will continue to pack as expected.

Tom

Re: Packing of union fields and bool_bf

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

On 10/13/2018 10:58 AM, Tom Tromey wrote:
> From https://docs.microsoft.com/en-us/cpp/c-language/padding-and-alignment-of-structure-members?view=vs-2017:
>
>     Adjacent bit fields are packed into the same 1-, 2-, or 4-byte
>     allocation unit if the integral types are the same size


It's silly to require the integral types to be the same size, since the
actual size is given by the number of bits in the bitfield. But if it's
the Microsoft API we can't change it. Does the attached patch work
around the problem?


[-- Attachment #2: emacs.diff --]
[-- Type: text/x-patch, Size: 804 bytes --]

diff --git a/src/conf_post.h b/src/conf_post.h
index 683a96f936..dbe5befcf9 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -54,10 +54,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
     ((v) < __GNUC__ + ((w) < __GNUC_MINOR__ + ((x) <= __GNUC_PATCHLEVEL__)))
 #endif
 
-/* The type of bool bitfields.  Needed to compile Objective-C with
-   standard GCC.  It was also needed to port to pre-C99 compilers,
-   although we don't care about that any more.  */
-#if NS_IMPL_GNUSTEP
+/* The type of bool bitfields.  Preferably bool for better debugging,
+   bu unsigned int on MS-Windows to avoid struct bloat, and unsigned
+   int for Objective-C with standard GCC.  */
+#if defined DOS_NT || NS_IMPL_GNUSTEP
 typedef unsigned int bool_bf;
 #else
 typedef bool bool_bf;

Re: Packing of union fields and bool_bf

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 13 Oct 2018 11:22:31 -0500
> 
> >     Adjacent bit fields are packed into the same 1-, 2-, or 4-byte
> >     allocation unit if the integral types are the same size
> 
> 
> It's silly to require the integral types to be the same size, since the
> actual size is given by the number of bits in the bitfield. But if it's
> the Microsoft API we can't change it. Does the attached patch work
> around the problem?

Yes, I installed something similar (__MINGW32__ instead of DOS_NT, as
I don't think the MSDOS build needs this).

Thanks.

Re: Packing of union fields and bool_bf

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 13 Oct 2018 09:58:00 -0600
> 
> So in this case, I think if the underlying type of bool_bf is "unsigned int",
> then it will continue to pack as expected.

Thanks, that works.