Re: RFC: coffread.c relocation fixes

[Elena Zannoni <ezannoni@redhat.com>]

Raoul Gough writes:
 > When a shared object can't be loaded at its preferred image base, it
 > gets relocated. There's a convoluted switch statement in
 > coff_symtab_read that handles relocation of the symbols, and I believe
 > it handles a couple of cases incorrectly:
 > 
 > o Static symbols (with c_sclass of C_STAT) never get relocated. I
 > think this is wrong, probably an oversight where the C_STAT cases fall
 > through to the C_EXT case and then don't get handled.
 > 
 > o Absolute symbols (with c_secnum of N_ABS) *do* get relocated, so
 > things like __minor_os_version__ get adjusted by the relocation
 > offset. Curiously, there is a fixme comment in the code not to do
 > this.
 > 
 > The attached exmple shows the problem in action and the attached diffs
 > fix both problems. Do these changes seem sensible to others?

I am not a coff expert, but the change seems sensible. 
See below for a typo. 
Did you run the gdb testsuite with your patch and w/o? Any differences?

elena


 > 
 > Regards,
 > Raoul Gough.
 > 2003-02-26  Raoul Gough  <RaoulGough@yahoo.co.uk>
 > 
 >  	* coffread.c(coff_symtab_read): DO relocate static symbols from PE
 > 	files, DON'T relocate absolute symbols (and DO use mst_abs).
 > Index: src/gdb/coffread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/coffread.c,v
 > retrieving revision 1.37
 > diff -c -p -r1.37 coffread.c
 > *** src/gdb/coffread.c	25 Feb 2003 21:36:17 -0000	1.37
 > --- src/gdb/coffread.c	26 Feb 2003 18:37:26 -0000
 > *************** coff_symtab_read (long symtab_offset, un
 > *** 869,875 ****
 >   	       print_address_symbolic work right without the (now
 >   	       gone) "set fast-symbolic-addr off" kludge.  */
 >   
 > - 	    /* FIXME: should use mst_abs, and not relocate, if absolute.  */
 >   	    enum minimal_symbol_type ms_type;
 >   	    int sec;
 >   
 > --- 869,874 ----
 > *************** coff_symtab_read (long symtab_offset, un
 > *** 891,902 ****
 >   		  || cs->c_sclass == C_THUMBEXT ?
 >   		  mst_bss : mst_file_bss;
 >   	      }
 >   	    else
 >   	      {
 >   		sec = cs_to_section (cs, objfile);
 >   		tmpaddr = cs->c_value;
 > ! 		if (cs->c_sclass == C_EXT || cs->c_sclass == C_THUMBEXTFUNC
 > ! 		    || cs->c_sclass == C_THUMBEXT)
 >   		  tmpaddr += ANOFFSET (objfile->section_offsets, sec);
 >   
 >   		if (sec == SECT_OFF_TEXT (objfile))
 > --- 890,913 ----
 >   		  || cs->c_sclass == C_THUMBEXT ?
 >   		  mst_bss : mst_file_bss;
 >   	      }
 > + 	    else if (cs->c_secnum == N_ABS)
 > + 	      {
 > + 		/* Use the correct minimal symbol type (and don't
 > + 		   relocate) for absolute vaules. */
                                          ^^^^^ typo
 
 > + 		ms_type = mst_abs;
 > + 		sec = cs_to_section (cs, objfile);
 > + 		tmpaddr = cs->c_value;
 > + 	      }
 >   	    else
 >   	      {
 >   		sec = cs_to_section (cs, objfile);
 >   		tmpaddr = cs->c_value;
 > ! 
 > ! 		/* Statics in a PE file also get relocated */
 > ! 		if (cs->c_sclass == C_EXT
 > ! 		    || cs->c_sclass == C_THUMBEXTFUNC
 > ! 		    || cs->c_sclass == C_THUMBEXT
 > ! 		    || (pe_file && (cs->c_sclass == C_STAT)))
 >   		  tmpaddr += ANOFFSET (objfile->section_offsets, sec);
 >   
 >   		if (sec == SECT_OFF_TEXT (objfile))