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>]

> > This patch is for addressing the case when the debugee is compiled with
> > xlc -qfuncsect or gcc --ffunction-sections. It also changes the data
type
> > of "fcn_start_address" from int to CORE_ADDR to make it large enough to
> > read XCOFF64 format.
>
> Since those seem to independent changes, it would probably be better to
> create separate patches for them; this makes also the discussion easier.

Sure, will create a separate patch entry and resend it.

> OK, changing fcn_start_addr to CORE_ADDR looks good.  However, I think
> we should change cs->c_value itself likewise to a CORE_ADDR type.
> When compiling xcoffread.c on a 64-bit system, the two types are
> equivalent.  However, xcoffread.c will also get built on other
> systems (e.g. when reading a 64-bit XCOFF core file on a 32-bit
> Linux/Windows machine).  In those cases, "long" would be a 4-byte
> type, and not enough to hold the full value.

Yes, I did not account for such a scenario. Changing the size of
cs->c_value to 'long long' will suffice ?

> As to the filename changes:
>
> > Also, because the options -qfuncsect and --ffunction-sections create
> > seperate sections for each function, the symtabs of these sections
> > do not get updated with the correct filename and remain as "_start_".
> > This is also addressed in this patch in a better way than what I had
> > previously suggested.
>
> 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.

Now read_xcoff_symtab, is called when, lets say I call
"break <func_name>" command. In this case, only the psymtab associated
with the <func_name> is searched in. Now, if the program is compiled
with "xlc -qfuncsect" or "gcc --function-sections" every function
is read as a seperate csect and hence has a different psymtab.
Now if the C_FILE entry is not present in this particular csect,
read_xcoff_symtab will assign filename as "_start_" even though
pst->filename has the correct file name.

Just a small example. Suppose I have 2 functions :- main and func1.
The file name is example.c. The symbol table entries are shown below :-

[76]    m   0x00000070     debug     3    FILE          C:COM     .file
[77]    a0                                                        example.c
..... (omitted because irrelevant)...
[88]    m   0x10000400     .text     2  unamex                    .func1
[89]    a1           0         0    52       0       0
[90]    a4  0x00000034       0    0     SD       PR    0    0
[91]    m   0x10000400     .text     2  extern                    .func1
[92]    a2           0              52    3168     100
[93]    a4  0x00000058       0    0     LD       PR    0    0
[94]    m   0x00000000     debug     0     fun                    func1:F-1
[95]    m   0x10000408     .text     1     fcn                    .bf
[96]    a1           0         8     0       0       0
[97]    m   0x00000058     debug     0    psym                    x:p-1
[98]    m   0x10000418     .text     1     fcn                    .ef
[99]    a1           0         9     0       0       0
[100]   m   0x10000380     .text     2  unamex                    .main
[101]   a1           0         0    88       0       0
[102]   a4  0x00000058       0    0     SD       PR    0    0
[103]   m   0x10000380     .text     2  extern                    .main
[104]   a2           0              88    3186     112
[105]   a4  0x00000064       0    0     LD       PR    0    0
[106]   m   0x00000000     debug     0     fun                    main:F-1
[107]   m   0x1000038c     .text     1     fcn                    .bf
[108]   a1           0         3     0       0       0
[109]   m   0x00000040     debug     0    lsym                    a:-1
[110]   m   0x100003c0     .text     1     fcn                    .ef
[111]   a1           0         5     0       0       0


Suppose we call 'break main', read_xcoff_symtab starts reading from the
psymtab
that contains 'main'. In this case it starts reading from symbol no [100].
So C_FILE entry (symbol no [76]) is not read. This is why it will assign it
to _start_.

But, If we call 'break func1', read_xcoff_symtab starts reading from symbol
[76]
and so it assigns it to the correct file name.

This is true even with gcc --function-sections.

Does this clear the air surrounding the need to address file name issue or
am I missing off track? You can just compile a small prog with two+
functions
using gcc --function-sections and set break points to reproduce the issue.


> As to the auxiliary entry changes:
>
> Given that the documentation seems to imply that there can be more
> than 2 aux entries, and the one we need is always the last, I don't
> think it is a good idea to hard-code two entries.  Instead, reading
> the *last* entry, like the original version of the patch did, is
> certainly preferable.
>
> As an aside, it seems in your current patch, nothing will ever
> reset aux_entry to 0 once it was set to 1

Thanks for pointing this out. Yes I agree that using cs->c_naux - 1
is better than hard coding to 2 entries.

> 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?

The below patch is working fine if I add the extra
ISFCN (cs->c_type) check to read the 1st aux entry.


> 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   16 Sep 2013 17:33:08 -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))
>         {
> @@ -1299,8 +1304,10 @@
>      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);
> +     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);
>       goto function_entry_point;
>     }

Thanks for the feedback.
Regards,
Raunaq M Bathija