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:

> > 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.
> 
> Yes, I did not account for such a scenario. Changing the size of
> cs->c_value to 'long long' will suffice ?

I think it really needs to be CORE_ADDR.  Note that "long long" shouldn't
be used in common code since on some systems / compilers this data type
isn't even available ...

> > 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?
> 
> According to my understanding scan_xcoff_symtab processes ALL the symbols
> and categorizes them into different psymtabs for each separate program
> csect. All these psymtabs will be associated with the same file name
> that is stored in pst->filename. This happens when we load a new
> file in GDB using the file command.

Ah, I see.  Thanks for the detailed explanation.  (As an aside, it would
probably be good to add an explanation as comment to the source code
as well ...)

However, I now have the reverse question: if we already (have to and can)
trust pst->filename, why do we even need the " _start_ " dummy string?
Is there any place where we *want* to see " _start_ "?

> > 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?
> 
> If you could just take a look at the symbol table entries for a function
> here -
> 
> [103]   m   0x10000380     .text     2  extern                    .main
> [104]   a2           0              88    3186     112
> [105]   a4  0x00000064       0    0     LD       PR    0    0
> 
> The first aux entry a2 contains value for function size i.e 88
> The second aux entry a4 just contains symbol type and sym class details.
> 
> If we read the last aux entry for functions we end up ignoring the
> function size and thus GDB will take its size as zero and hence not
> knowing what data is inside the function.
> 
> This is why I added an extra ISFCN check to ensure that we continue
> reading the 1st aux entry for function symbols as before.
> Does that sound right?

Ah, now I see.  However, the part of the code testing for

          switch (CSECT_SMTYP (&main_aux))
	    case XTY_SD:
                switch (CSECT_SCLAS (&main_aux))
                  case XMC_PR:

still accesses fields that are in the *second* entry; it seems odd
to just skip it ...

Maybe something like this is best solution:

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	17 Sep 2013 13:22:43 -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))
 	    {
@@ -1261,7 +1266,12 @@
 		  /* save the function header info, which will be used
 		     when `.bf' is seen.  */
 		  fcn_cs_saved = *cs;
-		  fcn_aux_saved = main_aux;
+
+		  /* Save function size, which is contained in the *first* aux
+		     entry -- note that we read in the last auxent above.  */
+		  bfd_coff_swap_aux_in (abfd, raw_auxptr,
+					cs->c_type, cs->c_sclass,
+					0, cs->c_naux, &fcn_aux_saved);
 		  continue;
 
 		case XMC_GL:
@@ -1298,11 +1308,7 @@
 	 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;
-	}
+	goto function_entry_point;
 
       switch (cs->c_sclass)
 	{

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com