Re: [PATCH RFA] Fixup SYMBOL_SECTION for objfiles_relocate()

[Elena Zannoni <ezannoni at cygnus dot com>]

Kevin Buettner writes:
 > I request permission to commit the patch below.
 > 
 > The OS for the project that I'm working on relocates the read/write
 > and the read-only sections by different amounts.  Thus it is critical
 > that objfile_relocate() actually have correct section indices during
 > the relocation process.
 > 
 >  I also discovered that for some symbol sym, SYMBOL_SECTION(sym) was
 > not providing the correct section index.  In fact, it was almost
 > always 0.  The same was true for partial symbols (upon which the same
 > macro magically works) as well.
 > 
 > The reason is that (at least for DWARF2) the SYMBOL_SECTION field
 > (section) was simply getting initialized to 0.  Furthermore, no
 > attempt was made to set things right later on.
 > 
 > The interesting thing is that SYMBOL_BFD_SECTION has exactly same
 > problem, but we have some code which attempts to set things right at
 > various points along the way.  The relevant functions are
 > fixup_symbol_section() and fixup_psymbol_section().  It seems to me
 > that we ought to be setting SYMBOL_SECTION at the same time that we're
 > setting BFD_SECTION and that is what the second hunk in the diff for
 > symtab.c is doing below.  [Thanks to Elena for bringing this to my
 > attention.]
 > 
 > Even once this is done, there is no guarantee that the symbols
 > and/or partial symbols will have been fixed up (with respect to
 > SYMBOL_SECTION) by the time that objfile_relocate() has been called.
 > So... now objfile_relocate() calls fixup_symbol_section() and
 > fixup_psymbol_section() as appropriate.
 > 
 > I'm not convinced that fixing these fields up at various random places
 > in the code where we need to access these fields is the right approach
 > to solving this problem.  It seems to me that it would be better to
 > attempt to set them correctly at the time that (or perhaps slightly
 > after) the symbol (or partial symbol) is created.  I made an attempt
 > at doing this, but I could not get it to work.  (Perhaps the minimal
 > symbols weren't available yet?) In any event, this is probably an
 > issue that the symbol table maintainers should consider in the future.
 > (grep for fixup_symbol_section() and look at all the places that
 > it's used.)
 > 

Yes, this is a cleanup that should be added to the TO DO file.
Andrew, Jimb?

 > 	* symtab.h (fixup_psymbol_section): Declare.
 > 	* symtab.c (fixup_psymbol_section): Make extern.
 > 	(fixup_section): Fix up section as well as bfd_section.
 > 	* objfiles.c (objfile_relocate): Call fixup_symbol_section
 > 	or fixup_psymbol_section before attempting to access
 > 	the SYMBOL_SECTION component of a symbol or partial symbol.
 > 

I think this is good. 
Elena

 > Index: objfiles.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/objfiles.c,v
 > retrieving revision 1.7
 > diff -u -p -r1.7 objfiles.c
 > --- objfiles.c	2000/07/30 01:48:26	1.7
 > +++ objfiles.c	2000/08/04 20:46:02
 > @@ -564,6 +564,9 @@ objfile_relocate (struct objfile *objfil
 >  	  for (j = 0; j < BLOCK_NSYMS (b); ++j)
 >  	    {
 >  	      struct symbol *sym = BLOCK_SYM (b, j);
 > +
 > +	      fixup_symbol_section (sym, objfile);
 > +
 >  	      /* The RS6000 code from which this was taken skipped
 >  	         any symbols in STRUCT_NAMESPACE or UNDEF_NAMESPACE.
 >  	         But I'm leaving out that test, on the theory that
 > @@ -606,15 +609,21 @@ objfile_relocate (struct objfile *objfil
 >      for (psym = objfile->global_psymbols.list;
 >  	 psym < objfile->global_psymbols.next;
 >  	 psym++)
 > -      if (SYMBOL_SECTION (*psym) >= 0)
 > -	SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
 > -						  SYMBOL_SECTION (*psym));
 > +      {
 > +	fixup_psymbol_section (*psym, objfile);
 > +	if (SYMBOL_SECTION (*psym) >= 0)
 > +	  SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
 > +						    SYMBOL_SECTION (*psym));
 > +      }
 >      for (psym = objfile->static_psymbols.list;
 >  	 psym < objfile->static_psymbols.next;
 >  	 psym++)
 > -      if (SYMBOL_SECTION (*psym) >= 0)
 > -	SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
 > -						  SYMBOL_SECTION (*psym));
 > +      {
 > +	fixup_psymbol_section (*psym, objfile);
 > +	if (SYMBOL_SECTION (*psym) >= 0)
 > +	  SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
 > +						    SYMBOL_SECTION (*psym));
 > +      }
 >    }
 >  
 >    {
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.10
 > diff -u -p -r1.10 symtab.c
 > --- symtab.c	2000/07/30 01:48:27	1.10
 > +++ symtab.c	2000/08/04 20:46:07
 > @@ -81,10 +81,6 @@ static struct partial_symbol *lookup_par
 >  						     const char *, int,
 >  						     namespace_enum);
 >  
 > -static struct partial_symbol *fixup_psymbol_section (struct
 > -						     partial_symbol *,
 > -						     struct objfile *);
 > -
 >  static struct symtab *lookup_symtab_1 (char *);
 >  
 >  static void cplusplus_hint (char *);
 > @@ -520,7 +516,10 @@ fixup_section (struct general_symbol_inf
 >    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
 >  
 >    if (msym)
 > -    ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
 > +    {
 > +      ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
 > +      ginfo->section = SYMBOL_SECTION (msym);
 > +    }
 >  }
 >  
 >  struct symbol *
 > @@ -537,7 +536,7 @@ fixup_symbol_section (struct symbol *sym
 >    return sym;
 >  }
 >  
 > -static struct partial_symbol *
 > +struct partial_symbol *
 >  fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
 >  {
 >    if (!psym)
 > Index: symtab.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.h,v
 > retrieving revision 1.11
 > diff -u -p -r1.11 symtab.h
 > --- symtab.h	2000/06/05 20:49:53	1.11
 > +++ symtab.h	2000/08/04 20:46:09
 > @@ -1414,6 +1414,10 @@ extern int in_prologue (CORE_ADDR pc, CO
 >  extern struct symbol *fixup_symbol_section (struct symbol *,
 >  					    struct objfile *);
 >  
 > +extern struct partial_symbol *fixup_psymbol_section (struct partial_symbol
 > +						     *psym,
 > +						     struct objfile *objfile);
 > +
 >  /* Symbol searching */
 >  
 >  /* When using search_symbols, a list of the following structs is returned.
 > 
 >