[PATCH 4/4] bfd.c: hancle aixcoff in bfd_get_sign_extend_vma.

[PATCH 4/4] bfd.c: hancle aixcoff in bfd_get_sign_extend_vma.

[Tristan Gingold]

To be submitted on binutils@, attached for completness.

bfd/
2011-05-23  Tristan Gingold  <gingold@adacore.com>

	* bfd.c (bfd_get_sign_extend_vma): Handle aixcoff.
---
 bfd/bfd.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/bfd/bfd.c b/bfd/bfd.c
index c729d63..b3e2c47 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -1031,7 +1031,8 @@ bfd_get_sign_extend_vma (bfd *abfd)
       || strcmp (name, "pe-x86-64") == 0
       || strcmp (name, "pei-x86-64") == 0
       || strcmp (name, "pe-arm-wince-little") == 0
-      || strcmp (name, "pei-arm-wince-little") == 0)
+      || strcmp (name, "pei-arm-wince-little") == 0
+      || strcmp (name, "aixcoff-rs6000") == 0)
     return 1;
 
   if (CONST_STRNEQ (name, "mach-o"))
-- 
1.7.3.GIT

[RFA] Add dwarf2 support to xcoff (aix)

[Tristan Gingold]

Hi,

with AIX 7, the xcoff binary file format now supports dwarf2.  This is a
huge step forward as the previous debugging format was a varation of stabs.

But (and there is always a but) this is not fully standard dwarf2 as the
section names differ. The first two patches deal with that in the most
elegant way I could figure.

The last two patches simply add the dwarf2 support for xcoff.

Manually tested on AIX.
No regressions on gnu/linux x86-64.

Ok for trunk ?

Tristan.

[PATCH 2/4] dwarf2read.c: handle alternate dwarf2 debug sections name.

[Tristan Gingold]

Unfortunately, some object file format cannot accept the standard dwarf2
debug section names.  This is in particular the case of XCOFF (aix), which
has only 8 chars section names.  In order to support dwarf2 on aix, we need
to allow alternate section names.

The change is in fact simple as sections are localized by the dwarf2_has_info
function.  So I added a new parameter to this function to accept alternate
names.  In order to make this obvious, the value NULL means standard names.

The alternate names is a record of 2 entries: one for normal section and one
for uncompressed section.

gdb/
2011-05-23  Tristan Gingold  <gingold@adacore.com>

	* symfile.h (struct dwarf2_section_names): New type.
	(struct dwarf2_debug_sections): New type.
	(dwarf2_has_info): Add parameter.
	* dwarf2read.c (dwarf2_elf_names): New variable.
	(INFO_SECTION, ABBREV_SECTION, LINE_SECTION, LOC_SECTION)
	(MACINFO_SECTION, STR_SECTION, RANGES_SECTION, TYPES_SECTION)
	(FRAME_SECTION, EH_FRAME_SECTION, GDB_INDEX_SECTION): Remove.
	(dwarf2_has_info): Add names parameter.  Pass names
	to dwarf2_locate_sections.
	(section_is_p): Rewrite using the names parameter.
	(dwarf2_locate_sections): Use section names from the names parameter.
	* coffread.c (coff_symfile_read): Adjust call to dwarf2_has_info.
	* elfread.c (read_psyms): Ditto.
	* machoread.c (macho_symfile_read): Ditto.
---
 gdb/coffread.c   |    2 +-
 gdb/dwarf2read.c |   85 ++++++++++++++++++++++++++++++++---------------------
 gdb/elfread.c    |    4 +-
 gdb/machoread.c  |    4 +-
 gdb/symfile.h    |   24 ++++++++++++++-
 5 files changed, 79 insertions(+), 40 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index b11dd73..16fd58f 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -637,7 +637,7 @@ coff_symfile_read (struct objfile *objfile, int symfile_flags)
 			       info->stabsects,
 			       info->stabstrsect->filepos, stabstrsize);
     }
-  if (dwarf2_has_info (objfile))
+  if (dwarf2_has_info (objfile, NULL))
     {
       /* DWARF2 sections.  */
       dwarf2_build_psymtabs (objfile);
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2003c46..7495d1f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -243,22 +243,24 @@ struct dwarf2_per_objfile
 
 static struct dwarf2_per_objfile *dwarf2_per_objfile;
 
-/* names of the debugging sections */
+/* Default names of the debugging sections.  */
 
 /* Note that if the debugging section has been compressed, it might
    have a name like .zdebug_info.  */
 
-#define INFO_SECTION     "debug_info"
-#define ABBREV_SECTION   "debug_abbrev"
-#define LINE_SECTION     "debug_line"
-#define LOC_SECTION      "debug_loc"
-#define MACINFO_SECTION  "debug_macinfo"
-#define STR_SECTION      "debug_str"
-#define RANGES_SECTION   "debug_ranges"
-#define TYPES_SECTION    "debug_types"
-#define FRAME_SECTION    "debug_frame"
-#define EH_FRAME_SECTION "eh_frame"
-#define GDB_INDEX_SECTION "gdb_index"
+static const struct dwarf2_debug_sections dwarf2_elf_names = {
+  { ".debug_info", ".zdebug_info" },
+  { ".debug_abbrev", ".zdebug_abbrev" },
+  { ".debug_line", ".zdebug_line" },
+  { ".debug_loc", ".zdebug_loc" },
+  { ".debug_macinfo", ".zdebug_macinfo" },
+  { ".debug_str", ".zdebug_str" },
+  { ".debug_ranges", ".zdebug_ranges" },
+  { ".debug_types", ".zdebug_types" },
+  { ".debug_frame", ".zdebug_frame" },
+  { ".eh_frame", NULL },
+  { ".gdb_index", ".zgdb_index" }
+};
 
 /* local data types */
 
@@ -1338,10 +1340,13 @@ static const char *dwarf2_physname (char *name, struct die_info *die,
 				    struct dwarf2_cu *cu);
 
 /* Try to locate the sections we need for DWARF 2 debugging
-   information and return true if we have enough to do something.  */
+   information and return true if we have enough to do something.
+   NAMES points to the dwarf2 section names, or is NULL if the standard
+   ELF names are used.  */
 
 int
-dwarf2_has_info (struct objfile *objfile)
+dwarf2_has_info (struct objfile *objfile,
+                 const struct dwarf2_debug_sections *names)
 {
   dwarf2_per_objfile = objfile_data (objfile, dwarf2_objfile_data_key);
   if (!dwarf2_per_objfile)
@@ -1354,23 +1359,28 @@ dwarf2_has_info (struct objfile *objfile)
       set_objfile_data (objfile, dwarf2_objfile_data_key, data);
       dwarf2_per_objfile = data;
 
-      bfd_map_over_sections (objfile->obfd, dwarf2_locate_sections, NULL);
+      bfd_map_over_sections (objfile->obfd, dwarf2_locate_sections,
+                             (void *)names);
       dwarf2_per_objfile->objfile = objfile;
     }
   return (dwarf2_per_objfile->info.asection != NULL
 	  && dwarf2_per_objfile->abbrev.asection != NULL);
 }
 
-/* When loading sections, we can either look for ".<name>", or for
- * ".z<name>", which indicates a compressed section.  */
+/* When loading sections, we look either for uncompressed section or for
+   compressed section names.  */
 
 static int
-section_is_p (const char *section_name, const char *name)
+section_is_p (const char *section_name,
+              const struct dwarf2_section_names *names)
 {
-  return (section_name[0] == '.'
-	  && (strcmp (section_name + 1, name) == 0
-	      || (section_name[1] == 'z'
-		  && strcmp (section_name + 2, name) == 0)));
+  if (names->normal != NULL
+      && strcmp (section_name, names->normal) == 0)
+    return 1;
+  if (names->compressed != NULL
+      && strcmp (section_name, names->compressed) == 0)
+    return 1;
+  return 0;
 }
 
 /* This function is mapped across the sections and remembers the
@@ -1378,44 +1388,51 @@ section_is_p (const char *section_name, const char *name)
    in.  */
 
 static void
-dwarf2_locate_sections (bfd *abfd, asection *sectp, void *ignore_ptr)
+dwarf2_locate_sections (bfd *abfd, asection *sectp, void *vnames)
 {
-  if (section_is_p (sectp->name, INFO_SECTION))
+  const struct dwarf2_debug_sections *names;
+
+  if (vnames == NULL)
+    names = &dwarf2_elf_names;
+  else
+    names = (const struct dwarf2_debug_sections *)vnames;
+
+  if (section_is_p (sectp->name, &names->info))
     {
       dwarf2_per_objfile->info.asection = sectp;
       dwarf2_per_objfile->info.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, ABBREV_SECTION))
+  else if (section_is_p (sectp->name, &names->abbrev))
     {
       dwarf2_per_objfile->abbrev.asection = sectp;
       dwarf2_per_objfile->abbrev.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, LINE_SECTION))
+  else if (section_is_p (sectp->name, &names->line))
     {
       dwarf2_per_objfile->line.asection = sectp;
       dwarf2_per_objfile->line.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, LOC_SECTION))
+  else if (section_is_p (sectp->name, &names->loc))
     {
       dwarf2_per_objfile->loc.asection = sectp;
       dwarf2_per_objfile->loc.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, MACINFO_SECTION))
+  else if (section_is_p (sectp->name, &names->macinfo))
     {
       dwarf2_per_objfile->macinfo.asection = sectp;
       dwarf2_per_objfile->macinfo.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, STR_SECTION))
+  else if (section_is_p (sectp->name, &names->str))
     {
       dwarf2_per_objfile->str.asection = sectp;
       dwarf2_per_objfile->str.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, FRAME_SECTION))
+  else if (section_is_p (sectp->name, &names->frame))
     {
       dwarf2_per_objfile->frame.asection = sectp;
       dwarf2_per_objfile->frame.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, EH_FRAME_SECTION))
+  else if (section_is_p (sectp->name, &names->eh_frame))
     {
       flagword aflag = bfd_get_section_flags (ignore_abfd, sectp);
 
@@ -1425,17 +1442,17 @@ dwarf2_locate_sections (bfd *abfd, asection *sectp, void *ignore_ptr)
           dwarf2_per_objfile->eh_frame.size = bfd_get_section_size (sectp);
         }
     }
-  else if (section_is_p (sectp->name, RANGES_SECTION))
+  else if (section_is_p (sectp->name, &names->ranges))
     {
       dwarf2_per_objfile->ranges.asection = sectp;
       dwarf2_per_objfile->ranges.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, TYPES_SECTION))
+  else if (section_is_p (sectp->name, &names->types))
     {
       dwarf2_per_objfile->types.asection = sectp;
       dwarf2_per_objfile->types.size = bfd_get_section_size (sectp);
     }
-  else if (section_is_p (sectp->name, GDB_INDEX_SECTION))
+  else if (section_is_p (sectp->name, &names->gdb_index))
     {
       dwarf2_per_objfile->gdb_index.asection = sectp;
       dwarf2_per_objfile->gdb_index.size = bfd_get_section_size (sectp);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 6c6a7af..825df0f 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1391,7 +1391,7 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags)
 				bfd_section_size (abfd, str_sect));
     }
 
-  if (dwarf2_has_info (objfile))
+  if (dwarf2_has_info (objfile, NULL))
     {
       /* elf_sym_fns_gdb_index cannot handle simultaneous non-DWARF debug
 	 information present in OBJFILE.  If there is such debug info present
@@ -1437,7 +1437,7 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags)
 static void
 read_psyms (struct objfile *objfile)
 {
-  if (dwarf2_has_info (objfile))
+  if (dwarf2_has_info (objfile, NULL))
     dwarf2_build_psymtabs (objfile);
 }
 
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 28cb958..1cfa21e 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -660,7 +660,7 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags)
       /* Try to read .eh_frame / .debug_frame.  */
       /* First, locate these sections.  We ignore the result status
 	 as it only checks for debug info.  */
-      dwarf2_has_info (objfile);
+      dwarf2_has_info (objfile, NULL);
       dwarf2_build_frame_info (objfile);
       
       /* Check for DSYM file.  */
@@ -702,7 +702,7 @@ macho_symfile_read (struct objfile *objfile, int symfile_flags)
 	}
     }
 
-  if (dwarf2_has_info (objfile))
+  if (dwarf2_has_info (objfile, NULL))
     {
       /* DWARF 2 sections */
       dwarf2_build_psymtabs (objfile);
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 83e807c..e6c4613 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -553,7 +553,29 @@ extern struct cleanup *increment_reading_symtab (void);
 
 /* From dwarf2read.c */
 
-extern int dwarf2_has_info (struct objfile *);
+/* Name of the dwarf2 sections.  */
+
+struct dwarf2_section_names {
+  const char *normal;
+  const char *compressed;
+};
+
+struct dwarf2_debug_sections {
+  struct dwarf2_section_names info;
+  struct dwarf2_section_names abbrev;
+  struct dwarf2_section_names line;
+  struct dwarf2_section_names loc;
+  struct dwarf2_section_names macinfo;
+  struct dwarf2_section_names str;
+  struct dwarf2_section_names ranges;
+  struct dwarf2_section_names types;
+  struct dwarf2_section_names frame;
+  struct dwarf2_section_names eh_frame;
+  struct dwarf2_section_names gdb_index;
+};
+
+extern int dwarf2_has_info (struct objfile *,
+                            const struct dwarf2_debug_sections *);
 
 /* Dwarf2 sections that can be accessed by dwarf2_get_section_info.  */
 enum dwarf2_section_enum {
-- 
1.7.3.GIT

[PATCH 1/4] Really make dwarf2_get_section_info public.

[Tristan Gingold]

This patch is a preliminary clean-up.  Instead of using the section name
to get the section info, use an enum literal.

It might be worth going farther and using an array of dwarf2_section_info
in dwarf2_per_objfile (instead of record fields).  I let this decision to
maintainers.

gdb/
2011-05-23  Tristan Gingold  <gingold@adacore.com>

	* symfile.h (enum dwarf2_section_enum): New type.
	(dwarf2_get_section_info): New prototype.
	* dwarf2read.c (dwarf2_get_section_info): Replace parameter
	section_name by sect.  Use a switch to select the info.
	* dwarf2-frame.c (warf2_get_section_info): Remove prototype.
	(dwarf2_build_frame_info): Adjust calls to dwarf2_get_section_info.
---
 gdb/dwarf2-frame.c |   10 ++--------
 gdb/dwarf2read.c   |   20 +++++++++++++-------
 gdb/symfile.h      |   11 +++++++++++
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index fe48713..9c329f9 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -2183,12 +2183,6 @@ Corrupt data in %s:%s; align 8 workaround apparently succeeded"),
   return ret;
 }
 \f
-
-/* Imported from dwarf2read.c.  */
-extern void dwarf2_get_section_info (struct objfile *, const char *,
-				     asection **, gdb_byte **,
-				     bfd_size_type *);
-
 static int
 qsort_fde_cmp (const void *a, const void *b)
 {
@@ -2233,7 +2227,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
   unit->dbase = 0;
   unit->tbase = 0;
 
-  dwarf2_get_section_info (objfile, ".eh_frame",
+  dwarf2_get_section_info (objfile, dwarf2_eh_frame,
                            &unit->dwarf_frame_section,
                            &unit->dwarf_frame_buffer,
                            &unit->dwarf_frame_size);
@@ -2269,7 +2263,7 @@ dwarf2_build_frame_info (struct objfile *objfile)
         }
     }
 
-  dwarf2_get_section_info (objfile, ".debug_frame",
+  dwarf2_get_section_info (objfile, dwarf2_debug_frame,
                            &unit->dwarf_frame_section,
                            &unit->dwarf_frame_buffer,
                            &unit->dwarf_frame_size);
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6558bfe..2003c46 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1636,7 +1636,8 @@ dwarf2_section_size (struct objfile *objfile,
    SECTION_NAME.  */
 
 void
-dwarf2_get_section_info (struct objfile *objfile, const char *section_name,
+dwarf2_get_section_info (struct objfile *objfile,
+                         enum dwarf2_section_enum sect,
                          asection **sectp, gdb_byte **bufp,
                          bfd_size_type *sizep)
 {
@@ -1653,12 +1654,17 @@ dwarf2_get_section_info (struct objfile *objfile, const char *section_name,
       *sizep = 0;
       return;
     }
-  if (section_is_p (section_name, EH_FRAME_SECTION))
-    info = &data->eh_frame;
-  else if (section_is_p (section_name, FRAME_SECTION))
-    info = &data->frame;
-  else
-    gdb_assert_not_reached ("unexpected section");
+  switch (sect)
+    {
+    case dwarf2_debug_frame:
+      info = &data->frame;
+      break;
+    case dwarf2_eh_frame:
+      info = &data->eh_frame;
+      break;
+    default:
+      gdb_assert_not_reached ("unexpected section");
+    }
 
   dwarf2_read_section (objfile, info);
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 3544475..83e807c 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -555,6 +555,17 @@ extern struct cleanup *increment_reading_symtab (void);
 
 extern int dwarf2_has_info (struct objfile *);
 
+/* Dwarf2 sections that can be accessed by dwarf2_get_section_info.  */
+enum dwarf2_section_enum {
+  dwarf2_debug_frame,
+  dwarf2_eh_frame
+};
+
+extern void dwarf2_get_section_info (struct objfile *,
+                                     enum dwarf2_section_enum,
+				     asection **, gdb_byte **,
+				     bfd_size_type *);
+
 extern int dwarf2_initialize_objfile (struct objfile *);
 extern void dwarf2_build_psymtabs (struct objfile *);
 extern void dwarf2_build_frame_info (struct objfile *);
-- 
1.7.3.GIT

[PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

This patch simply adds support for dwarf2 to AIX.  This is a matter of calling
the dwarf2 reader with the xcoff dwarf2 section names.

gdb/
2011-05-23  Tristan Gingold  <gingold@adacore.com>

	* xcoffread.c (dwarf2_xcoff_names): New variable.
	(aix_process_linenos): Add a guard.
	(xcoff_symfile_finish): Free dwarf2.
	(xcoff_initial_scan): Add dwarf2 support.
---
 gdb/xcoffread.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 23decae..bf3a9ec 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -152,6 +152,22 @@ struct coff_symfile_info
     CORE_ADDR toc_offset;
   };
 
+/* XCOFF names for dwarf sections.  There is no compressed sections.  */
+
+static const struct dwarf2_debug_sections dwarf2_xcoff_names = {
+  { ".dwinfo", NULL },
+  { ".dwabrev", NULL },
+  { ".dwline", NULL },
+  { ".dwloc", NULL },
+  { NULL, NULL }, /* debug_macinfo */
+  { ".dwstr", NULL },
+  { ".dwrnges", NULL },
+  { NULL, NULL }, /* debug_types */
+  { ".dwframe", NULL },
+  { NULL, NULL }, /* eh_frame */
+  { NULL, NULL } /* gdb_index */
+};
+
 static void
 bf_notfound_complaint (void)
 {
@@ -757,6 +773,9 @@ return_after_cleanup:
 static void
 aix_process_linenos (void)
 {
+  if (this_symtab_psymtab == NULL)
+    return;
+
   /* Process line numbers and enter them into line vector.  */
   process_linenos (last_source_start_addr, cur_src_end_addr);
 }
@@ -1910,6 +1929,8 @@ xcoff_symfile_finish (struct objfile *objfile)
       inclTable = NULL;
     }
   inclIndx = inclLength = inclDepth = 0;
+
+  dwarf2_free_objfile (objfile);
 }
 
 
@@ -3022,6 +3043,13 @@ xcoff_initial_scan (struct objfile *objfile, int symfile_flags)
 
   install_minimal_symbols (objfile);
 
+  /* DWARF2 sections.  */
+
+  if (dwarf2_has_info (objfile, &dwarf2_xcoff_names))
+    dwarf2_build_psymtabs (objfile);
+
+  dwarf2_build_frame_info (objfile);
+
   do_cleanups (back_to);
 }
 \f
-- 
1.7.3.GIT

Re: [PATCH 1/4] Really make dwarf2_get_section_info public.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan> This patch is a preliminary clean-up.  Instead of using the
Tristan> section name to get the section info, use an enum literal.

Looks reasonable, just one nit.

Tristan> It might be worth going farther and using an array of
Tristan> dwarf2_section_info in dwarf2_per_objfile (instead of record
Tristan> fields).  I let this decision to maintainers.

I think it is fine as is.

Tristan> +enum dwarf2_section_enum {
Tristan> +  dwarf2_debug_frame,
Tristan> +  dwarf2_eh_frame
Tristan> +};

It is customary for enum constants to be upper-case.
Ok with this change.

Tom

Re: [PATCH 2/4] dwarf2read.c: handle alternate dwarf2 debug sections name.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan> Unfortunately, some object file format cannot accept the
Tristan> standard dwarf2 debug section names.  This is in particular the
Tristan> case of XCOFF (aix), which has only 8 chars section names.  In
Tristan> order to support dwarf2 on aix, we need to allow alternate
Tristan> section names.

It seems reasonable to me, but I would like to understand why you took
this route, rather than the section-name-remapping approach taken by
bfd/mach-o.c.

The mach-o approach has the nice quality that it is encapsulated in BFD.

Tom

Re: [PATCH 2/4] dwarf2read.c: handle alternate dwarf2 debug sections name.

[Tristan Gingold]

On May 23, 2011, at 5:22 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan> Unfortunately, some object file format cannot accept the
> Tristan> standard dwarf2 debug section names.  This is in particular the
> Tristan> case of XCOFF (aix), which has only 8 chars section names.  In
> Tristan> order to support dwarf2 on aix, we need to allow alternate
> Tristan> section names.
> 
> It seems reasonable to me, but I would like to understand why you took
> this route, rather than the section-name-remapping approach taken by
> bfd/mach-o.c.
> 
> The mach-o approach has the nice quality that it is encapsulated in BFD.

Simply because the XCOFF maintainer was against this approach:
http://sourceware.org/ml/binutils/2011-04/msg00364.html

Both approach make sense...

Thank you for the reviews.

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan> +static const struct dwarf2_debug_sections dwarf2_xcoff_names = {
Tristan> +  { ".dwinfo", NULL },
Tristan> +  { ".dwabrev", NULL },
Tristan> +  { ".dwline", NULL },
Tristan> +  { ".dwloc", NULL },
Tristan> +  { NULL, NULL }, /* debug_macinfo */
Tristan> +  { ".dwstr", NULL },
Tristan> +  { ".dwrnges", NULL },
Tristan> +  { NULL, NULL }, /* debug_types */
Tristan> +  { ".dwframe", NULL },
Tristan> +  { NULL, NULL }, /* eh_frame */
Tristan> +  { NULL, NULL } /* gdb_index */
Tristan> +};

I don't like this approach, because it requires some non-obvious code
synchronization between this struct and dwarf2read.  I think some kind
of name mapping approach would be preferable -- that way, we would be
free to change dwarf2read without needing to update structs in other
parts of gdb.

If you don't want to do this in BFD, I think it would be fine to do it
locally in gdb.

What do you think?

Tom

Re: [PATCH 2/4] dwarf2read.c: handle alternate dwarf2 debug sections name.

[Tom Tromey]

Tristan> Simply because the XCOFF maintainer was against this approach:
Tristan> http://sourceware.org/ml/binutils/2011-04/msg00364.html

I see.  Onward, then.

Also I see my other note was completely in error.  Sigh.  Sorry about
that.  I'll reply to that to eliminate confusion.

I have a few nits with this patch, nothing serious.


Tristan> +      bfd_map_over_sections (objfile->obfd, dwarf2_locate_sections,
Tristan> +                             (void *)names);

Space after ")".

Tristan> +    names = (const struct dwarf2_debug_sections *)vnames;

You shouldn't need the cast, but if for some reason you do, there should
be a space after the ")".

Tristan> +struct dwarf2_section_names {
Tristan> +  const char *normal;
Tristan> +  const char *compressed;
Tristan> +};
Tristan> +
Tristan> +struct dwarf2_debug_sections {

These should both have introductory comments explaining the purpose of
the structs.

Ok with those changes.

Tom

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tom Tromey]

Tom> I don't like this approach, because it requires some non-obvious code
Tom> synchronization between this struct and dwarf2read.

No, I'm pretty wrong here.  Reordering the struct would indeed cause
problems, because all the fields have the same type -- but this is also
not a very likely transformation.  Adding or removing a field should be
ok, which is what matters.  I don't think this is such a big deal after
all.

Tom

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

On May 23, 2011, at 5:42 PM, Tom Tromey wrote:

> Tom> I don't like this approach, because it requires some non-obvious code
> Tom> synchronization between this struct and dwarf2read.
> 
> No, I'm pretty wrong here.  Reordering the struct would indeed cause
> problems, because all the fields have the same type -- but this is also
> not a very likely transformation.  Adding or removing a field should be
> ok, which is what matters.  I don't think this is such a big deal after
> all.

Ok, thank you for reconsidering your note.  That's nice.

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan>  static void
Tristan>  aix_process_linenos (void)
Tristan>  {
Tristan> +  if (this_symtab_psymtab == NULL)
Tristan> +    return;

I don't understand this change.

The rest looks ok to me.

Tom

Re: [RFA] Add dwarf2 support to xcoff (aix)

[Yao Qi]

On 05/23/2011 06:29 PM, Tristan Gingold wrote:
> Hi,
> 
> with AIX 7, the xcoff binary file format now supports dwarf2.  This is a
> huge step forward as the previous debugging format was a varation of stabs.
> 
> But (and there is always a but) this is not fully standard dwarf2 as the
> section names differ. The first two patches deal with that in the most
> elegant way I could figure.
> 
> The last two patches simply add the dwarf2 support for xcoff.
> 
> Manually tested on AIX.

Is there any reason that prevent you running testsuite on AIX?  If
dwarf2 has been supported on AIX, we may have to add corresponding
target triplet into proc dwarf2_support in testsuite/lib/dwarf.exp.

-- 
Yao (齐尧)

Re: [RFA] Add dwarf2 support to xcoff (aix)

[Tristan Gingold]

On May 23, 2011, at 5:59 PM, Yao Qi wrote:

> On 05/23/2011 06:29 PM, Tristan Gingold wrote:
>> Hi,
>> 
>> with AIX 7, the xcoff binary file format now supports dwarf2.  This is a
>> huge step forward as the previous debugging format was a varation of stabs.
>> 
>> But (and there is always a but) this is not fully standard dwarf2 as the
>> section names differ. The first two patches deal with that in the most
>> elegant way I could figure.
>> 
>> The last two patches simply add the dwarf2 support for xcoff.
>> 
>> Manually tested on AIX.
> 
> Is there any reason that prevent you running testsuite on AIX?

Short answer: I was never able to have a reliable run: expect fails randomly.

>  If
> dwarf2 has been supported on AIX, we may have to add corresponding
> target triplet into proc dwarf2_support in testsuite/lib/dwarf.exp.

Only if dwarf2 is enabled by default, which is not true in the official gcc.

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

On May 23, 2011, at 5:49 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan>  static void
> Tristan>  aix_process_linenos (void)
> Tristan>  {
> Tristan> +  if (this_symtab_psymtab == NULL)
> Tristan> +    return;
> 
> I don't understand this change.

This is just a guard.
process_linenos (called by aix_process_linenos) will dereference this_symtab_psymtab, which is set by read_xcoff_symtab.
However, if the psymtab is built by dwarf2, the variable will never be initialized.

Do you think the issue is somewhere else ?

> 
> The rest looks ok to me.

Thank you for reviewing.

Tristan.

Re: [PATCH 1/4] Really make dwarf2_get_section_info public.

[Tristan Gingold]

On May 23, 2011, at 5:15 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan> This patch is a preliminary clean-up.  Instead of using the
> Tristan> section name to get the section info, use an enum literal.
> 
> Looks reasonable, just one nit.
> 
> Tristan> It might be worth going farther and using an array of
> Tristan> dwarf2_section_info in dwarf2_per_objfile (instead of record
> Tristan> fields).  I let this decision to maintainers.
> 
> I think it is fine as is.
> 
> Tristan> +enum dwarf2_section_enum {
> Tristan> +  dwarf2_debug_frame,
> Tristan> +  dwarf2_eh_frame
> Tristan> +};
> 
> It is customary for enum constants to be upper-case.
> Ok with this change.

Thank, committed with the enum constants in upper-case.

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan> +  if (this_symtab_psymtab == NULL)
Tristan> +    return;

Tom> I don't understand this change.

Tristan> This is just a guard.  process_linenos (called by
Tristan> aix_process_linenos) will dereference this_symtab_psymtab,
Tristan> which is set by read_xcoff_symtab.  However, if the psymtab is
Tristan> built by dwarf2, the variable will never be initialized.

Tristan> Do you think the issue is somewhere else ?

It is very unclear to me whether this can be non-NULL even with DWARF.
In the DWARF case wouldn't the line information come from the DWARF line
table?  In which case it seems like xcoffread.c should have a second
struct sym_fns, with a NULL entry for this method.

Tom

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

On May 24, 2011, at 3:36 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan> +  if (this_symtab_psymtab == NULL)
> Tristan> +    return;
> 
> Tom> I don't understand this change.
> 
> Tristan> This is just a guard.  process_linenos (called by
> Tristan> aix_process_linenos) will dereference this_symtab_psymtab,
> Tristan> which is set by read_xcoff_symtab.  However, if the psymtab is
> Tristan> built by dwarf2, the variable will never be initialized.
> 
> Tristan> Do you think the issue is somewhere else ?
> 
> It is very unclear to me whether this can be non-NULL even with DWARF.
> In the DWARF case wouldn't the line information come from the DWARF line
> table?

Yes, that's correct.  DWARF line table will be used.

>  In which case it seems like xcoffread.c should have a second
> struct sym_fns, with a NULL entry for this method.

But how would that work ?  find_sym_fns finds the sym_fns struct according to the bfd flavour, and there is only
one xcoff flavour.

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tom> In which case it seems like xcoffread.c should have a second
Tom> struct sym_fns, with a NULL entry for this method.

Tristan> But how would that work ?  find_sym_fns finds the sym_fns
Tristan> struct according to the bfd flavour, and there is only one
Tristan> xcoff flavour.

You can change the pointer to an unregistered sym_fns.
See how elfread.c handles this.

I don't actually know anything about XCOFF or xcoffread.c.
If you think your current approach is safe enough, I am ok with it.

Tom

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

On May 24, 2011, at 4:55 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tom> In which case it seems like xcoffread.c should have a second
> Tom> struct sym_fns, with a NULL entry for this method.
> 
> Tristan> But how would that work ?  find_sym_fns finds the sym_fns
> Tristan> struct according to the bfd flavour, and there is only one
> Tristan> xcoff flavour.
> 
> You can change the pointer to an unregistered sym_fns.
> See how elfread.c handles this.

Thank you for the pointer.

> I don't actually know anything about XCOFF or xcoffread.c.
> If you think your current approach is safe enough, I am ok with it.

Well, I think it is safe enough, but I'd prefer to clarify the point.
Let me think about that a little bit.

Tristan.

Re: [PATCH 2/4] dwarf2read.c: handle alternate dwarf2 debug sections name.

[Tristan Gingold]

On May 23, 2011, at 5:40 PM, Tom Tromey wrote:

> Tristan> Simply because the XCOFF maintainer was against this approach:
> Tristan> http://sourceware.org/ml/binutils/2011-04/msg00364.html
> 
> I see.  Onward, then.
> 
> Also I see my other note was completely in error.  Sigh.  Sorry about
> that.  I'll reply to that to eliminate confusion.
> 
> I have a few nits with this patch, nothing serious.
> 
> 
> Tristan> +      bfd_map_over_sections (objfile->obfd, dwarf2_locate_sections,
> Tristan> +                             (void *)names);
> 
> Space after ")".
> 
> Tristan> +    names = (const struct dwarf2_debug_sections *)vnames;
> 
> You shouldn't need the cast, but if for some reason you do, there should
> be a space after the ")".
> 
> Tristan> +struct dwarf2_section_names {
> Tristan> +  const char *normal;
> Tristan> +  const char *compressed;
> Tristan> +};
> Tristan> +
> Tristan> +struct dwarf2_debug_sections {
> 
> These should both have introductory comments explaining the purpose of
> the structs.
> 
> Ok with those changes.

Thanks, committed with these changes.

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

On May 24, 2011, at 3:36 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan> +  if (this_symtab_psymtab == NULL)
> Tristan> +    return;
> 
> Tom> I don't understand this change.
> 
> Tristan> This is just a guard.  process_linenos (called by
> Tristan> aix_process_linenos) will dereference this_symtab_psymtab,
> Tristan> which is set by read_xcoff_symtab.  However, if the psymtab is
> Tristan> built by dwarf2, the variable will never be initialized.
> 
> Tristan> Do you think the issue is somewhere else ?
> 
> It is very unclear to me whether this can be non-NULL even with DWARF.
> In the DWARF case wouldn't the line information come from the DWARF line
> table?  In which case it seems like xcoffread.c should have a second
> struct sym_fns, with a NULL entry for this method.

Tom,

here is my understanding of the issue.

aix_process_linenos is called by end_symtab.
In the case of a standard xcoff binary, a symtab is created only if debugging symbols are present and everything is consistent.
In the case of an xcoff binary compiled without debug info, no symtab is created.

In the case of a dwarf xcoff binary, a symtab is created (by dwarf2read), and the call to aix_process_linenos crashes because of
a non-consistent state within xcoffread.  I am not sure that we could have a second sym_fns, as an objfile can have both dwarf2 and
stabs debug info.

In fact this_symtab_psymtab is NULL with dwarf, as it is set only by read_xcoff_symtab, which is called only numsyms != 0.  This happens only when linenos are present.

I fear that adding a second sym_fns will be harder to manage than the proposed guard.

What do you think ?

Tristan.

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:

Tristan> I fear that adding a second sym_fns will be harder to manage
Tristan> than the proposed guard.

Tristan> What do you think ?

Your patch is fine by me.

thanks,
Tom

Re: [PATCH 3/4] aix: add support for dwarf2.

[Tristan Gingold]

On Jun 2, 2011, at 9:10 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@adacore.com> writes:
> 
> Tristan> I fear that adding a second sym_fns will be harder to manage
> Tristan> than the proposed guard.
> 
> Tristan> What do you think ?
> 
> Your patch is fine by me.

Ok, committed with an additional comment in aix_process_linenos.

Tristan.