From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16970 invoked by alias); 6 Nov 2002 20:41:11 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 16959 invoked from network); 6 Nov 2002 20:41:09 -0000 Received: from unknown (HELO localhost.redhat.com) (216.138.202.10) by sources.redhat.com with SMTP; 6 Nov 2002 20:41:09 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 142B93E60; Wed, 6 Nov 2002 15:41:11 -0500 (EST) Message-ID: <3DC97E66.70807@redhat.com> Date: Wed, 06 Nov 2002 12:41:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.0) Gecko/20020824 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com Subject: Re: RFA/types: Clean up use of field bitsize References: <20020930010515.GA27762@nevyn.them.org> <20021030234148.GA22769@nevyn.them.org> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-11/txt/msg00121.txt.bz2 > Does anyone have a comment on this patch? If not, I'll commit it in a > couple of days, after I'm added to the global write list. > > (The type code has no specific maintainer, the debug reader and > language parts I consider obvious, and the patch is over a month old > now.) I'm mainly wondering if we're that desperate for memory space. I thought a data structure was added to GDB so that it could spot duplicate type info and, hence, keep its memory size down. Andrew >> Right now, we have this really disturbing comment: >> >> /* Size of this field, in bits, or zero if not packed. >> For an unpacked field, the field's type's length >> says how many bytes the field occupies. >> A value of -1 or -2 indicates a static field; -1 means the location >> is specified by the label loc.physname; -2 means that loc.physaddr >> specifies the actual address. */ >> >> Think about this for a moment. While in practice a static member is never >> going to be packed, and in at least C++ can not be a bit-field, that's not >> logically obvious for other languages. I don't know Ada but I wouldn't be >> surprised if there were some construct which violated this assumption. >> >> Worse, all sorts of places don't check for negative bitsize at all. It may >> be that they're all safe - I didn't spend a lot of time working out problem >> cases - but I have my doubts. >> >> So, since I needed to gain a new field here anyway, and since I have no >> compunctions about shrinking this field a little (packed bitfields of size >> greater than a couple of words are allowed in some languages IIRC (including >> GNU C maybe? Although they are not allowed in ISO C99), but they're >> definitely dodgy), and since signed bitfields are not portable, I cleaned up >> the construct. It turned out to be painless except for making sure symbol >> readers initialized it, which was a little tedious. >> >> This patch: >> Moves 'artificial' out from 'loc' and makes it a bitfield >> Creates a 'static_kind' bitfield >> Makes 'bitsize' into a bitfield >> >> The goal is to allow more kinds of fields to be marked artificial - >> particularly data members. After this patch I'll submit the followup to >> mark DW_AT_artificial members as artificial types. >> >> OK? >> >> -- >> Daniel Jacobowitz >> MontaVista Software Debian GNU/Linux Developer >> >> 2002-09-29 Daniel Jacobowitz >> >> * gdbtypes.h (struct main_type): Move artificial flag out of >> loc. New member of ``struct field'' named static_kind. Reduce >> overloaded meaning of bitsize. >> (FIELD_ARTIFICIAL, SET_FIELD_PHYSNAME, SET_FIELD_PHYSADDR) >> (TYPE_FIELD_STATIC, TYPE_FIELD_STATIC_HAS_ADDR): Likewise. >> (FIELD_STATIC_KIND, TYPE_FIELD_STATIC_KIND): New macros. >> >> * ada-lang.c (fill_in_ada_prototype): Initialize static_kind for >> new fields. >> (template_to_fixed_record_type, template_to_static_fixed_type) >> (to_record_with_fixed_variant_part): Likewise. >> * coffread.c (coff_read_struct_type, coff_read_enum_type): Likewise. >> * dwarf2read.c (dwarf2_add_field, read_enumeration): Likewise. >> * dwarfread.c (struct_type, enum_type): Likewise. >> * hpread.c (hpread_read_enum_type) >> (hpread_read_function_type, hpread_read_doc_function_type) >> (hpread_read_struct_type): Likewise. >> * mdebugread.c (parse_symbol): Likewise. >> >> Index: ada-lang.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/ada-lang.c,v >> retrieving revision 1.9 >> diff -u -p -r1.9 ada-lang.c >> --- ada-lang.c 8 Sep 2002 17:43:26 -0000 1.9 >> +++ ada-lang.c 30 Sep 2002 00:43:30 -0000 >> @@ -4189,6 +4189,7 @@ fill_in_ada_prototype (struct symbol *fu >> case LOC_REGPARM_ADDR: >> TYPE_FIELD_BITPOS (ftype, nargs) = nargs; >> TYPE_FIELD_BITSIZE (ftype, nargs) = 0; >> + TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0; >> TYPE_FIELD_TYPE (ftype, nargs) = >> lookup_pointer_type (check_typedef (SYMBOL_TYPE (sym))); >> TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym); >> @@ -4202,6 +4203,7 @@ fill_in_ada_prototype (struct symbol *fu >> case LOC_BASEREG_ARG: >> TYPE_FIELD_BITPOS (ftype, nargs) = nargs; >> TYPE_FIELD_BITSIZE (ftype, nargs) = 0; >> + TYPE_FIELD_STATIC_KIND (ftype, nargs) = 0; >> TYPE_FIELD_TYPE (ftype, nargs) = check_typedef (SYMBOL_TYPE (sym)); >> TYPE_FIELD_NAME (ftype, nargs) = SYMBOL_NAME (sym); >> nargs += 1; >> @@ -6046,6 +6048,7 @@ template_to_fixed_record_type (struct ty >> * rediscover why we needed field_offset and fix it properly. */ >> TYPE_FIELD_BITPOS (rtype, f) = off; >> TYPE_FIELD_BITSIZE (rtype, f) = 0; >> + TYPE_FIELD_STATIC_KIND (rtype, f) = 0; >> >> if (ada_is_variant_part (type, f)) >> { >> @@ -6149,6 +6152,7 @@ template_to_static_fixed_type (struct ty >> { >> TYPE_FIELD_BITPOS (type, f) = 0; >> TYPE_FIELD_BITSIZE (type, f) = 0; >> + TYPE_FIELD_STATIC_KIND (type, f) = 0; >> >> if (is_dynamic_field (templ_type, f)) >> { >> @@ -6218,6 +6222,7 @@ to_record_with_fixed_variant_part (struc >> TYPE_FIELD_TYPE (rtype, nfields - 1) = branch_type; >> TYPE_FIELD_NAME (rtype, nfields - 1) = "S"; >> TYPE_FIELD_BITSIZE (rtype, nfields - 1) = 0; >> + TYPE_FIELD_STATIC_KIND (rtype, nfields - 1) = 0; >> TYPE_LENGTH (rtype) += TYPE_LENGTH (branch_type); >> -TYPE_LENGTH (TYPE_FIELD_TYPE (type, nfields - 1)); >> } >> Index: coffread.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/coffread.c,v >> retrieving revision 1.29 >> diff -u -p -r1.29 coffread.c >> --- coffread.c 22 Aug 2002 05:50:11 -0000 1.29 >> +++ coffread.c 30 Sep 2002 00:43:30 -0000 >> @@ -1997,6 +1997,7 @@ coff_read_struct_type (int index, int le >> FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux); >> FIELD_BITPOS (list->field) = 8 * ms->c_value; >> FIELD_BITSIZE (list->field) = 0; >> + FIELD_STATIC_KIND (list->field) = 0; >> nfields++; >> break; >> >> @@ -2015,6 +2016,7 @@ coff_read_struct_type (int index, int le >> FIELD_TYPE (list->field) = decode_type (ms, ms->c_type, &sub_aux); >> FIELD_BITPOS (list->field) = ms->c_value; >> FIELD_BITSIZE (list->field) = sub_aux.x_sym.x_misc.x_lnsz.x_size; >> + FIELD_STATIC_KIND (list->field) = 0; >> nfields++; >> break; >> >> @@ -2135,6 +2137,7 @@ coff_read_enum_type (int index, int leng >> if (SYMBOL_VALUE (xsym) < 0) >> unsigned_enum = 0; >> TYPE_FIELD_BITSIZE (type, n) = 0; >> + TYPE_FIELD_STATIC_KIND (type, n) = 0; >> } >> if (syms == osyms) >> break; >> Index: dwarf2read.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/dwarf2read.c,v >> retrieving revision 1.66 >> diff -u -p -r1.66 dwarf2read.c >> --- dwarf2read.c 3 Sep 2002 17:32:11 -0000 1.66 >> +++ dwarf2read.c 30 Sep 2002 00:43:33 -0000 >> @@ -2055,6 +2055,8 @@ dwarf2_add_field (struct field_info *fip >> /* Get type of field. */ >> fp->type = die_type (die, objfile, cu_header); >> >> + FIELD_STATIC_KIND (*fp) = 0; >> + >> /* Get bit size of field (zero if none). */ >> attr = dwarf_attr (die, DW_AT_bit_size); >> if (attr) >> @@ -2163,6 +2165,7 @@ dwarf2_add_field (struct field_info *fip >> FIELD_BITPOS (*fp) = (decode_locdesc (DW_BLOCK (attr), objfile, cu_header) >> * bits_per_byte); >> FIELD_BITSIZE (*fp) = 0; >> + FIELD_STATIC_KIND (*fp) = 0; >> FIELD_TYPE (*fp) = die_type (die, objfile, cu_header); >> FIELD_NAME (*fp) = type_name_no_tag (fp->type); >> fip->nbaseclasses++; >> @@ -2667,6 +2670,7 @@ read_enumeration (struct die_info *die, >> FIELD_TYPE (fields[num_fields]) = NULL; >> FIELD_BITPOS (fields[num_fields]) = SYMBOL_VALUE (sym); >> FIELD_BITSIZE (fields[num_fields]) = 0; >> + FIELD_STATIC_KIND (fields[num_fields]) = 0; >> >> num_fields++; >> } >> Index: dwarfread.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/dwarfread.c,v >> retrieving revision 1.14 >> diff -u -p -r1.14 dwarfread.c >> --- dwarfread.c 1 Aug 2002 17:18:32 -0000 1.14 >> +++ dwarfread.c 30 Sep 2002 00:43:33 -0000 >> @@ -1027,6 +1027,7 @@ struct_type (struct dieinfo *dip, char * >> &objfile->type_obstack); >> FIELD_TYPE (list->field) = decode_die_type (&mbr); >> FIELD_BITPOS (list->field) = 8 * locval (&mbr); >> + FIELD_STATIC_KIND (list->field) = 0; >> /* Handle bit fields. */ >> FIELD_BITSIZE (list->field) = mbr.at_bit_size; >> if (BITS_BIG_ENDIAN) >> @@ -1694,6 +1695,7 @@ enum_type (struct dieinfo *dip, struct o >> list = new; >> FIELD_TYPE (list->field) = NULL; >> FIELD_BITSIZE (list->field) = 0; >> + FIELD_STATIC_KIND (list->field) = 0; >> FIELD_BITPOS (list->field) = >> target_to_host (scan, TARGET_FT_LONG_SIZE (objfile), GET_SIGNED, >> objfile); >> Index: gdbtypes.h >> =================================================================== >> RCS file: /cvs/src/src/gdb/gdbtypes.h,v >> retrieving revision 1.36 >> diff -u -p -r1.36 gdbtypes.h >> --- gdbtypes.h 14 Sep 2002 02:09:39 -0000 1.36 >> +++ gdbtypes.h 30 Sep 2002 00:43:34 -0000 >> @@ -381,22 +381,25 @@ struct main_type >> >> CORE_ADDR physaddr; >> char *physname; >> - >> - /* For a function or member type, this is 1 if the argument is marked >> - artificial. Artificial arguments should not be shown to the >> - user. */ >> - int artificial; >> } >> loc; >> >> + /* For a function or member type, this is 1 if the argument is marked >> + artificial. Artificial arguments should not be shown to the >> + user. */ >> + unsigned int artificial : 1; >> + >> + /* This flag is zero for non-static fields, 1 for fields whose location >> + is specified by the label loc.physname, and 2 for fields whose location >> + is specified by loc.physaddr. */ >> + >> + unsigned int static_kind : 2; >> + >> /* Size of this field, in bits, or zero if not packed. >> For an unpacked field, the field's type's length >> - says how many bytes the field occupies. >> - A value of -1 or -2 indicates a static field; -1 means the location >> - is specified by the label loc.physname; -2 means that loc.physaddr >> - specifies the actual address. */ >> + says how many bytes the field occupies. */ >> >> - int bitsize; >> + unsigned int bitsize : 29; >> >> /* In a struct or union type, type of this field. >> In a function or member type, type of this argument. >> @@ -793,14 +796,15 @@ extern void allocate_cplus_struct_type ( >> #define FIELD_TYPE(thisfld) ((thisfld).type) >> #define FIELD_NAME(thisfld) ((thisfld).name) >> #define FIELD_BITPOS(thisfld) ((thisfld).loc.bitpos) >> -#define FIELD_ARTIFICIAL(thisfld) ((thisfld).loc.artificial) >> +#define FIELD_ARTIFICIAL(thisfld) ((thisfld).artificial) >> #define FIELD_BITSIZE(thisfld) ((thisfld).bitsize) >> +#define FIELD_STATIC_KIND(thisfld) ((thisfld).static_kind) >> #define FIELD_PHYSNAME(thisfld) ((thisfld).loc.physname) >> #define FIELD_PHYSADDR(thisfld) ((thisfld).loc.physaddr) >> #define SET_FIELD_PHYSNAME(thisfld, name) \ >> - ((thisfld).bitsize = -1, FIELD_PHYSNAME(thisfld) = (name)) >> + ((thisfld).static_kind = 1, FIELD_PHYSNAME(thisfld) = (name)) >> #define SET_FIELD_PHYSADDR(thisfld, name) \ >> - ((thisfld).bitsize = -2, FIELD_PHYSADDR(thisfld) = (name)) >> + ((thisfld).static_kind = 2, FIELD_PHYSADDR(thisfld) = (name)) >> #define TYPE_FIELD(thistype, n) TYPE_MAIN_TYPE(thistype)->fields[n] >> #define TYPE_FIELD_TYPE(thistype, n) FIELD_TYPE(TYPE_FIELD(thistype, n)) >> #define TYPE_FIELD_NAME(thistype, n) FIELD_NAME(TYPE_FIELD(thistype, n)) >> @@ -840,8 +844,9 @@ extern void allocate_cplus_struct_type ( >> (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \ >> : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (n))) >> >> -#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize < 0) >> -#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].bitsize == -2) >> +#define TYPE_FIELD_STATIC(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind != 0) >> +#define TYPE_FIELD_STATIC_KIND(thistype, n) TYPE_MAIN_TYPE (thistype)->fields[n].static_kind >> +#define TYPE_FIELD_STATIC_HAS_ADDR(thistype, n) (TYPE_MAIN_TYPE (thistype)->fields[n].static_kind == 2) >> #define TYPE_FIELD_STATIC_PHYSNAME(thistype, n) FIELD_PHYSNAME(TYPE_FIELD(thistype, n)) >> #define TYPE_FIELD_STATIC_PHYSADDR(thistype, n) FIELD_PHYSADDR(TYPE_FIELD(thistype, n)) >> >> Index: hpread.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/hpread.c,v >> retrieving revision 1.22 >> diff -u -p -r1.22 hpread.c >> --- hpread.c 4 Aug 2002 19:11:12 -0000 1.22 >> +++ hpread.c 30 Sep 2002 00:43:35 -0000 >> @@ -3186,6 +3186,7 @@ hpread_read_enum_type (dnttpointer hp_ty >> TYPE_FIELD_NAME (type, n) = SYMBOL_NAME (xsym); >> TYPE_FIELD_BITPOS (type, n) = SYMBOL_VALUE (xsym); >> TYPE_FIELD_BITSIZE (type, n) = 0; >> + TYPE_FIELD_STATIC_KIND (type, n) = 0; >> } >> if (syms == osyms) >> break; >> @@ -3347,6 +3348,7 @@ hpread_read_function_type (dnttpointer h >> TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym); >> TYPE_FIELD_ARTIFICIAL (type, n) = 0; >> TYPE_FIELD_BITSIZE (type, n) = 0; >> + TYPE_FIELD_STATIC_KIND (type, n) = 0; >> } >> } >> /* Mark it as having been processed */ >> @@ -3520,6 +3522,7 @@ hpread_read_doc_function_type (dnttpoint >> TYPE_FIELD_TYPE (type, n) = SYMBOL_TYPE (xsym); >> TYPE_FIELD_ARTIFICIAL (type, n) = 0; >> TYPE_FIELD_BITSIZE (type, n) = 0; >> + TYPE_FIELD_STATIC_KIND (type, n) = 0; >> } >> } >> >> @@ -3704,6 +3707,7 @@ hpread_read_struct_type (dnttpointer hp_ >> list = new; >> >> FIELD_BITSIZE (list->field) = 0; >> + FIELD_STATIC_KIND (list->field) = 0; >> >> /* The "classname" field is actually a DNTT pointer to the base class */ >> baseclass = hpread_type_lookup (parentp->dinheritance.classname, >> @@ -4101,6 +4105,7 @@ hpread_read_struct_type (dnttpointer hp_ >> list->field.name = VT (objfile) + fn_fieldp->dsvar.name; >> FIELD_BITPOS (list->field) = 0; /* FIXME is this always true? */ >> FIELD_BITSIZE (list->field) = 0; /* use length from type */ >> + FIELD_STATIC_KIND (list->field) = 0; >> memtype = hpread_type_lookup (fn_fieldp->dsvar.type, objfile); >> list->field.type = memtype; >> list->attributes = 0; >> @@ -4120,6 +4125,7 @@ hpread_read_struct_type (dnttpointer hp_ >> list->field.name = VT (objfile) + fn_fieldp->ddvar.name; >> FIELD_BITPOS (list->field) = 0; /* FIXME is this always true? */ >> FIELD_BITSIZE (list->field) = 0; /* use length from type */ >> + FIELD_STATIC_KIND (list->field) = 0; >> memtype = hpread_type_lookup (fn_fieldp->ddvar.type, objfile); >> list->field.type = memtype; >> list->attributes = 0; >> @@ -4168,6 +4174,7 @@ hpread_read_struct_type (dnttpointer hp_ >> >> >> /* A FIELD by itself (without a GENFIELD) can also be a static member */ >> + FIELD_STATIC_KIND (list->field) = 0; >> if (fieldp->dfield.staticmem) >> { >> FIELD_BITPOS (list->field) = -1; >> Index: mdebugread.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/mdebugread.c,v >> retrieving revision 1.29 >> diff -u -p -r1.29 mdebugread.c >> --- mdebugread.c 18 Sep 2002 20:47:39 -0000 1.29 >> +++ mdebugread.c 30 Sep 2002 00:43:36 -0000 >> @@ -1092,6 +1092,7 @@ parse_symbol (SYMR *sh, union aux_ext *a >> FIELD_TYPE (*f) = t; >> FIELD_NAME (*f) = debug_info->ss + cur_fdr->issBase + tsym.iss; >> FIELD_BITSIZE (*f) = 0; >> + FIELD_STATIC_KIND (*f) = 0; >> >> enum_sym = ((struct symbol *) >> obstack_alloc (¤t_objfile->symbol_obstack, >> @@ -1284,6 +1285,7 @@ parse_symbol (SYMR *sh, union aux_ext *a >> bitsize = 0; >> FIELD_TYPE (*f) = parse_type (cur_fd, ax, sh->index, &bitsize, bigend, name); >> FIELD_BITSIZE (*f) = bitsize; >> + FIELD_STATIC_KIND (*f) = 0; >> break; >> >> case stIndirect: /* forward declaration on Irix5 */ >> > > > -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer