RFA/types: Clean up use of field bitsize

RFA/types: Clean up use of field bitsize

[Daniel Jacobowitz]

Right now, we have this really disturbing comment:

     /* Size of this field, in bits, or zero if not packed.
        For an unpacked field, the field's type's length
        says how many bytes the field occupies.
        A value of -1 or -2 indicates a static field;  -1 means the location
        is specified by the label loc.physname;  -2 means that loc.physaddr
        specifies the actual address. */

Think about this for a moment.  While in practice a static member is never
going to be packed, and in at least C++ can not be a bit-field, that's not
logically obvious for other languages.  I don't know Ada but I wouldn't be
surprised if there were some construct which violated this assumption.

Worse, all sorts of places don't check for negative bitsize at all.  It may
be that they're all safe - I didn't spend a lot of time working out problem
cases - but I have my doubts.

So, since I needed to gain a new field here anyway, and since I have no
compunctions about shrinking this field a little (packed bitfields of size
greater than a couple of words are allowed in some languages IIRC (including
GNU C maybe?  Although they are not allowed in ISO C99), but they're
definitely dodgy), and since signed bitfields are not portable, I cleaned up
the construct.  It turned out to be painless except for making sure symbol
readers initialized it, which was a little tedious.

This patch:
  Moves 'artificial' out from 'loc' and makes it a bitfield
  Creates a 'static_kind' bitfield
  Makes 'bitsize' into a bitfield

The goal is to allow more kinds of fields to be marked artificial -
particularly data members.  After this patch I'll submit the followup to
mark DW_AT_artificial members as artificial types.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2002-09-29  Daniel Jacobowitz  <drow@mvista.com>

	* gdbtypes.h (struct main_type): Move artificial flag out of
	loc.  New member of ``struct field'' named static_kind.  Reduce
	overloaded meaning of bitsize.
	(FIELD_ARTIFICIAL, SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR)
	(TYPE_FIELD_STATIC, TYPE_FIELD_STATIC_HAS_ADDR): Likewise.
	(FIELD_STATIC_KIND, TYPE_FIELD_STATIC_KIND): New macros.

	* ada-lang.c (fill_in_ada_prototype): Initialize static_kind for
	new fields.
	(template_to_fixed_record_type, template_to_static_fixed_type)
	(to_record_with_fixed_variant_part): Likewise.
	* coffread.c (coff_read_struct_type, coff_read_enum_type): Likewise.
	* dwarf2read.c (dwarf2_add_field, read_enumeration): Likewise.
	* dwarfread.c (struct_type, enum_type): Likewise.
	* hpread.c (hpread_read_enum_type)
	(hpread_read_function_type, hpread_read_doc_function_type)
	(hpread_read_struct_type): Likewise.
	* mdebugread.c (parse_symbol): Likewise.

Index: ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.9
diff -u -p -r1.9 ada-lang.c
--- ada-lang.c	8 Sep 2002 17:43:26 -0000	1.9
+++ ada-lang.c	30 Sep 2002 00:43:30 -0000
@@ -4189,6 +4189,7 @@ fill_in_ada_prototype (struct symbol *fu
       case LOC_REGPARM_ADDR:
 	TYPE_FIELD_BITPOS (ftype, nargs) = nargs;
 	TYPE_FIELD_BITSIZE (ftype, nargs) = 0;
+	TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0;
 	TYPE_FIELD_TYPE (ftype, nargs) =
 	  lookup_pointer_type (check_typedef (SYMBOL_TYPE (sym)));
 	TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym);
@@ -4202,6 +4203,7 @@ fill_in_ada_prototype (struct symbol *fu
       case LOC_BASEREG_ARG:
 	TYPE_FIELD_BITPOS (ftype, nargs) = nargs;
 	TYPE_FIELD_BITSIZE (ftype, nargs) = 0;
+	TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0;
 	TYPE_FIELD_TYPE (ftype, nargs) = check_typedef (SYMBOL_TYPE (sym));
 	TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym);
 	nargs += 1;
@@ -6046,6 +6048,7 @@ template_to_fixed_record_type (struct ty
        * rediscover why we needed field_offset and fix it properly. */
       TYPE_FIELD_BITPOS (rtype, f) = off;
       TYPE_FIELD_BITSIZE (rtype, f) = 0;
+      TYPE_FIELD_STATIC_KIND (rtype, f) = 0;
 
       if (ada_is_variant_part (type, f))
 	{
@@ -6149,6 +6152,7 @@ template_to_static_fixed_type (struct ty
     {
       TYPE_FIELD_BITPOS (type, f) = 0;
       TYPE_FIELD_BITSIZE (type, f) = 0;
+      TYPE_FIELD_STATIC_KIND (type, f) = 0;
 
       if (is_dynamic_field (templ_type, f))
 	{
@@ -6218,6 +6222,7 @@ to_record_with_fixed_variant_part (struc
       TYPE_FIELD_TYPE (rtype, nfields - 1) = branch_type;
       TYPE_FIELD_NAME (rtype, nfields - 1) = "S";
       TYPE_FIELD_BITSIZE (rtype, nfields - 1) = 0;
+      TYPE_FIELD_STATIC_KIND (rtype, nfields - 1) = 0;
       TYPE_LENGTH (rtype) += TYPE_LENGTH (branch_type);
       -TYPE_LENGTH (TYPE_FIELD_TYPE (type, nfields - 1));
     }
Index: coffread.c
===================================================================
RCS file: /cvs/src/src/gdb/coffread.c,v
retrieving revision 1.29
diff -u -p -r1.29 coffread.c
--- coffread.c	22 Aug 2002 05:50:11 -0000	1.29
+++ coffread.c	30 Sep 2002 00:43:30 -0000
@@ -1997,6 +1997,7 @@ coff_read_struct_type (int index, int le
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux);
 	  FIELD_BITPOS (list->field) = 8 * ms->c_value;
 	  FIELD_BITSIZE (list->field) = 0;
+	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -2015,6 +2016,7 @@ coff_read_struct_type (int index, int le
 	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux);
 	  FIELD_BITPOS (list->field) = ms->c_value;
 	  FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size;
+	  FIELD_STATIC_KIND (list->field) = 0;
 	  nfields++;
 	  break;
 
@@ -2135,6 +2137,7 @@ coff_read_enum_type (int index, int leng
 	  if (SYMBOL_VALUE (xsym) < 0)
 	    unsigned_enum = 0;
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
+	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
       if (syms == osyms)
 	break;
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.66
diff -u -p -r1.66 dwarf2read.c
--- dwarf2read.c	3 Sep 2002 17:32:11 -0000	1.66
+++ dwarf2read.c	30 Sep 2002 00:43:33 -0000
@@ -2055,6 +2055,8 @@ dwarf2_add_field (struct field_info *fip
       /* Get type of field.  */
       fp->type = die_type (die, objfile, cu_header);
 
+      FIELD_STATIC_KIND (*fp) = 0;
+
       /* Get bit size of field (zero if none).  */
       attr = dwarf_attr (die, DW_AT_bit_size);
       if (attr)
@@ -2163,6 +2165,7 @@ dwarf2_add_field (struct field_info *fip
 	FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), objfile, cu_header)
 			      * bits_per_byte);
       FIELD_BITSIZE (*fp) = 0;
+      FIELD_STATIC_KIND (*fp) = 0;
       FIELD_TYPE (*fp) = die_type (die, objfile, cu_header);
       FIELD_NAME (*fp) = type_name_no_tag (fp->type);
       fip->nbaseclasses++;
@@ -2667,6 +2670,7 @@ read_enumeration (struct die_info *die, 
 		  FIELD_TYPE (fields[num_fields]) = NULL;
 		  FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
 		  FIELD_BITSIZE (fields[num_fields]) = 0;
+		  FIELD_STATIC_KIND (fields[num_fields]) = 0;
 
 		  num_fields++;
 		}
Index: dwarfread.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarfread.c,v
retrieving revision 1.14
diff -u -p -r1.14 dwarfread.c
--- dwarfread.c	1 Aug 2002 17:18:32 -0000	1.14
+++ dwarfread.c	30 Sep 2002 00:43:33 -0000
@@ -1027,6 +1027,7 @@ struct_type (struct dieinfo *dip, char *
 			  &objfile->type_obstack);
 	  FIELD_TYPE (list->field) = decode_die_type (&mbr);
 	  FIELD_BITPOS (list->field) = 8 * locval (&mbr);
+	  FIELD_STATIC_KIND (list->field) = 0;
 	  /* Handle bit fields. */
 	  FIELD_BITSIZE (list->field) = mbr.at_bit_size;
 	  if (BITS_BIG_ENDIAN)
@@ -1694,6 +1695,7 @@ enum_type (struct dieinfo *dip, struct o
 	  list = new;
 	  FIELD_TYPE (list->field) = NULL;
 	  FIELD_BITSIZE (list->field) = 0;
+	  FIELD_STATIC_KIND (list->field) = 0;
 	  FIELD_BITPOS (list->field) =
 	    target_to_host (scan, TARGET_FT_LONG_SIZE (objfile), GET_SIGNED,
 			    objfile);
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.36
diff -u -p -r1.36 gdbtypes.h
--- gdbtypes.h	14 Sep 2002 02:09:39 -0000	1.36
+++ gdbtypes.h	30 Sep 2002 00:43:34 -0000
@@ -381,22 +381,25 @@ struct main_type
 
       CORE_ADDR physaddr;
       char *physname;
-
-      /* For a function or member type, this is 1 if the argument is marked
-	 artificial.  Artificial arguments should not be shown to the
-	 user.  */
-      int artificial;
     }
     loc;
 
+    /* For a function or member type, this is 1 if the argument is marked
+       artificial.  Artificial arguments should not be shown to the
+       user.  */
+    unsigned int artificial : 1;
+
+    /* This flag is zero for non-static fields, 1 for fields whose location
+       is specified by the label loc.physname, and 2 for fields whose location
+       is specified by loc.physaddr.  */
+
+    unsigned int static_kind : 2;
+
     /* Size of this field, in bits, or zero if not packed.
        For an unpacked field, the field's type's length
-       says how many bytes the field occupies.
-       A value of -1 or -2 indicates a static field;  -1 means the location
-       is specified by the label loc.physname;  -2 means that loc.physaddr
-       specifies the actual address. */
+       says how many bytes the field occupies.  */
 
-    int bitsize;
+    unsigned int bitsize : 29;
 
     /* In a struct or union type, type of this field.
        In a function or member type, type of this argument.
@@ -793,14 +796,15 @@ extern void allocate_cplus_struct_type (
 #define FIELD_TYPE(thisfld) ((thisfld).type)
 #define FIELD_NAME(thisfld) ((thisfld).name)
 #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
-#define FIELD_ARTIFICIAL(thisfld) ((thisfld).loc.artificial)
+#define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial)
 #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize)
+#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
 #define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
 #define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
 #define SET_FIELD_PHYSNAME(thisfld, name) \
-  ((thisfld).bitsize = -1, FIELD_PHYSNAME(thisfld) = (name))
+  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
 #define SET_FIELD_PHYSADDR(thisfld, name) \
-  ((thisfld).bitsize = -2, FIELD_PHYSADDR(thisfld) = (name))
+  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
 #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n]
 #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
@@ -840,8 +844,9 @@ extern void allocate_cplus_struct_type (
   (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
     : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
 
-#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize < 0)
-#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize == -2)
+#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
+#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
+#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
 #define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
 #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
 
Index: hpread.c
===================================================================
RCS file: /cvs/src/src/gdb/hpread.c,v
retrieving revision 1.22
diff -u -p -r1.22 hpread.c
--- hpread.c	4 Aug 2002 19:11:12 -0000	1.22
+++ hpread.c	30 Sep 2002 00:43:35 -0000
@@ -3186,6 +3186,7 @@ hpread_read_enum_type (dnttpointer hp_ty
 	  TYPE_FIELD_NAME (type, n) = SYMBOL_NAME (xsym);
 	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
+	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
       if (syms == osyms)
 	break;
@@ -3347,6 +3348,7 @@ hpread_read_function_type (dnttpointer h
 	  TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym);
 	  TYPE_FIELD_ARTIFICIAL (type, n) = 0;
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
+	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
     }
   /* Mark it as having been processed */
@@ -3520,6 +3522,7 @@ hpread_read_doc_function_type (dnttpoint
 	  TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym);
 	  TYPE_FIELD_ARTIFICIAL (type, n) = 0;
 	  TYPE_FIELD_BITSIZE (type, n) = 0;
+	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
 	}
     }
 
@@ -3704,6 +3707,7 @@ hpread_read_struct_type (dnttpointer hp_
 	  list = new;
 
 	  FIELD_BITSIZE (list->field) = 0;
+	  FIELD_STATIC_KIND (list->field) = 0;
 
 	  /* The "classname" field is actually a DNTT pointer to the base class */
 	  baseclass = hpread_type_lookup (parentp->dinheritance.classname,
@@ -4101,6 +4105,7 @@ hpread_read_struct_type (dnttpointer hp_
 	      list->field.name = VT (objfile) + fn_fieldp->dsvar.name;
 	      FIELD_BITPOS (list->field) = 0;	/* FIXME is this always true? */
 	      FIELD_BITSIZE (list->field) = 0;	/* use length from type */
+	      FIELD_STATIC_KIND (list->field) = 0;
 	      memtype = hpread_type_lookup (fn_fieldp->dsvar.type, objfile);
 	      list->field.type = memtype;
 	      list->attributes = 0;
@@ -4120,6 +4125,7 @@ hpread_read_struct_type (dnttpointer hp_
 	      list->field.name = VT (objfile) + fn_fieldp->ddvar.name;
 	      FIELD_BITPOS (list->field) = 0;	/* FIXME is this always true? */
 	      FIELD_BITSIZE (list->field) = 0;	/* use length from type */
+	      FIELD_STATIC_KIND (list->field) = 0;
 	      memtype = hpread_type_lookup (fn_fieldp->ddvar.type, objfile);
 	      list->field.type = memtype;
 	      list->attributes = 0;
@@ -4168,6 +4174,7 @@ hpread_read_struct_type (dnttpointer hp_
 
 
 	  /* A FIELD by itself (without a GENFIELD) can also be a static member */
+	  FIELD_STATIC_KIND (list->field) = 0;
 	  if (fieldp->dfield.staticmem)
 	    {
 	      FIELD_BITPOS (list->field) = -1;
Index: mdebugread.c
===================================================================
RCS file: /cvs/src/src/gdb/mdebugread.c,v
retrieving revision 1.29
diff -u -p -r1.29 mdebugread.c
--- mdebugread.c	18 Sep 2002 20:47:39 -0000	1.29
+++ mdebugread.c	30 Sep 2002 00:43:36 -0000
@@ -1092,6 +1092,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
 		FIELD_TYPE (*f) = t;
 		FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss;
 		FIELD_BITSIZE (*f) = 0;
+		FIELD_STATIC_KIND (*f) = 0;
 
 		enum_sym = ((struct symbol *)
 			    obstack_alloc (&current_objfile->symbol_obstack,
@@ -1284,6 +1285,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
       bitsize = 0;
       FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name);
       FIELD_BITSIZE (*f) = bitsize;
+      FIELD_STATIC_KIND (*f) = 0;
       break;
 
     case stIndirect:		/* forward declaration on Irix5 */

Re: RFA/types: Clean up use of field bitsize

[Paul N. Hilfinger]

> So, since I needed to gain a new field here anyway, and since I have no
> compunctions about shrinking this field a little (packed bitfields of size
> greater than a couple of words are allowed in some languages IIRC (including
> GNU C maybe?  Although they are not allowed in ISO C99), but they're
> definitely dodgy), and since signed bitfields are not portable, I cleaned up
> the construct.  It turned out to be painless except for making sure symbol
> readers initialized it, which was a little tedious.

Well, as it happens, Ada IS able to handle bit fields of arbitrary size.
However, given that you have left 29 bits (unsigned), and that the gain from
decreasing one's memory usage by 0.000006% (31 bits of 2**29) is insignificant
compared to the enormous cost of extracting 2**29-bit, arbitrarily aligned
fields, I'd say that this change is reasonable (:->).

P. Hilfinger
Ada Core Technologies, Inc.

Re: RFA/types: Clean up use of field bitsize

[Daniel Jacobowitz]

Does anyone have a comment on this patch?  If not, I'll commit it in a
couple of days, after I'm added to the global write list.

(The type code has no specific maintainer, the debug reader and
language parts I consider obvious, and the patch is over a month old
now.)

On Sun, Sep 29, 2002 at 09:05:15PM -0400, Daniel Jacobowitz wrote:
> Right now, we have this really disturbing comment:
> 
>      /* Size of this field, in bits, or zero if not packed.
>         For an unpacked field, the field's type's length
>         says how many bytes the field occupies.
>         A value of -1 or -2 indicates a static field;  -1 means the location
>         is specified by the label loc.physname;  -2 means that loc.physaddr
>         specifies the actual address. */
> 
> Think about this for a moment.  While in practice a static member is never
> going to be packed, and in at least C++ can not be a bit-field, that's not
> logically obvious for other languages.  I don't know Ada but I wouldn't be
> surprised if there were some construct which violated this assumption.
> 
> Worse, all sorts of places don't check for negative bitsize at all.  It may
> be that they're all safe - I didn't spend a lot of time working out problem
> cases - but I have my doubts.
> 
> So, since I needed to gain a new field here anyway, and since I have no
> compunctions about shrinking this field a little (packed bitfields of size
> greater than a couple of words are allowed in some languages IIRC (including
> GNU C maybe?  Although they are not allowed in ISO C99), but they're
> definitely dodgy), and since signed bitfields are not portable, I cleaned up
> the construct.  It turned out to be painless except for making sure symbol
> readers initialized it, which was a little tedious.
> 
> This patch:
>   Moves 'artificial' out from 'loc' and makes it a bitfield
>   Creates a 'static_kind' bitfield
>   Makes 'bitsize' into a bitfield
> 
> The goal is to allow more kinds of fields to be marked artificial -
> particularly data members.  After this patch I'll submit the followup to
> mark DW_AT_artificial members as artificial types.
> 
> OK?
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2002-09-29  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* gdbtypes.h (struct main_type): Move artificial flag out of
> 	loc.  New member of ``struct field'' named static_kind.  Reduce
> 	overloaded meaning of bitsize.
> 	(FIELD_ARTIFICIAL, SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR)
> 	(TYPE_FIELD_STATIC, TYPE_FIELD_STATIC_HAS_ADDR): Likewise.
> 	(FIELD_STATIC_KIND, TYPE_FIELD_STATIC_KIND): New macros.
> 
> 	* ada-lang.c (fill_in_ada_prototype): Initialize static_kind for
> 	new fields.
> 	(template_to_fixed_record_type, template_to_static_fixed_type)
> 	(to_record_with_fixed_variant_part): Likewise.
> 	* coffread.c (coff_read_struct_type, coff_read_enum_type): Likewise.
> 	* dwarf2read.c (dwarf2_add_field, read_enumeration): Likewise.
> 	* dwarfread.c (struct_type, enum_type): Likewise.
> 	* hpread.c (hpread_read_enum_type)
> 	(hpread_read_function_type, hpread_read_doc_function_type)
> 	(hpread_read_struct_type): Likewise.
> 	* mdebugread.c (parse_symbol): Likewise.
> 
> Index: ada-lang.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/ada-lang.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 ada-lang.c
> --- ada-lang.c	8 Sep 2002 17:43:26 -0000	1.9
> +++ ada-lang.c	30 Sep 2002 00:43:30 -0000
> @@ -4189,6 +4189,7 @@ fill_in_ada_prototype (struct symbol *fu
>        case LOC_REGPARM_ADDR:
>  	TYPE_FIELD_BITPOS (ftype, nargs) = nargs;
>  	TYPE_FIELD_BITSIZE (ftype, nargs) = 0;
> +	TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0;
>  	TYPE_FIELD_TYPE (ftype, nargs) =
>  	  lookup_pointer_type (check_typedef (SYMBOL_TYPE (sym)));
>  	TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym);
> @@ -4202,6 +4203,7 @@ fill_in_ada_prototype (struct symbol *fu
>        case LOC_BASEREG_ARG:
>  	TYPE_FIELD_BITPOS (ftype, nargs) = nargs;
>  	TYPE_FIELD_BITSIZE (ftype, nargs) = 0;
> +	TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0;
>  	TYPE_FIELD_TYPE (ftype, nargs) = check_typedef (SYMBOL_TYPE (sym));
>  	TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym);
>  	nargs += 1;
> @@ -6046,6 +6048,7 @@ template_to_fixed_record_type (struct ty
>         * rediscover why we needed field_offset and fix it properly. */
>        TYPE_FIELD_BITPOS (rtype, f) = off;
>        TYPE_FIELD_BITSIZE (rtype, f) = 0;
> +      TYPE_FIELD_STATIC_KIND (rtype, f) = 0;
>  
>        if (ada_is_variant_part (type, f))
>  	{
> @@ -6149,6 +6152,7 @@ template_to_static_fixed_type (struct ty
>      {
>        TYPE_FIELD_BITPOS (type, f) = 0;
>        TYPE_FIELD_BITSIZE (type, f) = 0;
> +      TYPE_FIELD_STATIC_KIND (type, f) = 0;
>  
>        if (is_dynamic_field (templ_type, f))
>  	{
> @@ -6218,6 +6222,7 @@ to_record_with_fixed_variant_part (struc
>        TYPE_FIELD_TYPE (rtype, nfields - 1) = branch_type;
>        TYPE_FIELD_NAME (rtype, nfields - 1) = "S";
>        TYPE_FIELD_BITSIZE (rtype, nfields - 1) = 0;
> +      TYPE_FIELD_STATIC_KIND (rtype, nfields - 1) = 0;
>        TYPE_LENGTH (rtype) += TYPE_LENGTH (branch_type);
>        -TYPE_LENGTH (TYPE_FIELD_TYPE (type, nfields - 1));
>      }
> Index: coffread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/coffread.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 coffread.c
> --- coffread.c	22 Aug 2002 05:50:11 -0000	1.29
> +++ coffread.c	30 Sep 2002 00:43:30 -0000
> @@ -1997,6 +1997,7 @@ coff_read_struct_type (int index, int le
>  	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux);
>  	  FIELD_BITPOS (list->field) = 8 * ms->c_value;
>  	  FIELD_BITSIZE (list->field) = 0;
> +	  FIELD_STATIC_KIND (list->field) = 0;
>  	  nfields++;
>  	  break;
>  
> @@ -2015,6 +2016,7 @@ coff_read_struct_type (int index, int le
>  	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux);
>  	  FIELD_BITPOS (list->field) = ms->c_value;
>  	  FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size;
> +	  FIELD_STATIC_KIND (list->field) = 0;
>  	  nfields++;
>  	  break;
>  
> @@ -2135,6 +2137,7 @@ coff_read_enum_type (int index, int leng
>  	  if (SYMBOL_VALUE (xsym) < 0)
>  	    unsigned_enum = 0;
>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>  	}
>        if (syms == osyms)
>  	break;
> Index: dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 dwarf2read.c
> --- dwarf2read.c	3 Sep 2002 17:32:11 -0000	1.66
> +++ dwarf2read.c	30 Sep 2002 00:43:33 -0000
> @@ -2055,6 +2055,8 @@ dwarf2_add_field (struct field_info *fip
>        /* Get type of field.  */
>        fp->type = die_type (die, objfile, cu_header);
>  
> +      FIELD_STATIC_KIND (*fp) = 0;
> +
>        /* Get bit size of field (zero if none).  */
>        attr = dwarf_attr (die, DW_AT_bit_size);
>        if (attr)
> @@ -2163,6 +2165,7 @@ dwarf2_add_field (struct field_info *fip
>  	FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), objfile, cu_header)
>  			      * bits_per_byte);
>        FIELD_BITSIZE (*fp) = 0;
> +      FIELD_STATIC_KIND (*fp) = 0;
>        FIELD_TYPE (*fp) = die_type (die, objfile, cu_header);
>        FIELD_NAME (*fp) = type_name_no_tag (fp->type);
>        fip->nbaseclasses++;
> @@ -2667,6 +2670,7 @@ read_enumeration (struct die_info *die, 
>  		  FIELD_TYPE (fields[num_fields]) = NULL;
>  		  FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
>  		  FIELD_BITSIZE (fields[num_fields]) = 0;
> +		  FIELD_STATIC_KIND (fields[num_fields]) = 0;
>  
>  		  num_fields++;
>  		}
> Index: dwarfread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarfread.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 dwarfread.c
> --- dwarfread.c	1 Aug 2002 17:18:32 -0000	1.14
> +++ dwarfread.c	30 Sep 2002 00:43:33 -0000
> @@ -1027,6 +1027,7 @@ struct_type (struct dieinfo *dip, char *
>  			  &objfile->type_obstack);
>  	  FIELD_TYPE (list->field) = decode_die_type (&mbr);
>  	  FIELD_BITPOS (list->field) = 8 * locval (&mbr);
> +	  FIELD_STATIC_KIND (list->field) = 0;
>  	  /* Handle bit fields. */
>  	  FIELD_BITSIZE (list->field) = mbr.at_bit_size;
>  	  if (BITS_BIG_ENDIAN)
> @@ -1694,6 +1695,7 @@ enum_type (struct dieinfo *dip, struct o
>  	  list = new;
>  	  FIELD_TYPE (list->field) = NULL;
>  	  FIELD_BITSIZE (list->field) = 0;
> +	  FIELD_STATIC_KIND (list->field) = 0;
>  	  FIELD_BITPOS (list->field) =
>  	    target_to_host (scan, TARGET_FT_LONG_SIZE (objfile), GET_SIGNED,
>  			    objfile);
> Index: gdbtypes.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtypes.h,v
> retrieving revision 1.36
> diff -u -p -r1.36 gdbtypes.h
> --- gdbtypes.h	14 Sep 2002 02:09:39 -0000	1.36
> +++ gdbtypes.h	30 Sep 2002 00:43:34 -0000
> @@ -381,22 +381,25 @@ struct main_type
>  
>        CORE_ADDR physaddr;
>        char *physname;
> -
> -      /* For a function or member type, this is 1 if the argument is marked
> -	 artificial.  Artificial arguments should not be shown to the
> -	 user.  */
> -      int artificial;
>      }
>      loc;
>  
> +    /* For a function or member type, this is 1 if the argument is marked
> +       artificial.  Artificial arguments should not be shown to the
> +       user.  */
> +    unsigned int artificial : 1;
> +
> +    /* This flag is zero for non-static fields, 1 for fields whose location
> +       is specified by the label loc.physname, and 2 for fields whose location
> +       is specified by loc.physaddr.  */
> +
> +    unsigned int static_kind : 2;
> +
>      /* Size of this field, in bits, or zero if not packed.
>         For an unpacked field, the field's type's length
> -       says how many bytes the field occupies.
> -       A value of -1 or -2 indicates a static field;  -1 means the location
> -       is specified by the label loc.physname;  -2 means that loc.physaddr
> -       specifies the actual address. */
> +       says how many bytes the field occupies.  */
>  
> -    int bitsize;
> +    unsigned int bitsize : 29;
>  
>      /* In a struct or union type, type of this field.
>         In a function or member type, type of this argument.
> @@ -793,14 +796,15 @@ extern void allocate_cplus_struct_type (
>  #define FIELD_TYPE(thisfld) ((thisfld).type)
>  #define FIELD_NAME(thisfld) ((thisfld).name)
>  #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
> -#define FIELD_ARTIFICIAL(thisfld) ((thisfld).loc.artificial)
> +#define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial)
>  #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize)
> +#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
>  #define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
>  #define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
>  #define SET_FIELD_PHYSNAME(thisfld, name) \
> -  ((thisfld).bitsize = -1, FIELD_PHYSNAME(thisfld) = (name))
> +  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
>  #define SET_FIELD_PHYSADDR(thisfld, name) \
> -  ((thisfld).bitsize = -2, FIELD_PHYSADDR(thisfld) = (name))
> +  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
>  #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n]
>  #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
>  #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
> @@ -840,8 +844,9 @@ extern void allocate_cplus_struct_type (
>    (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
>      : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
>  
> -#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize < 0)
> -#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize == -2)
> +#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
> +#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
> +#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
>  #define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
>  #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
>  
> Index: hpread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hpread.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 hpread.c
> --- hpread.c	4 Aug 2002 19:11:12 -0000	1.22
> +++ hpread.c	30 Sep 2002 00:43:35 -0000
> @@ -3186,6 +3186,7 @@ hpread_read_enum_type (dnttpointer hp_ty
>  	  TYPE_FIELD_NAME (type, n) = SYMBOL_NAME (xsym);
>  	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>  	}
>        if (syms == osyms)
>  	break;
> @@ -3347,6 +3348,7 @@ hpread_read_function_type (dnttpointer h
>  	  TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym);
>  	  TYPE_FIELD_ARTIFICIAL (type, n) = 0;
>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>  	}
>      }
>    /* Mark it as having been processed */
> @@ -3520,6 +3522,7 @@ hpread_read_doc_function_type (dnttpoint
>  	  TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym);
>  	  TYPE_FIELD_ARTIFICIAL (type, n) = 0;
>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>  	}
>      }
>  
> @@ -3704,6 +3707,7 @@ hpread_read_struct_type (dnttpointer hp_
>  	  list = new;
>  
>  	  FIELD_BITSIZE (list->field) = 0;
> +	  FIELD_STATIC_KIND (list->field) = 0;
>  
>  	  /* The "classname" field is actually a DNTT pointer to the base class */
>  	  baseclass = hpread_type_lookup (parentp->dinheritance.classname,
> @@ -4101,6 +4105,7 @@ hpread_read_struct_type (dnttpointer hp_
>  	      list->field.name = VT (objfile) + fn_fieldp->dsvar.name;
>  	      FIELD_BITPOS (list->field) = 0;	/* FIXME is this always true? */
>  	      FIELD_BITSIZE (list->field) = 0;	/* use length from type */
> +	      FIELD_STATIC_KIND (list->field) = 0;
>  	      memtype = hpread_type_lookup (fn_fieldp->dsvar.type, objfile);
>  	      list->field.type = memtype;
>  	      list->attributes = 0;
> @@ -4120,6 +4125,7 @@ hpread_read_struct_type (dnttpointer hp_
>  	      list->field.name = VT (objfile) + fn_fieldp->ddvar.name;
>  	      FIELD_BITPOS (list->field) = 0;	/* FIXME is this always true? */
>  	      FIELD_BITSIZE (list->field) = 0;	/* use length from type */
> +	      FIELD_STATIC_KIND (list->field) = 0;
>  	      memtype = hpread_type_lookup (fn_fieldp->ddvar.type, objfile);
>  	      list->field.type = memtype;
>  	      list->attributes = 0;
> @@ -4168,6 +4174,7 @@ hpread_read_struct_type (dnttpointer hp_
>  
>  
>  	  /* A FIELD by itself (without a GENFIELD) can also be a static member */
> +	  FIELD_STATIC_KIND (list->field) = 0;
>  	  if (fieldp->dfield.staticmem)
>  	    {
>  	      FIELD_BITPOS (list->field) = -1;
> Index: mdebugread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mdebugread.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 mdebugread.c
> --- mdebugread.c	18 Sep 2002 20:47:39 -0000	1.29
> +++ mdebugread.c	30 Sep 2002 00:43:36 -0000
> @@ -1092,6 +1092,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
>  		FIELD_TYPE (*f) = t;
>  		FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss;
>  		FIELD_BITSIZE (*f) = 0;
> +		FIELD_STATIC_KIND (*f) = 0;
>  
>  		enum_sym = ((struct symbol *)
>  			    obstack_alloc (&current_objfile->symbol_obstack,
> @@ -1284,6 +1285,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
>        bitsize = 0;
>        FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name);
>        FIELD_BITSIZE (*f) = bitsize;
> +      FIELD_STATIC_KIND (*f) = 0;
>        break;
>  
>      case stIndirect:		/* forward declaration on Irix5 */
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/types: Clean up use of field bitsize

[Andrew Cagney]

> Does anyone have a comment on this patch?  If not, I'll commit it in a
> couple of days, after I'm added to the global write list.
> 
> (The type code has no specific maintainer, the debug reader and
> language parts I consider obvious, and the patch is over a month old
> now.)

I'm mainly wondering if we're that desperate for memory space.

I thought a data structure was added to GDB so that it could spot 
duplicate type info and, hence, keep its memory size down.

Andrew


>> Right now, we have this really disturbing comment:
>> 
>>      /* Size of this field, in bits, or zero if not packed.
>>         For an unpacked field, the field's type's length
>>         says how many bytes the field occupies.
>>         A value of -1 or -2 indicates a static field;  -1 means the location
>>         is specified by the label loc.physname;  -2 means that loc.physaddr
>>         specifies the actual address. */
>> 
>> Think about this for a moment.  While in practice a static member is never
>> going to be packed, and in at least C++ can not be a bit-field, that's not
>> logically obvious for other languages.  I don't know Ada but I wouldn't be
>> surprised if there were some construct which violated this assumption.
>> 
>> Worse, all sorts of places don't check for negative bitsize at all.  It may
>> be that they're all safe - I didn't spend a lot of time working out problem
>> cases - but I have my doubts.
>> 
>> So, since I needed to gain a new field here anyway, and since I have no
>> compunctions about shrinking this field a little (packed bitfields of size
>> greater than a couple of words are allowed in some languages IIRC (including
>> GNU C maybe?  Although they are not allowed in ISO C99), but they're
>> definitely dodgy), and since signed bitfields are not portable, I cleaned up
>> the construct.  It turned out to be painless except for making sure symbol
>> readers initialized it, which was a little tedious.
>> 
>> This patch:
>>   Moves 'artificial' out from 'loc' and makes it a bitfield
>>   Creates a 'static_kind' bitfield
>>   Makes 'bitsize' into a bitfield
>> 
>> The goal is to allow more kinds of fields to be marked artificial -
>> particularly data members.  After this patch I'll submit the followup to
>> mark DW_AT_artificial members as artificial types.
>> 
>> OK?
>> 
>> -- 
>> Daniel Jacobowitz
>> MontaVista Software                         Debian GNU/Linux Developer
>> 
>> 2002-09-29  Daniel Jacobowitz  <drow@mvista.com>
>> 
>> 	* gdbtypes.h (struct main_type): Move artificial flag out of
>> 	loc.  New member of ``struct field'' named static_kind.  Reduce
>> 	overloaded meaning of bitsize.
>> 	(FIELD_ARTIFICIAL, SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR)
>> 	(TYPE_FIELD_STATIC, TYPE_FIELD_STATIC_HAS_ADDR): Likewise.
>> 	(FIELD_STATIC_KIND, TYPE_FIELD_STATIC_KIND): New macros.
>> 
>> 	* ada-lang.c (fill_in_ada_prototype): Initialize static_kind for
>> 	new fields.
>> 	(template_to_fixed_record_type, template_to_static_fixed_type)
>> 	(to_record_with_fixed_variant_part): Likewise.
>> 	* coffread.c (coff_read_struct_type, coff_read_enum_type): Likewise.
>> 	* dwarf2read.c (dwarf2_add_field, read_enumeration): Likewise.
>> 	* dwarfread.c (struct_type, enum_type): Likewise.
>> 	* hpread.c (hpread_read_enum_type)
>> 	(hpread_read_function_type, hpread_read_doc_function_type)
>> 	(hpread_read_struct_type): Likewise.
>> 	* mdebugread.c (parse_symbol): Likewise.
>> 
>> Index: ada-lang.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/ada-lang.c,v
>> retrieving revision 1.9
>> diff -u -p -r1.9 ada-lang.c
>> --- ada-lang.c	8 Sep 2002 17:43:26 -0000	1.9
>> +++ ada-lang.c	30 Sep 2002 00:43:30 -0000
>> @@ -4189,6 +4189,7 @@ fill_in_ada_prototype (struct symbol *fu
>>        case LOC_REGPARM_ADDR:
>>  	TYPE_FIELD_BITPOS (ftype, nargs) = nargs;
>>  	TYPE_FIELD_BITSIZE (ftype, nargs) = 0;
>> +	TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0;
>>  	TYPE_FIELD_TYPE (ftype, nargs) =
>>  	  lookup_pointer_type (check_typedef (SYMBOL_TYPE (sym)));
>>  	TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym);
>> @@ -4202,6 +4203,7 @@ fill_in_ada_prototype (struct symbol *fu
>>        case LOC_BASEREG_ARG:
>>  	TYPE_FIELD_BITPOS (ftype, nargs) = nargs;
>>  	TYPE_FIELD_BITSIZE (ftype, nargs) = 0;
>> +	TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0;
>>  	TYPE_FIELD_TYPE (ftype, nargs) = check_typedef (SYMBOL_TYPE (sym));
>>  	TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym);
>>  	nargs += 1;
>> @@ -6046,6 +6048,7 @@ template_to_fixed_record_type (struct ty
>>         * rediscover why we needed field_offset and fix it properly. */
>>        TYPE_FIELD_BITPOS (rtype, f) = off;
>>        TYPE_FIELD_BITSIZE (rtype, f) = 0;
>> +      TYPE_FIELD_STATIC_KIND (rtype, f) = 0;
>>  
>>        if (ada_is_variant_part (type, f))
>>  	{
>> @@ -6149,6 +6152,7 @@ template_to_static_fixed_type (struct ty
>>      {
>>        TYPE_FIELD_BITPOS (type, f) = 0;
>>        TYPE_FIELD_BITSIZE (type, f) = 0;
>> +      TYPE_FIELD_STATIC_KIND (type, f) = 0;
>>  
>>        if (is_dynamic_field (templ_type, f))
>>  	{
>> @@ -6218,6 +6222,7 @@ to_record_with_fixed_variant_part (struc
>>        TYPE_FIELD_TYPE (rtype, nfields - 1) = branch_type;
>>        TYPE_FIELD_NAME (rtype, nfields - 1) = "S";
>>        TYPE_FIELD_BITSIZE (rtype, nfields - 1) = 0;
>> +      TYPE_FIELD_STATIC_KIND (rtype, nfields - 1) = 0;
>>        TYPE_LENGTH (rtype) += TYPE_LENGTH (branch_type);
>>        -TYPE_LENGTH (TYPE_FIELD_TYPE (type, nfields - 1));
>>      }
>> Index: coffread.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/coffread.c,v
>> retrieving revision 1.29
>> diff -u -p -r1.29 coffread.c
>> --- coffread.c	22 Aug 2002 05:50:11 -0000	1.29
>> +++ coffread.c	30 Sep 2002 00:43:30 -0000
>> @@ -1997,6 +1997,7 @@ coff_read_struct_type (int index, int le
>>  	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux);
>>  	  FIELD_BITPOS (list->field) = 8 * ms->c_value;
>>  	  FIELD_BITSIZE (list->field) = 0;
>> +	  FIELD_STATIC_KIND (list->field) = 0;
>>  	  nfields++;
>>  	  break;
>>  
>> @@ -2015,6 +2016,7 @@ coff_read_struct_type (int index, int le
>>  	  FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux);
>>  	  FIELD_BITPOS (list->field) = ms->c_value;
>>  	  FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size;
>> +	  FIELD_STATIC_KIND (list->field) = 0;
>>  	  nfields++;
>>  	  break;
>>  
>> @@ -2135,6 +2137,7 @@ coff_read_enum_type (int index, int leng
>>  	  if (SYMBOL_VALUE (xsym) < 0)
>>  	    unsigned_enum = 0;
>>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
>> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>>  	}
>>        if (syms == osyms)
>>  	break;
>> Index: dwarf2read.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
>> retrieving revision 1.66
>> diff -u -p -r1.66 dwarf2read.c
>> --- dwarf2read.c	3 Sep 2002 17:32:11 -0000	1.66
>> +++ dwarf2read.c	30 Sep 2002 00:43:33 -0000
>> @@ -2055,6 +2055,8 @@ dwarf2_add_field (struct field_info *fip
>>        /* Get type of field.  */
>>        fp->type = die_type (die, objfile, cu_header);
>>  
>> +      FIELD_STATIC_KIND (*fp) = 0;
>> +
>>        /* Get bit size of field (zero if none).  */
>>        attr = dwarf_attr (die, DW_AT_bit_size);
>>        if (attr)
>> @@ -2163,6 +2165,7 @@ dwarf2_add_field (struct field_info *fip
>>  	FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), objfile, cu_header)
>>  			      * bits_per_byte);
>>        FIELD_BITSIZE (*fp) = 0;
>> +      FIELD_STATIC_KIND (*fp) = 0;
>>        FIELD_TYPE (*fp) = die_type (die, objfile, cu_header);
>>        FIELD_NAME (*fp) = type_name_no_tag (fp->type);
>>        fip->nbaseclasses++;
>> @@ -2667,6 +2670,7 @@ read_enumeration (struct die_info *die, 
>>  		  FIELD_TYPE (fields[num_fields]) = NULL;
>>  		  FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
>>  		  FIELD_BITSIZE (fields[num_fields]) = 0;
>> +		  FIELD_STATIC_KIND (fields[num_fields]) = 0;
>>  
>>  		  num_fields++;
>>  		}
>> Index: dwarfread.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/dwarfread.c,v
>> retrieving revision 1.14
>> diff -u -p -r1.14 dwarfread.c
>> --- dwarfread.c	1 Aug 2002 17:18:32 -0000	1.14
>> +++ dwarfread.c	30 Sep 2002 00:43:33 -0000
>> @@ -1027,6 +1027,7 @@ struct_type (struct dieinfo *dip, char *
>>  			  &objfile->type_obstack);
>>  	  FIELD_TYPE (list->field) = decode_die_type (&mbr);
>>  	  FIELD_BITPOS (list->field) = 8 * locval (&mbr);
>> +	  FIELD_STATIC_KIND (list->field) = 0;
>>  	  /* Handle bit fields. */
>>  	  FIELD_BITSIZE (list->field) = mbr.at_bit_size;
>>  	  if (BITS_BIG_ENDIAN)
>> @@ -1694,6 +1695,7 @@ enum_type (struct dieinfo *dip, struct o
>>  	  list = new;
>>  	  FIELD_TYPE (list->field) = NULL;
>>  	  FIELD_BITSIZE (list->field) = 0;
>> +	  FIELD_STATIC_KIND (list->field) = 0;
>>  	  FIELD_BITPOS (list->field) =
>>  	    target_to_host (scan, TARGET_FT_LONG_SIZE (objfile), GET_SIGNED,
>>  			    objfile);
>> Index: gdbtypes.h
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/gdbtypes.h,v
>> retrieving revision 1.36
>> diff -u -p -r1.36 gdbtypes.h
>> --- gdbtypes.h	14 Sep 2002 02:09:39 -0000	1.36
>> +++ gdbtypes.h	30 Sep 2002 00:43:34 -0000
>> @@ -381,22 +381,25 @@ struct main_type
>>  
>>        CORE_ADDR physaddr;
>>        char *physname;
>> -
>> -      /* For a function or member type, this is 1 if the argument is marked
>> -	 artificial.  Artificial arguments should not be shown to the
>> -	 user.  */
>> -      int artificial;
>>      }
>>      loc;
>>  
>> +    /* For a function or member type, this is 1 if the argument is marked
>> +       artificial.  Artificial arguments should not be shown to the
>> +       user.  */
>> +    unsigned int artificial : 1;
>> +
>> +    /* This flag is zero for non-static fields, 1 for fields whose location
>> +       is specified by the label loc.physname, and 2 for fields whose location
>> +       is specified by loc.physaddr.  */
>> +
>> +    unsigned int static_kind : 2;
>> +
>>      /* Size of this field, in bits, or zero if not packed.
>>         For an unpacked field, the field's type's length
>> -       says how many bytes the field occupies.
>> -       A value of -1 or -2 indicates a static field;  -1 means the location
>> -       is specified by the label loc.physname;  -2 means that loc.physaddr
>> -       specifies the actual address. */
>> +       says how many bytes the field occupies.  */
>>  
>> -    int bitsize;
>> +    unsigned int bitsize : 29;
>>  
>>      /* In a struct or union type, type of this field.
>>         In a function or member type, type of this argument.
>> @@ -793,14 +796,15 @@ extern void allocate_cplus_struct_type (
>>  #define FIELD_TYPE(thisfld) ((thisfld).type)
>>  #define FIELD_NAME(thisfld) ((thisfld).name)
>>  #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
>> -#define FIELD_ARTIFICIAL(thisfld) ((thisfld).loc.artificial)
>> +#define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial)
>>  #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize)
>> +#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
>>  #define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
>>  #define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
>>  #define SET_FIELD_PHYSNAME(thisfld, name) \
>> -  ((thisfld).bitsize = -1, FIELD_PHYSNAME(thisfld) = (name))
>> +  ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
>>  #define SET_FIELD_PHYSADDR(thisfld, name) \
>> -  ((thisfld).bitsize = -2, FIELD_PHYSADDR(thisfld) = (name))
>> +  ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
>>  #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n]
>>  #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n))
>>  #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n))
>> @@ -840,8 +844,9 @@ extern void allocate_cplus_struct_type (
>>    (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
>>      : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n)))
>>  
>> -#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize < 0)
>> -#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize == -2)
>> +#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
>> +#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
>> +#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
>>  #define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
>>  #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
>>  
>> Index: hpread.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/hpread.c,v
>> retrieving revision 1.22
>> diff -u -p -r1.22 hpread.c
>> --- hpread.c	4 Aug 2002 19:11:12 -0000	1.22
>> +++ hpread.c	30 Sep 2002 00:43:35 -0000
>> @@ -3186,6 +3186,7 @@ hpread_read_enum_type (dnttpointer hp_ty
>>  	  TYPE_FIELD_NAME (type, n) = SYMBOL_NAME (xsym);
>>  	  TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
>>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
>> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>>  	}
>>        if (syms == osyms)
>>  	break;
>> @@ -3347,6 +3348,7 @@ hpread_read_function_type (dnttpointer h
>>  	  TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym);
>>  	  TYPE_FIELD_ARTIFICIAL (type, n) = 0;
>>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
>> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>>  	}
>>      }
>>    /* Mark it as having been processed */
>> @@ -3520,6 +3522,7 @@ hpread_read_doc_function_type (dnttpoint
>>  	  TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym);
>>  	  TYPE_FIELD_ARTIFICIAL (type, n) = 0;
>>  	  TYPE_FIELD_BITSIZE (type, n) = 0;
>> +	  TYPE_FIELD_STATIC_KIND (type, n) = 0;
>>  	}
>>      }
>>  
>> @@ -3704,6 +3707,7 @@ hpread_read_struct_type (dnttpointer hp_
>>  	  list = new;
>>  
>>  	  FIELD_BITSIZE (list->field) = 0;
>> +	  FIELD_STATIC_KIND (list->field) = 0;
>>  
>>  	  /* The "classname" field is actually a DNTT pointer to the base class */
>>  	  baseclass = hpread_type_lookup (parentp->dinheritance.classname,
>> @@ -4101,6 +4105,7 @@ hpread_read_struct_type (dnttpointer hp_
>>  	      list->field.name = VT (objfile) + fn_fieldp->dsvar.name;
>>  	      FIELD_BITPOS (list->field) = 0;	/* FIXME is this always true? */
>>  	      FIELD_BITSIZE (list->field) = 0;	/* use length from type */
>> +	      FIELD_STATIC_KIND (list->field) = 0;
>>  	      memtype = hpread_type_lookup (fn_fieldp->dsvar.type, objfile);
>>  	      list->field.type = memtype;
>>  	      list->attributes = 0;
>> @@ -4120,6 +4125,7 @@ hpread_read_struct_type (dnttpointer hp_
>>  	      list->field.name = VT (objfile) + fn_fieldp->ddvar.name;
>>  	      FIELD_BITPOS (list->field) = 0;	/* FIXME is this always true? */
>>  	      FIELD_BITSIZE (list->field) = 0;	/* use length from type */
>> +	      FIELD_STATIC_KIND (list->field) = 0;
>>  	      memtype = hpread_type_lookup (fn_fieldp->ddvar.type, objfile);
>>  	      list->field.type = memtype;
>>  	      list->attributes = 0;
>> @@ -4168,6 +4174,7 @@ hpread_read_struct_type (dnttpointer hp_
>>  
>>  
>>  	  /* A FIELD by itself (without a GENFIELD) can also be a static member */
>> +	  FIELD_STATIC_KIND (list->field) = 0;
>>  	  if (fieldp->dfield.staticmem)
>>  	    {
>>  	      FIELD_BITPOS (list->field) = -1;
>> Index: mdebugread.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/mdebugread.c,v
>> retrieving revision 1.29
>> diff -u -p -r1.29 mdebugread.c
>> --- mdebugread.c	18 Sep 2002 20:47:39 -0000	1.29
>> +++ mdebugread.c	30 Sep 2002 00:43:36 -0000
>> @@ -1092,6 +1092,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
>>  		FIELD_TYPE (*f) = t;
>>  		FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss;
>>  		FIELD_BITSIZE (*f) = 0;
>> +		FIELD_STATIC_KIND (*f) = 0;
>>  
>>  		enum_sym = ((struct symbol *)
>>  			    obstack_alloc (&current_objfile->symbol_obstack,
>> @@ -1284,6 +1285,7 @@ parse_symbol (SYMR *sh, union aux_ext *a
>>        bitsize = 0;
>>        FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name);
>>        FIELD_BITSIZE (*f) = bitsize;
>> +      FIELD_STATIC_KIND (*f) = 0;
>>        break;
>>  
>>      case stIndirect:		/* forward declaration on Irix5 */
>> 
> 
> 
> -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 

Re: RFA/types: Clean up use of field bitsize

[Daniel Jacobowitz]

On Wed, Nov 06, 2002 at 03:41:10PM -0500, Andrew Cagney wrote:
> >Does anyone have a comment on this patch?  If not, I'll commit it in a
> >couple of days, after I'm added to the global write list.
> >
> >(The type code has no specific maintainer, the debug reader and
> >language parts I consider obvious, and the patch is over a month old
> >now.)
> 
> I'm mainly wondering if we're that desperate for memory space.
> 
> I thought a data structure was added to GDB so that it could spot 
> duplicate type info and, hence, keep its memory size down.

If so, I don't see it.  The debug readers will create a new copy when
they hit a new definition.

Besides, wasting memory is still bad.  And that's not the reason I did
it, anyway:

> >>The goal is to allow more kinds of fields to be marked artificial -
> >>particularly data members.  After this patch I'll submit the followup to
> >>mark DW_AT_artificial members as artificial types.

... in other words, moving artificial out of loc without wasting an
additional 32 bits.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA/types: Clean up use of field bitsize

[Andrew Cagney]

> On Wed, Nov 06, 2002 at 03:41:10PM -0500, Andrew Cagney wrote:
> 
>> >Does anyone have a comment on this patch?  If not, I'll commit it in a
>> >couple of days, after I'm added to the global write list.
>> >
>> >(The type code has no specific maintainer, the debug reader and
>> >language parts I consider obvious, and the patch is over a month old
>> >now.)
> 
>> 
>> I'm mainly wondering if we're that desperate for memory space.
>> 
>> I thought a data structure was added to GDB so that it could spot 
>> duplicate type info and, hence, keep its memory size down.
> 
> 
> If so, I don't see it.  The debug readers will create a new copy when
> they hit a new definition.

Sigh, looks depressingly like a proposal that fell flat :-(
There are bcache's for macro and psymbol stuff but not types.

> Besides, wasting memory is still bad.  And that's not the reason I did
> it, anyway:

True, the real problem is (true?) the type duplication - gdb is wasting 
memory by duplicating type information - fixing that eliminates the 
problem removing the need for a micro optomization?

Can't find the discussion :-(

>> >>The goal is to allow more kinds of fields to be marked artificial -
>> >>particularly data members.  After this patch I'll submit the followup to
>> >>mark DW_AT_artificial members as artificial types.
> 
> 
> ... in other words, moving artificial out of loc without wasting an
> additional 32 bits.

I'm definitly not questioning this.

Andrew

Re: RFA/types: Clean up use of field bitsize

[Daniel Jacobowitz]

On Sun, Sep 29, 2002 at 09:05:15PM -0400, Daniel Jacobowitz wrote:
> Right now, we have this really disturbing comment:
> 
>      /* Size of this field, in bits, or zero if not packed.
>         For an unpacked field, the field's type's length
>         says how many bytes the field occupies.
>         A value of -1 or -2 indicates a static field;  -1 means the location
>         is specified by the label loc.physname;  -2 means that loc.physaddr
>         specifies the actual address. */
> 
> Think about this for a moment.  While in practice a static member is never
> going to be packed, and in at least C++ can not be a bit-field, that's not
> logically obvious for other languages.  I don't know Ada but I wouldn't be
> surprised if there were some construct which violated this assumption.
> 
> Worse, all sorts of places don't check for negative bitsize at all.  It may
> be that they're all safe - I didn't spend a lot of time working out problem
> cases - but I have my doubts.
> 
> So, since I needed to gain a new field here anyway, and since I have no
> compunctions about shrinking this field a little (packed bitfields of size
> greater than a couple of words are allowed in some languages IIRC (including
> GNU C maybe?  Although they are not allowed in ISO C99), but they're
> definitely dodgy), and since signed bitfields are not portable, I cleaned up
> the construct.  It turned out to be painless except for making sure symbol
> readers initialized it, which was a little tedious.
> 
> This patch:
>   Moves 'artificial' out from 'loc' and makes it a bitfield
>   Creates a 'static_kind' bitfield
>   Makes 'bitsize' into a bitfield
> 
> The goal is to allow more kinds of fields to be marked artificial -
> particularly data members.  After this patch I'll submit the followup to
> mark DW_AT_artificial members as artificial types.
> 
> OK?

It's in.  Now to do the followon for artificial members; this will let
us hide _vptr members in type output if requested.  And probably a set
option to toggle the artificial behavior...

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer