This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: read correct xcoff auxiliary entry & skip reading @fix entries.
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: sangamesh dot swamy at in dot ibm dot com (Sangamesh Mallayya)
- Cc: gdb-patches at sourceware dot org, Ulrich dot Weigand at de dot ibm dot com (Ulrich Weigand)
- Date: Fri, 30 Sep 2016 17:00:00 +0200 (CEST)
- Subject: Re: read correct xcoff auxiliary entry & skip reading @fix entries.
- Authentication-results: sourceware.org; auth=none
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.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com