[PATCH 1/2] gdb/dwarf: move loops into locate_dw{o,z}_sections

[PATCH 1/2] gdb/dwarf: move loops into locate_dw{o,z}_sections

[Simon Marchi]

For a subsequent patch, it would be easier if the loop over sections
inside locate_dwo_sections (I want to maintain some state for the
duration of the loop).  Move the for loop in there.  And because
locate_dwz_sections is very similar, modify that one too, to keep both
in sync.

Change-Id: I90b3d44184910cc2d86af265bb4b41828a5d2c2e
---
 gdb/dwarf2/dwz.c  | 60 ++++++++++++++++++-------------------
 gdb/dwarf2/read.c | 75 ++++++++++++++++++++++++-----------------------
 gdb/dwarf2/read.h |  3 +-
 3 files changed, 69 insertions(+), 69 deletions(-)

diff --git a/gdb/dwarf2/dwz.c b/gdb/dwarf2/dwz.c
index 583103b4bae4..59fe8e4886b4 100644
--- a/gdb/dwarf2/dwz.c
+++ b/gdb/dwarf2/dwz.c
@@ -56,35 +56,37 @@ dwz_file::read_string (struct objfile *objfile, LONGEST str_offset)
 /* A helper function to find the sections for a .dwz file.  */
 
 static void
-locate_dwz_sections (struct objfile *objfile, bfd *abfd, asection *sectp,
-		     dwz_file *dwz_file)
+locate_dwz_sections (objfile *objfile, dwz_file &dwz_file)
 {
-  dwarf2_section_info *sect = nullptr;
-
-  /* Note that we only support the standard ELF names, because .dwz
-     is ELF-only (at the time of writing).  */
-  if (dwarf2_elf_names.abbrev.matches (sectp->name))
-    sect = &dwz_file->abbrev;
-  else if (dwarf2_elf_names.info.matches (sectp->name))
-    sect = &dwz_file->info;
-  else if (dwarf2_elf_names.str.matches (sectp->name))
-    sect = &dwz_file->str;
-  else if (dwarf2_elf_names.line.matches (sectp->name))
-    sect = &dwz_file->line;
-  else if (dwarf2_elf_names.macro.matches (sectp->name))
-    sect = &dwz_file->macro;
-  else if (dwarf2_elf_names.gdb_index.matches (sectp->name))
-    sect = &dwz_file->gdb_index;
-  else if (dwarf2_elf_names.debug_names.matches (sectp->name))
-    sect = &dwz_file->debug_names;
-  else if (dwarf2_elf_names.types.matches (sectp->name))
-    sect = &dwz_file->types;
-
-  if (sect != nullptr)
+  for (asection *sec : gdb_bfd_sections (dwz_file.dwz_bfd))
     {
-      sect->s.section = sectp;
-      sect->size = bfd_section_size (sectp);
-      sect->read (objfile);
+      dwarf2_section_info *sect = nullptr;
+
+      /* Note that we only support the standard ELF names, because .dwz
+     is ELF-only (at the time of writing).  */
+      if (dwarf2_elf_names.abbrev.matches (sec->name))
+	sect = &dwz_file.abbrev;
+      else if (dwarf2_elf_names.info.matches (sec->name))
+	sect = &dwz_file.info;
+      else if (dwarf2_elf_names.str.matches (sec->name))
+	sect = &dwz_file.str;
+      else if (dwarf2_elf_names.line.matches (sec->name))
+	sect = &dwz_file.line;
+      else if (dwarf2_elf_names.macro.matches (sec->name))
+	sect = &dwz_file.macro;
+      else if (dwarf2_elf_names.gdb_index.matches (sec->name))
+	sect = &dwz_file.gdb_index;
+      else if (dwarf2_elf_names.debug_names.matches (sec->name))
+	sect = &dwz_file.debug_names;
+      else if (dwarf2_elf_names.types.matches (sec->name))
+	sect = &dwz_file.types;
+
+      if (sect != nullptr)
+	{
+	  sect->s.section = sec;
+	  sect->size = bfd_section_size (sec);
+	  sect->read (objfile);
+	}
     }
 }
 
@@ -392,9 +394,7 @@ dwz_file::read_dwz_file (dwarf2_per_objfile *per_objfile)
 
   dwz_file_up result (new dwz_file (std::move (dwz_bfd)));
 
-  for (asection *sec : gdb_bfd_sections (result->dwz_bfd))
-    locate_dwz_sections (per_objfile->objfile, result->dwz_bfd.get (),
-			 sec, result.get ());
+  locate_dwz_sections (per_objfile->objfile, *result);
 
   gdb_bfd_record_inclusion (per_bfd->obfd, result->dwz_bfd.get ());
   bfd_cache_close (result->dwz_bfd.get ());
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 71fa793315ff..550968abc95c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -7512,45 +7512,48 @@ cutu_reader::open_dwo_file (dwarf2_per_bfd *per_bfd, const char *file_name,
    size of each of the DWO debugging sections we are interested in.  */
 
 void
-cutu_reader::locate_dwo_sections (struct objfile *objfile, bfd *abfd,
-				  asection *sectp, dwo_sections *dwo_sections)
+cutu_reader::locate_dwo_sections (objfile *objfile, dwo_file &dwo_file)
 {
   const struct dwop_section_names *names = &dwop_section_names;
+  dwo_sections &dwo_sections = dwo_file.sections;
 
-  struct dwarf2_section_info *dw_sect = nullptr;
-
-  if (names->abbrev_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->abbrev;
-  else if (names->info_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->infos.emplace_back (dwarf2_section_info {});
-  else if (names->line_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->line;
-  else if (names->loc_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->loc;
-  else if (names->loclists_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->loclists;
-  else if (names->macinfo_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->macinfo;
-  else if (names->macro_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->macro;
-  else if (names->rnglists_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->rnglists;
-  else if (names->str_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->str;
-  else if (names->str_offsets_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->str_offsets;
-  else if (names->types_dwo.matches (sectp->name))
-    dw_sect = &dwo_sections->types.emplace_back (dwarf2_section_info {});
-
-  if (dw_sect != nullptr)
+  for (asection *sec : gdb_bfd_sections (dwo_file.dbfd))
     {
-      /* Make sure we don't overwrite a section info that has been filled in
-	 already.  */
-      gdb_assert (!dw_sect->readin);
+      struct dwarf2_section_info *dw_sect = nullptr;
 
-      dw_sect->s.section = sectp;
-      dw_sect->size = bfd_section_size (sectp);
-      dw_sect->read (objfile);
+      if (names->abbrev_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.abbrev;
+      else if (names->info_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.infos.emplace_back (dwarf2_section_info {});
+      else if (names->line_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.line;
+      else if (names->loc_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.loc;
+      else if (names->loclists_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.loclists;
+      else if (names->macinfo_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.macinfo;
+      else if (names->macro_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.macro;
+      else if (names->rnglists_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.rnglists;
+      else if (names->str_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.str;
+      else if (names->str_offsets_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.str_offsets;
+      else if (names->types_dwo.matches (sec->name))
+	dw_sect = &dwo_sections.types.emplace_back (dwarf2_section_info {});
+
+      if (dw_sect != nullptr)
+	{
+	  /* Make sure we don't overwrite a section info that has been filled in
+	 already.  */
+	  gdb_assert (!dw_sect->readin);
+
+	  dw_sect->s.section = sec;
+	  dw_sect->size = bfd_section_size (sec);
+	  dw_sect->read (objfile);
+	}
     }
 }
 
@@ -7578,9 +7581,7 @@ cutu_reader::open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
   dwo_file->comp_dir = comp_dir;
   dwo_file->dbfd = std::move (dbfd);
 
-  for (asection *sec : gdb_bfd_sections (dwo_file->dbfd))
-    this->locate_dwo_sections (per_objfile->objfile, dwo_file->dbfd.get (), sec,
-			       &dwo_file->sections);
+  this->locate_dwo_sections (per_objfile->objfile, *dwo_file);
 
   /* There is normally just one .debug_info.dwo section in a DWO file.  But when
      building with -fdebug-types-section, gcc produces multiple .debug_info.dwo
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 5b4c8f673a0f..aaac5e7cbbfc 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -1029,8 +1029,7 @@ class cutu_reader
   dwo_file_up open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
 				      const char *comp_dir);
 
-  void locate_dwo_sections (struct objfile *objfile, bfd *abfd, asection *sectp,
-			    struct dwo_sections *dwo_sections);
+  void locate_dwo_sections (objfile *objfile, dwo_file &dwo_file);
 
   void create_dwo_unit_hash_tables (dwo_file &dwo_file, dwarf2_cu &skeleton_cu,
 				    dwarf2_section_info &section,

base-commit: 998165ba99a3bcc80ba0b196223f4d49623e87b4
-- 
2.49.0

[PATCH 2/2] gdb/dwarf: skip broken .debug_macro.dwo

[Simon Marchi]

Running gdb.base/errno.exp with gcc <= 13 with split DWARF results in:

    $ make check TESTS="gdb.base/errno.exp" RUNTESTFLAGS="CC_FOR_TARGET=gcc-13 --target_board=fission"
    (gdb) break -qualified main
    /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:7549: internal-error: locate_dwo_sections: Assertion `!dw_sect->readin' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    ...
    FAIL: gdb.base/errno.exp: macros: gdb_breakpoint: set breakpoint at main (GDB internal error)

The assert being hit has been added in 28f15782adab ("gdb/dwarf: read
multiple .debug_info.dwo sections"), but it merely exposed an existing
problem.

gcc versions <= 13 are affected by this bug:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111409

Basically, it produces .dwo files with multiple .debug_macro.dwo
sections, with some unresolved links between them.  I think that this
macro debug info is unusable, and all we can do is ignore it.

In locate_dwo_sections, if we detect a second .debug_macro.dwo section,
forget about the previous .debug_macro.dwo and any subsequent one.  This
will effectively make it as if the macro debug info wasn't there at all.

The errno test seems happy with it:

    # of expected passes            84
    # of expected failures          8

Change-Id: I6489b4713954669bf69f6e91865063ddcd1ac2c8
---
 gdb/dwarf2/read.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 550968abc95c..63c0c007563e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -7516,6 +7516,7 @@ cutu_reader::locate_dwo_sections (objfile *objfile, dwo_file &dwo_file)
 {
   const struct dwop_section_names *names = &dwop_section_names;
   dwo_sections &dwo_sections = dwo_file.sections;
+  bool complained_about_macro_already = false;
 
   for (asection *sec : gdb_bfd_sections (dwo_file.dbfd))
     {
@@ -7534,7 +7535,24 @@ cutu_reader::locate_dwo_sections (objfile *objfile, dwo_file &dwo_file)
       else if (names->macinfo_dwo.matches (sec->name))
 	dw_sect = &dwo_sections.macinfo;
       else if (names->macro_dwo.matches (sec->name))
-	dw_sect = &dwo_sections.macro;
+	{
+	  /* gcc versions <= 13 generate multiple .debug_macro.dwo sections with
+	     some unresolved links between them.  It's not usable, so do as if
+	     there were not there.  */
+	  if (!complained_about_macro_already)
+	    {
+	      if (dwo_sections.macro.s.section == nullptr)
+		dw_sect = &dwo_sections.macro;
+	      else
+		{
+		  complaint (_("Multiple .debug_macro.dwo sections found in "
+			       "%s, ignoring them."), dwo_file.dbfd->filename);
+
+		  dwo_sections.macro = dwarf2_section_info {};
+		  complained_about_macro_already = true;
+		}
+	    }
+	}
       else if (names->rnglists_dwo.matches (sec->name))
 	dw_sect = &dwo_sections.rnglists;
       else if (names->str_dwo.matches (sec->name))
-- 
2.49.0

Re: [PATCH 2/2] gdb/dwarf: skip broken .debug_macro.dwo

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> +		  complaint (_("Multiple .debug_macro.dwo sections found in "
Simon> +			       "%s, ignoring them."), dwo_file.dbfd->filename);

complaints are only really visible to gdb developers, since most users
don't (and shouldn't) enable them.

However this is a situation where the issue could cause a user-relevant
problem (macros not working) and has a fix the user could perhaps enact
(different compiler).  So maybe a warning would be more appropriate?

Tom

Re: [PATCH 2/2] gdb/dwarf: skip broken .debug_macro.dwo

[Simon Marchi]

On 5/12/25 1:55 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> +		  complaint (_("Multiple .debug_macro.dwo sections found in "
> Simon> +			       "%s, ignoring them."), dwo_file.dbfd->filename);
> 
> complaints are only really visible to gdb developers, since most users
> don't (and shouldn't) enable them.
> 
> However this is a situation where the issue could cause a user-relevant
> problem (macros not working) and has a fix the user could perhaps enact
> (different compiler).  So maybe a warning would be more appropriate?

This would be fine with me.  The line would become:

		  warning (_("Multiple .debug_macro.dwo sections found in "
			     "%s, ignoring them."), dwo_file.dbfd->filename);

The output looks like this:

    (gdb) b main
    During symbol reading: Multiple .debug_macro.dwo sections found in /home/smarchi/build/binutils-gdb/gdb/test.dwo, ignoring them.
    Breakpoint 1 at 0x11b2: file test.cpp, line 4.

Is the patch ok with that change?

Simon

Re: [PATCH 2/2] gdb/dwarf: skip broken .debug_macro.dwo

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> Is the patch ok with that change?

Yeah, it's fine by me either way -- I was really asking for your opinion
though, not requesting a change.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 2/2] gdb/dwarf: skip broken .debug_macro.dwo

[Simon Marchi]

On 5/12/25 2:48 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> Is the patch ok with that change?
> 
> Yeah, it's fine by me either way -- I was really asking for your opinion
> though, not requesting a change.

I don't really know the rule of thumb here, so I trust you.  Warning
seems fine to me.

> Approved-By: Tom Tromey <tom@tromey.com>

Thanks, will push.

Simon

Re: [PATCH 2/2] gdb/dwarf: skip broken .debug_macro.dwo

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

>> Yeah, it's fine by me either way -- I was really asking for your opinion
>> though, not requesting a change.

Simon> I don't really know the rule of thumb here, so I trust you.  Warning
Simon> seems fine to me.

I hold the somewhat extreme view that complaints are useless and we
should remove them.  But anyway they are normally used to report
pedantic DWARF bugs that aren't user-actionable.

Tom

Re: [PATCH 1/2] gdb/dwarf: move loops into locate_dw{o,z}_sections

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> For a subsequent patch, it would be easier if the loop over sections
Simon> inside locate_dwo_sections (I want to maintain some state for the
Simon> duration of the loop).  Move the for loop in there.  And because
Simon> locate_dwz_sections is very similar, modify that one too, to keep both
Simon> in sync.

Looks good, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom