* [gdb] Fortran dynamic arrays @ 2008-08-15 15:06 Tobias Burnus 2008-08-18 11:12 ` Joel Brobecker 2008-09-07 11:59 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil 0 siblings, 2 replies; 34+ messages in thread From: Tobias Burnus @ 2008-08-15 15:06 UTC (permalink / raw) To: gdb-patches; +Cc: Jan Kratochvil, Joel Brobecker, Jim Blandy, jimb Hello all, I'm badly missing the support for Fortran dynamic arrays in gdb. Jan Kratochvil had written a couple of patches which implement this support. They had been tested with several compilers, including GCC/gfortran (4.3.0+), IBM's xlf and Intel's ifort. The problem is that the patches are quite bulky and no one stepped forward to review them. I'm willing to re-diff them, but only if someone is willing to review them. (Patch history: Initial patch 16 Nov. 2007, updated patch 24 Nov. 2007 (see links below); pinged for Dec 2007, Jan/Feb/March 2008.) The original patch is available from: 0: http://sourceware.org/ml/gdb-patches/2007-11/msg00438.html 1: (checked in) 2: http://sourceware.org/ml/gdb-patches/2007-11/msg00440.html 3: http://sourceware.org/ml/gdb-patches/2007-11/msg00441.html 4: http://sourceware.org/ml/gdb-patches/2007-11/msg00442.html 5: http://sourceware.org/ml/gdb-patches/2007-11/msg00443.html 6: http://sourceware.org/ml/gdb-patches/2007-11/msg00444.html An updated (i.e. rediffed), unsplit version is available from: http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/gdb/devel/gdb-6.8-bz377541-fortran-dynamic-arrays.patch But I were not surprised if that one needs rediffing as well, regarding that URL cf. also http://sourceware.org/ml/gdb-patches/2008-03/msg00115.html Tobias ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [gdb] Fortran dynamic arrays 2008-08-15 15:06 [gdb] Fortran dynamic arrays Tobias Burnus @ 2008-08-18 11:12 ` Joel Brobecker 2008-08-18 15:54 ` Ulrich Weigand 2008-09-07 11:59 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil 1 sibling, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-08-18 11:12 UTC (permalink / raw) To: Tobias Burnus; +Cc: gdb-patches, Jan Kratochvil, Jim Blandy, jimb > The problem is that the patches are quite bulky and no one stepped > forward to review them. I'm sorry about this. I will try to see if I feel more comfortable reviewing them today than I did back then. We badly need more reviewers... Please send an updated set of patches (and if you haven't done so already, could you show us how Fortran arrays are described in DWARF?). -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [gdb] Fortran dynamic arrays 2008-08-18 11:12 ` Joel Brobecker @ 2008-08-18 15:54 ` Ulrich Weigand 0 siblings, 0 replies; 34+ messages in thread From: Ulrich Weigand @ 2008-08-18 15:54 UTC (permalink / raw) To: Joel Brobecker Cc: Tobias Burnus, gdb-patches, Jan Kratochvil, Jim Blandy, jimb Joel Brobecker wrote: > > The problem is that the patches are quite bulky and no one stepped > > forward to review them. > > I'm sorry about this. I will try to see if I feel more comfortable > reviewing them today than I did back then. We badly need more > reviewers... > > Please send an updated set of patches (and if you haven't done so already, > could you show us how Fortran arrays are described in DWARF?). As this is an area I'm interested as well, I'll try to help reviewing those patches too ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 34+ messages in thread
* [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-08-15 15:06 [gdb] Fortran dynamic arrays Tobias Burnus 2008-08-18 11:12 ` Joel Brobecker @ 2008-09-07 11:59 ` Jan Kratochvil 2008-09-08 15:32 ` Tom Tromey ` (2 more replies) 1 sibling, 3 replies; 34+ messages in thread From: Jan Kratochvil @ 2008-09-07 11:59 UTC (permalink / raw) To: gdb-patches Cc: Tobias Burnus, Joel Brobecker, Ulrich Weigand, Jim Blandy, jimb [-- Attachment #1: Type: text/plain, Size: 3900 bytes --] Hi, On Mon, 18 Aug 2008 13:11:20 +0200, Joel Brobecker wrote: > could you show us how Fortran arrays are described in DWARF?). They use regular DWARF BLOCKs. Just so far GDB supported these blocks only for DW_AT_location while for Fortran they are used even for other attributes. There is also new DW_OP_push_object_address, DW_AT_data_location, DW_AT_associated and DW_AT_allocated. <2><6e>: Abbrev Number: 5 (DW_TAG_variable) <6f> DW_AT_name : (indirect string, offset: 0x63): varv <73> DW_AT_decl_file : 1 <74> DW_AT_decl_line : 23 <75> DW_AT_type : <0x197> <79> DW_AT_location : 3 byte block: 91 f0 7e (DW_OP_fbreg: -144) <1><197>: Abbrev Number: 13 (DW_TAG_array_type) <198> DW_AT_name : (indirect string, offset: 0x107): array3_real(kind=4) <19c> DW_AT_ordering : 1 (column major) <19d> DW_AT_data_location: 2 byte block: 97 6 (DW_OP_push_object_address; DW_OP_deref) <1a0> DW_AT_associated : 4 byte block: 97 6 30 2e (DW_OP_push_object_address; DW_OP_deref; DW_OP_lit0; DW_OP_ne) <1a5> DW_AT_type : <0x190> <1a9> DW_AT_sibling : <0x1e4> <2><1ad>: Abbrev Number: 14 (DW_TAG_subrange_type) <1ae> DW_AT_lower_bound : 4 byte block: 97 23 20 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref) <1b3> DW_AT_upper_bound : 4 byte block: 97 23 28 6 (DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref) <1b8> DW_AT_stride : 6 byte block: 97 23 18 6 34 1e (DW_OP_push_object_address; DW_OP_plus_uconst: 24; DW_OP_deref; DW_OP_lit4; DW_OP_mul) The patchset for HEAD as a single file is at: http://people.redhat.com/jkratoch/fortran-dynamic-arrays-HEAD-7.patch (just FYI, not for a review as it is not split) Attaching its first part - a very useless one on its own - its target is to get a new storage space for two bits of info (TYPE_BOUND_IS_DWARF_BLOCK and TYPE_BOUND_IS_DWARF_BLOCK). I do not think it is the best way but it is a minimal change without expanding the structure size. One of such spare space there is `enum array_bound_type' Except for BOUND_CANNOT_BE_DETERMINED it is never set anywhere. BOUND_BY_* are read only in f-valprint.c. In fact it could be probably used for the new bits TYPE_BOUND_IS_DWARF_BLOCK and possibly TYPE_HIGH_BOUND_IS_COUNT but I would rather like to later remove `enum array_bound_type' and its fields at all. The patch modifies a lot of sources trying to follow the new style of main_type bitfields instead of the former FLAGS variable. I do not think current main_type layout is right but the patch tries to follow the current main_type style. The original version of this patch (in current Fedora Rawhide) used the FLAGS style access: +#define TYPE_BOUND_IS_DWARF_BLOCK_MASK 1 +#define TYPE_BOUND_IS_DWARF_BLOCK_VAR(range_type, fieldno) \ + TYPE_FIELD_STATIC_KIND (range_type, fieldno) +#define TYPE_HIGH_BOUND_IS_COUNT_MASK 2 +#define TYPE_HIGH_BOUND_IS_COUNT_VAR(range_type) \ + TYPE_FIELD_STATIC_KIND (range_type, 1) + TYPE_BOUND_IS_DWARF_BLOCK_VAR (range_type, 1) + |= TYPE_BOUND_IS_DWARF_BLOCK_MASK; + if ((TYPE_BOUND_IS_DWARF_BLOCK_VAR (range_type, 0) + & TYPE_BOUND_IS_DWARF_BLOCK_MASK) != 0 Also I do not understand why exist all the macros like this one at all: #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type Why we cannot use it expanded? This way it is always one (or more) "tags"-jump (VIM ctrl-]) indirections while navigating the source files. In the attached patch FIELD_STATIC_KIND was removed so the pairs like - FIELD_BITPOS (*f) = sh->value; - FIELD_STATIC_KIND (*f) = 0; were replaced by + SET_FIELD_BITPOS (*f, sh->value); but still just assignments to FIELD_BITPOS (without an assignment to FIELD_STATIC_KIND) were left as they are. Regards, Jan [-- Attachment #2: fortran7-bit01.patch --] [-- Type: text/plain, Size: 9489 bytes --] 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. --- ./gdb/gdbtypes.h 2008-08-26 20:24:20.000000000 +0200 +++ ./gdb/gdbtypes.h 2008-09-07 13:39:11.000000000 +0200 @@ -463,11 +463,14 @@ struct main_type 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. */ + /* For a field of structure: + BIT0 is 0 and BIT1 is 0: Static field using LOC.BITPOS. + BIT0 is 1 and BIT1 is 0: Dynamic field using LOC.PHYSADDR. + BIT0 is 0 and BIT1 is 1: Dynamic field using LOC.PHYSNAME. + BIT0 is 1 and BIT1 is 1: Undefined. */ - unsigned int static_kind : 2; + unsigned int bit0 : 1; + unsigned int bit1 : 1; /* Size of this field, in bits, or zero if not packed. For an unpacked field, the field's type's length @@ -858,13 +861,16 @@ extern void allocate_cplus_struct_type ( #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos) #define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial) #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize) -#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind) +#define FIELD_BIT0(thisfld) ((thisfld).bit0) +#define FIELD_BIT1(thisfld) ((thisfld).bit1) #define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname) #define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr) +#define SET_FIELD_BITPOS(thisfld, bitpos) \ + ((thisfld).bit0 = 0, (thisfld).bit1 = 0, FIELD_PHYSADDR(thisfld) = (bitpos)) +#define SET_FIELD_PHYSADDR(thisfld, addr) \ + ((thisfld).bit0 = 1, (thisfld).bit1 = 0, FIELD_PHYSADDR(thisfld) = (addr)) #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)) + ((thisfld).bit0 = 0, (thisfld).bit1 = 1, FIELD_PHYSNAME(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)) @@ -903,11 +909,14 @@ 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_BIT0(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bit0) +#define TYPE_FIELD_BIT1(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bit1) +#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) TYPE_FIELD_BIT0 (thistype, n) +#define TYPE_FIELD_STATIC_HAS_NAME(thistype, n) TYPE_FIELD_BIT1 (thistype, n) +#define TYPE_FIELD_STATIC(thistype, n) (TYPE_FIELD_STATIC_HAS_ADDR (thistype, n) \ + || TYPE_FIELD_STATIC_HAS_NAME (thistype, n)) #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n)) +#define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(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] --- ./gdb/coffread.c 2008-08-26 20:24:18.000000000 +0200 +++ ./gdb/coffread.c 2008-09-07 13:34:35.000000000 +0200 @@ -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 2008-09-02 00:21:45.000000000 +0200 +++ ./gdb/dwarf2read.c 2008-09-07 13:34:35.000000000 +0200 @@ -3498,8 +3498,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) @@ -3527,10 +3525,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) { @@ -3622,10 +3620,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++; @@ -4343,9 +4340,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 2008-08-21 22:13:08.000000000 +0200 +++ ./gdb/eval.c 2008-09-07 13:34:35.000000000 +0200 @@ -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 2008-08-26 20:24:20.000000000 +0200 +++ ./gdb/gdbtypes.c 2008-09-07 13:34:35.000000000 +0200 @@ -3013,11 +3013,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/mdebugread.c 2008-08-26 20:24:20.000000000 +0200 +++ ./gdb/mdebugread.c 2008-09-07 13:34:35.000000000 +0200 @@ -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 (¤t_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 */ ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-07 11:59 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil @ 2008-09-08 15:32 ` Tom Tromey 2008-09-08 17:27 ` Jan Kratochvil 2008-09-19 6:04 ` Joel Brobecker 2008-09-19 22:13 ` Joel Brobecker 2 siblings, 1 reply; 34+ messages in thread From: Tom Tromey @ 2008-09-08 15:32 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: Jan> One of such spare space there is `enum array_bound_type' Except Jan> for BOUND_CANNOT_BE_DETERMINED it is never set anywhere. Jan> BOUND_BY_* are read only in f-valprint.c. In fact it could be Jan> probably used for the new bits TYPE_BOUND_IS_DWARF_BLOCK and Jan> possibly TYPE_HIGH_BOUND_IS_COUNT but I would rather like to Jan> later remove `enum array_bound_type' and its fields at all. This sounds like a good cleanup to me. Jan> The patch modifies a lot of sources trying to follow the new Jan> style of main_type bitfields instead of the former FLAGS Jan> variable. I do not think current main_type layout is right but Jan> the patch tries to follow the current main_type style. Do you mean the main_type layout after this patch, or before this patch? If before -- could you be more specific? Tom ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-08 15:32 ` Tom Tromey @ 2008-09-08 17:27 ` Jan Kratochvil 2008-09-19 22:29 ` Joel Brobecker 0 siblings, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-09-08 17:27 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Mon, 08 Sep 2008 17:30:47 +0200, Tom Tromey wrote: > >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: > > Jan> The patch modifies a lot of sources trying to follow the new > Jan> style of main_type bitfields instead of the former FLAGS > Jan> variable. I do not think current main_type layout is right but > Jan> the patch tries to follow the current main_type style. > > Do you mean the main_type layout after this patch, or before this patch? > If before -- could you be more specific? Both before and it gets a bit worse after. There should be a union with fields for each very specific TYPE_CODE_*. TYPE_CODE_ARRAY is terrible - it has always exactly NFIELDS==1 with fields[0].type->main_type pointing to TYPE_CODE_RANGE having exactly NFIELDS==2 (for the lower and upper bound). If we try so much to save some main_type memory: * fields[0].loc is not used. * nfields everywhere is not used. * main_type for TYPE_CODE_RANGE is not used as DW_TAG_subrange_type's DW_AT_type is stored into TYPE_CODE_RANGE's TYPE_TARGET_TYPE. We should have just one TYPE_CODE_ARRAY main_type pointing to the element-type (DW_TAG_array_type's DW_AT_type - which is right as its target_type) also also one simple pointer to the index type (DW_TAG_subrange_type's DW_AT_type - currently stored in TYPE_CODE_RANGE's target_type). target_type, vptr_fieldno, vptr_basetype and some cases of fields[] is used only for some types and for other types they are left unused. These fields should be moved to the type_specific union. fields[] with nfields should be moved into the type_specific union for specific types TYPE_CODE_FUNC, TYPE_CODE_STRUCT and some others. Simple union would make it all more clear, optimal for the memory size and even allocatable with variable length. Some better encapsulation would benefit the dynamic bounds I hacked into the current code. There should be a virtual method with MAIN_TYPE containing generic abstract class type. class debuginfo_value { virtual LONGEST get() = 0; }; class dwarf2_integer : debuginfo_value { virtual LONGEST get() { ... }; }; class dwarf2_block : debuginfo_value { virtual LONGEST get() { ... }; }; class main_type { ...; debuginfo_value upper_bound_value; ... }; LONGEST is not right as we need to return also unbound/undefined results. Just C++ requires pointer (or reference) as upper_bound_value which is inefficient so a C implementation (knowing the maximum size of all the inherited classes) should be better here, right? If the memory of main_type is so critical (I doubt so but i did not measure it) in my Fortran/dynamic-bounds patch main_type had to get three new pointers almost always being NULL. These three pointers may need to be set for any type so the base struct main_type has to be extended for them. We could instead use the Amiga "tags" data format with variable block size allocation (but sure slow access time which should not matter here): struct main_type { ...; void *tags[1]; }; memcpy (main_type->tags, { DW_AT_allocated, 0x601010, DW_AT_associated, 0x610348, NULL /* terminator */ }); Regards, Jan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-08 17:27 ` Jan Kratochvil @ 2008-09-19 22:29 ` Joel Brobecker 2008-09-26 23:04 ` Tom Tromey 0 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-09-19 22:29 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Tom Tromey, gdb-patches > Both before and it gets a bit worse after. There should be a union with > fields for each very specific TYPE_CODE_*. I would tend to agree, but I haven't thought about it much. I might be biased by the fact that Ada's use of variant record is relatively common. For instance, our structure implemented in Ada might roughly look like this: type GDB_Type_Kind is (Type_Code_Enum, Type_Code_Range, Type_Code_Array, ... -- All possible kinds of types. type GDB_Type (Kind : GDB_Type_Kind) is Boolean : Stub; Boolean : Incomplete; -- ... Any other field that's common to all types case Kind is when Type_Code_Enum => Enumerates : List_Of_Enumerates; when Type_Code_Range | Type_Code_Int => Low, High : Bound_Type; when Type_Code_Array => Element_Type : GDB_Type_Pointer; Index : Range_Type_Pointer; when ... => [etc] As you suggest, this is easily achievable in C as well by using unions, with the one difference is that Ada does some access check and makes sure that we access fields that are consistent with the discriminant (field named "Kind" in the example above). I think that this would help clarify the whole structure, and tell us what is and what is not used for each kind of type. But I am not sure of the drawbacks... For sure this would be a pretty significant change througout GDB, but maybe an interesting one. (that was my windy way of saying: I think I agree) -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-19 22:29 ` Joel Brobecker @ 2008-09-26 23:04 ` Tom Tromey 2008-09-27 14:53 ` Joel Brobecker 0 siblings, 1 reply; 34+ messages in thread From: Tom Tromey @ 2008-09-26 23:04 UTC (permalink / raw) To: Joel Brobecker; +Cc: Jan Kratochvil, gdb-patches >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes: [ type rearrangements ] Joel> As you suggest, this is easily achievable in C as well by using Joel> unions, with the one difference is that Ada does some access Joel> check and makes sure that we access fields that are consistent Joel> with the discriminant (field named "Kind" in the example above). GCC accomplishes this using checking in the accessor macros. For the most part this works well -- it certainly catches bugs. Joel> I think that this would help clarify the whole structure, and tell us Joel> what is and what is not used for each kind of type. But I am not sure Joel> of the drawbacks... For sure this would be a pretty significant change Joel> througout GDB, but maybe an interesting one. Joel> (that was my windy way of saying: I think I agree) I think it would be a good idea provided that there's some data indicating that the space savings is worthwhile. I'm not sure what would qualify here. Tom ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-26 23:04 ` Tom Tromey @ 2008-09-27 14:53 ` Joel Brobecker 0 siblings, 0 replies; 34+ messages in thread From: Joel Brobecker @ 2008-09-27 14:53 UTC (permalink / raw) To: Tom Tromey; +Cc: Jan Kratochvil, gdb-patches > Joel> (that was my windy way of saying: I think I agree) > > I think it would be a good idea provided that there's some data > indicating that the space savings is worthwhile. I'm not sure what > would qualify here. I actually wasn't thinking about space savings, just code clarity. If we can save space too, great. But I think the first goal in itself is worthwhile. -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-07 11:59 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil 2008-09-08 15:32 ` Tom Tromey @ 2008-09-19 6:04 ` Joel Brobecker 2008-09-22 15:25 ` Jan Kratochvil 2008-09-19 22:13 ` Joel Brobecker 2 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-09-19 6:04 UTC (permalink / raw) To: Jan Kratochvil Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb > One of such spare space there is `enum array_bound_type' Except for > BOUND_CANNOT_BE_DETERMINED it is never set anywhere. BOUND_BY_* are > read only in f-valprint.c. In fact it could be probably used for the > new bits TYPE_BOUND_IS_DWARF_BLOCK and possibly > TYPE_HIGH_BOUND_IS_COUNT but I would rather like to later remove `enum > array_bound_type' and its fields at all. I haven't started looking at the actual patch yet, but hope to get to it either tonight or tomorrow. But I would approve a separate patch that garbage-collected these fields. I did verify that, indeed, they are not used at all. -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-19 6:04 ` Joel Brobecker @ 2008-09-22 15:25 ` Jan Kratochvil 2008-09-24 19:15 ` Joel Brobecker 0 siblings, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-09-22 15:25 UTC (permalink / raw) To: Joel Brobecker Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb [-- Attachment #1: Type: text/plain, Size: 723 bytes --] On Fri, 19 Sep 2008 08:03:36 +0200, Joel Brobecker wrote: > > One of such spare space there is `enum array_bound_type' Except for > > BOUND_CANNOT_BE_DETERMINED it is never set anywhere. BOUND_BY_* are > > read only in f-valprint.c. In fact it could be probably used for the > > new bits TYPE_BOUND_IS_DWARF_BLOCK and possibly > > TYPE_HIGH_BOUND_IS_COUNT but I would rather like to later remove `enum > > array_bound_type' and its fields at all. > > I haven't started looking at the actual patch yet, but hope to get to it > either tonight or tomorrow. But I would approve a separate patch that > garbage-collected these fields. I did verify that, indeed, they are not > used at all. Here is the patch. Thanks, Jan [-- Attachment #2: gdb-cvs-nobounds.patch --] [-- Type: text/plain, Size: 21821 bytes --] 2008-09-22 Jan Kratochvil <jan.kratochvil@redhat.com> Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined. * gdb/c-typeprint.c (c_type_print_varspec_suffix), gdb/m2-typeprint.c (m2_array), gdb/p-typeprint.c (pascal_type_print_varspec_prefix), gdb/valops.c (value_cast), gdb/varobj.c (c_number_of_children): Replace TYPE_ARRAY_UPPER_BOUND_TYPE compared to BOUND_CANNOT_BE_DETERMINED by TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. * gdb/parse.c (follow_types): Use TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. * gdb/f-valprint.c (f77_get_dynamic_upperbound): Replace with ... (f77_get_upperbound): ... this function handling now only TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. (f77_get_dynamic_lowerbound): Replace with ... (f77_get_lowerbound): ... this function handling now only TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED. (f77_get_dynamic_length_of_aggregate, f77_create_arrayprint_offset_tbl): Update their callers. * gdb/eval.c (evaluate_subexp_standard): Update their callers. * gdb/f-lang.h (f77_get_dynamic_upperbound, f77_get_upperbound) (f77_get_dynamic_lowerbound, f77_get_lowerbound): Update their prototypes. (BOUND_FETCH_OK, BOUND_FETCH_ERROR): Remove. * gdb/f-typeprint.c (f_type_print_varspec_suffix, f_type_print_base): Remove the lower_bound_was_default variable. Update the f77_get_dynamic_upperbound, f77_get_upperbound and TYPE_ARRAY_UPPER_BOUND_TYPE calls. * gdb/gdbtypes.c (print_bound_type): Remove the function. (recursive_dump_type): Replace the print_bound_type calls by simple prints. * gdb/gdbtypes.h (enum array_bound_type): Remove. (struct main_type): Remove the fields upper_bound_type and lower_bound_type. Comment the new overload of the field artificial. (TYPE_ARRAY_UPPER_BOUND_TYPE): Replace by ... (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED): ... this macro. (TYPE_ARRAY_LOWER_BOUND_TYPE): Replace by ... (TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED): ... this macro. * gdb/testsuite/gdb.base/break.c (array): New variable. (main): Artificially access ARRAY. * gdb/testsuite/gdb.base/maint.exp (maint print type): Rename to ... (maint print type argc): ... this testcase name, remove its removed upper_bound_type and lower_bound_type expectations. (maint print type array): New testcase. --- gdb/c-typeprint.c 3 May 2008 22:20:13 -0000 1.42 +++ gdb/c-typeprint.c 22 Sep 2008 14:55:28 -0000 @@ -542,7 +542,7 @@ c_type_print_varspec_suffix (struct type fprintf_filtered (stream, "["); if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintf_filtered (stream, "%d", (TYPE_LENGTH (type) / TYPE_LENGTH (TYPE_TARGET_TYPE (type)))); --- gdb/eval.c 11 Sep 2008 14:27:34 -0000 1.98 +++ gdb/eval.c 22 Sep 2008 14:55:32 -0000 @@ -1994,13 +1994,8 @@ evaluate_subexp_standard (struct type *e /* Internal type of array is arranged right to left */ for (i = 0; i < nargs; i++) { - retcode = f77_get_dynamic_upperbound (tmp_type, &upper); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic upper bound")); - - retcode = f77_get_dynamic_lowerbound (tmp_type, &lower); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic lower bound")); + upper = f77_get_upperbound (tmp_type); + lower = f77_get_lowerbound (tmp_type); array_size_array[nargs - i - 1] = upper - lower + 1; --- gdb/f-lang.h 11 Sep 2008 14:31:43 -0000 1.12 +++ gdb/f-lang.h 22 Sep 2008 14:55:32 -0000 @@ -83,9 +83,6 @@ extern SAVED_F77_COMMON_PTR find_common_ #define BLANK_COMMON_NAME_MF77 "__BLNK__" /* MF77 assigned */ #define BLANK_COMMON_NAME_LOCAL "__BLANK" /* Local GDB */ -#define BOUND_FETCH_OK 1 -#define BOUND_FETCH_ERROR -999 - /* When reasonable array bounds cannot be fetched, such as when you ask to 'mt print symbols' and there is no stack frame and therefore no way of knowing the bounds of stack-based arrays, @@ -97,9 +94,9 @@ extern SAVED_F77_COMMON_PTR find_common_ extern char *real_main_name; /* Name of main function */ extern int real_main_c_value; /* C_value field of main function */ -extern int f77_get_dynamic_upperbound (struct type *, int *); +extern int f77_get_upperbound (struct type *); -extern int f77_get_dynamic_lowerbound (struct type *, int *); +extern int f77_get_lowerbound (struct type *); extern void f77_get_dynamic_array_length (struct type *); --- gdb/f-typeprint.c 22 Apr 2008 06:53:18 -0000 1.23 +++ gdb/f-typeprint.c 22 Sep 2008 14:55:32 -0000 @@ -153,7 +153,6 @@ f_type_print_varspec_suffix (struct type int show, int passed_a_ptr, int demangled_args) { int upper_bound, lower_bound; - int lower_bound_was_default = 0; static int arrayprint_recurse_level = 0; int retcode; @@ -176,35 +175,19 @@ f_type_print_varspec_suffix (struct type if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY) f_type_print_varspec_suffix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0); - retcode = f77_get_dynamic_lowerbound (type, &lower_bound); - - lower_bound_was_default = 0; - - if (retcode == BOUND_FETCH_ERROR) - fprintf_filtered (stream, "???"); - else if (lower_bound == 1) /* The default */ - lower_bound_was_default = 1; - else - fprintf_filtered (stream, "%d", lower_bound); - - if (lower_bound_was_default) - lower_bound_was_default = 0; - else - fprintf_filtered (stream, ":"); + lower_bound = f77_get_lowerbound (type); + if (lower_bound != 1) /* Not the default. */ + fprintf_filtered (stream, "%d:", lower_bound); /* Make sure that, if we have an assumed size array, we print out a warning and print the upperbound as '*' */ - if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED) + if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintf_filtered (stream, "*"); else { - retcode = f77_get_dynamic_upperbound (type, &upper_bound); - - if (retcode == BOUND_FETCH_ERROR) - fprintf_filtered (stream, "???"); - else - fprintf_filtered (stream, "%d", upper_bound); + upper_bound = f77_get_upperbound (type); + fprintf_filtered (stream, "%d", upper_bound); } if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_ARRAY) @@ -354,16 +337,12 @@ f_type_print_base (struct type *type, st case TYPE_CODE_STRING: /* Strings may have dynamic upperbounds (lengths) like arrays. */ - if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED) + if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintfi_filtered (level, stream, "character*(*)"); else { - retcode = f77_get_dynamic_upperbound (type, &upper_bound); - - if (retcode == BOUND_FETCH_ERROR) - fprintf_filtered (stream, "character*???"); - else - fprintf_filtered (stream, "character*%d", upper_bound); + upper_bound = f77_get_upperbound (type); + fprintf_filtered (stream, "character*%d", upper_bound); } break; --- gdb/f-valprint.c 11 Sep 2008 14:28:47 -0000 1.45 +++ gdb/f-valprint.c 22 Sep 2008 14:55:32 -0000 @@ -61,132 +61,28 @@ int f77_array_offset_tbl[MAX_FORTRAN_DIM #define F77_DIM_OFFSET(n) (f77_array_offset_tbl[n][0]) int -f77_get_dynamic_lowerbound (struct type *type, int *lower_bound) +f77_get_lowerbound (struct type *type) { - struct frame_info *frame; - CORE_ADDR current_frame_addr; - CORE_ADDR ptr_to_lower_bound; - - switch (TYPE_ARRAY_LOWER_BOUND_TYPE (type)) - { - case BOUND_BY_VALUE_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - *lower_bound = - read_memory_integer (current_frame_addr + - TYPE_ARRAY_LOWER_BOUND_VALUE (type), - 4); - } - else - { - *lower_bound = DEFAULT_LOWER_BOUND; - return BOUND_FETCH_ERROR; - } - break; - - case BOUND_SIMPLE: - *lower_bound = TYPE_ARRAY_LOWER_BOUND_VALUE (type); - break; - - case BOUND_CANNOT_BE_DETERMINED: - error (_("Lower bound may not be '*' in F77")); - break; + if (TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)) + error (_("Lower bound may not be '*' in F77")); - case BOUND_BY_REF_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - struct gdbarch *arch = get_frame_arch (frame); - ptr_to_lower_bound = - read_memory_typed_address (current_frame_addr + - TYPE_ARRAY_LOWER_BOUND_VALUE (type), - builtin_type (arch)->builtin_data_ptr); - *lower_bound = read_memory_integer (ptr_to_lower_bound, 4); - } - else - { - *lower_bound = DEFAULT_LOWER_BOUND; - return BOUND_FETCH_ERROR; - } - break; - - case BOUND_BY_REF_IN_REG: - case BOUND_BY_VALUE_IN_REG: - default: - error (_("??? unhandled dynamic array bound type ???")); - break; - } - return BOUND_FETCH_OK; + return TYPE_ARRAY_LOWER_BOUND_VALUE (type); } int -f77_get_dynamic_upperbound (struct type *type, int *upper_bound) +f77_get_upperbound (struct type *type) { - struct frame_info *frame; - CORE_ADDR current_frame_addr = 0; - CORE_ADDR ptr_to_upper_bound; - - switch (TYPE_ARRAY_UPPER_BOUND_TYPE (type)) - { - case BOUND_BY_VALUE_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - *upper_bound = - read_memory_integer (current_frame_addr + - TYPE_ARRAY_UPPER_BOUND_VALUE (type), - 4); - } - else - { - *upper_bound = DEFAULT_UPPER_BOUND; - return BOUND_FETCH_ERROR; - } - break; - - case BOUND_SIMPLE: - *upper_bound = TYPE_ARRAY_UPPER_BOUND_VALUE (type); - break; - - case BOUND_CANNOT_BE_DETERMINED: - /* we have an assumed size array on our hands. Assume that - upper_bound == lower_bound so that we show at least - 1 element.If the user wants to see more elements, let - him manually ask for 'em and we'll subscript the - array and show him */ - f77_get_dynamic_lowerbound (type, upper_bound); - break; - - case BOUND_BY_REF_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - struct gdbarch *arch = get_frame_arch (frame); - ptr_to_upper_bound = - read_memory_typed_address (current_frame_addr + - TYPE_ARRAY_UPPER_BOUND_VALUE (type), - builtin_type (arch)->builtin_data_ptr); - *upper_bound = read_memory_integer (ptr_to_upper_bound, 4); - } - else - { - *upper_bound = DEFAULT_UPPER_BOUND; - return BOUND_FETCH_ERROR; - } - break; + if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) + { + /* We have an assumed size array on our hands. Assume that + upper_bound == lower_bound so that we show at least 1 element. + If the user wants to see more elements, let him manually ask for 'em + and we'll subscript the array and show him. */ - case BOUND_BY_REF_IN_REG: - case BOUND_BY_VALUE_IN_REG: - default: - error (_("??? unhandled dynamic array bound type ???")); - break; + return f77_get_lowerbound (type); } - return BOUND_FETCH_OK; + + return TYPE_ARRAY_UPPER_BOUND_VALUE (type); } /* Obtain F77 adjustable array dimensions */ @@ -212,13 +108,8 @@ f77_get_dynamic_length_of_aggregate (str f77_get_dynamic_length_of_aggregate (TYPE_TARGET_TYPE (type)); /* Recursion ends here, start setting up lengths. */ - retcode = f77_get_dynamic_lowerbound (type, &lower_bound); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain valid array lower bound")); - - retcode = f77_get_dynamic_upperbound (type, &upper_bound); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain valid array upper bound")); + lower_bound = f77_get_lowerbound (type); + upper_bound = f77_get_upperbound (type); /* Patch in a valid length value. */ @@ -241,16 +132,8 @@ f77_create_arrayprint_offset_tbl (struct while ((TYPE_CODE (tmp_type) == TYPE_CODE_ARRAY)) { - if (TYPE_ARRAY_UPPER_BOUND_TYPE (tmp_type) == BOUND_CANNOT_BE_DETERMINED) - fprintf_filtered (stream, "<assumed size array> "); - - retcode = f77_get_dynamic_upperbound (tmp_type, &upper); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic upper bound")); - - retcode = f77_get_dynamic_lowerbound (tmp_type, &lower); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic lower bound")); + upper = f77_get_upperbound (tmp_type); + lower = f77_get_lowerbound (tmp_type); F77_DIM_SIZE (ndimen) = upper - lower + 1; --- gdb/gdbtypes.c 11 Sep 2008 14:24:27 -0000 1.150 +++ gdb/gdbtypes.c 22 Sep 2008 14:55:34 -0000 @@ -2556,35 +2556,6 @@ print_cplus_stuff (struct type *type, in } } -static void -print_bound_type (int bt) -{ - switch (bt) - { - case BOUND_CANNOT_BE_DETERMINED: - printf_filtered ("(BOUND_CANNOT_BE_DETERMINED)"); - break; - case BOUND_BY_REF_ON_STACK: - printf_filtered ("(BOUND_BY_REF_ON_STACK)"); - break; - case BOUND_BY_VALUE_ON_STACK: - printf_filtered ("(BOUND_BY_VALUE_ON_STACK)"); - break; - case BOUND_BY_REF_IN_REG: - printf_filtered ("(BOUND_BY_REF_IN_REG)"); - break; - case BOUND_BY_VALUE_IN_REG: - printf_filtered ("(BOUND_BY_VALUE_IN_REG)"); - break; - case BOUND_SIMPLE: - printf_filtered ("(BOUND_SIMPLE)"); - break; - default: - printf_filtered (_("(unknown bound type)")); - break; - } -} - static struct obstack dont_print_type_obstack; void @@ -2719,14 +2690,13 @@ recursive_dump_type (struct type *type, } puts_filtered ("\n"); printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type)); - printfi_filtered (spaces, "upper_bound_type 0x%x ", - TYPE_ARRAY_UPPER_BOUND_TYPE (type)); - print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type)); - puts_filtered ("\n"); - printfi_filtered (spaces, "lower_bound_type 0x%x ", - TYPE_ARRAY_LOWER_BOUND_TYPE (type)); - print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type)); - puts_filtered ("\n"); + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) + { + printfi_filtered (spaces, "upper bound undefined is %d\n", + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)); + printfi_filtered (spaces, "lower bound undefined is %d\n", + TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)); + } printfi_filtered (spaces, "objfile "); gdb_print_host_address (TYPE_OBJFILE (type), gdb_stdout); printf_filtered ("\n"); --- gdb/gdbtypes.h 11 Sep 2008 14:31:43 -0000 1.91 +++ gdb/gdbtypes.h 22 Sep 2008 14:55:36 -0000 @@ -316,18 +316,6 @@ enum type_instance_flag_value #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) - -/* Array bound type. */ -enum array_bound_type -{ - BOUND_SIMPLE = 0, - BOUND_BY_VALUE_IN_REG, - BOUND_BY_REF_IN_REG, - BOUND_BY_VALUE_ON_STACK, - BOUND_BY_REF_ON_STACK, - BOUND_CANNOT_BE_DETERMINED -}; - /* This structure is space-critical. Its layout has been tweaked to reduce the space used. */ @@ -337,12 +325,6 @@ struct main_type ENUM_BITFIELD(type_code) code : 8; - /* Array bounds. These fields appear at this location because - they pack nicely here. */ - - ENUM_BITFIELD(array_bound_type) upper_bound_type : 4; - ENUM_BITFIELD(array_bound_type) lower_bound_type : 4; - /* Flags about this type. These fields appear at this location because they packs nicely here. See the TYPE_* macros for documentation about these fields. */ @@ -460,7 +442,8 @@ struct main_type /* For a function or member type, this is 1 if the argument is marked artificial. Artificial arguments should not be shown to the - user. */ + user. For TYPE_CODE_RANGE it is set if the specific bound is not + defined. */ unsigned int artificial : 1; /* This flag is zero for non-static fields, 1 for fields whose location @@ -817,10 +800,10 @@ extern void allocate_cplus_struct_type ( /* Moto-specific stuff for FORTRAN arrays */ -#define TYPE_ARRAY_UPPER_BOUND_TYPE(thistype) \ - TYPE_MAIN_TYPE(thistype)->upper_bound_type -#define TYPE_ARRAY_LOWER_BOUND_TYPE(thistype) \ - TYPE_MAIN_TYPE(thistype)->lower_bound_type +#define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \ + (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),1)) +#define TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED(arraytype) \ + (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),0)) #define TYPE_ARRAY_UPPER_BOUND_VALUE(arraytype) \ (TYPE_FIELD_BITPOS((TYPE_FIELD_TYPE((arraytype),0)),1)) --- gdb/m2-typeprint.c 11 Sep 2008 14:22:32 -0000 1.16 +++ gdb/m2-typeprint.c 22 Sep 2008 14:55:36 -0000 @@ -202,7 +202,7 @@ static void m2_array (struct type *type, { fprintf_filtered (stream, "ARRAY ["); if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) { if (TYPE_INDEX_TYPE (type) != 0) { --- gdb/p-typeprint.c 11 Sep 2008 14:22:32 -0000 1.22 +++ gdb/p-typeprint.c 22 Sep 2008 14:55:37 -0000 @@ -251,7 +251,7 @@ pascal_type_print_varspec_prefix (struct fprintf_filtered (stream, "("); fprintf_filtered (stream, "array "); if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintf_filtered (stream, "[%d..%d] ", TYPE_ARRAY_LOWER_BOUND_VALUE (type), TYPE_ARRAY_UPPER_BOUND_VALUE (type) --- gdb/parse.c 11 Sep 2008 14:22:32 -0000 1.79 +++ gdb/parse.c 22 Sep 2008 14:55:37 -0000 @@ -1268,8 +1268,7 @@ follow_types (struct type *follow_type) create_array_type ((struct type *) NULL, follow_type, range_type); if (array_size < 0) - TYPE_ARRAY_UPPER_BOUND_TYPE (follow_type) - = BOUND_CANNOT_BE_DETERMINED; + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (follow_type) = 1; break; case tp_function: /* FIXME-type-allocation: need a way to free this type when we are --- gdb/valops.c 11 Sep 2008 14:27:34 -0000 1.200 +++ gdb/valops.c 22 Sep 2008 14:55:41 -0000 @@ -361,8 +361,7 @@ value_cast (struct type *type, struct va { struct type *element_type = TYPE_TARGET_TYPE (type); unsigned element_length = TYPE_LENGTH (check_typedef (element_type)); - if (element_length > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED) + if (element_length > 0 && TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) { struct type *range_type = TYPE_INDEX_TYPE (type); int val_length = TYPE_LENGTH (type2); --- gdb/varobj.c 13 Sep 2008 23:37:05 -0000 1.117 +++ gdb/varobj.c 22 Sep 2008 14:55:43 -0000 @@ -1951,7 +1951,7 @@ c_number_of_children (struct varobj *var { case TYPE_CODE_ARRAY: if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (target) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) children = TYPE_LENGTH (type) / TYPE_LENGTH (target); else /* If we don't know how many elements there are, don't display --- gdb/testsuite/gdb.base/break.c 6 Aug 2008 12:52:07 -0000 1.11 +++ gdb/testsuite/gdb.base/break.c 22 Sep 2008 14:55:46 -0000 @@ -62,6 +62,9 @@ extern void marker3 (); extern void marker4 (); #endif +/* Test of the array boundaries debug information. */ +static char array[2]; + /* * This simple classical example of recursion is useful for * testing stack backtraces and such. @@ -96,6 +99,7 @@ char *argv[], **envp; /* We're used by a test that requires malloc, so make sure it is in the executable. */ (void)malloc (1); + array[0] = array[1]; argc = (argc == 12345); /* This is silly, but we can step off of it */ /* set breakpoint 2 here */ return argc; /* set breakpoint 10 here */ --- gdb/testsuite/gdb.base/maint.exp 24 Aug 2008 16:39:57 -0000 1.35 +++ gdb/testsuite/gdb.base/maint.exp 22 Sep 2008 14:55:48 -0000 @@ -399,13 +399,11 @@ gdb_expect { timeout { fail "(timeout) maint print symbols" } } -send_gdb "maint print type argc\n" -gdb_expect { - -re "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nupper_bound_type $hex \\(BOUND_SIMPLE\\)\r\nlower_bound_type $hex \\(BOUND_SIMPLE\\)\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex\r\n$gdb_prompt $"\ - { pass "maint print type" } - -re ".*$gdb_prompt $" { fail "maint print type" } - timeout { fail "(timeout) maint print type" } - } +gdb_test "maint print type argc" "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex" + +# TYPE_CODE_ARRAY has the additional two fields: `upper bound undefined is %d' +# and `lower bound undefined is %d' +gdb_test "maint print type array" "code $hex \\(TYPE_CODE_ARRAY\\)\r\nlength \[2\]\r\nupper bound undefined is 0\r\nlower bound undefined is 0\r\nobjfile $hex.*" if [istarget "hppa*-*-11*"] { setup_xfail hppa*-*-*11* CLLbs14860 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-22 15:25 ` Jan Kratochvil @ 2008-09-24 19:15 ` Joel Brobecker 2008-09-26 5:03 ` Jan Kratochvil 2008-09-26 12:52 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz 0 siblings, 2 replies; 34+ messages in thread From: Joel Brobecker @ 2008-09-24 19:15 UTC (permalink / raw) To: Jan Kratochvil Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb > Here is the patch. Argh, it took me a long time to break through the patch because I had missed the fact that the upper-bound was indeed set to BOUND_CANNOT_BE_DETERMINED once. I think you were pretty clear about this in your previous message, but my (hopefully then) fried brain didn't register. I'm sorry I missed that somehow, because it changes slightly my point of view. To make it up to you, I will try to help as much as I can. Anyway, so the patch is using the TYPE_FIELD_ARTIFICIAL bit to detect range types where a given bound value cannot be determined. The patch as it is would work, and I wouldn't object to having it checked in as a small step forward (with the agreement from at least one other maintainer). However, I think we can implement this slightly differently to make it more general. struct field has the following two components: /* 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; In practice, these are really what we call "discriminants" in Ada, which means a value that we read in order to know what field to use in the "loc" union. I propose that, instead of re-using the "artificial" flag for undefined bounds, we merge the two components "artificial" and "static_kind" together into a 3-bit field whose value would be an enum that tells us what field to use for our field loc. Because this field would be 3-bit long, it would allow us to have 7 different possible fields, in addition to the case where the bound is undefined or the argument is artificial. That would leave more than enough room to then add a DWARF block as a possible field loc. What does everyone think? Some comments on your patch, that I did review pretty carefully: > 2008-09-22 Jan Kratochvil <jan.kratochvil@redhat.com> > > Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined. > * gdb/c-typeprint.c (c_type_print_varspec_suffix), gdb/m2-typeprint.c > (m2_array), gdb/p-typeprint.c (pascal_type_print_varspec_prefix), > gdb/valops.c (value_cast), gdb/varobj.c (c_number_of_children): Replace > TYPE_ARRAY_UPPER_BOUND_TYPE compared to BOUND_CANNOT_BE_DETERMINED by > TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. [...] I am not sure about the reason why you listed all the filenames with the gdb/ subdirectory in it, but the gdb/ subdir has its own ChangeLog, so it shouldn't be part of the file name. Perhaps it's because you wanted to make it clear that the changes are in the gdb/ directory. Usually, it's obvious enough that I don't bother. When I make changes to more than one directory such that it will cause more than one ChangeLog to be updated, I sometimes write something like this: gdb/ 2008-09-22 Joel Brobecker <....> [...] gdb/testsuite/ 2008-09-22 Joel Brobecker <...> Most of the time, I don't even bother, as it is clear enough. I do make sure to write each ChangeLog separately as shown above, though. I noticed that you grouped all your changes, including your testsuite changes into one CL entry. > - retcode = f77_get_dynamic_lowerbound (type, &lower_bound); > - > - lower_bound_was_default = 0; > - > - if (retcode == BOUND_FETCH_ERROR) > - fprintf_filtered (stream, "???"); > - else if (lower_bound == 1) /* The default */ > - lower_bound_was_default = 1; > - else > - fprintf_filtered (stream, "%d", lower_bound); > - > - if (lower_bound_was_default) > - lower_bound_was_default = 0; > - else > - fprintf_filtered (stream, ":"); > + lower_bound = f77_get_lowerbound (type); > + if (lower_bound != 1) /* Not the default. */ > + fprintf_filtered (stream, "%d:", lower_bound); Here, it looks like we're slightly modifying the behavior - if the lower bound was undefined, then we're throwing an error when we used not to. I think that would be unfriendly if this issue is caused by bad debug info. Should we check that the lower bound is not undefined first, and print "???" if it is? > @@ -2719,14 +2690,13 @@ recursive_dump_type (struct type *type, > } > puts_filtered ("\n"); > printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type)); > - printfi_filtered (spaces, "upper_bound_type 0x%x ", > - TYPE_ARRAY_UPPER_BOUND_TYPE (type)); > - print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type)); > - puts_filtered ("\n"); > - printfi_filtered (spaces, "lower_bound_type 0x%x ", > - TYPE_ARRAY_LOWER_BOUND_TYPE (type)); > - print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type)); > - puts_filtered ("\n"); > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) > + { > + printfi_filtered (spaces, "upper bound undefined is %d\n", > + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)); > + printfi_filtered (spaces, "lower bound undefined is %d\n", > + TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)); > + } I think this is only repetitive, and shouldn't be displayed. The artificial field value will be displayed a little later, and that should be good enough. No strong objection, though. -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-24 19:15 ` Joel Brobecker @ 2008-09-26 5:03 ` Jan Kratochvil 2008-09-26 22:12 ` Joel Brobecker 2008-09-26 12:52 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz 1 sibling, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-09-26 5:03 UTC (permalink / raw) To: Joel Brobecker Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb On Wed, 24 Sep 2008 21:15:04 +0200, Joel Brobecker wrote: > struct field has the following two components: > > /* 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; > > In practice, these are really what we call "discriminants" in Ada, which > means a value that we read in order to know what field to use in the > "loc" union. I agree "static_kind" is "discriminant" but not "artificial". Artificial is just additional flag existing only for FIELDS of type TYPE_CODE_FUNC or type TYPE_CODE_METHOD: FIELDs: artificial static_kind!=0 TYPE_CODE_ENUM no no TYPE_CODE_FUNC or TYPE_CODE_METHOD yes no TYPE_CODE_STRUCT yes(_vptr) yes(static field) Somehow I do not see how these two items are related - just each of them exists for some types and does not exist for some other types. Moreover the split into the big union gets more complicated when we merge them. Should I really rework the patch into the enum merging artificial+static_kind? > What does everyone think? I expect MAIN_TYPE may get reworked into a big union with per-type fields and all the bit-size alignment gets different etc. But otherwise I accept any decision on the layout of these. > > 2008-09-22 Jan Kratochvil <jan.kratochvil@redhat.com> > > > > Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined. > > * gdb/c-typeprint.c (c_type_print_varspec_suffix), gdb/m2-typeprint.c ... > I am not sure about the reason why you listed all the filenames with > the gdb/ subdirectory in it, Sorry, a typo from the ChangeLog script http://sourceware.org/ml/gdb-patches/2006-08/msg00195.html , put there at least a warning now: warn "Extra directory" if $dfile=~m{(?:gdb|bfd|opcodes|libiberty)/}; > I do make sure to write each ChangeLog separately as shown above, though. > I noticed that you grouped all your changes, including your testsuite > changes into one CL entry. Sorry, also a typo, already written the split way many types: http://sourceware.org/ml/gdb-patches/2008-07/msg00145.html I put there the directory prefixes sometimes only in the CVS commits: http://sourceware.org/ml/gdb-cvs/2008-07/msg00048.html > > - retcode = f77_get_dynamic_lowerbound (type, &lower_bound); > > - > > - lower_bound_was_default = 0; > > - > > - if (retcode == BOUND_FETCH_ERROR) > > - fprintf_filtered (stream, "???"); > > - else if (lower_bound == 1) /* The default */ > > - lower_bound_was_default = 1; > > - else > > - fprintf_filtered (stream, "%d", lower_bound); > > - > > - if (lower_bound_was_default) > > - lower_bound_was_default = 0; > > - else > > - fprintf_filtered (stream, ":"); > > + lower_bound = f77_get_lowerbound (type); > > + if (lower_bound != 1) /* Not the default. */ > > + fprintf_filtered (stream, "%d:", lower_bound); > > Here, it looks like we're slightly modifying the behavior - if the lower > bound was undefined, then we're throwing an error when we used not to. That was a dead code. BOUND_FETCH_ERROR could never have been returned by f77_get_dynamic_lowerbound(): BOUND_SIMPLE returns BOUND_FETCH_OK. BOUND_CANNOT_BE_DETERMINED already error()s itself and never returns. Other BOUND_* cases were never set. Another question is if BOUND_CANNOT_BE_DETERMINED should error() as currently does or if it rather should return BOUND_FETCH_ERROR. But that decision is outside of the scope of my patch and changes the current GDB behavior. > > @@ -2719,14 +2690,13 @@ recursive_dump_type (struct type *type, > > } > > puts_filtered ("\n"); > > printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type)); > > - printfi_filtered (spaces, "upper_bound_type 0x%x ", > > - TYPE_ARRAY_UPPER_BOUND_TYPE (type)); > > - print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type)); > > - puts_filtered ("\n"); > > - printfi_filtered (spaces, "lower_bound_type 0x%x ", > > - TYPE_ARRAY_LOWER_BOUND_TYPE (type)); > > - print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type)); > > - puts_filtered ("\n"); > > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) > > + { > > + printfi_filtered (spaces, "upper bound undefined is %d\n", > > + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)); > > + printfi_filtered (spaces, "lower bound undefined is %d\n", > > + TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)); > > + } > > I think this is only repetitive, and shouldn't be displayed. The > artificial field value will be displayed a little later, and that > should be good enough. No strong objection, though. As I hope the "artificial" field may start to exist only for some types I tried to make the patch more abstract to the specific implementation of TYPE_ARRAY_{UPPER,LOWER}_BOUND_IS_UNDEFINED. But no strong opinion on either way. Thanks, Jan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-26 5:03 ` Jan Kratochvil @ 2008-09-26 22:12 ` Joel Brobecker 2008-10-02 22:13 ` [patch] Fortran obsolete bounds type [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil 0 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-09-26 22:12 UTC (permalink / raw) To: Jan Kratochvil Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb > I agree "static_kind" is "discriminant" but not "artificial". Artificial is > just additional flag existing only for FIELDS of type TYPE_CODE_FUNC or type > TYPE_CODE_METHOD: Yes, I don't know why I was thinking that artificial was a discriminant (meaning "no location"). > Moreover the split into the big union gets more complicated when we merge > them. Should I really rework the patch into the enum merging > artificial+static_kind? No that won't be necessary. I agree with Daniel that we should go with your patch for now. Improving the clarity of the discriminant can be done separately. > Sorry, a typo from the ChangeLog script > http://sourceware.org/ml/gdb-patches/2006-08/msg00195.html > , put there at least a warning now: > warn "Extra directory" if $dfile=~m{(?:gdb|bfd|opcodes|libiberty)/}; No problem, please just remember to fix these before you commit. > > > + lower_bound = f77_get_lowerbound (type); > > > + if (lower_bound != 1) /* Not the default. */ > > > + fprintf_filtered (stream, "%d:", lower_bound); > > > > Here, it looks like we're slightly modifying the behavior - if the lower > > bound was undefined, then we're throwing an error when we used not to. > > That was a dead code. BOUND_FETCH_ERROR could never have been returned by > f77_get_dynamic_lowerbound(): > BOUND_SIMPLE returns BOUND_FETCH_OK. > BOUND_CANNOT_BE_DETERMINED already error()s itself and never returns. > Other BOUND_* cases were never set. Hmmm, you are right! So that piece was fine too. > Another question is if BOUND_CANNOT_BE_DETERMINED should error() as > currently does or if it rather should return BOUND_FETCH_ERROR. But > that decision is outside of the scope of my patch and changes the > current GDB behavior. Absolutely. > > > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) > > > + { > > > + printfi_filtered (spaces, "upper bound undefined is %d\n", > > > + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)); > > > + printfi_filtered (spaces, "lower bound undefined is %d\n", > > > + TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)); > > > + } > > > > I think this is only repetitive, and shouldn't be displayed. The > > artificial field value will be displayed a little later, and that > > should be good enough. No strong objection, though. > > As I hope the "artificial" field may start to exist only for some types > I tried to make the patch more abstract to the specific implementation of > TYPE_ARRAY_{UPPER,LOWER}_BOUND_IS_UNDEFINED. But no strong opinion on > either way. That's only my opinion too, and there is no real reason why my opinion should count too. But to me, this function does a low-level dump of our type structure, and thus should follow closely the implementation. If we start to introduce a higher-level view of this type, I think it can potentially be confusing. For now, I would prefer to leave that part out. Your patch is approved with the few corrections I mentioned. Thank you. -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* [patch] Fortran obsolete bounds type [Re: [patch] static_kind -> bit0, bit1] 2008-09-26 22:12 ` Joel Brobecker @ 2008-10-02 22:13 ` Jan Kratochvil 0 siblings, 0 replies; 34+ messages in thread From: Jan Kratochvil @ 2008-10-02 22:13 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy [-- Attachment #1: Type: text/plain, Size: 869 bytes --] On Sat, 27 Sep 2008 00:11:19 +0200, Joel Brobecker wrote: > > > > + if (TYPE_CODE (type) == TYPE_CODE_ARRAY) > > > > + { > > > > + printfi_filtered (spaces, "upper bound undefined is %d\n", > > > > + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)); > > > > + printfi_filtered (spaces, "lower bound undefined is %d\n", > > > > + TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)); > > > > + } ... > That's only my opinion too, and there is no real reason why my opinion > should count too. But to me, this function does a low-level dump of > our type structure, and thus should follow closely the implementation. > If we start to introduce a higher-level view of this type, I think > it can potentially be confusing. For now, I would prefer to leave > that part out. Removed. Committed. http://sourceware.org/ml/gdb-cvs/2008-10/msg00019.html Thanks, Jan [-- Attachment #2: gdb-cvs-nobounds2.patch --] [-- Type: text/plain, Size: 19339 bytes --] gdb/ 2008-10-02 Jan Kratochvil <jan.kratochvil@redhat.com> Replace TYPE_ARRAY_{UPPER,LOWER}_BOUND_TYPE by a bit if {un,}defined. * c-typeprint.c (c_type_print_varspec_suffix), m2-typeprint.c (m2_array), p-typeprint.c (pascal_type_print_varspec_prefix), valops.c (value_cast), varobj.c (c_number_of_children): Replace TYPE_ARRAY_UPPER_BOUND_TYPE compared to BOUND_CANNOT_BE_DETERMINED by TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. * parse.c (follow_types): Use TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. * f-valprint.c (f77_get_dynamic_upperbound): Replace with ... (f77_get_upperbound): ... this function handling now only TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED. (f77_get_dynamic_lowerbound): Replace with ... (f77_get_lowerbound): ... this function handling now only TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED. (f77_get_dynamic_length_of_aggregate, f77_create_arrayprint_offset_tbl): Update their callers. * eval.c (evaluate_subexp_standard): Update their callers. * f-lang.h (f77_get_dynamic_upperbound, f77_get_upperbound) (f77_get_dynamic_lowerbound, f77_get_lowerbound): Update their prototypes. (BOUND_FETCH_OK, BOUND_FETCH_ERROR): Remove. * f-typeprint.c (f_type_print_varspec_suffix, f_type_print_base): Remove the lower_bound_was_default variable. Update the f77_get_dynamic_upperbound, f77_get_upperbound and TYPE_ARRAY_UPPER_BOUND_TYPE calls. * gdbtypes.c (print_bound_type): Remove the function. (recursive_dump_type): Remove its calls printing UPPER_BOUND_TYPE and LOWER_BOUND_TYPE. * gdbtypes.h (enum array_bound_type): Remove. (struct main_type): Remove the fields upper_bound_type and lower_bound_type. Comment the new overload of the field artificial. (TYPE_ARRAY_UPPER_BOUND_TYPE): Replace by ... (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED): ... this macro. (TYPE_ARRAY_LOWER_BOUND_TYPE): Replace by ... (TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED): ... this macro. gdb/testsuite/ 2008-10-02 Jan Kratochvil <jan.kratochvil@redhat.com> * gdb.base/maint.exp (maint print type): Remove printing UPPER_BOUND_TYPE and LOWER_BOUND_TYPE. --- src/gdb/c-typeprint.c 2008/09/27 21:29:29 1.43 +++ src/gdb/c-typeprint.c 2008/10/02 22:06:07 1.44 @@ -560,7 +560,7 @@ fprintf_filtered (stream, "["); if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintf_filtered (stream, "%d", (TYPE_LENGTH (type) / TYPE_LENGTH (TYPE_TARGET_TYPE (type)))); --- src/gdb/eval.c 2008/09/11 14:27:34 1.98 +++ src/gdb/eval.c 2008/10/02 22:06:07 1.99 @@ -1994,13 +1994,8 @@ /* Internal type of array is arranged right to left */ for (i = 0; i < nargs; i++) { - retcode = f77_get_dynamic_upperbound (tmp_type, &upper); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic upper bound")); - - retcode = f77_get_dynamic_lowerbound (tmp_type, &lower); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic lower bound")); + upper = f77_get_upperbound (tmp_type); + lower = f77_get_lowerbound (tmp_type); array_size_array[nargs - i - 1] = upper - lower + 1; --- src/gdb/f-lang.h 2008/09/11 14:31:43 1.12 +++ src/gdb/f-lang.h 2008/10/02 22:06:07 1.13 @@ -83,9 +83,6 @@ #define BLANK_COMMON_NAME_MF77 "__BLNK__" /* MF77 assigned */ #define BLANK_COMMON_NAME_LOCAL "__BLANK" /* Local GDB */ -#define BOUND_FETCH_OK 1 -#define BOUND_FETCH_ERROR -999 - /* When reasonable array bounds cannot be fetched, such as when you ask to 'mt print symbols' and there is no stack frame and therefore no way of knowing the bounds of stack-based arrays, @@ -97,9 +94,9 @@ extern char *real_main_name; /* Name of main function */ extern int real_main_c_value; /* C_value field of main function */ -extern int f77_get_dynamic_upperbound (struct type *, int *); +extern int f77_get_upperbound (struct type *); -extern int f77_get_dynamic_lowerbound (struct type *, int *); +extern int f77_get_lowerbound (struct type *); extern void f77_get_dynamic_array_length (struct type *); --- src/gdb/f-typeprint.c 2008/04/22 06:53:18 1.23 +++ src/gdb/f-typeprint.c 2008/10/02 22:06:07 1.24 @@ -153,7 +153,6 @@ int show, int passed_a_ptr, int demangled_args) { int upper_bound, lower_bound; - int lower_bound_was_default = 0; static int arrayprint_recurse_level = 0; int retcode; @@ -176,35 +175,19 @@ if (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY) f_type_print_varspec_suffix (TYPE_TARGET_TYPE (type), stream, 0, 0, 0); - retcode = f77_get_dynamic_lowerbound (type, &lower_bound); - - lower_bound_was_default = 0; - - if (retcode == BOUND_FETCH_ERROR) - fprintf_filtered (stream, "???"); - else if (lower_bound == 1) /* The default */ - lower_bound_was_default = 1; - else - fprintf_filtered (stream, "%d", lower_bound); - - if (lower_bound_was_default) - lower_bound_was_default = 0; - else - fprintf_filtered (stream, ":"); + lower_bound = f77_get_lowerbound (type); + if (lower_bound != 1) /* Not the default. */ + fprintf_filtered (stream, "%d:", lower_bound); /* Make sure that, if we have an assumed size array, we print out a warning and print the upperbound as '*' */ - if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED) + if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintf_filtered (stream, "*"); else { - retcode = f77_get_dynamic_upperbound (type, &upper_bound); - - if (retcode == BOUND_FETCH_ERROR) - fprintf_filtered (stream, "???"); - else - fprintf_filtered (stream, "%d", upper_bound); + upper_bound = f77_get_upperbound (type); + fprintf_filtered (stream, "%d", upper_bound); } if (TYPE_CODE (TYPE_TARGET_TYPE (type)) != TYPE_CODE_ARRAY) @@ -354,16 +337,12 @@ case TYPE_CODE_STRING: /* Strings may have dynamic upperbounds (lengths) like arrays. */ - if (TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED) + if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintfi_filtered (level, stream, "character*(*)"); else { - retcode = f77_get_dynamic_upperbound (type, &upper_bound); - - if (retcode == BOUND_FETCH_ERROR) - fprintf_filtered (stream, "character*???"); - else - fprintf_filtered (stream, "character*%d", upper_bound); + upper_bound = f77_get_upperbound (type); + fprintf_filtered (stream, "character*%d", upper_bound); } break; --- src/gdb/f-valprint.c 2008/09/11 14:28:47 1.45 +++ src/gdb/f-valprint.c 2008/10/02 22:06:07 1.46 @@ -61,132 +61,28 @@ #define F77_DIM_OFFSET(n) (f77_array_offset_tbl[n][0]) int -f77_get_dynamic_lowerbound (struct type *type, int *lower_bound) +f77_get_lowerbound (struct type *type) { - struct frame_info *frame; - CORE_ADDR current_frame_addr; - CORE_ADDR ptr_to_lower_bound; - - switch (TYPE_ARRAY_LOWER_BOUND_TYPE (type)) - { - case BOUND_BY_VALUE_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - *lower_bound = - read_memory_integer (current_frame_addr + - TYPE_ARRAY_LOWER_BOUND_VALUE (type), - 4); - } - else - { - *lower_bound = DEFAULT_LOWER_BOUND; - return BOUND_FETCH_ERROR; - } - break; - - case BOUND_SIMPLE: - *lower_bound = TYPE_ARRAY_LOWER_BOUND_VALUE (type); - break; - - case BOUND_CANNOT_BE_DETERMINED: - error (_("Lower bound may not be '*' in F77")); - break; + if (TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED (type)) + error (_("Lower bound may not be '*' in F77")); - case BOUND_BY_REF_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - struct gdbarch *arch = get_frame_arch (frame); - ptr_to_lower_bound = - read_memory_typed_address (current_frame_addr + - TYPE_ARRAY_LOWER_BOUND_VALUE (type), - builtin_type (arch)->builtin_data_ptr); - *lower_bound = read_memory_integer (ptr_to_lower_bound, 4); - } - else - { - *lower_bound = DEFAULT_LOWER_BOUND; - return BOUND_FETCH_ERROR; - } - break; - - case BOUND_BY_REF_IN_REG: - case BOUND_BY_VALUE_IN_REG: - default: - error (_("??? unhandled dynamic array bound type ???")); - break; - } - return BOUND_FETCH_OK; + return TYPE_ARRAY_LOWER_BOUND_VALUE (type); } int -f77_get_dynamic_upperbound (struct type *type, int *upper_bound) +f77_get_upperbound (struct type *type) { - struct frame_info *frame; - CORE_ADDR current_frame_addr = 0; - CORE_ADDR ptr_to_upper_bound; - - switch (TYPE_ARRAY_UPPER_BOUND_TYPE (type)) - { - case BOUND_BY_VALUE_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - *upper_bound = - read_memory_integer (current_frame_addr + - TYPE_ARRAY_UPPER_BOUND_VALUE (type), - 4); - } - else - { - *upper_bound = DEFAULT_UPPER_BOUND; - return BOUND_FETCH_ERROR; - } - break; - - case BOUND_SIMPLE: - *upper_bound = TYPE_ARRAY_UPPER_BOUND_VALUE (type); - break; - - case BOUND_CANNOT_BE_DETERMINED: - /* we have an assumed size array on our hands. Assume that - upper_bound == lower_bound so that we show at least - 1 element.If the user wants to see more elements, let - him manually ask for 'em and we'll subscript the - array and show him */ - f77_get_dynamic_lowerbound (type, upper_bound); - break; - - case BOUND_BY_REF_ON_STACK: - frame = deprecated_safe_get_selected_frame (); - current_frame_addr = get_frame_base (frame); - if (current_frame_addr > 0) - { - struct gdbarch *arch = get_frame_arch (frame); - ptr_to_upper_bound = - read_memory_typed_address (current_frame_addr + - TYPE_ARRAY_UPPER_BOUND_VALUE (type), - builtin_type (arch)->builtin_data_ptr); - *upper_bound = read_memory_integer (ptr_to_upper_bound, 4); - } - else - { - *upper_bound = DEFAULT_UPPER_BOUND; - return BOUND_FETCH_ERROR; - } - break; + if (TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) + { + /* We have an assumed size array on our hands. Assume that + upper_bound == lower_bound so that we show at least 1 element. + If the user wants to see more elements, let him manually ask for 'em + and we'll subscript the array and show him. */ - case BOUND_BY_REF_IN_REG: - case BOUND_BY_VALUE_IN_REG: - default: - error (_("??? unhandled dynamic array bound type ???")); - break; + return f77_get_lowerbound (type); } - return BOUND_FETCH_OK; + + return TYPE_ARRAY_UPPER_BOUND_VALUE (type); } /* Obtain F77 adjustable array dimensions */ @@ -212,13 +108,8 @@ f77_get_dynamic_length_of_aggregate (TYPE_TARGET_TYPE (type)); /* Recursion ends here, start setting up lengths. */ - retcode = f77_get_dynamic_lowerbound (type, &lower_bound); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain valid array lower bound")); - - retcode = f77_get_dynamic_upperbound (type, &upper_bound); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain valid array upper bound")); + lower_bound = f77_get_lowerbound (type); + upper_bound = f77_get_upperbound (type); /* Patch in a valid length value. */ @@ -241,16 +132,8 @@ while ((TYPE_CODE (tmp_type) == TYPE_CODE_ARRAY)) { - if (TYPE_ARRAY_UPPER_BOUND_TYPE (tmp_type) == BOUND_CANNOT_BE_DETERMINED) - fprintf_filtered (stream, "<assumed size array> "); - - retcode = f77_get_dynamic_upperbound (tmp_type, &upper); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic upper bound")); - - retcode = f77_get_dynamic_lowerbound (tmp_type, &lower); - if (retcode == BOUND_FETCH_ERROR) - error (_("Cannot obtain dynamic lower bound")); + upper = f77_get_upperbound (tmp_type); + lower = f77_get_lowerbound (tmp_type); F77_DIM_SIZE (ndimen) = upper - lower + 1; --- src/gdb/gdbtypes.c 2008/09/11 14:24:27 1.150 +++ src/gdb/gdbtypes.c 2008/10/02 22:06:07 1.151 @@ -2556,35 +2556,6 @@ } } -static void -print_bound_type (int bt) -{ - switch (bt) - { - case BOUND_CANNOT_BE_DETERMINED: - printf_filtered ("(BOUND_CANNOT_BE_DETERMINED)"); - break; - case BOUND_BY_REF_ON_STACK: - printf_filtered ("(BOUND_BY_REF_ON_STACK)"); - break; - case BOUND_BY_VALUE_ON_STACK: - printf_filtered ("(BOUND_BY_VALUE_ON_STACK)"); - break; - case BOUND_BY_REF_IN_REG: - printf_filtered ("(BOUND_BY_REF_IN_REG)"); - break; - case BOUND_BY_VALUE_IN_REG: - printf_filtered ("(BOUND_BY_VALUE_IN_REG)"); - break; - case BOUND_SIMPLE: - printf_filtered ("(BOUND_SIMPLE)"); - break; - default: - printf_filtered (_("(unknown bound type)")); - break; - } -} - static struct obstack dont_print_type_obstack; void @@ -2719,14 +2690,6 @@ } puts_filtered ("\n"); printfi_filtered (spaces, "length %d\n", TYPE_LENGTH (type)); - printfi_filtered (spaces, "upper_bound_type 0x%x ", - TYPE_ARRAY_UPPER_BOUND_TYPE (type)); - print_bound_type (TYPE_ARRAY_UPPER_BOUND_TYPE (type)); - puts_filtered ("\n"); - printfi_filtered (spaces, "lower_bound_type 0x%x ", - TYPE_ARRAY_LOWER_BOUND_TYPE (type)); - print_bound_type (TYPE_ARRAY_LOWER_BOUND_TYPE (type)); - puts_filtered ("\n"); printfi_filtered (spaces, "objfile "); gdb_print_host_address (TYPE_OBJFILE (type), gdb_stdout); printf_filtered ("\n"); --- src/gdb/gdbtypes.h 2008/09/11 14:31:43 1.91 +++ src/gdb/gdbtypes.h 2008/10/02 22:06:07 1.92 @@ -316,18 +316,6 @@ #define TYPE_ADDRESS_CLASS_ALL(t) (TYPE_INSTANCE_FLAGS(t) \ & TYPE_INSTANCE_FLAG_ADDRESS_CLASS_ALL) - -/* Array bound type. */ -enum array_bound_type -{ - BOUND_SIMPLE = 0, - BOUND_BY_VALUE_IN_REG, - BOUND_BY_REF_IN_REG, - BOUND_BY_VALUE_ON_STACK, - BOUND_BY_REF_ON_STACK, - BOUND_CANNOT_BE_DETERMINED -}; - /* This structure is space-critical. Its layout has been tweaked to reduce the space used. */ @@ -337,12 +325,6 @@ ENUM_BITFIELD(type_code) code : 8; - /* Array bounds. These fields appear at this location because - they pack nicely here. */ - - ENUM_BITFIELD(array_bound_type) upper_bound_type : 4; - ENUM_BITFIELD(array_bound_type) lower_bound_type : 4; - /* Flags about this type. These fields appear at this location because they packs nicely here. See the TYPE_* macros for documentation about these fields. */ @@ -460,7 +442,8 @@ /* For a function or member type, this is 1 if the argument is marked artificial. Artificial arguments should not be shown to the - user. */ + user. For TYPE_CODE_RANGE it is set if the specific bound is not + defined. */ unsigned int artificial : 1; /* This flag is zero for non-static fields, 1 for fields whose location @@ -817,10 +800,10 @@ /* Moto-specific stuff for FORTRAN arrays */ -#define TYPE_ARRAY_UPPER_BOUND_TYPE(thistype) \ - TYPE_MAIN_TYPE(thistype)->upper_bound_type -#define TYPE_ARRAY_LOWER_BOUND_TYPE(thistype) \ - TYPE_MAIN_TYPE(thistype)->lower_bound_type +#define TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED(arraytype) \ + (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),1)) +#define TYPE_ARRAY_LOWER_BOUND_IS_UNDEFINED(arraytype) \ + (TYPE_FIELD_ARTIFICIAL((TYPE_FIELD_TYPE((arraytype),0)),0)) #define TYPE_ARRAY_UPPER_BOUND_VALUE(arraytype) \ (TYPE_FIELD_BITPOS((TYPE_FIELD_TYPE((arraytype),0)),1)) --- src/gdb/m2-typeprint.c 2008/09/27 21:29:30 1.17 +++ src/gdb/m2-typeprint.c 2008/10/02 22:06:07 1.18 @@ -222,7 +222,7 @@ { fprintf_filtered (stream, "ARRAY ["); if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) { if (TYPE_INDEX_TYPE (type) != 0) { --- src/gdb/p-typeprint.c 2008/09/27 21:29:30 1.23 +++ src/gdb/p-typeprint.c 2008/10/02 22:06:07 1.24 @@ -266,7 +266,7 @@ fprintf_filtered (stream, "("); fprintf_filtered (stream, "array "); if (TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) fprintf_filtered (stream, "[%d..%d] ", TYPE_ARRAY_LOWER_BOUND_VALUE (type), TYPE_ARRAY_UPPER_BOUND_VALUE (type) --- src/gdb/parse.c 2008/10/01 16:56:52 1.80 +++ src/gdb/parse.c 2008/10/02 22:06:07 1.81 @@ -1268,8 +1268,7 @@ create_array_type ((struct type *) NULL, follow_type, range_type); if (array_size < 0) - TYPE_ARRAY_UPPER_BOUND_TYPE (follow_type) - = BOUND_CANNOT_BE_DETERMINED; + TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (follow_type) = 1; break; case tp_function: /* FIXME-type-allocation: need a way to free this type when we are --- src/gdb/valops.c 2008/09/11 14:27:34 1.200 +++ src/gdb/valops.c 2008/10/02 22:06:07 1.201 @@ -361,8 +361,7 @@ { struct type *element_type = TYPE_TARGET_TYPE (type); unsigned element_length = TYPE_LENGTH (check_typedef (element_type)); - if (element_length > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) == BOUND_CANNOT_BE_DETERMINED) + if (element_length > 0 && TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) { struct type *range_type = TYPE_INDEX_TYPE (type); int val_length = TYPE_LENGTH (type2); --- src/gdb/varobj.c 2008/09/13 23:37:05 1.117 +++ src/gdb/varobj.c 2008/10/02 22:06:07 1.118 @@ -1951,7 +1951,7 @@ { case TYPE_CODE_ARRAY: if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (target) > 0 - && TYPE_ARRAY_UPPER_BOUND_TYPE (type) != BOUND_CANNOT_BE_DETERMINED) + && !TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (type)) children = TYPE_LENGTH (type) / TYPE_LENGTH (target); else /* If we don't know how many elements there are, don't display --- src/gdb/testsuite/gdb.base/maint.exp 2008/08/24 16:39:57 1.35 +++ src/gdb/testsuite/gdb.base/maint.exp 2008/10/02 22:06:08 1.36 @@ -401,7 +401,7 @@ send_gdb "maint print type argc\n" gdb_expect { - -re "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nupper_bound_type $hex \\(BOUND_SIMPLE\\)\r\nlower_bound_type $hex \\(BOUND_SIMPLE\\)\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex\r\n$gdb_prompt $"\ + -re "type node $hex\r\nname .int. \\($hex\\)\r\ntagname .<NULL>. \\($hex\\)\r\ncode $hex \\(TYPE_CODE_INT\\)\r\nlength \[24\]\r\nobjfile $hex\r\ntarget_type $hex\r\npointer_type $hex\r\nreference_type $hex\r\ntype_chain $hex\r\ninstance_flags $hex\r\nflags\r\nnfields 0 $hex\r\nvptr_basetype $hex\r\nvptr_fieldno -1\r\ntype_specific $hex\r\n$gdb_prompt $"\ { pass "maint print type" } -re ".*$gdb_prompt $" { fail "maint print type" } timeout { fail "(timeout) maint print type" } ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-24 19:15 ` Joel Brobecker 2008-09-26 5:03 ` Jan Kratochvil @ 2008-09-26 12:52 ` Daniel Jacobowitz 2008-09-26 22:15 ` Joel Brobecker 1 sibling, 1 reply; 34+ messages in thread From: Daniel Jacobowitz @ 2008-09-26 12:52 UTC (permalink / raw) To: Joel Brobecker Cc: Jan Kratochvil, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb On Wed, Sep 24, 2008 at 12:15:04PM -0700, Joel Brobecker wrote: > Anyway, so the patch is using the TYPE_FIELD_ARTIFICIAL bit to > detect range types where a given bound value cannot be determined. > The patch as it is would work, and I wouldn't object to having it > checked in as a small step forward (with the agreement from at least > one other maintainer). However, I think we can implement this slightly > differently to make it more general. In my opinion, we should go with Jan's patch. I don't think that artificial is a discriminant; it has a clear meaning as a flag and nothing to do with the location or the loc union. What GCC does in this case is have two accessor macros for the same flag bit, so that it is clear at the usage site what is meant by checking the bit. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-26 12:52 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz @ 2008-09-26 22:15 ` Joel Brobecker 2008-09-26 22:20 ` Daniel Jacobowitz 0 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-09-26 22:15 UTC (permalink / raw) To: Jan Kratochvil, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb > In my opinion, we should go with Jan's patch. I don't think that > artificial is a discriminant; it has a clear meaning as a flag and > nothing to do with the location or the loc union. Agreed. Just a thought (independent from this patch): If this is a flag, should we try moving this flag up together with the rest of the flags? -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-26 22:15 ` Joel Brobecker @ 2008-09-26 22:20 ` Daniel Jacobowitz 0 siblings, 0 replies; 34+ messages in thread From: Daniel Jacobowitz @ 2008-09-26 22:20 UTC (permalink / raw) To: Joel Brobecker Cc: Jan Kratochvil, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb On Fri, Sep 26, 2008 at 03:14:32PM -0700, Joel Brobecker wrote: > > In my opinion, we should go with Jan's patch. I don't think that > > artificial is a discriminant; it has a clear meaning as a flag and > > nothing to do with the location or the loc union. > > Agreed. Just a thought (independent from this patch): If this is > a flag, should we try moving this flag up together with the rest > of the flags? Aren't the others on the type? This really is a flag on the field. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-07 11:59 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil 2008-09-08 15:32 ` Tom Tromey 2008-09-19 6:04 ` Joel Brobecker @ 2008-09-19 22:13 ` Joel Brobecker 2008-09-26 5:06 ` Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil ` (2 more replies) 2 siblings, 3 replies; 34+ messages in thread From: Joel Brobecker @ 2008-09-19 22:13 UTC (permalink / raw) To: Jan Kratochvil Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb > Also I do not understand why exist all the macros like this one at all: > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type > Why we cannot use it expanded? This way it is always one (or more) > "tags"-jump (VIM ctrl-]) indirections while navigating the source files. Personally, I find them to be very useful to quickly find who is using field "main_type" in struct type. > 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. The names are yet to be decided, but it's just to give you an idea of what they would mean. 4 possible values mean that, for now, we have room in the 2-bit field to store it. This "field_loc_kind" would then be checked for the types were the loc kind can be ambiguous (range types would be the next addition to the current list). Next, we add a new field in the "loc" union that would contain a pointer to the dwarf block, and probably create some kind of routine to create these range types. After that, we can update the DWARF reader to handle DWARF blocks for range type bounds, and finally update Fortran to make use of them. What do others think? -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] 2008-09-19 22:13 ` Joel Brobecker @ 2008-09-26 5:06 ` Jan Kratochvil 2008-09-26 12:55 ` Daniel Jacobowitz 2008-09-26 23:15 ` Tom Tromey 2008-09-26 12:58 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz 2008-10-04 20:28 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil 2 siblings, 2 replies; 34+ messages in thread From: Jan Kratochvil @ 2008-09-26 5:06 UTC (permalink / raw) To: Joel Brobecker Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote: > > Also I do not understand why exist all the macros like this one at all: > > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type > > Why we cannot use it expanded? This way it is always one (or more) > > "tags"-jump (VIM ctrl-]) indirections while navigating the source files. > > Personally, I find them to be very useful to quickly find who is using > field "main_type" in struct type. How does it differ from grep -- '->main_type\>' *.[ch] ? [ other part of the mail left after update of the patch ] Regards, Jan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] 2008-09-26 5:06 ` Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil @ 2008-09-26 12:55 ` Daniel Jacobowitz 2008-10-02 20:59 ` Jan Kratochvil 2008-09-26 23:15 ` Tom Tromey 1 sibling, 1 reply; 34+ messages in thread From: Daniel Jacobowitz @ 2008-09-26 12:55 UTC (permalink / raw) To: Jan Kratochvil Cc: Joel Brobecker, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb On Fri, Sep 26, 2008 at 06:43:09AM +0200, Jan Kratochvil wrote: > On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote: > > > Also I do not understand why exist all the macros like this one at all: > > > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type > > > Why we cannot use it expanded? This way it is always one (or more) > > > "tags"-jump (VIM ctrl-]) indirections while navigating the source files. > > > > Personally, I find them to be very useful to quickly find who is using > > field "main_type" in struct type. > > How does it differ from > grep -- '->main_type\>' *.[ch] > ? "in struct type" is the difference. For main_type it's not too bad. But for e.g. TYPE_FIELD_TYPE you can see the problem; there's lots of things that have a type. If you're looking just for the ones that come from a "struct field", it can be pretty awful to locate them all. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] 2008-09-26 12:55 ` Daniel Jacobowitz @ 2008-10-02 20:59 ` Jan Kratochvil 2008-10-02 21:05 ` Daniel Jacobowitz 0 siblings, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-10-02 20:59 UTC (permalink / raw) To: Daniel Jacobowitz Cc: Joel Brobecker, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy On Fri, 26 Sep 2008 14:54:22 +0200, Daniel Jacobowitz wrote: > On Fri, Sep 26, 2008 at 06:43:09AM +0200, Jan Kratochvil wrote: > > On Sat, 20 Sep 2008 00:12:21 +0200, Joel Brobecker wrote: > > > > Also I do not understand why exist all the macros like this one at all: > > > > #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type > > > > Why we cannot use it expanded? This way it is always one (or more) > > > > "tags"-jump (VIM ctrl-]) indirections while navigating the source files. > > > > > > Personally, I find them to be very useful to quickly find who is using > > > field "main_type" in struct type. > > > > How does it differ from > > grep -- '->main_type\>' *.[ch] > > ? > > "in struct type" is the difference. * Either to use static getters/setters (Gnome): struct main_type *type_main_type_get (struct type *type); * or in the C++ way (optional get/set prefixes instead of the overloading): class type { private: struct main_type *_main_type; public: struct main_type *main_type () { return _main_type; } public: void main_type (struct main_type *v) { _main_type = v; } }; One can get here even the automatic reference counting / memory management. * or to use semiunique fields prefix: struct type { struct main_type *tp_main_type; ... }; > For main_type it's not too bad. But for e.g. TYPE_FIELD_TYPE you can > see the problem; there's lots of things that have a type. If you're > looking just for the ones that come from a "struct field", it can be > pretty awful to locate them all. There is not the mess of mixed prefix (macros) vs. postfix (fields): xml-tdesc.c: if (TYPE_VECTOR (TYPE_FIELD_TYPE (data->current_union, i))) It gets expanded to: if ((((((data->current_union)->main_type->fields[i]).type))->main_type->flag_vector)) if (data->current_union->main_type->fields[i].type->main_type->flag_vector) Which could be simplified with C++ (do you also find it more clear?): if (data->current_union->fields()[i]->type()->flag_vector()) Regards, Jan ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] 2008-10-02 20:59 ` Jan Kratochvil @ 2008-10-02 21:05 ` Daniel Jacobowitz 0 siblings, 0 replies; 34+ messages in thread From: Daniel Jacobowitz @ 2008-10-02 21:05 UTC (permalink / raw) To: Jan Kratochvil Cc: Joel Brobecker, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy On Thu, Oct 02, 2008 at 10:57:22PM +0200, Jan Kratochvil wrote: > > "in struct type" is the difference. [list of other ways to do the same thing] But this is the one that GDB uses. If you think there's a good reason to switch, propose patches, but I don't see a point. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] 2008-09-26 5:06 ` Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil 2008-09-26 12:55 ` Daniel Jacobowitz @ 2008-09-26 23:15 ` Tom Tromey 1 sibling, 0 replies; 34+ messages in thread From: Tom Tromey @ 2008-09-26 23:15 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Joel Brobecker, gdb-patches >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes: [accessor macros] Joel> Personally, I find them to be very useful to quickly find who is using Joel> field "main_type" in struct type. Jan> How does it differ from Jan> grep -- '->main_type\>' *.[ch] Jan> ? Like Daniel said, this is a pain if unrelated fields have the same name. Also it is a pain if there are formatting oddities. However, if I am really desperate I just rename the field I want to find to something odd, then "make -k". The compiler will tell me all (or most) of the spots it is used :-) One thing that is nice about the accessor macros is that it makes it much simpler to play with certain kinds of field rearrangements. This isn't common, of course, but I did do it recently. It turns out you can push fields into struct general_symbol_info and save a word from minimal_symbol and symbol. Trying this out was easy since I only had to modify a few macros (well, once I fixed the code not using them :-) Anyway, I don't really care either way about this stuff. It seems to be a GNU cultural thing more than anything technical. Tom ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-19 22:13 ` Joel Brobecker 2008-09-26 5:06 ` Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil @ 2008-09-26 12:58 ` Daniel Jacobowitz [not found] ` <20081006200928.GD3588@adacore.com> 2008-10-04 20:28 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil 2 siblings, 1 reply; 34+ messages in thread From: Daniel Jacobowitz @ 2008-09-26 12:58 UTC (permalink / raw) To: Joel Brobecker Cc: Jan Kratochvil, gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb On Fri, Sep 19, 2008 at 03:12:21PM -0700, Joel Brobecker wrote: > Next, we add a new field in the "loc" union that would contain a pointer > to the dwarf block, and probably create some kind of routine to create > these range types. After that, we can update the DWARF reader to handle > DWARF blocks for range type bounds, and finally update Fortran to make > use of them. > > What do others think? Agreed. I would suggest LOC_COMPUTED as the role model, except I don't see where to stash an ops pointer. Maybe in TYPE_SPECIFIC, or is that ugly? -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <20081006200928.GD3588@adacore.com>]
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] [not found] ` <20081006200928.GD3588@adacore.com> @ 2008-10-06 20:26 ` Jan Kratochvil 2008-10-07 23:22 ` type/main_type/field size [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil 1 sibling, 0 replies; 34+ messages in thread From: Jan Kratochvil @ 2008-10-06 20:26 UTC (permalink / raw) To: Joel Brobecker Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy, jimb Hi, you were probably describing it already in your mail but more simply: SYMBOL_OPS can currently have only two values: dwarf2_loclist_funcs or dwarf2_locexpr_funcs. Which is moreover dependent on ENUM_BITFIELD(address_class). So the whole ops field has a useful size of 1 bit. In fact 0 bits as there can be LOC_COMPUTED_LOCLIST and LOC_COMPUTED_LOCEXPR instead of LOC_COMPUTED. But I would prefer to keep this part as a different patch. Thanks, Jan ^ permalink raw reply [flat|nested] 34+ messages in thread
* type/main_type/field size [Re: [patch] static_kind -> bit0, bit1] [not found] ` <20081006200928.GD3588@adacore.com> 2008-10-06 20:26 ` Jan Kratochvil @ 2008-10-07 23:22 ` Jan Kratochvil 2008-10-08 3:32 ` Joel Brobecker 1 sibling, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-10-07 23:22 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Daniel Jacobowitz, Tom Tromey [-- Attachment #1: Type: text/plain, Size: 929 bytes --] On Mon, 06 Oct 2008 22:09:28 +0200, Joel Brobecker wrote: > I think it is kind of ugly, and i tried to think about it for a while, > but I don't see many possible solutions if we don't want to increase > the gdbtype struct size. BTW are there some measurements if sizeof (type / main_type / field) matters? On Tue, 07 Oct 2008 19:48:53 +0200, Tom Tromey wrote: > For the lazy psymtab thing I had two related test cases: > > 1. Start OO.o writer, then time 'attach'. > 2. The same, plus time "thread apply all bt full". FYI I found (on Firefox) the difference of reading full symbols for this command `thread apply all bt full' is: valgrind --tool=massif: mem_heap_B=393798138 mem_heap_B=395820353 == 2MB == only 0.5% size increase (by the attached patch if it is right) Just `thread apply all bt full' may not be the typical debugging session, a more representative application / command set would be useful. Regards, Jan [-- Attachment #2: gdb-readneverfull.patch --] [-- Type: text/plain, Size: 706 bytes --] --- gdb/dwarf2read.c 30 Sep 2008 16:57:37 -0000 1.285 +++ gdb/dwarf2read.c 7 Oct 2008 20:50:07 -0000 @@ -2432,6 +2432,7 @@ locate_pdi_sibling (struct partial_die_i static void dwarf2_psymtab_to_symtab (struct partial_symtab *pst) { +return; /* FIXME: This is barely more than a stub. */ if (pst != NULL) { --- gdb/symtab.c 1 Oct 2008 17:25:22 -0000 1.199 +++ gdb/symtab.c 7 Oct 2008 20:50:10 -0000 @@ -1527,6 +1527,8 @@ lookup_symbol_aux_psymtabs (int block_in psymtab_index, domain)) { s = PSYMTAB_TO_SYMTAB (ps); + if (!s) + continue; bv = BLOCKVECTOR (s); block = BLOCKVECTOR_BLOCK (bv, block_index); sym = lookup_block_symbol (block, name, linkage_name, domain); ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: type/main_type/field size [Re: [patch] static_kind -> bit0, bit1] 2008-10-07 23:22 ` type/main_type/field size [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil @ 2008-10-08 3:32 ` Joel Brobecker 2008-10-08 23:56 ` Tom Tromey 0 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-10-08 3:32 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches, Daniel Jacobowitz, Tom Tromey > > I think it is kind of ugly, and i tried to think about it for a while, > > but I don't see many possible solutions if we don't want to increase > > the gdbtype struct size. > > BTW are there some measurements if sizeof (type / main_type / field) > matters? Once in a while, Tom will post some numbers with each of his size improvements. He worked pretty hard at reducing the sizes by some bytes, so I'd like to know what he thinks of the increase before adding a pointer back (4-8 bytes). (2MB seems like a small and reasonable increase to me) -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: type/main_type/field size [Re: [patch] static_kind -> bit0, bit1] 2008-10-08 3:32 ` Joel Brobecker @ 2008-10-08 23:56 ` Tom Tromey 0 siblings, 0 replies; 34+ messages in thread From: Tom Tromey @ 2008-10-08 23:56 UTC (permalink / raw) To: Joel Brobecker; +Cc: Jan Kratochvil, gdb-patches, Daniel Jacobowitz >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes: Joel> Once in a while, Tom will post some numbers with each of his size Joel> improvements. He worked pretty hard at reducing the sizes by some Joel> bytes, so I'd like to know what he thinks of the increase before Joel> adding a pointer back (4-8 bytes). Joel> (2MB seems like a small and reasonable increase to me) The main_type shrinkage patch was here: http://sourceware.org/ml/gdb-patches/2008-08/msg00464.html According to the note this saved about 1% on "gdb -readnow cc1". However, this is pretty artificial. -readnow is not something normal users do. So, offhand I would say that type and main_type are not extremely size-sensitive. I suppose it is a judgment call as to whether you would rather use more memory and have the code be clearer, or whether you would rather save memory and use a bit to indicate (IIUC) a special dwarf-specific callback. Tom ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-09-19 22:13 ` Joel Brobecker 2008-09-26 5:06 ` Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil 2008-09-26 12:58 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz @ 2008-10-04 20:28 ` Jan Kratochvil 2008-10-06 20:00 ` Joel Brobecker 2 siblings, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-10-04 20:28 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy [-- 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 (¤t_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)); ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-10-04 20:28 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil @ 2008-10-06 20:00 ` Joel Brobecker 2008-10-07 23:18 ` Jan Kratochvil 0 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-10-06 20:00 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy > 2008-10-04 Jan Kratochvil <jan.kratochvil@redhat.com> > > Convert static_kind into loc_kind enum. [...] Thanks for the patch. I have a few tiny comments left, but the principle looks good to me. > --- 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 */ Is it worth forcing the value of FIELD_LOC_KIND_BITPOS to zero? FIELD_LOC_KIND_BITPOS = 0, (this is where my lack of knowledge of C shows up) > + /* For variable length arrays. Passed to dwarf_locexpr_baton_eval. */ > + struct dwarf2_locexpr_baton *dwarf_block; I would prefer a more generic explaination, because this could be used in any situation where the field location is not known at compile time. /* The field location can be computed by evaluating the following DWARF block. This can be used in Fortran variable-length arrays, for instance. */ > +#define FIELD_STATIC(thisfld) \ > + (FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSNAME \ > + || FIELD_LOC_KIND (thisfld) == FIELD_LOC_KIND_PHYSADDR) With the new scheme, this is no longer an accessor macro, but more like a logical deduction based on field.loc_kind. So I suggest we implement it as a function instead rather than following the accessor-macro style. In theory, using the loc discriminant seems a little iffy to me, compared to having a dedicated flag for that. But it'll work in practice, at least for now. Back to the function, I propose: int field_is_static (struct field *f) { /* "static" fields are the fields whose location is not relative to the address of the enclosing struct. It would be nice to have a dedicated flag that would be set for static fields when the type is being created. But in practice, checking the field loc_kind should give us an accurate answer (at least as long as we assume that DWARF block locations are not going to be used for static fields). FIXME? */ return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR); } > +#define TYPE_FIELD_STATIC(thistype, n) FIELD_STATIC (TYPE_FIELD (thistype, n)) If we implement field_is_static, then I propose we delete this macro entirely. > --- 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 One interesting consequence of your change is that it should help when we want to deal with fields whose position is described with a location list. > /* 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); Can we replace the "FIELD_STATIC_KIND = 0" line by "SET_FIELD_BITPOS (*fp, 0);" and then remove the "else" part below? I am convinced that your change is correct, and the kind will always be set because it is set in both branches of the "if/else", but this way feels safer against future changes. -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-10-06 20:00 ` Joel Brobecker @ 2008-10-07 23:18 ` Jan Kratochvil 2008-10-08 3:28 ` Joel Brobecker 0 siblings, 1 reply; 34+ messages in thread From: Jan Kratochvil @ 2008-10-07 23:18 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy [-- Attachment #1: Type: text/plain, Size: 1532 bytes --] On Mon, 06 Oct 2008 22:00:01 +0200, Joel Brobecker wrote: > > +enum field_loc_kind > > + { > > + FIELD_LOC_KIND_BITPOS, /* bitpos */ > > Is it worth forcing the value of FIELD_LOC_KIND_BITPOS to zero? > > FIELD_LOC_KIND_BITPOS = 0, > > (this is where my lack of knowledge of C shows up) No, according to ISO C99 enums with no assignments get values 0, 1, 2... > int > field_is_static (struct field *f) > { > /* "static" fields are the fields whose location is not relative > to the address of the enclosing struct. It would be nice to > have a dedicated flag that would be set for static fields when > the type is being created. But in practice, checking the field > loc_kind should give us an accurate answer (at least as long as > we assume that DWARF block locations are not going to be used > for static fields). FIXME? */ > return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME > || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR); > } Included. FIELD_LOC_KIND_DWARF_BLOCK could be even asserted-out as the current patchset uses FIELD_LOC_KIND_DWARF_BLOCK only for TYPE_CODE_RANGE fields (low/high/stride) upon whose field_is_static() is never called. > Can we replace the "FIELD_STATIC_KIND = 0" line by "SET_FIELD_BITPOS > (*fp, 0);" and then remove the "else" part below? Done. Included you as a co-author of this patch. OK to apply? Regression-tested only on x86_64-unknown-linux-gnu. Thanks, Jan [-- Attachment #2: fortran9-bit01.patch --] [-- Type: text/plain, Size: 23278 bytes --] 2008-10-08 Jan Kratochvil <jan.kratochvil@redhat.com> Joel Brobecker <brobecker@adacore.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. (TYPE_FIELD_STATIC): Remove. (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. (field_is_static): New declaration. * gdbtypes.c (field_is_static): New function. (copy_type_recursive): Update throughout. * amd64-tdep.c, c-typeprint.c, coffread.c, cp-valprint.c, dwarf2read.c, eval.c, jv-typeprint.c, jv-valprint.c, mdebugread.c, p-typeprint.c, p-valprint.c, valops.c, value.c, varobj.c: Update throughout. --- gdb/amd64-tdep.c 11 Sep 2008 14:23:15 -0000 1.54 +++ gdb/amd64-tdep.c 7 Oct 2008 22:25:44 -0000 @@ -316,7 +316,7 @@ amd64_classify_aggregate (struct type *t enum amd64_reg_class subclass[2]; /* Ignore static fields. */ - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) continue; gdb_assert (pos == 0 || pos == 1); --- gdb/c-typeprint.c 2 Oct 2008 22:06:07 -0000 1.44 +++ gdb/c-typeprint.c 7 Oct 2008 22:25:46 -0000 @@ -890,14 +890,12 @@ c_type_print_base (struct type *type, st } print_spaces_filtered (level + 4, stream); - if (TYPE_FIELD_STATIC (type, i)) - { - fprintf_filtered (stream, "static "); - } + if (field_is_static (&TYPE_FIELD (type, i))) + fprintf_filtered (stream, "static "); c_print_type (TYPE_FIELD_TYPE (type, i), TYPE_FIELD_NAME (type, i), stream, show - 1, level + 4); - if (!TYPE_FIELD_STATIC (type, i) + if (!field_is_static (&TYPE_FIELD (type, i)) && TYPE_FIELD_PACKED (type, i)) { /* It is a bitfield. This code does not attempt --- gdb/coffread.c 1 Oct 2008 16:41:27 -0000 1.86 +++ gdb/coffread.c 7 Oct 2008 22:25:47 -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/cp-valprint.c 11 Sep 2008 14:24:27 -0000 1.54 +++ gdb/cp-valprint.c 7 Oct 2008 22:25:47 -0000 @@ -192,7 +192,8 @@ cp_print_value_fields (struct type *type for (i = n_baseclasses; i < len; i++) { /* If requested, skip printing of static fields. */ - if (!static_field_print && TYPE_FIELD_STATIC (type, i)) + if (!static_field_print + && field_is_static (&TYPE_FIELD (type, i))) continue; if (fields_seen) @@ -225,7 +226,7 @@ cp_print_value_fields (struct type *type fputs_filtered ("\"( ptr \"", stream); else fputs_filtered ("\"( nodef \"", stream); - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fputs_filtered ("static ", stream); fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i), current_language->la_language, @@ -240,7 +241,7 @@ cp_print_value_fields (struct type *type { annotate_field_begin (TYPE_FIELD_TYPE (type, i)); - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fputs_filtered ("static ", stream); fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i), current_language->la_language, @@ -252,7 +253,8 @@ cp_print_value_fields (struct type *type annotate_field_value (); } - if (!TYPE_FIELD_STATIC (type, i) && TYPE_FIELD_PACKED (type, i)) + if (!field_is_static (&TYPE_FIELD (type, i)) + && TYPE_FIELD_PACKED (type, i)) { struct value *v; @@ -278,7 +280,7 @@ cp_print_value_fields (struct type *type { fputs_filtered ("<optimized out or zero length>", stream); } - else if (TYPE_FIELD_STATIC (type, i)) + else if (field_is_static (&TYPE_FIELD (type, i))) { struct value *v = value_static_field (type, i); if (v == NULL) --- gdb/dwarf2read.c 6 Oct 2008 22:23:17 -0000 1.287 +++ gdb/dwarf2read.c 7 Oct 2008 22:25:55 -0000 @@ -3588,7 +3588,7 @@ dwarf2_add_field (struct field_info *fip /* Get type of field. */ fp->type = die_type (die, cu); - FIELD_STATIC_KIND (*fp) = 0; + SET_FIELD_BITPOS (*fp, 0); /* Get bit size of field (zero if none). */ attr = dwarf2_attr (die, DW_AT_bit_size, cu); @@ -3617,10 +3617,8 @@ 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; attr = dwarf2_attr (die, DW_AT_bit_offset, cu); if (attr) { @@ -3712,10 +3710,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++; @@ -4433,9 +4430,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 7 Oct 2008 22:25:56 -0000 @@ -319,7 +319,8 @@ evaluate_struct_tuple (struct value *str fieldno++; /* Skip static fields. */ while (fieldno < TYPE_NFIELDS (struct_type) - && TYPE_FIELD_STATIC_KIND (struct_type, fieldno)) + && field_is_static (&TYPE_FIELD (struct_type, + fieldno))) fieldno++; subfieldno = fieldno; if (fieldno >= TYPE_NFIELDS (struct_type)) --- gdb/gdbtypes.c 3 Oct 2008 22:00:46 -0000 1.152 +++ gdb/gdbtypes.c 7 Oct 2008 22:26:00 -0000 @@ -2434,6 +2434,20 @@ print_arg_types (struct field *args, int } } +int +field_is_static (struct field *f) +{ + /* "static" fields are the fields whose location is not relative + to the address of the enclosing struct. It would be nice to + have a dedicated flag that would be set for static fields when + the type is being created. But in practice, checking the field + loc_kind should give us an accurate answer (at least as long as + we assume that DWARF block locations are not going to be used + for static fields). FIXME? */ + return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME + || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR); +} + static void dump_fn_fieldlists (struct type *type, int spaces) { @@ -2975,18 +2989,23 @@ 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)) - SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i), - TYPE_FIELD_STATIC_PHYSADDR (type, i)); - else if (TYPE_FIELD_STATIC (type, i)) - SET_FIELD_PHYSNAME (TYPE_FIELD (new_type, i), - xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type, - i))); - else + switch (TYPE_FIELD_LOC_KIND (type, i)) { - TYPE_FIELD_BITPOS (new_type, i) = - TYPE_FIELD_BITPOS (type, i); - TYPE_FIELD_STATIC_KIND (new_type, i) = 0; + case FIELD_LOC_KIND_BITPOS: + SET_FIELD_BITPOS (TYPE_FIELD (new_type, i), + TYPE_FIELD_BITPOS (type, i)); + break; + case FIELD_LOC_KIND_PHYSADDR: + SET_FIELD_PHYSADDR (TYPE_FIELD (new_type, i), + TYPE_FIELD_STATIC_PHYSADDR (type, i)); + break; + case FIELD_LOC_KIND_PHYSNAME: + SET_FIELD_PHYSNAME (TYPE_FIELD (new_type, i), + xstrdup (TYPE_FIELD_STATIC_PHYSNAME (type, + i))); + break; + default: + gdb_assert (0); } } } --- gdb/gdbtypes.h 2 Oct 2008 22:06:07 -0000 1.92 +++ gdb/gdbtypes.h 7 Oct 2008 22:26:00 -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 +447,12 @@ struct main_type CORE_ADDR physaddr; char *physname; + + /* The field location can be computed by evaluating the following DWARF + block. This can be used in Fortran variable-length arrays, for + instance. */ + + struct dwarf2_locexpr_baton *dwarf_block; } loc; @@ -446,11 +462,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 +851,34 @@ 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_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_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 +913,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 @@ -1221,6 +1242,8 @@ extern int rank_one_type (struct type *, extern void recursive_dump_type (struct type *, int); +extern int field_is_static (struct field *); + /* printcmd.c */ extern void print_scalar_formatted (const void *, struct type *, int, int, --- gdb/jv-typeprint.c 1 Jan 2008 22:53:11 -0000 1.14 +++ gdb/jv-typeprint.c 7 Oct 2008 22:26:00 -0000 @@ -185,7 +185,7 @@ java_type_print_base (struct type *type, fprintf_filtered (stream, "public "); } - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fprintf_filtered (stream, "static "); java_print_type (TYPE_FIELD_TYPE (type, i), --- gdb/jv-valprint.c 6 May 2008 21:34:59 -0000 1.32 +++ gdb/jv-valprint.c 7 Oct 2008 22:26:01 -0000 @@ -304,7 +304,7 @@ java_print_value_fields (struct type *ty for (i = n_baseclasses; i < len; i++) { /* If requested, skip printing of static fields. */ - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) { char *name = TYPE_FIELD_NAME (type, i); if (!static_field_print) @@ -342,7 +342,7 @@ java_print_value_fields (struct type *ty fputs_filtered ("\"( ptr \"", stream); else fputs_filtered ("\"( nodef \"", stream); - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fputs_filtered ("static ", stream); fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i), language_cplus, @@ -357,7 +357,7 @@ java_print_value_fields (struct type *ty { annotate_field_begin (TYPE_FIELD_TYPE (type, i)); - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fputs_filtered ("static ", stream); fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i), language_cplus, @@ -367,7 +367,8 @@ java_print_value_fields (struct type *ty annotate_field_value (); } - if (!TYPE_FIELD_STATIC (type, i) && TYPE_FIELD_PACKED (type, i)) + if (!field_is_static (&TYPE_FIELD (type, i)) + && TYPE_FIELD_PACKED (type, i)) { struct value *v; @@ -392,7 +393,7 @@ java_print_value_fields (struct type *ty { fputs_filtered ("<optimized out or zero length>", stream); } - else if (TYPE_FIELD_STATIC (type, i)) + else if (field_is_static (&TYPE_FIELD (type, i))) { struct value *v = value_static_field (type, i); if (v == NULL) --- gdb/mdebugread.c 1 Oct 2008 16:41:27 -0000 1.95 +++ gdb/mdebugread.c 7 Oct 2008 22:26:05 -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 (¤t_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/p-typeprint.c 2 Oct 2008 22:06:07 -0000 1.24 +++ gdb/p-typeprint.c 7 Oct 2008 22:26:07 -0000 @@ -590,14 +590,12 @@ pascal_type_print_base (struct type *typ } print_spaces_filtered (level + 4, stream); - if (TYPE_FIELD_STATIC (type, i)) - { - fprintf_filtered (stream, "static "); - } + if (field_is_static (&TYPE_FIELD (type, i))) + fprintf_filtered (stream, "static "); pascal_print_type (TYPE_FIELD_TYPE (type, i), TYPE_FIELD_NAME (type, i), stream, show - 1, level + 4); - if (!TYPE_FIELD_STATIC (type, i) + if (!field_is_static (&TYPE_FIELD (type, i)) && TYPE_FIELD_PACKED (type, i)) { /* It is a bitfield. This code does not attempt --- gdb/p-valprint.c 11 Sep 2008 14:17:24 -0000 1.56 +++ gdb/p-valprint.c 7 Oct 2008 22:26:07 -0000 @@ -671,7 +671,8 @@ pascal_object_print_value_fields (struct for (i = n_baseclasses; i < len; i++) { /* If requested, skip printing of static fields. */ - if (!pascal_static_field_print && TYPE_FIELD_STATIC (type, i)) + if (!pascal_static_field_print + && field_is_static (&TYPE_FIELD (type, i))) continue; if (fields_seen) fprintf_filtered (stream, ", "); @@ -703,7 +704,7 @@ pascal_object_print_value_fields (struct fputs_filtered ("\"( ptr \"", stream); else fputs_filtered ("\"( nodef \"", stream); - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fputs_filtered ("static ", stream); fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i), language_cplus, @@ -718,7 +719,7 @@ pascal_object_print_value_fields (struct { annotate_field_begin (TYPE_FIELD_TYPE (type, i)); - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) fputs_filtered ("static ", stream); fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i), language_cplus, @@ -728,7 +729,8 @@ pascal_object_print_value_fields (struct annotate_field_value (); } - if (!TYPE_FIELD_STATIC (type, i) && TYPE_FIELD_PACKED (type, i)) + if (!field_is_static (&TYPE_FIELD (type, i)) + && TYPE_FIELD_PACKED (type, i)) { struct value *v; @@ -753,7 +755,7 @@ pascal_object_print_value_fields (struct { fputs_filtered ("<optimized out or zero length>", stream); } - else if (TYPE_FIELD_STATIC (type, i)) + else if (field_is_static (&TYPE_FIELD (type, i))) { /* struct value *v = value_static_field (type, i); v4.17 specific */ struct value *v; --- gdb/valops.c 2 Oct 2008 22:06:07 -0000 1.201 +++ gdb/valops.c 7 Oct 2008 22:26:10 -0000 @@ -1459,7 +1459,7 @@ search_struct_field (char *name, struct if (t_field_name && (strcmp_iw (t_field_name, name) == 0)) { struct value *v; - if (TYPE_FIELD_STATIC (type, i)) + if (field_is_static (&TYPE_FIELD (type, i))) { v = value_static_field (type, i); if (v == 0) @@ -2540,7 +2540,7 @@ value_struct_elt_for_reference (struct t if (t_field_name && strcmp (t_field_name, name) == 0) { - if (TYPE_FIELD_STATIC (t, i)) + if (field_is_static (&TYPE_FIELD (t, i))) { v = value_static_field (t, i); if (v == NULL) --- gdb/value.c 11 Sep 2008 14:22:33 -0000 1.66 +++ gdb/value.c 7 Oct 2008 22:26:12 -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)); --- gdb/varobj.c 2 Oct 2008 22:06:07 -0000 1.118 +++ gdb/varobj.c 7 Oct 2008 22:26:13 -0000 @@ -2013,7 +2013,7 @@ value_struct_element_index (struct value TRY_CATCH (e, RETURN_MASK_ERROR) { - if (TYPE_FIELD_STATIC (type, type_index)) + if (field_is_static (&TYPE_FIELD (type, type_index))) result = value_static_field (type, type_index); else result = value_primitive_field (value, 0, type_index, type); ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-10-07 23:18 ` Jan Kratochvil @ 2008-10-08 3:28 ` Joel Brobecker 2008-10-08 12:54 ` Jan Kratochvil 0 siblings, 1 reply; 34+ messages in thread From: Joel Brobecker @ 2008-10-08 3:28 UTC (permalink / raw) To: Jan Kratochvil; +Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy > No, according to ISO C99 enums with no assignments get values 0, 1, 2... Yeah, you're right. I checked C90 as well, since I believe we're still restricted to using C90 for GDB for now, and it says the same thing. > Included you as a co-author of this patch. OK to apply? That's very kind, but you really didn't need to do that. You should get sole credit for this work, as you clearly guided me as much as I tried to guide you. > 2008-10-08 Jan Kratochvil <jan.kratochvil@redhat.com> > Joel Brobecker <brobecker@adacore.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. > (TYPE_FIELD_STATIC): Remove. > (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. > (field_is_static): New declaration. > * gdbtypes.c (field_is_static): New function. > (copy_type_recursive): Update throughout. > * amd64-tdep.c, c-typeprint.c, coffread.c, cp-valprint.c, dwarf2read.c, > eval.c, jv-typeprint.c, jv-valprint.c, mdebugread.c, p-typeprint.c, > p-valprint.c, valops.c, value.c, varobj.c: Update throughout. Almost there: Pre-approved with the following nit fixed... [in copy_type_recursive] > + break; > + default: > + gdb_assert (0); Can you change that to using an internal_error, instead? For instance, internal_error (__FILE__, __LINE__, _("Unexpected ... kind: %d"), TYPE_FIELD_LOC_KIND (type, i)); Something like that. Thank you, -- Joel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] 2008-10-08 3:28 ` Joel Brobecker @ 2008-10-08 12:54 ` Jan Kratochvil 0 siblings, 0 replies; 34+ messages in thread From: Jan Kratochvil @ 2008-10-08 12:54 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Tobias Burnus, Ulrich Weigand, Jim Blandy On Wed, 08 Oct 2008 05:27:31 +0200, Joel Brobecker wrote: > [in copy_type_recursive] > > + break; > > + default: > > + gdb_assert (0); > > Can you change that to using an internal_error, instead? For instance, > > internal_error (__FILE__, __LINE__, _("Unexpected ... kind: %d"), > TYPE_FIELD_LOC_KIND (type, i)); > > Something like that. Used: + internal_error (__FILE__, __LINE__, + _("Unexpected type field location kind: %d"), + TYPE_FIELD_LOC_KIND (type, i)); And committed: http://sourceware.org/ml/gdb-cvs/2008-10/msg00099.html Going to post the real parts of the patchset rebased on this change and split. Thanks, Jan ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2008-10-08 23:56 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-15 15:06 [gdb] Fortran dynamic arrays Tobias Burnus
2008-08-18 11:12 ` Joel Brobecker
2008-08-18 15:54 ` Ulrich Weigand
2008-09-07 11:59 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil
2008-09-08 15:32 ` Tom Tromey
2008-09-08 17:27 ` Jan Kratochvil
2008-09-19 22:29 ` Joel Brobecker
2008-09-26 23:04 ` Tom Tromey
2008-09-27 14:53 ` Joel Brobecker
2008-09-19 6:04 ` Joel Brobecker
2008-09-22 15:25 ` Jan Kratochvil
2008-09-24 19:15 ` Joel Brobecker
2008-09-26 5:03 ` Jan Kratochvil
2008-09-26 22:12 ` Joel Brobecker
2008-10-02 22:13 ` [patch] Fortran obsolete bounds type [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil
2008-09-26 12:52 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz
2008-09-26 22:15 ` Joel Brobecker
2008-09-26 22:20 ` Daniel Jacobowitz
2008-09-19 22:13 ` Joel Brobecker
2008-09-26 5:06 ` Accessor macro wrappers removal [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil
2008-09-26 12:55 ` Daniel Jacobowitz
2008-10-02 20:59 ` Jan Kratochvil
2008-10-02 21:05 ` Daniel Jacobowitz
2008-09-26 23:15 ` Tom Tromey
2008-09-26 12:58 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Daniel Jacobowitz
[not found] ` <20081006200928.GD3588@adacore.com>
2008-10-06 20:26 ` Jan Kratochvil
2008-10-07 23:22 ` type/main_type/field size [Re: [patch] static_kind -> bit0, bit1] Jan Kratochvil
2008-10-08 3:32 ` Joel Brobecker
2008-10-08 23:56 ` Tom Tromey
2008-10-04 20:28 ` [patch] static_kind -> bit0, bit1 [Re: [gdb] Fortran dynamic arrays] Jan Kratochvil
2008-10-06 20:00 ` Joel Brobecker
2008-10-07 23:18 ` Jan Kratochvil
2008-10-08 3:28 ` Joel Brobecker
2008-10-08 12:54 ` Jan Kratochvil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox