Re: [rfc] msymbol.size

Elena Zannoni
2003-11-07T20:06:55Z

Michael Elizabeth Chastain writes:
> This is a cleanup patch for MSYMBOL_SIZE. But I need help from
> someone with a Solaris machine to test it.
>
> struct minimal_symbol has this icky field:
>
> struct minimal_symbol
> {
> ...
>
> /* The info field is available for caching machine-specific information
> so it doesn't have to rederive the info constantly (over a serial line).
> It is initialized to zero and stays that way until target-dependent code
> sets it. Storage for any data pointed to by this field should be allo-
> cated on the symbol_obstack for the associated objfile.
> The type would be "void *" except for reasons of compatibility with older
> compilers. This field is optional.
>
> Currently, the AMD 29000 tdep.c uses it to remember things it has decoded
> from the instructions in the function header, and the MIPS-16 code uses
> it to identify 16-bit procedures. */
>
> char *info;
>
> ...
> };
>
> The comment is disconnected from reality. In reality, there are two
> uses for msym.info:
>
> (A) The elf reader stores the size of each minimal symbol into this
> field. Later on, the stabs reader reads this size to calculate
> the 'texthigh' of a partial symbol table from the address of the
> last symbol plus the size of the last symbol.
>
> (B) Several targets store flag bits into msym.info for their own
> internal purposes.
>
> This patch cleans up (A) by adding a new msym.size field with the
> proper data type of unsigned long.
>
> msym's are not space critical so it's okay to add fields to them.
>
> In the long run it might be better to calculate psymbol->texthigh
> some other way entirely. This patch makes that job easier.
>
> Another benefit of this patch is that there was only one caller
> of prim_record_minimal_symbol_and_info with a non-NULL 'info'.
> After this patch there are no such callers.
>

yeah. I was going to do this today, too!

> Testing: I am testing the *-tdep.c patches just by running gdb_mbuild.sh
> (in progress). If you grep on MSYMBOL_SIZE you will see that *-tdep.c
> never use these values. So if gdb builds, it ought to be fine.
>
> For the elfread.c and dbxread.c changes, I ran the test suite on
> native i686-pc-linux-gnu with gcc 2 and gcc 3, dwarf-2 and stabs+.
> Unfortunately, this is not good test coverage, because on this
> platform finish_psymtab does not need the logic "address + size
> of last symbol".
>
> Is there anyone with a Solaris machine who can help me?
>

let me see if I can get at one.

elena

> Michael C
>
> ===
>
> 2003-11-07 Michael Chastain <mec@shout.net>
>
> * symtab.h (struct minimal_symbol): Add size.
> * dbxread.c: Use it.
> * elfread.c: (record_minimal_symbol_and_info): Do not use info.
> Rename to record_minimal_symbol.
> (elf_symtab_read): Set MSYMBOL_SIZE explicitly.
> * minsyms.c (prim_record_minimal_symbol_and_info): Initialize MSYMBOL_SIZE.
> (install_minimal_symbols): Ditto.
> * objfiles.c (terminate_minimal_symbol_table): Ditto.
> * arm-tdep.c: Delete unused MSYMBOL_SIZE.
> * m68hc11-tdep.c: Ditto.
> * mips-tdep.c: Ditto.
> * sh64-tdep.c: Ditto.
>
> Index: symtab.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.h,v
> retrieving revision 1.82
> diff -c -3 -p -r1.82 symtab.h
> *** symtab.h 4 Nov 2003 22:22:31 -0000 1.82
> --- symtab.h 7 Nov 2003 17:45:07 -0000
> *************** struct minimal_symbol
> *** 325,330 ****
> --- 325,336 ----
>
> char *info;
>
> + /* Size of this symbol. end_psymtab in dbxread.c uses this
> + information to calculate the end of the partial symtab based on the
> + address of the last symbol plus the size of the last symbol. */
> +
> + unsigned long size;
> +
> #ifdef SOFUN_ADDRESS_MAYBE_MISSING
> /* Which source file is this symbol in? Only relevant for mst_file_*. */
> char *filename;
> *************** struct minimal_symbol
> *** 346,351 ****
> --- 352,358 ----
> };
>
> #define MSYMBOL_INFO(msymbol) (msymbol)->info
> + #define MSYMBOL_SIZE(msymbol) (msymbol)->size
> #define MSYMBOL_TYPE(msymbol) (msymbol)->type
>
> \f
> Index: dbxread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dbxread.c,v
> retrieving revision 1.58
> diff -c -3 -p -r1.58 dbxread.c
> *** dbxread.c 6 Nov 2003 22:54:01 -0000 1.58
> --- dbxread.c 7 Nov 2003 17:45:08 -0000
> ***************
> *** 64,77 ****
> #include "aout/stab_gnu.h" /* We always use GNU stabs, not native, now */
> \f
>
> - /* This macro returns the size field of a minimal symbol, which is normally
> - stored in the "info" field. The macro can be overridden for specific
> - targets (e.g. MIPS16) that use the info field for other purposes. */
> - #ifndef MSYMBOL_SIZE
> - #define MSYMBOL_SIZE(msym) ((long) MSYMBOL_INFO (msym))
> - #endif
> -
> -
> /* We put a pointer to this structure in the read_symtab_private field
> of the psymtab. */
>
> --- 64,69 ----
> Index: elfread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/elfread.c,v
> retrieving revision 1.37
> diff -c -3 -p -r1.37 elfread.c
> *** elfread.c 6 Nov 2003 02:52:27 -0000 1.37
> --- elfread.c 7 Nov 2003 17:45:08 -0000
> *************** elf_locate_sections (bfd *ignore_abfd, a
> *** 105,119 ****
> }
>
> static struct minimal_symbol *
> ! record_minimal_symbol_and_info (char *name, CORE_ADDR address,
> ! enum minimal_symbol_type ms_type, char *info, /* FIXME, is this really char *? */
> ! asection *bfd_section, struct objfile *objfile)
> {
> if (ms_type == mst_text || ms_type == mst_file_text)
> address = SMASH_TEXT_ADDRESS (address);
>
> return prim_record_minimal_symbol_and_info
> ! (name, address, ms_type, info, bfd_section->index, bfd_section, objfile);
> }
>
> /*
> --- 105,119 ----
> }
>
> static struct minimal_symbol *
> ! record_minimal_symbol (char *name, CORE_ADDR address,
> ! enum minimal_symbol_type ms_type,
> ! asection *bfd_section, struct objfile *objfile)
> {
> if (ms_type == mst_text || ms_type == mst_file_text)
> address = SMASH_TEXT_ADDRESS (address);
>
> return prim_record_minimal_symbol_and_info
> ! (name, address, ms_type, NULL, bfd_section->index, bfd_section, objfile);
> }
>
> /*
> *************** elf_symtab_read (struct objfile *objfile
> *** 163,169 ****
> char *filesymname = obsavestring ("", 0, &objfile->symbol_obstack);
> #endif
> struct dbx_symfile_info *dbx = objfile->sym_stab_info;
> - unsigned long size;
> int stripped = (bfd_get_symcount (objfile->obfd) == 0);
>
> if (dynamic)
> --- 163,168 ----
> *************** elf_symtab_read (struct objfile *objfile
> *** 223,231 ****
> if (symaddr == 0)
> continue;
> symaddr += offset;
> ! msym = record_minimal_symbol_and_info
> ((char *) sym->name, symaddr,
> ! mst_solib_trampoline, NULL, sym->section, objfile);
> #ifdef SOFUN_ADDRESS_MAYBE_MISSING
> if (msym != NULL)
> msym->filename = filesymname;
> --- 222,230 ----
> if (symaddr == 0)
> continue;
> symaddr += offset;
> ! msym = record_minimal_symbol
> ((char *) sym->name, symaddr,
> ! mst_solib_trampoline, sym->section, objfile);
> #ifdef SOFUN_ADDRESS_MAYBE_MISSING
> if (msym != NULL)
> msym->filename = filesymname;
> *************** elf_symtab_read (struct objfile *objfile
> *** 436,446 ****
> /* ms_type = mst_unknown; */
> continue; /* Skip this symbol. */
> }
> ! /* Pass symbol size field in via BFD. FIXME!!! */
> ! size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
> ! msym = record_minimal_symbol_and_info
> ((char *) sym->name, symaddr,
> ! ms_type, (void *) size, sym->section, objfile);
> #ifdef SOFUN_ADDRESS_MAYBE_MISSING
> if (msym != NULL)
> msym->filename = filesymname;
> --- 435,449 ----
> /* ms_type = mst_unknown; */
> continue; /* Skip this symbol. */
> }
> ! msym = record_minimal_symbol
> ((char *) sym->name, symaddr,
> ! ms_type, sym->section, objfile);
> ! if (msym)
> ! {
> ! /* Pass symbol size field in via BFD. FIXME!!! */
> ! unsigned long size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
> ! MSYMBOL_SIZE(msym) = size;
> ! }
> #ifdef SOFUN_ADDRESS_MAYBE_MISSING
> if (msym != NULL)
> msym->filename = filesymname;
> Index: minsyms.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/minsyms.c,v
> retrieving revision 1.37
> diff -c -3 -p -r1.37 minsyms.c
> *** minsyms.c 6 Nov 2003 22:54:01 -0000 1.37
> --- minsyms.c 7 Nov 2003 18:04:01 -0000
> *************** prim_record_minimal_symbol_and_info (con
> *** 610,615 ****
> --- 610,616 ----
> MSYMBOL_TYPE (msymbol) = ms_type;
> /* FIXME: This info, if it remains, needs its own field. */
> MSYMBOL_INFO (msymbol) = info; /* FIXME! */
> + MSYMBOL_SIZE (msymbol) = 0;
>
> /* The hash pointers must be cleared! If they're not,
> add_minsym_to_hash_table will NOT add this msymbol to the hash table. */
> *************** install_minimal_symbols (struct objfile
> *** 889,894 ****
> --- 890,896 ----
> SYMBOL_LINKAGE_NAME (&msymbols[mcount]) = NULL;
> SYMBOL_VALUE_ADDRESS (&msymbols[mcount]) = 0;
> MSYMBOL_INFO (&msymbols[mcount]) = NULL;
> + MSYMBOL_SIZE (&msymbols[mcount]) = 0;
> MSYMBOL_TYPE (&msymbols[mcount]) = mst_unknown;
> SYMBOL_INIT_LANGUAGE_SPECIFIC (&msymbols[mcount], language_unknown);
>
> Index: objfiles.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/objfiles.c,v
> retrieving revision 1.40
> diff -c -3 -p -r1.40 objfiles.c
> *** objfiles.c 6 Nov 2003 02:52:27 -0000 1.40
> --- objfiles.c 7 Nov 2003 17:45:09 -0000
> *************** terminate_minimal_symbol_table (struct o
> *** 386,391 ****
> --- 386,392 ----
> DEPRECATED_SYMBOL_NAME (m) = NULL;
> SYMBOL_VALUE_ADDRESS (m) = 0;
> MSYMBOL_INFO (m) = NULL;
> + MSYMBOL_SIZE (m) = 0;
> MSYMBOL_TYPE (m) = mst_unknown;
> SYMBOL_INIT_LANGUAGE_SPECIFIC (m, language_unknown);
> }
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.154
> diff -c -3 -p -r1.154 arm-tdep.c
> *** arm-tdep.c 31 Oct 2003 23:47:17 -0000 1.154
> --- arm-tdep.c 7 Nov 2003 17:45:10 -0000
> *************** static int arm_debug;
> *** 81,95 ****
>
> /* Macros for setting and testing a bit in a minimal symbol that marks
> it as Thumb function. The MSB of the minimal symbol's "info" field
> ! is used for this purpose. This field is already being used to store
> ! the symbol size, so the assumption is that the symbol size cannot
> ! exceed 2^31.
>
> MSYMBOL_SET_SPECIAL Actually sets the "special" bit.
> ! MSYMBOL_IS_SPECIAL Tests the "special" bit in a minimal symbol.
> ! MSYMBOL_SIZE Returns the size of the minimal symbol,
> ! i.e. the "info" field with the "special" bit
> ! masked out. */
>
> #define MSYMBOL_SET_SPECIAL(msym) \
> MSYMBOL_INFO (msym) = (char *) (((long) MSYMBOL_INFO (msym)) \
> --- 81,90 ----
>
> /* Macros for setting and testing a bit in a minimal symbol that marks
> it as Thumb function. The MSB of the minimal symbol's "info" field
> ! is used for this purpose.
>
> MSYMBOL_SET_SPECIAL Actually sets the "special" bit.
> ! MSYMBOL_IS_SPECIAL Tests the "special" bit in a minimal symbol. */
>
> #define MSYMBOL_SET_SPECIAL(msym) \
> MSYMBOL_INFO (msym) = (char *) (((long) MSYMBOL_INFO (msym)) \
> *************** static int arm_debug;
> *** 97,105 ****
>
> #define MSYMBOL_IS_SPECIAL(msym) \
> (((long) MSYMBOL_INFO (msym) & 0x80000000) != 0)
> -
> - #define MSYMBOL_SIZE(msym) \
> - ((long) MSYMBOL_INFO (msym) & 0x7fffffff)
>
> /* The list of available "set arm ..." and "show arm ..." commands. */
> static struct cmd_list_element *setarmcmdlist = NULL;
> --- 92,97 ----
> Index: m68hc11-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68hc11-tdep.c,v
> retrieving revision 1.92
> diff -c -3 -p -r1.92 m68hc11-tdep.c
> *** m68hc11-tdep.c 31 Oct 2003 23:47:17 -0000 1.92
> --- m68hc11-tdep.c 7 Nov 2003 17:45:10 -0000
> *************** Foundation, Inc., 59 Temple Place - Suit
> *** 50,65 ****
> analysis to compute correct stack frame layout.
>
> The MSB of the minimal symbol's "info" field is used for this purpose.
> - This field is already being used to store the symbol size, so the
> - assumption is that the symbol size cannot exceed 2^30.
>
> MSYMBOL_SET_RTC Actually sets the "RTC" bit.
> MSYMBOL_SET_RTI Actually sets the "RTI" bit.
> MSYMBOL_IS_RTC Tests the "RTC" bit in a minimal symbol.
> ! MSYMBOL_IS_RTI Tests the "RTC" bit in a minimal symbol.
> ! MSYMBOL_SIZE Returns the size of the minimal symbol,
> ! i.e. the "info" field with the "special" bit
> ! masked out. */
>
> #define MSYMBOL_SET_RTC(msym) \
> MSYMBOL_INFO (msym) = (char *) (((long) MSYMBOL_INFO (msym)) \
> --- 50,60 ----
> analysis to compute correct stack frame layout.
>
> The MSB of the minimal symbol's "info" field is used for this purpose.
>
> MSYMBOL_SET_RTC Actually sets the "RTC" bit.
> MSYMBOL_SET_RTI Actually sets the "RTI" bit.
> MSYMBOL_IS_RTC Tests the "RTC" bit in a minimal symbol.
> ! MSYMBOL_IS_RTI Tests the "RTC" bit in a minimal symbol. */
>
> #define MSYMBOL_SET_RTC(msym) \
> MSYMBOL_INFO (msym) = (char *) (((long) MSYMBOL_INFO (msym)) \
> *************** Foundation, Inc., 59 Temple Place - Suit
> *** 74,82 ****
>
> #define MSYMBOL_IS_RTI(msym) \
> (((long) MSYMBOL_INFO (msym) & 0x40000000) != 0)
> -
> - #define MSYMBOL_SIZE(msym) \
> - ((long) MSYMBOL_INFO (msym) & 0x3fffffff)
>
> enum insn_return_kind {
> RETURN_RTS,
> --- 69,74 ----
> Index: mips-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-tdep.c,v
> retrieving revision 1.243
> diff -c -3 -p -r1.243 mips-tdep.c
> *** mips-tdep.c 6 Nov 2003 02:52:27 -0000 1.243
> --- mips-tdep.c 7 Nov 2003 17:45:11 -0000
> *************** mips_saved_regsize (void)
> *** 226,242 ****
>
> /* Functions for setting and testing a bit in a minimal symbol that
> marks it as 16-bit function. The MSB of the minimal symbol's
> ! "info" field is used for this purpose. This field is already
> ! being used to store the symbol size, so the assumption is
> ! that the symbol size cannot exceed 2^31.
>
> ELF_MAKE_MSYMBOL_SPECIAL tests whether an ELF symbol is "special",
> i.e. refers to a 16-bit function, and sets a "special" bit in a
> minimal symbol to mark it as a 16-bit function
>
> ! MSYMBOL_IS_SPECIAL tests the "special" bit in a minimal symbol
> ! MSYMBOL_SIZE returns the size of the minimal symbol, i.e.
> ! the "info" field with the "special" bit masked out */
>
> static void
> mips_elf_make_msymbol_special (asymbol *sym, struct minimal_symbol *msym)
> --- 226,238 ----
>
> /* Functions for setting and testing a bit in a minimal symbol that
> marks it as 16-bit function. The MSB of the minimal symbol's
> ! "info" field is used for this purpose.
>
> ELF_MAKE_MSYMBOL_SPECIAL tests whether an ELF symbol is "special",
> i.e. refers to a 16-bit function, and sets a "special" bit in a
> minimal symbol to mark it as a 16-bit function
>
> ! MSYMBOL_IS_SPECIAL tests the "special" bit in a minimal symbol */
>
> static void
> mips_elf_make_msymbol_special (asymbol *sym, struct minimal_symbol *msym)
> Index: sh64-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/sh64-tdep.c,v
> retrieving revision 1.11
> diff -c -3 -p -r1.11 sh64-tdep.c
> *** sh64-tdep.c 10 Oct 2003 07:13:11 -0000 1.11
> --- sh64-tdep.c 7 Nov 2003 17:45:11 -0000
> *************** sh_sh64_register_name (int reg_nr)
> *** 197,213 ****
>
> /* Macros and functions for setting and testing a bit in a minimal
> symbol that marks it as 32-bit function. The MSB of the minimal
> ! symbol's "info" field is used for this purpose. This field is
> ! already being used to store the symbol size, so the assumption is
> ! that the symbol size cannot exceed 2^31.
>
> ELF_MAKE_MSYMBOL_SPECIAL
> tests whether an ELF symbol is "special", i.e. refers
> to a 32-bit function, and sets a "special" bit in a
> minimal symbol to mark it as a 32-bit function
> ! MSYMBOL_IS_SPECIAL tests the "special" bit in a minimal symbol
> ! MSYMBOL_SIZE returns the size of the minimal symbol, i.e.
> ! the "info" field with the "special" bit masked out */
>
> #define MSYMBOL_IS_SPECIAL(msym) \
> (((long) MSYMBOL_INFO (msym) & 0x80000000) != 0)
> --- 197,209 ----
>
> /* Macros and functions for setting and testing a bit in a minimal
> symbol that marks it as 32-bit function. The MSB of the minimal
> ! symbol's "info" field is used for this purpose.
>
> ELF_MAKE_MSYMBOL_SPECIAL
> tests whether an ELF symbol is "special", i.e. refers
> to a 32-bit function, and sets a "special" bit in a
> minimal symbol to mark it as a 32-bit function
> ! MSYMBOL_IS_SPECIAL tests the "special" bit in a minimal symbol */
>
> #define MSYMBOL_IS_SPECIAL(msym) \
> (((long) MSYMBOL_INFO (msym) & 0x80000000) != 0)