[PATCH] Fix build with GCC 7.5

[PATCH] Fix build with GCC 7.5

[Tom Tromey]

A recent change to 'struct field' caused a build failure with GCC
7.5.0, as reported by Tom de Vries:

/data/vries/gdb/src/gdb/gdbtypes.h:721:51: error:
‘field::m_accessibility’ is too small to hold all values of ‘enum
class accessibility’ [-Werror]
   ENUM_BITFIELD (accessibility) m_accessibility : 2;
                                                   ^

Mark Wielaard pointed out that this was a GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242

This patch works around the bug by changing this member not to be a
bitfield.  It reduces the size of the enum a bit instead.  I also
changed m_bitsize to no longer be a bitfield -- that was done for
packing reasons in ancient times, but with m_accessibility not being a
bitfield, this no longer matters.

This patch does not change the size of struct field on 64-bit hosts.
---
 gdb/gdbtypes.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 34ca1ac99e5..411916e7b3f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -534,7 +534,7 @@ union field_location
 };
 
 /* Accessibility of a member.  */
-enum class accessibility : unsigned
+enum class accessibility : unsigned char
 {
   /* It's important that this be 0 so that fields default to
      public.  */
@@ -717,8 +717,6 @@ struct field
 
   unsigned int m_artificial : 1;
 
-  /* Accessibility of the field.  */
-  ENUM_BITFIELD (accessibility) m_accessibility : 2;
   /* Whether the field is 'virtual'.  */
   bool m_virtual : 1;
   /* Whether the field is 'ignored'.  */
@@ -728,13 +726,16 @@ struct field
 
   ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
 
+  /* Accessibility of the field.  */
+  enum accessibility m_accessibility;
+
   /* * Size of this field, in bits, or zero if not packed.
      If non-zero in an array type, indicates the element size in
      bits (used only in Ada at the moment).
      For an unpacked field, the field's type's length
      says how many bytes the field occupies.  */
 
-  unsigned int m_bitsize : 28;
+  unsigned int m_bitsize;
 
   /* * In a struct or union type, type of this field.
      - In a function or member type, type of this argument.
-- 
2.41.0

Re: [PATCH] Fix build with GCC 7.5

[Tom de Vries]

On 11/22/23 15:03, Tom Tromey wrote:
> A recent change to 'struct field' caused a build failure with GCC
> 7.5.0, as reported by Tom de Vries:
> 
> /data/vries/gdb/src/gdb/gdbtypes.h:721:51: error:
> ‘field::m_accessibility’ is too small to hold all values of ‘enum
> class accessibility’ [-Werror]
>     ENUM_BITFIELD (accessibility) m_accessibility : 2;
>                                                     ^
> 
> Mark Wielaard pointed out that this was a GCC bug:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242
> 
> This patch works around the bug by changing this member not to be a
> bitfield.  It reduces the size of the enum a bit instead.  I also
> changed m_bitsize to no longer be a bitfield -- that was done for
> packing reasons in ancient times, but with m_accessibility not being a
> bitfield, this no longer matters.
> 
> This patch does not change the size of struct field on 64-bit hosts.

Hi,

thanks for looking into this.

This fixes one of the three warnings.

But I still get:
...
/data/vries/gdb/src/gdb/gdbtypes.h:1616:49: error: 
‘fn_field::accessibility’ is too small to hold all values of ‘enum class 
accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) accessibility : 2;
                                                  ^
/data/vries/gdb/src/gdb/gdbtypes.h:1662:49: error: 
‘decl_field::accessibility’ is too small to hold all values of ‘enum 
class accessibility’ [-Werror]
    ENUM_BITFIELD (accessibility) accessibility : 2;
                                                  ^
...

Thanks,
- Tom

Re: [PATCH] Fix build with GCC 7.5

[Tom Tromey]

Tom> This fixes one of the three warnings.

Indeed, duh.  Sorry about that.

Here is v2.

Tom

commit 0447f3d09267f6fb90237b42aa6e1ec137a09c87
Author: Tom Tromey <tromey@adacore.com>
Date:   Wed Nov 22 06:54:40 2023 -0700

    Fix build with GCC 7.5
    
    A recent change to 'struct field' caused a build failure with GCC
    7.5.0, as reported by Tom de Vries:
    
    /data/vries/gdb/src/gdb/gdbtypes.h:721:51: error:
    ‘field::m_accessibility’ is too small to hold all values of ‘enum
    class accessibility’ [-Werror]
       ENUM_BITFIELD (accessibility) m_accessibility : 2;
                                                       ^
    
    Mark Wielaard pointed out that this was a GCC bug:
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242
    
    This patch works around the bug by changing several members not to be
    bitfields.  It reduces the size of the enum's underlying type,
    instead.
    
    I also changed m_bitsize to no longer be a bitfield -- that was done
    for packing reasons in ancient times, but with m_accessibility not
    being a bitfield, this no longer matters.
    
    I removed fn_field::dummy.  In earlier times it was somewhat normal in
    gdb to have these dummy fields to keep track of any available padding.
    However, since the advent of "ptype/o", there doesn't seem to be any
    need for this.
    
    This patch does not change the size of struct field, fn_field, or
    decl_field on 64-bit hosts.

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 34ca1ac99e5..eca92196364 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -534,7 +534,7 @@ union field_location
 };
 
 /* Accessibility of a member.  */
-enum class accessibility : unsigned
+enum class accessibility : unsigned char
 {
   /* It's important that this be 0 so that fields default to
      public.  */
@@ -717,8 +717,6 @@ struct field
 
   unsigned int m_artificial : 1;
 
-  /* Accessibility of the field.  */
-  ENUM_BITFIELD (accessibility) m_accessibility : 2;
   /* Whether the field is 'virtual'.  */
   bool m_virtual : 1;
   /* Whether the field is 'ignored'.  */
@@ -728,13 +726,16 @@ struct field
 
   ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
 
+  /* Accessibility of the field.  */
+  enum accessibility m_accessibility;
+
   /* * Size of this field, in bits, or zero if not packed.
      If non-zero in an array type, indicates the element size in
      bits (used only in Ada at the moment).
      For an unpacked field, the field's type's length
      says how many bytes the field occupies.  */
 
-  unsigned int m_bitsize : 28;
+  unsigned int m_bitsize;
 
   /* * In a struct or union type, type of this field.
      - In a function or member type, type of this argument.
@@ -1611,8 +1612,6 @@ struct fn_field
 
   unsigned int is_const:1;
   unsigned int is_volatile:1;
-  /* Accessibility of the field.  */
-  ENUM_BITFIELD (accessibility) accessibility : 2;
   unsigned int is_artificial:1;
 
   /* * A stub method only has some fields valid (but they are enough
@@ -1633,9 +1632,8 @@ struct fn_field
 
   ENUM_BITFIELD (dwarf_defaulted_attribute) defaulted : 2;
 
-  /* * Unused.  */
-
-  unsigned int dummy:6;
+  /* Accessibility of the field.  */
+  enum accessibility accessibility;
 
   /* * Index into that baseclass's virtual function table, minus 2;
      else if static: VOFFSET_STATIC; else: 0.  */
@@ -1658,7 +1656,7 @@ struct decl_field
   struct type *type;
 
   /* Accessibility of the field.  */
-  ENUM_BITFIELD (accessibility) accessibility : 2;
+  enum accessibility accessibility;
 };
 
 /* * C++ language-specific information for TYPE_CODE_STRUCT and

Re: [PATCH] Fix build with GCC 7.5

[Tom de Vries]

On 11/22/23 16:17, Tom Tromey wrote:
> Tom> This fixes one of the three warnings.
> 
> Indeed, duh.  Sorry about that.
> 
> Here is v2.
> 

With this patch I was able to finish the build using system gcc 7.5.0.

Thanks,
- Tom

> Tom
> 
> commit 0447f3d09267f6fb90237b42aa6e1ec137a09c87
> Author: Tom Tromey <tromey@adacore.com>
> Date:   Wed Nov 22 06:54:40 2023 -0700
> 
>      Fix build with GCC 7.5
>      
>      A recent change to 'struct field' caused a build failure with GCC
>      7.5.0, as reported by Tom de Vries:
>      
>      /data/vries/gdb/src/gdb/gdbtypes.h:721:51: error:
>      ‘field::m_accessibility’ is too small to hold all values of ‘enum
>      class accessibility’ [-Werror]
>         ENUM_BITFIELD (accessibility) m_accessibility : 2;
>                                                         ^
>      
>      Mark Wielaard pointed out that this was a GCC bug:
>      https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242
>      
>      This patch works around the bug by changing several members not to be
>      bitfields.  It reduces the size of the enum's underlying type,
>      instead.
>      
>      I also changed m_bitsize to no longer be a bitfield -- that was done
>      for packing reasons in ancient times, but with m_accessibility not
>      being a bitfield, this no longer matters.
>      
>      I removed fn_field::dummy.  In earlier times it was somewhat normal in
>      gdb to have these dummy fields to keep track of any available padding.
>      However, since the advent of "ptype/o", there doesn't seem to be any
>      need for this.
>      
>      This patch does not change the size of struct field, fn_field, or
>      decl_field on 64-bit hosts.
> 
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 34ca1ac99e5..eca92196364 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -534,7 +534,7 @@ union field_location
>   };
>   
>   /* Accessibility of a member.  */
> -enum class accessibility : unsigned
> +enum class accessibility : unsigned char
>   {
>     /* It's important that this be 0 so that fields default to
>        public.  */
> @@ -717,8 +717,6 @@ struct field
>   
>     unsigned int m_artificial : 1;
>   
> -  /* Accessibility of the field.  */
> -  ENUM_BITFIELD (accessibility) m_accessibility : 2;
>     /* Whether the field is 'virtual'.  */
>     bool m_virtual : 1;
>     /* Whether the field is 'ignored'.  */
> @@ -728,13 +726,16 @@ struct field
>   
>     ENUM_BITFIELD(field_loc_kind) m_loc_kind : 3;
>   
> +  /* Accessibility of the field.  */
> +  enum accessibility m_accessibility;
> +
>     /* * Size of this field, in bits, or zero if not packed.
>        If non-zero in an array type, indicates the element size in
>        bits (used only in Ada at the moment).
>        For an unpacked field, the field's type's length
>        says how many bytes the field occupies.  */
>   
> -  unsigned int m_bitsize : 28;
> +  unsigned int m_bitsize;
>   
>     /* * In a struct or union type, type of this field.
>        - In a function or member type, type of this argument.
> @@ -1611,8 +1612,6 @@ struct fn_field
>   
>     unsigned int is_const:1;
>     unsigned int is_volatile:1;
> -  /* Accessibility of the field.  */
> -  ENUM_BITFIELD (accessibility) accessibility : 2;
>     unsigned int is_artificial:1;
>   
>     /* * A stub method only has some fields valid (but they are enough
> @@ -1633,9 +1632,8 @@ struct fn_field
>   
>     ENUM_BITFIELD (dwarf_defaulted_attribute) defaulted : 2;
>   
> -  /* * Unused.  */
> -
> -  unsigned int dummy:6;
> +  /* Accessibility of the field.  */
> +  enum accessibility accessibility;
>   
>     /* * Index into that baseclass's virtual function table, minus 2;
>        else if static: VOFFSET_STATIC; else: 0.  */
> @@ -1658,7 +1656,7 @@ struct decl_field
>     struct type *type;
>   
>     /* Accessibility of the field.  */
> -  ENUM_BITFIELD (accessibility) accessibility : 2;
> +  enum accessibility accessibility;
>   };
>   
>   /* * C++ language-specific information for TYPE_CODE_STRUCT and