Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays]

Jan Kratochvil
2008-10-04T20:28:31Z

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

On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote:
> > 2008-09-07 Jan Kratochvil <jan.kratochvil@redhat.com>
> >
> > Split two-bit STATIC_KIND to two separate bitfield single bits.
> > * gdbtypes.h (struct main_type <static_kind>): Split to ...
> > (struct main_type <bit0>, <bit1>): ... here.
> > (FIELD_STATIC_KIND): Split to ...
> > (FIELD_BIT0, FIELD_BIT1): ... here.
> > (SET_FIELD_BITPOS): New.
> > (SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR, TYPE_FIELD_STATIC)
> > (TYPE_FIELD_STATIC_HAS_ADDR): Update for the split.
> > (TYPE_FIELD_STATIC_KIND): Remove.
> > (TYPE_FIELD_STATIC_HAS_NAME): New.
> > * coffread.c, dwarf2read.c, eval.c, gdbtypes.c, mdebugread.c: Update
> > throughout.
>
> It would be interesting to hear what others think, but I don't really
> like the idea of splitting the 2-bit static_kind field into two one-bit
> fields named bit0 & bit1. I just find it obscures things even more.
>
> The first goal, as I understand it, is to be able to extend range types
> so that the bounds can be DWARF blocks. Here is a thought: Rename the
> static_kind field into "field_loc_kind" which would be an enum with
> 4 possible values: loc_in_bitpos, loc_in_physaddr, loc_in_physname,
> and loc_in_block.

Done. Including below reformatted diff to make the changes more related to
each other.

It already introduces the new field into the loc union:
+ /* For variable-sized arrays. Passed to DWARF_LOCEXPR_BATON_EVAL. */
+ struct dwarf2_locexpr_baton *dwarf_block;

as otherwise this change is not much useful. The other way is to implement
there enum address_class but I find it as another larger patch to properly
separate it out from symbols, drop the `struct symbol_ops *ops' field etc.

Thanks,
Jan

Unsplit patch now at:
http://people.redhat.com/jkratoch/vla/fortran-dynamic-arrays-HEAD-b.patch

-#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind)
+#define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
-#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind
+#define TYPE_FIELD_LOC_KIND(thistype, n) FIELD_LOC_KIND (TYPE_FIELD (thistype, n))

-#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2)
+#define FIELD_STATIC(thisfld) (FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSNAME || FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSADDR)
-#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0)
+#define TYPE_FIELD_STATIC(thistype, n) FIELD_STATIC (TYPE_FIELD (thistype, n))

-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS(TYPE_FIELD(thistype,n))
+#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
+#define SET_FIELD_BITPOS(thisfld, bitpos) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_BITPOS, FIELD_BITPOS (thisfld) = (bitpos))

-#define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
+#define FIELD_STATIC_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
-#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n))
+#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_STATIC_PHYSADDR (TYPE_FIELD (thistype, n))
-#define SET_FIELD_PHYSADDR(thisfld, name) ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name))
+#define SET_FIELD_PHYSADDR(thisfld, addr) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSADDR, FIELD_STATIC_PHYSADDR (thisfld) = (addr))

-#define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname)
+#define FIELD_STATIC_PHYSNAME(thisfld) ((thisfld).loc.physname)
-#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n))
+#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_STATIC_PHYSNAME (TYPE_FIELD (thistype, n))
-#define SET_FIELD_PHYSNAME(thisfld, name) ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
+#define SET_FIELD_PHYSNAME(thisfld, name) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSNAME, FIELD_STATIC_PHYSNAME (thisfld) = (name))

+#define FIELD_DWARF_BLOCK(thisfld) ((thisfld).loc.dwarf_block)
+#define TYPE_FIELD_DWARF_BLOCK(thistype, n) FIELD_DWARF_BLOCK (TYPE_FIELD (thistype, n))
+#define SET_FIELD_DWARF_BLOCK(thisfld, addr) (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_DWARF_BLOCK, FIELD_DWARF_BLOCK (thisfld) = (addr))

[-- Attachment #2: fortran8-bit01.patch --]
[-- Type: text/plain, Size: 12911 bytes --]

2008-10-04 Jan Kratochvil <jan.kratochvil@redhat.com>

Convert static_kind into loc_kind enum.
* gdbtypes.h (enum field_loc_kind): New.
(union field_location): New field dwarf_block.
(struct field): Rename static_kind as loc_kind.
(FIELD_STATIC_KIND): Rename to ...
(FIELD_LOC_KIND): ... here.
(TYPE_FIELD_STATIC_KIND): Rename to ...
(TYPE_FIELD_LOC_KIND): ... here and use there now new FIELD_LOC_KIND.
(TYPE_FIELD_STATIC_HAS_ADDR): Remove.
(FIELD_STATIC): New.
(TYPE_FIELD_STATIC): Base now on FIELD_STATIC.
(TYPE_FIELD_BITPOS): Reformat.
(SET_FIELD_BITPOS): New.
(FIELD_PHYSADDR): Rename to ...
(FIELD_STATIC_PHYSADDR): ... here.
(TYPE_FIELD_STATIC_PHYSADDR): Follow the FIELD_PHYSADDR rename.
(SET_FIELD_PHYSADDR): Use new FIELD_LOC_KIND.
(FIELD_PHYSNAME): Rename to ...
(FIELD_STATIC_PHYSNAME): ... here.
(TYPE_FIELD_STATIC_PHYSNAME): Follow the FIELD_PHYSNAME rename.
(SET_FIELD_PHYSNAME): Use new FIELD_LOC_KIND.
(FIELD_DWARF_BLOCK, TYPE_FIELD_DWARF_BLOCK, SET_FIELD_DWARF_BLOCK): New.
* coffread.c, dwarf2read.c, eval.c, gdbtypes.c, mdebugread.c, value.c:
Update throughout.

--- gdb/coffread.c 1 Oct 2008 16:41:27 -0000 1.86
+++ gdb/coffread.c 4 Oct 2008 19:44:09 -0000
@@ -1943,9 +1943,8 @@ coff_read_struct_type (int index, int le
obsavestring (name, strlen (name), &objfile->objfile_obstack);
FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
objfile);
- FIELD_BITPOS (list->field) = 8 * ms->c_value;
+ SET_FIELD_BITPOS (list->field, 8 * ms->c_value);
FIELD_BITSIZE (list->field) = 0;
- FIELD_STATIC_KIND (list->field) = 0;
nfields++;
break;

@@ -1961,9 +1960,8 @@ coff_read_struct_type (int index, int le
obsavestring (name, strlen (name), &objfile->objfile_obstack);
FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux,
objfile);
- FIELD_BITPOS (list->field) = ms->c_value;
+ SET_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;

@@ -2080,11 +2078,10 @@ coff_read_enum_type (int index, int leng
struct symbol *xsym = syms->symbol[j];
SYMBOL_TYPE (xsym) = type;
TYPE_FIELD_NAME (type, n) = SYMBOL_LINKAGE_NAME (xsym);
- TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym);
+ SET_FIELD_BITPOS (TYPE_FIELD (type, n), SYMBOL_VALUE (xsym));
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;
--- gdb/dwarf2read.c 30 Sep 2008 16:57:37 -0000 1.285
+++ gdb/dwarf2read.c 4 Oct 2008 19:44:18 -0000
@@ -3561,8 +3561,6 @@ dwarf2_add_field (struct field_info *fip
/* Get type of field. */
fp->type = die_type (die, cu);

- FIELD_STATIC_KIND (*fp) = 0;
-
/* Get bit size of field (zero if none). */
attr = dwarf2_attr (die, DW_AT_bit_size, cu);
if (attr)
@@ -3590,10 +3588,10 @@ dwarf2_add_field (struct field_info *fip
else
byte_offset = decode_locdesc (DW_BLOCK (attr), cu);

- FIELD_BITPOS (*fp) = byte_offset * bits_per_byte;
+ SET_FIELD_BITPOS (*fp, byte_offset * bits_per_byte);
}
else
- FIELD_BITPOS (*fp) = 0;
+ SET_FIELD_BITPOS (*fp, 0);
attr = dwarf2_attr (die, DW_AT_bit_offset, cu);
if (attr)
{
@@ -3685,10 +3683,9 @@ dwarf2_add_field (struct field_info *fip
/* C++ base class field. */
attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
if (attr)
- FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), cu)
- * bits_per_byte);
+ SET_FIELD_BITPOS (*fp, decode_locdesc (DW_BLOCK (attr), cu)
+ * bits_per_byte);
FIELD_BITSIZE (*fp) = 0;
- FIELD_STATIC_KIND (*fp) = 0;
FIELD_TYPE (*fp) = die_type (die, cu);
FIELD_NAME (*fp) = type_name_no_tag (fp->type);
fip->nbaseclasses++;
@@ -4406,9 +4403,8 @@ process_enumeration_scope (struct die_in

FIELD_NAME (fields[num_fields]) = SYMBOL_LINKAGE_NAME (sym);
FIELD_TYPE (fields[num_fields]) = NULL;
- FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym);
+ SET_FIELD_BITPOS (fields[num_fields], SYMBOL_VALUE (sym));
FIELD_BITSIZE (fields[num_fields]) = 0;
- FIELD_STATIC_KIND (fields[num_fields]) = 0;

num_fields++;
}
--- gdb/eval.c 2 Oct 2008 22:06:07 -0000 1.99
+++ gdb/eval.c 4 Oct 2008 19:44:22 -0000
@@ -319,7 +319,7 @@ evaluate_struct_tuple (struct value *str
fieldno++;
/* Skip static fields. */
while (fieldno < TYPE_NFIELDS (struct_type)
- && TYPE_FIELD_STATIC_KIND (struct_type, fieldno))
+ && TYPE_FIELD_STATIC (struct_type, fieldno))
fieldno++;
subfieldno = fieldno;
if (fieldno >= TYPE_NFIELDS (struct_type))
--- gdb/gdbtypes.c 2 Oct 2008 22:06:07 -0000 1.151
+++ gdb/gdbtypes.c 4 Oct 2008 19:44:26 -0000
@@ -2975,7 +2975,7 @@ copy_type_recursive (struct objfile *obj
if (TYPE_FIELD_NAME (type, i))
TYPE_FIELD_NAME (new_type, i) =
xstrdup (TYPE_FIELD_NAME (type, i));
- if (TYPE_FIELD_STATIC_HAS_ADDR (type, i))
+ if (TYPE_FIELD_LOC_KIND (type, i) == FIELD_LOC_KIND_PHYSADDR)
SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i),
TYPE_FIELD_STATIC_PHYSADDR (type, i));
else if (TYPE_FIELD_STATIC (type, i))
@@ -2983,11 +2983,8 @@ copy_type_recursive (struct objfile *obj
xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type,
i)));
else
- {
- TYPE_FIELD_BITPOS (new_type, i) =
- TYPE_FIELD_BITPOS (type, i);
- TYPE_FIELD_STATIC_KIND (new_type, i) = 0;
- }
+ SET_FIELD_BITPOS (TYPE_FIELD (new_type, i),
+ TYPE_FIELD_BITPOS (type, i));
}
}

--- gdb/gdbtypes.h 2 Oct 2008 22:06:07 -0000 1.92
+++ gdb/gdbtypes.h 4 Oct 2008 19:44:27 -0000
@@ -316,6 +316,16 @@ enum type_instance_flag_value
#define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \
& TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL)

+/* Determine which field of the union main_type.fields[x].loc is used. */
+
+enum field_loc_kind
+ {
+ FIELD_LOC_KIND_BITPOS, /* bitpos */
+ FIELD_LOC_KIND_PHYSADDR, /* physaddr */
+ FIELD_LOC_KIND_PHYSNAME, /* physname */
+ FIELD_LOC_KIND_DWARF_BLOCK /* dwarf_block */
+ };
+
/* This structure is space-critical.
Its layout has been tweaked to reduce the space used. */

@@ -437,6 +448,9 @@ struct main_type

CORE_ADDR physaddr;
char *physname;
+
+ /* For variable length arrays. Passed to dwarf_locexpr_baton_eval. */
+ struct dwarf2_locexpr_baton *dwarf_block;
}
loc;

@@ -446,11 +460,8 @@ struct main_type
defined. */
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;
+ /* Discriminant for union field_location. */
+ ENUM_BITFIELD(field_loc_kind) loc_kind : 2;

/* Size of this field, in bits, or zero if not packed.
For an unpacked field, the field's type's length
@@ -838,20 +849,38 @@ extern void allocate_cplus_struct_type (

#define FIELD_TYPE(thisfld) ((thisfld).type)
#define FIELD_NAME(thisfld) ((thisfld).name)
+#define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
+#define FIELD_STATIC(thisfld) \
+ (FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSNAME \
+ || FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSADDR)
#define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos)
+#define FIELD_STATIC_PHYSNAME(thisfld) ((thisfld).loc.physname)
+#define FIELD_STATIC_PHYSADDR(thisfld) ((thisfld).loc.physaddr)
+#define FIELD_DWARF_BLOCK(thisfld) ((thisfld).loc.dwarf_block)
+#define SET_FIELD_BITPOS(thisfld, bitpos) \
+ (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_BITPOS, \
+ FIELD_BITPOS (thisfld) = (bitpos))
+#define SET_FIELD_PHYSNAME(thisfld, name) \
+ (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSNAME, \
+ FIELD_STATIC_PHYSNAME (thisfld) = (name))
+#define SET_FIELD_PHYSADDR(thisfld, addr) \
+ (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_PHYSADDR, \
+ FIELD_STATIC_PHYSADDR (thisfld) = (addr))
+#define SET_FIELD_DWARF_BLOCK(thisfld, addr) \
+ (FIELD_LOC_KIND (thisfld) = FIELD_LOC_KIND_DWARF_BLOCK, \
+ FIELD_DWARF_BLOCK (thisfld) = (addr))
#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).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name))
-#define SET_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))
-#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS(TYPE_FIELD(thistype,n))
+#define TYPE_FIELD_LOC_KIND(thistype, n) FIELD_LOC_KIND (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC(thistype, n) FIELD_STATIC (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_BITPOS(thistype, n) FIELD_BITPOS (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_STATIC_PHYSNAME (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_STATIC_PHYSADDR (TYPE_FIELD (thistype, n))
+#define TYPE_FIELD_DWARF_BLOCK(thistype, n) FIELD_DWARF_BLOCK (TYPE_FIELD (thistype, n))
#define TYPE_FIELD_ARTIFICIAL(thistype, n) FIELD_ARTIFICIAL(TYPE_FIELD(thistype,n))
#define TYPE_FIELD_BITSIZE(thistype, n) FIELD_BITSIZE(TYPE_FIELD(thistype,n))
#define TYPE_FIELD_PACKED(thistype, n) (FIELD_BITSIZE(TYPE_FIELD(thistype,n))!=0)
@@ -886,12 +915,6 @@ 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].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))
-
#define TYPE_FN_FIELDLISTS(thistype) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists
#define TYPE_FN_FIELDLIST(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n]
#define TYPE_FN_FIELDLIST1(thistype, n) TYPE_CPLUS_SPECIFIC(thistype)->fn_fieldlists[n].fn_fields
--- gdb/mdebugread.c 1 Oct 2008 16:41:27 -0000 1.95
+++ gdb/mdebugread.c 4 Oct 2008 19:44:31 -0000
@@ -1054,11 +1054,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
if (tsym.st != stMember)
break;

- FIELD_BITPOS (*f) = tsym.value;
+ SET_FIELD_BITPOS (*f, tsym.value);
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->objfile_obstack,
@@ -1241,11 +1240,10 @@ parse_symbol (SYMR *sh, union aux_ext *a
case stMember: /* member of struct or union */
f = &TYPE_FIELDS (top_stack->cur_type)[top_stack->cur_field++];
FIELD_NAME (*f) = name;
- FIELD_BITPOS (*f) = sh->value;
+ SET_FIELD_BITPOS (*f, sh->value);
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 */
@@ -1777,12 +1775,9 @@ upgrade_type (int fd, struct type **tpp,
ignore the erroneous bitsize from the auxiliary entry safely.
dbx seems to ignore it too. */

- /* TYPE_FLAG_TARGET_STUB now takes care of the zero TYPE_LENGTH
- problem. */
+ /* TYPE_TARGET_STUB now takes care of the zero TYPE_LENGTH problem. */
if (TYPE_LENGTH (*tpp) == 0)
- {
- TYPE_TARGET_STUB (t) = 1;
- }
+ TYPE_TARGET_STUB (t) = 1;

*tpp = t;
return 4 + off;
--- gdb/value.c 11 Sep 2008 14:22:33 -0000 1.66
+++ gdb/value.c 4 Oct 2008 19:44:40 -0000
@@ -1236,7 +1236,7 @@ value_static_field (struct type *type, i
{
struct value *retval;

- if (TYPE_FIELD_STATIC_HAS_ADDR (type, fieldno))
+ if (TYPE_FIELD_LOC_KIND (type, fieldno) == FIELD_LOC_KIND_PHYSADDR)
{
retval = value_at (TYPE_FIELD_TYPE (type, fieldno),
TYPE_FIELD_STATIC_PHYSADDR (type, fieldno));