Re: [PATCH] xcoff64 support, read correct aux entries in case of xlc, qfuncsect and ffunction-sections filename update.

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Raunaq 12 wrote:

> This patch is for addressing the case when the debugee is compiled with
> xlc -qfuncsect or gcc --ffunction-sections. It also changes the data type
> of "fcn_start_address" from int to CORE_ADDR to make it large enough to
> read XCOFF64 format.

Since those seem to independent changes, it would probably be better to
create separate patches for them; this makes also the discussion easier.


As to the address size changes:

> Ulrich, I changed the data type of fcn_start_addr to CORE_ADDR
> as I noticed that other address related variables that derive their
> value from cs->c_value also are of this type. In XCOFF64 format,
> the section length uses 8 Bytes instead of 4. Therefore, 'long'
> data type for cs->c_value suffices. Just the fcn_start_addr was of
> a smaller size so I took care of that.

OK, changing fcn_start_addr to CORE_ADDR looks good.  However, I think
we should change cs->c_value itself likewise to a CORE_ADDR type.
When compiling xcoffread.c on a 64-bit system, the two types are
equivalent.  However, xcoffread.c will also get built on other
systems (e.g. when reading a 64-bit XCOFF core file on a 32-bit
Linux/Windows machine).  In those cases, "long" would be a 4-byte
type, and not enough to hold the full value.


As to the filename changes:

> Also, because the options -qfuncsect and --ffunction-sections create
> seperate sections for each function, the symtabs of these sections
> do not get updated with the correct filename and remain as "_start_".
> This is also addressed in this patch in a better way than what I had
> previously suggested.

I still don't understand why this is needed.  You set the filename
to pst->filename; but this information was itself retrieved from
the XCOFF symbol table at the time the GDB partial symtab was
created in scan_xcoff_symtab.

Why isn't read_xcoff_symtab able to recover the same information
back from the C_FILE entries, where scan_xcoff_symtab got it from
originally?


As to the auxiliary entry changes:

Given that the documentation seems to imply that there can be more
than 2 aux entries, and the one we need is always the last, I don't
think it is a good idea to hard-code two entries.  Instead, reading
the *last* entry, like the original version of the patch did, is
certainly preferable.

As an aside, it seems in your current patch, nothing will ever
reset aux_entry to 0 once it was set to 1 ...

Also, I don't understand the handling of ISFCN.  Why do you still
read the first entry for those?   Also, there still seems to be
two places to handle ISFCN.

I'd have expected something like the patch appended below to work;
is there anything I'm missing here?

Bye,
Ulrich

Index: xcoffread.c
===================================================================
RCS file: /cvs/src/src/gdb/xcoffread.c,v
retrieving revision 1.116
diff -u -r1.116 xcoffread.c
--- xcoffread.c	26 Aug 2013 15:28:28 -0000	1.116
+++ xcoffread.c	16 Sep 2013 17:33:08 -0000
@@ -1022,6 +1022,8 @@
   char *strtbl = xcoff->strtbl;
   char *debugsec = xcoff->debugsec;
   const char *debugfmt = bfd_xcoff_is_xcoff64 (abfd) ? "XCOFF64" : "XCOFF";
+  unsigned int local_auxesz;
+  unsigned int aux_entry;
 
   struct internal_syment symbol[1];
   union internal_auxent main_aux;
@@ -1051,6 +1053,7 @@
   /* Get the appropriate COFF "constants" related to the file we're
      handling.  */
   local_symesz = coff_data (abfd)->local_symesz;
+  local_auxesz = coff_data (abfd)->local_auxesz;
 
   set_last_source_file (NULL);
   last_csect_name = 0;
@@ -1128,7 +1131,7 @@
 	/* Skip all the auxents associated with this symbol.  */
 	for (ii = symbol->n_numaux; ii; --ii)
 	  {
-	    raw_symbol += coff_data (abfd)->local_auxesz;
+	    raw_symbol += local_auxesz;
 	    ++symnum;
 	  }
       }
@@ -1155,7 +1158,7 @@
 	}
 
       if ((cs->c_sclass == C_EXT || cs->c_sclass == C_HIDEXT)
-	  && cs->c_naux == 1)
+	  && cs->c_naux > 0)
 	{
 	  /* Dealing with a symbol with a csect entry.  */
 
@@ -1165,9 +1168,11 @@
 #define	CSECT_SMTYP(PP) (SMTYP_SMTYP(CSECT(PP).x_smtyp))
 #define	CSECT_SCLAS(PP) (CSECT(PP).x_smclas)
 
-	  /* Convert the auxent to something we can access.  */
-	  bfd_coff_swap_aux_in (abfd, raw_auxptr, cs->c_type, cs->c_sclass,
-				0, cs->c_naux, &main_aux);
+	  /* Convert the last auxent to something we can access.  */
+	  aux_entry = cs->c_naux - 1;
+	  bfd_coff_swap_aux_in (abfd, raw_auxptr + aux_entry * local_auxesz,
+				cs->c_type, cs->c_sclass,
+				aux_entry, cs->c_naux, &main_aux);
 
 	  switch (CSECT_SMTYP (&main_aux))
 	    {
@@ -1299,8 +1304,10 @@
 	 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);
+	  aux_entry = cs->c_naux - 1;
+	  bfd_coff_swap_aux_in (abfd, raw_auxptr + aux_entry * local_auxesz,
+				cs->c_type, cs->c_sclass,
+				aux_entry, cs->c_naux, &main_aux);
 	  goto function_entry_point;
 	}
 
-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com