This is the mail archive of the gdb-patches@sourceware.org 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: [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


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