Re: read correct xcoff auxiliary entry & skip reading @fix entries.

["Sangamesh Mallayya" <sangamesh dot swamy at in dot ibm dot com>]

Hi Ulrich,

Yes. I think better to have discussion on each patch separately.

I will send a separate patch for.

1) Skipping @fix entries.
   There seems to be mistake in using "!" with startswith function and 
extremely sorry for this silly mistake.
   I will have this corrected and have the regression runs of every patch. 

2) setting filename in a psymtab.
3) Reading multiple auxiliary entries.

> 
> Sangamesh Mallayya wrote:
> 
> > Here is the information about each patch and the changes.
> 
> To make discussion of the separate changes easier, please send them in
> different emails in the future.  Thanks!
> 
> > Patch #1:
> >  It seems that @fix entries are generated when extra code is needed 
for
> > every reference to a TOC symbol that cannot be addressed with a 16-bit
> > offset.
> >  So looks like we can safely ignore these symbols.
> >  Used a startswith function instead of directly using strncmp function 
and
> > little bit code rearrangement to have proper readability.
> 
> >-      /* if symbol name starts with ".$" or "$", ignore it.  */
> >+      /* if symbol name starts with ".$" or "$", ignore it.
> >+         We also need to skip symbols starting with @FIX,
> >+         which are used for TOC references. */
> >       if (cs->c_name[0] == '$'
> >+          || !startswith (cs->c_name, "@FIX")
> >      || (cs->c_name[1] == '$' && cs->c_name[0] == '.'))
> >    continue;
> 
> Are you sure you want the ! here?  This would appear to reject
> any function except those starting with @FIX ...   Did you run a
> regression test with this patch applied?
> 
> A minor issue: the line you inserted looks indented incorrectly.
> GDB coding style requires use of TAB characters instead of 8 spaces.
> Also, please move the new line last, so that the code sequence
> matches the sequence in the comment above it.
> 
> As a more substantiative comment, I'm wondering whether it really
> correct to just complely reject all @FIX symbols from consideration.
> I'm not really very familiar with the XCOFF format, but I do notice
> that there are several places in the rest of the code that appear to
> want to do something with @FIX symbols:
> 
>      /* If explicitly specified as a function, treat is as one.  This 
check
>          evaluates to true for @FIX* bigtoc CSECT symbols, so it must 
occur
>          after the above CSECT check.  */
> [...]
>                     /* Activate the misc_func_recorded mechanism for
>                        compiler- and linker-generated CSECTs like 
".strcmp"
>                        and "@FIX1".  */
> [...]
>                 /* We need only the minimal symbols for these
>                    loader-generated definitions.  Keeping the global
>                    symbols leads to "in psymbols but not in symbols"
>                    errors.  */
>                 if (startswith (namestring, "@FIX"))
>                   continue;
> 
> Can you explain how your new patch interacts with those existing
> places w.r.t. handling of @FIX symbols?
> 
> 
> > -  char *filestring = " _start_ ";      /* Name of the current file. 
*/
> > +  char *filestring = pst->filename;    /* Name of the current file. 
*/
> > 
> > When -qfuncsect options is used each function csect associated with 
each
> > psymtab.
> > So each psymtab will have the it's corresponding c filename entries 
set.
> 
> This looks like a reasonable change.  It should probably go in as a
> separate patch on its own.
> 
> 
> > Here is the offical xcoff documentation say about csect auxilirary 
entry.
> > 
> > And we always need to read last auxiliary entry as symbols having a
> > storage class of C_EXT, C_WEAKEXT or C_HIDEXT
> > and more than one auxiliary entry must have the csect auxiliary entry 
as
> > the last auxiliary entry.
> > 
> > So we need to read the last auxiliary entry which is having the SD/PR 
in
> > case of qfuncsect or LD/PR is case of no csects.
> 
> I agree that multiple auxillary entries need to be handled, but the code
> looks a bit of a hack ...   Can't we just do a loop over all aux 
entries,
> check the type of each, and handle it accordingly?   That would make a
> lot less assumptions about particular compiler behaviour, and would also
> make future maintenance easier.