This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH3/5] 64 bit support in xcoffread + reading auxillary entries correctly
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: raunaq12 at in dot ibm dot com (Raunaq 12)
- Cc: gdb-patches at sourceware dot org, brobecker at adacore dot com, tromey at redhat dot com, palves at redhat dot com (Pedro Alves)
- Date: Wed, 7 Aug 2013 17:44:48 +0200 (CEST)
- Subject: Re: [PATCH3/5] 64 bit support in xcoffread + reading auxillary entries correctly
Raunaq 12 wrote:
> unsigned int max_symnum;
> int just_started = 1;
> int depth = 0;
> - int fcn_start_addr = 0;
> + file_ptr fcn_start_addr = 0;
This seems to address only part of the problem. This variable is
initialized from cs->c_value, which itself is declared as "long".
If that needs to hold a 64-bit value, shouldn't c_value itself
be using "file_ptr"? In theory we could build a cross-debugger
hosted on a 32-bit platform to debug 64-bit AIX ...
Also, several other places copy "c_value" contents into variables
of type "int". Don't those also need changing?
> union internal_auxent fcn_aux_saved = main_aux;
> struct context_stack *new;
>
> - char *filestring = " _start_ "; /* Name of the current file. */
> + char *filestring = pst -> filename; /* Name of the current file. */
Isn't the code in the C_FILE switch case supposed to handle
detecting the correct file name(s)? Why doesn't that work
for you? Maybe that place also needs to be adapted to
handle multiple aux entries?
> @@ -1113,7 +1113,8 @@ read_xcoff_symtab (struct objfile *objfi
> }
>
> /* if symbol name starts with ".$" or "$", ignore it. */
> - if (cs->c_name[0] == '$'
> + /* We also need to skip symbols starting with @FIX, which are used
> for TOC reference */
> + if (cs->c_name[0] == '$' || !strncmp(cs->c_name, "@FIX", 4)
> || (cs->c_name[1] == '$' && cs->c_name[0] == '.'))
> continue;
There appear to be various places in the code that look for @FIX
symbols and handle them differently. Why do you just completely
ignore those symbols here?
> @@ -1133,8 +1134,7 @@ read_xcoff_symtab (struct objfile *objfi
> /* Done with all files, everything from here on is globals. */
> }
>
> - if ((cs->c_sclass == C_EXT || cs->c_sclass == C_HIDEXT)
> - && cs->c_naux == 1)
> + if (cs->c_sclass == C_EXT || cs->c_sclass == C_HIDEXT)
Probably cleaner to change the check to cs->c_naux > 0 instead
of completely removing it and then handling the == 0 case later.
> - bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
> - 0, cs->c_naux, &main_aux);
> + /* xcoff can have more than 1 auxent */
> + if (cs->c_naux > 1)
> + {
> + 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;
> + }
This seems to duplicate another check right after this switch:
/* 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. */
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;
}
Why can't we arrange things so as to avoid this duplication?
> + else
> + bfd_coff_swap_aux_in (abfd,
> + raw_auxptr + ((coff_data (abfd)->
> local_symesz) * (cs->c_naux - 1)),
> + cs->c_type, cs->c_sclass, cs->
> c_naux - 1, cs->c_naux, &main_aux);
> + }
> + else if (cs->c_naux == 1)
> + bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->
> c_sclass,
> + 0, cs->c_naux, &main_aux);
Note that the cs->c_naux == 1 case actually is just a special case
of the cs->c_naux > 1 case, so they should be merged ...
Also, those lines seem to be longer than 80 chars.
Just like I commented on the 2/5 patch, it would be good to test this
for improvements/regressions using both GCC and XLC.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com