Re: [PATCH] xcoff64 support, read correct aux entries in case of xlc, qfuncsect and ffunction-sections filename update.

[Raunaq 12 <raunaq12 at in dot ibm dot com>]

> > > I still don't understand why this is needed.  You set the filename
> > > to pst->filename; but this information was itself retrieved from
> > > the XCOFF symbol table at the time the GDB partial symtab was
> > > created in scan_xcoff_symtab.
> > >
> > > Why isn't read_xcoff_symtab able to recover the same information
> > > back from the C_FILE entries, where scan_xcoff_symtab got it from
> > > originally?
> >
> > According to my understanding scan_xcoff_symtab processes ALL the
symbols
> > and categorizes them into different psymtabs for each separate program
> > csect. All these psymtabs will be associated with the same file name
> > that is stored in pst->filename. This happens when we load a new
> > file in GDB using the file command.
>
> Ah, I see.  Thanks for the detailed explanation.  (As an aside, it would
> probably be good to add an explanation as comment to the source code
> as well ...)

Yes, will add comments related to this in the final patch once this
discussion
on ISFCN check and filename is sorted.

> However, I now have the reverse question: if we already (have to and can)
> trust pst->filename, why do we even need the " _start_ " dummy string?
> Is there any place where we *want* to see " _start_ "?

This was the question I asked myself too when I initially thought of
hard-coding filesrtring to pst->filename directly.
Going through the xcoffread.c code I see that no place really is looking
for
" _start_ ". Its just used as a default filename if no C_FILE entries are
seen
while calling read_xcoff_symtab.

My guess is that sometimes scan_xcoff_symtab creates psymtabs without
assigning
pst->filename as no C_FILE entries are seen. Is that a possibility ?
If so, then the default _start_ value will be assigned to it.

That is why in my patch, I check first if we have entered a valid program
csect, then if the filename is still " _start_ " I assign it to pst->
filename.

> > > Also, I don't understand the handling of ISFCN.  Why do you still
> > > read the first entry for those?   Also, there still seems to be
> > > two places to handle ISFCN.
> > >
> > > I'd have expected something like the patch appended below to work;
> > > is there anything I'm missing here?
> >
> > If you could just take a look at the symbol table entries for a
function
> > here -
> >
> > [103]   m   0x10000380     .text     2  extern                    .main
> > [104]   a2           0              88    3186     112
> > [105]   a4  0x00000064       0    0     LD       PR    0    0
> >
> > The first aux entry a2 contains value for function size i.e 88
> > The second aux entry a4 just contains symbol type and sym class
details.
> >
> > If we read the last aux entry for functions we end up ignoring the
> > function size and thus GDB will take its size as zero and hence not
> > knowing what data is inside the function.
> >
> > This is why I added an extra ISFCN check to ensure that we continue
> > reading the 1st aux entry for function symbols as before.
> > Does that sound right?
>
> Ah, now I see.  However, the part of the code testing for
>
>           switch (CSECT_SMTYP (&main_aux))
>        case XTY_SD:
>                 switch (CSECT_SCLAS (&main_aux))
>                   case XMC_PR:
>
> still accesses fields that are in the *second* entry; it seems odd
> to just skip it ...
>
> Maybe something like this is best solution:

The way the code has been designed, there are 2 ways to reach
function_entry_point:

1) There is a check for IFSCN
	-> if (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF)
   which directly takes it to function_entry_point.

2) The symbol type is read :-

           switch (CSECT_SMTYP (&main_aux))
      	case XTY_SD:
		   ....
            case XTY_LD:

              switch (CSECT_SCLAS (&main_aux))
                {
                case XMC_PR:

And this also takes us to function_entry_point.

If we just read the first aux entry, it reaches function_entry_point
through the option 1. With the fsize already read.
This is what was happening with my initail patch.

If we use the method you mentioned below, It will read the last aux
entry, realize that it is a function, take the way I mentioned in (2)
above, and then read the 1st aux entry for the function size.

In either case, the outcome is exactly the same and we are not losing
any important info as the last aux entry for function start symbols
just contain the symbol type and class and no other info.

Which way is better?

Thanks & Regards,
Raunaq M Bathija
> Index: xcoffread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/xcoffread.c,v
> retrieving revision 1.116
> diff -u -r1.116 xcoffread.c
> --- xcoffread.c   26 Aug 2013 15:28:28 -0000   1.116
> +++ xcoffread.c   17 Sep 2013 13:22:43 -0000
> @@ -1022,6 +1022,8 @@
>    char *strtbl = xcoff->strtbl;
>    char *debugsec = xcoff->debugsec;
>    const char *debugfmt = bfd_xcoff_is_xcoff64 (abfd) ? "XCOFF64" :
"XCOFF";
> +  unsigned int local_auxesz;
> +  unsigned int aux_entry;
>
>    struct internal_syment symbol[1];
>    union internal_auxent main_aux;
> @@ -1051,6 +1053,7 @@
>    /* Get the appropriate COFF "constants" related to the file we're
>       handling.  */
>    local_symesz = coff_data (abfd)->local_symesz;
> +  local_auxesz = coff_data (abfd)->local_auxesz;
>
>    set_last_source_file (NULL);
>    last_csect_name = 0;
> @@ -1128,7 +1131,7 @@
>     /* Skip all the auxents associated with this symbol.  */
>     for (ii = symbol->n_numaux; ii; --ii)
>       {
> -       raw_symbol += coff_data (abfd)->local_auxesz;
> +       raw_symbol += local_auxesz;
>         ++symnum;
>       }
>        }
> @@ -1155,7 +1158,7 @@
>     }
>
>        if ((cs->c_sclass == C_EXT || cs->c_sclass == C_HIDEXT)
> -     && cs->c_naux == 1)
> +     && cs->c_naux > 0)
>     {
>       /* Dealing with a symbol with a csect entry.  */
>
> @@ -1165,9 +1168,11 @@
>  #define   CSECT_SMTYP(PP) (SMTYP_SMTYP(CSECT(PP).x_smtyp))
>  #define   CSECT_SCLAS(PP) (CSECT(PP).x_smclas)
>
> -     /* Convert the auxent to something we can access.  */
> -     bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
> -            0, cs->c_naux, &main_aux);
> +     /* Convert the last auxent to something we can access.  */
> +     aux_entry = cs->c_naux - 1;
> +     bfd_coff_swap_aux_in (abfd, raw_auxptr + aux_entry * local_auxesz,
> +            cs->c_type, cs->c_sclass,
> +            aux_entry, cs->c_naux, &main_aux);
>
>       switch (CSECT_SMTYP (&main_aux))
>         {
> @@ -1261,7 +1266,12 @@
>          /* save the function header info, which will be used
>             when `.bf' is seen.  */
>          fcn_cs_saved = *cs;
> -        fcn_aux_saved = main_aux;
> +
> +        /* Save function size, which is contained in the *first* aux
> +           entry -- note that we read in the last auxent above.  */
> +        bfd_coff_swap_aux_in (abfd, raw_auxptr,
> +               cs->c_type, cs->c_sclass,
> +               0, cs->c_naux, &fcn_aux_saved);
>          continue;
>
>        case XMC_GL:
> @@ -1298,11 +1308,7 @@
>      evaluates to true for @FIX* bigtoc CSECT symbols, so it must occur
>      after the above CSECT check.  */
>        if (ISFCN (cs->c_type) && cs->c_sclass != C_TPDEF)
> -   {
> -     bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
> -            0, cs->c_naux, &main_aux);
> -     goto function_entry_point;
> -   }
> +   goto function_entry_point;
>
>        switch (cs->c_sclass)
>     {
>
> Bye,
> Ulrich
>
> --
>   Dr. Ulrich Weigand
>   GNU/Linux compilers and toolchain
>   Ulrich.Weigand@de.ibm.com