This is the mail archive of the gdb@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

msym->info is wack


Here are some notes on msym->info.  I am going to submit a couple
of patches based on these notes.

Michael C

struct minimal_symbol has this 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 outdated.  I looked for all the uses of minimal_symbol.info.
Here is what I found:

=== COFFREAD.C

coffread.c stores "storage class" into msym->info:

      if (cs->c_name[0] != '@' /* Skip tdesc symbols */ )
	{
	  struct minimal_symbol *msym;

	  /* FIXME: cagney/2001-02-01: The nasty (int) -> (long)
	     -> (void*) cast is to ensure that that the value of
	     cs->c_sclass can be correctly stored in a void
	     pointer in MSYMBOL_INFO.  Better solutions
	     welcome. */
	  gdb_assert (sizeof (void *) >= sizeof (cs->c_sclass));
	  msym = prim_record_minimal_symbol_and_info
	    (cs->c_name, tmpaddr, ms_type, (void *) (long) cs->c_sclass,
	     sec, NULL, objfile);
	  if (msym)
	    COFF_MAKE_MSYMBOL_SPECIAL (cs->c_sclass, msym);
	}

In the call to prim_record_minimal_symbol_and_info, the fourth parameter
goes into msym->info.  However, nothing ever reads back this "info" as a
c_sclass, anywhere!

Back in gdb 4.17, arm_pc_is_thumb in arm-tdep.c used this information
to determine whether a symbol is a Thumb or Arm function.  But this
mechanism was replaced in gdb 4.18 with a new mechanism based on
COFF_MAKE_MSYMBOL_SPECIAL, which sets a flag bit instead of storing
the sclass and reading it later.  That enables other readers besides
the coff reader to mark functions as special.  The new mechanism
continues to this day.

So we can just kill the whole FIXME and nasty type conversion and do:

  msym = prim_record_minimal_symbol_and_info
    (cs->c_name, tmpaddr, ms_type, NULL, sec, NULL, objfile);

=== DBXREAD.C

dbxread.c uses the 'info' field to store the size of the msym.
Later on, it retrieves the size from msym->info.

dbxread.c does this when SOFUN_ADDRESS_MAYBE_MISSING is set
(which is often the case).  The comment is:

  /* Under Solaris, the N_SO symbols always have a value of 0,
     instead of the usual address of the .o file.  Therefore,
     we have to do some tricks to fill in texthigh and textlow.
     The first trick is: if we see a static
     or global function, and the textlow for the current pst
     is not set (ie: textlow_not_set), then we use that function's
     address for the textlow of the pst.  */

  /* Now, to fill in texthigh, we remember the last function seen
     in the .o file.  Also, there's a hack in
     bfd/elf.c and gdb/elfread.c to pass the ELF st_size field
     to here via the misc_info field.  Therefore, we can fill in
     a reliable texthigh by taking the address plus size of the
     last function in the file.  */

Instead of abusing msym->info, I would like to replace this with
an explicit field msym->size.  This would get rid of some
conversions back and forth between 'long' and 'void *'.

The difficulty here is that I can't find any code that ever sets
the size to a non-zero value.  So this needs more investigation
before touching the code.

A little more deeply, it's important to ask whether msym->size
is useful in its own right.  If it is, great, let's have it.
But if it is used only for this purpose, let's not store information
in every minsym that is used just transiently to initialize
pst->texthigh.

=== ARM-TDEP.C, M68HC11-TDEP.C, MIPS-TDEP.C, SH-TDEP.C

These *-tdep.c files use msym->info as a place to store flag bits.
So, first, they are converting 'void *' to 'long' and back again.

The straightforward solution is to provide msym->tdep_flags as
an explicit place for tdep files to store flag bits.  Then they
can store their bits without nasty casting.

Plus the code becomes a lot more readable when it uses
MSYMBOL_TDEP_FLAGS instead of MSYMBOL_INFO.

They can also use less space.  These files use 1 bit or 2 bits
of flags.  We can provide 8 bits and be happy, instead of providing
a whole pointer.  But that's not very important in the minsyms.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]