This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: read correct xcoff auxiliary entry & skip reading @fix 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"))

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.


  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]