This is the mail archive of the
mailing list for the GDB project.
Re: read correct xcoff auxiliary entry & skip reading @fix entries.
- From: "Sangamesh Mallayya" <sangamesh dot swamy at in dot ibm dot com>
- To: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- Cc: gdb-patches at sourceware dot org, Ulrich dot Weigand at de dot ibm dot com (Ulrich Weigand)
- Date: Mon, 10 Oct 2016 17:51:02 +0530
- Subject: Re: read correct xcoff auxiliary entry & skip reading @fix entries.
- Authentication-results: sourceware.org; auth=none
- References: <OF0968463E.9D92B5ED-ON6525803E.004CFA9A-6525803E.004D741A@notes.na.collabserv.com> from "Sangamesh Mallayya" at Sep 30, 2016 02:06:00 PM <20160930150000.561D511C24D@oc8523832656.ibm.com>
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
> > 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
> > 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 == '$'
> >+ || !startswith (cs->c_name, "@FIX")
> > || (cs->c_name == '$' && cs->c_name == '.'))
> > 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
> evaluates to true for @FIX* bigtoc CSECT symbols, so it must
> after the above CSECT check. */
> /* Activate the misc_func_recorded mechanism for
> compiler- and linker-generated CSECTs like
> 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"))
> 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
> > psymtab.
> > So each psymtab will have the it's corresponding c filename entries
> 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
> > 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
> > the last auxiliary entry.
> > So we need to read the last auxiliary entry which is having the SD/PR
> > 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
> 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.