[PATCH][gdb/symtab] Support .debug_names section with TUs in .debug_info

[PATCH][gdb/symtab] Support .debug_names section with TUs in .debug_info

[Tom de Vries]

Hi,

[ Requires "[gdb/symtab] Fix assert in set_length" submitted here (
https://sourceware.org/pipermail/gdb-patches/2022-August/191313.html ). ]

When running test-case gdb.cp/cpexprs-debug-types.exp on target board
cc-with-debug-names/gdb:debug_flags=-gdwarf-5, we get an executable with
a .debug_names section, but no .debug_types section.  For dwarf-5, the TUs
are no longer put in a separate unit, but instead they're put in the
.debug_info section.

When loading the executable, the .debug_names section is silently ignored
because of this check in dwarf2_read_debug_names:
...
  if (map->tu_count != 0)
    {
      /* We can only handle a single .debug_types when we have an
         index.  */
      if (per_bfd->types.size () != 1)
        return false;
...
which triggers because per_bfd->types.size () == 0.

The intention of the check is to make sure we don't have both .debug_types and
.zdebug_types sections.

Fix this by:
- changing the check condition to "per_bfd->types.size () > 1", and
- handling per_bfd->types.size () == 0.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29385

Any comments?

Thanks,
- Tom

[gdb/symtab] Support .debug_names section with TUs in .debug_info

---
 gdb/dwarf2/read.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6c6ca96f8d9..8e82f464293 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4736,13 +4736,16 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
     {
       /* We can only handle a single .debug_types when we have an
 	 index.  */
-      if (per_bfd->types.size () != 1)
+      if (per_bfd->types.size () > 1)
 	{
 	  per_bfd->all_comp_units.clear ();
 	  return false;
 	}
 
-      dwarf2_section_info *section = &per_bfd->types[0];
+      dwarf2_section_info *section
+	= (per_bfd->types.size () == 1
+	   ? &per_bfd->types[0]
+	   : &per_bfd->info);
 
       create_signatured_type_table_from_debug_names
 	(per_objfile, *map, section, &per_bfd->abbrev);

Re: [PATCH][gdb/symtab] Support .debug_names section with TUs in .debug_info

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On 8/12/22 17:48, Tom de Vries via Gdb-patches wrote:
> Hi,
> 
> [ Requires "[gdb/symtab] Fix assert in set_length" submitted here (
> https://sourceware.org/pipermail/gdb-patches/2022-August/191313.html ). ]
> 
> When running test-case gdb.cp/cpexprs-debug-types.exp on target board
> cc-with-debug-names/gdb:debug_flags=-gdwarf-5, we get an executable with
> a .debug_names section, but no .debug_types section.  For dwarf-5, the TUs
> are no longer put in a separate unit, but instead they're put in the
> .debug_info section.
> 
> When loading the executable, the .debug_names section is silently ignored
> because of this check in dwarf2_read_debug_names:
> ...
>    if (map->tu_count != 0)
>      {
>        /* We can only handle a single .debug_types when we have an
>           index.  */
>        if (per_bfd->types.size () != 1)
>          return false;
> ...
> which triggers because per_bfd->types.size () == 0.
> 
> The intention of the check is to make sure we don't have both .debug_types and
> .zdebug_types sections.
> 

Well, that's what I thought when I read the code, but I actually found 
back the corresponding PR: PR12984, and it turns out it's about object 
files, which can contain:
...
$ grep "\.debug_types" 11.s
         .section 
.debug_types,"G",@progbits,wt.75c042c23a9a07ee,comdat
         .section 
.debug_types,"G",@progbits,wt.c59c413bf50a4607,comdat
...

I've updated the rationale to reflect this.

Also, I've added a test-case.

Committed as attached.

Thanks,
- Tom

[-- Attachment #2: 0003-gdb-symtab-Support-.debug_names-section-with-TUs-in-.debug_info.patch --]
[-- Type: text/x-patch, Size: 9959 bytes --]

[gdb/symtab] Support .debug_names section with TUs in .debug_info

When running test-case gdb.cp/cpexprs-debug-types.exp on target board
cc-with-debug-names/gdb:debug_flags=-gdwarf-5, we get an executable with
a .debug_names section, but no .debug_types section.  For dwarf-5, the TUs
are no longer put in a separate unit, but instead they're put in the
.debug_info section.

When loading the executable, the .debug_names section is silently ignored
because of this check in dwarf2_read_debug_names:
...
  if (map->tu_count != 0)
    {
      /* We can only handle a single .debug_types when we have an
         index.  */
      if (per_bfd->types.size () != 1)
        return false;
...
which triggers because per_bfd->types.size () == 0.

The intention of the check is to make sure we don't have more that one
.debug_types section, as can happen in a object file (see PR12984):
...
$ grep "\.debug_types" 11.s
        .section        .debug_types,"G",@progbits,wt.75c042c23a9a07ee,comdat
        .section        .debug_types,"G",@progbits,wt.c59c413bf50a4607,comdat
...

Fix this by:
- changing the check condition to "per_bfd->types.size () > 1", and
- handling per_bfd->types.size () == 0.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29385

---
 gdb/dwarf2/read.c                                  |  7 +-
 gdb/testsuite/gdb.dwarf2/debug-names-tu-dwarf5.exp | 18 +++++
 gdb/testsuite/gdb.dwarf2/debug-names-tu.exp        | 79 +-----------------
 gdb/testsuite/gdb.dwarf2/debug-names-tu.exp.tcl    | 94 ++++++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp                        | 21 ++++-
 5 files changed, 136 insertions(+), 83 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 08dd0b76b23..3ca441c4cae 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4736,13 +4736,16 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
     {
       /* We can only handle a single .debug_types when we have an
 	 index.  */
-      if (per_bfd->types.size () != 1)
+      if (per_bfd->types.size () > 1)
 	{
 	  per_bfd->all_comp_units.clear ();
 	  return false;
 	}
 
-      dwarf2_section_info *section = &per_bfd->types[0];
+      dwarf2_section_info *section
+	= (per_bfd->types.size () == 1
+	   ? &per_bfd->types[0]
+	   : &per_bfd->info);
 
       create_signatured_type_table_from_debug_names
 	(per_objfile, *map, section, &per_bfd->abbrev);
diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-tu-dwarf5.exp b/gdb/testsuite/gdb.dwarf2/debug-names-tu-dwarf5.exp
new file mode 100644
index 00000000000..ac3b459db08
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/debug-names-tu-dwarf5.exp
@@ -0,0 +1,18 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set dwarf_version 5
+
+source $srcdir/$subdir/debug-names-tu.exp.tcl
diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp
index 34fecff7b9d..b796a7320be 100644
--- a/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp
+++ b/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp
@@ -13,81 +13,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-load_lib dwarf.exp
+set dwarf_version 4
 
-# This test can only be run on targets which support DWARF-2 and use gas.
-if {![dwarf2_support]} {
-    return 0
-}
-
-standard_testfile _start.c debug-names.S
-
-set func_info_vars \
-    [get_func_info _start [list debug additional_flags=-nostartfiles]]
-
-# Create the DWARF.
-set asm_file [standard_output_file $srcfile2]
-Dwarf::assemble {
-    filename $asm_file
-    add_dummy_cus 0
-} {
-    global func_info_vars
-    foreach var $func_info_vars {
-	global $var
-    }
-
-    cu { label cu_label } {
-	compile_unit {{language @DW_LANG_C}} {
-	    subprogram {
-		{DW_AT_name _start}
-		{DW_AT_low_pc $_start_start DW_FORM_addr}
-		{DW_AT_high_pc $_start_end DW_FORM_addr}
-	    }
-	}
-    }
-
-    tu { label tu_label } 0x8ece66f4224fddb3 "" {
-	type_unit {} {
-	    declare_labels int_type
-
-	    structure_type {
-		{name struct_with_int_member}
-		{byte_size 4 sdata}
-	    } {
-		member {
-		    {name member}
-		    {type :$int_type}
-		}
-	    }
-	    int_type: base_type {
-		{name int}
-		{encoding @DW_ATE_signed}
-		{byte_size 4 sdata}
-	    }
-	}
-    }
-
-    debug_names {} {
-	cu cu_label
-	tu tu_label
-	name _start subprogram cu_label 0xEDDB6232
-	name struct_with_int_member structure_type tu_label 0x53A2AE86
-    }
-}
-
-if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \
-	[list additional_flags=-nostartfiles]] {
-    return -1
-}
-
-# Verify that .debug_names section is not ignored.
-set index [have_index $binfile]
-gdb_assert { [string equal $index "debug_names"] } ".debug_names used"
-
-# Verify that we can find the type in the type unit.
-set re \
-    [multi_line \
-	 "type = struct struct_with_int_member {" \
-	 "    int member;" \
-	 "}"]
-gdb_test "ptype struct struct_with_int_member" $re
+source $srcdir/$subdir/debug-names-tu.exp.tcl
diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp.tcl b/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp.tcl
new file mode 100644
index 00000000000..738ecc47c5a
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/debug-names-tu.exp.tcl
@@ -0,0 +1,94 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile _start.c debug-names.S
+
+set func_info_vars \
+    [get_func_info _start [list debug additional_flags=-nostartfiles]]
+
+# Create the DWARF.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble {
+    filename $asm_file
+    add_dummy_cus 0
+} {
+    global dwarf_version
+    global func_info_vars
+    foreach var $func_info_vars {
+	global $var
+    }
+
+    cu { label cu_label version $dwarf_version } {
+	compile_unit {{language @DW_LANG_C}} {
+	    subprogram {
+		{DW_AT_name _start}
+		{DW_AT_low_pc $_start_start DW_FORM_addr}
+		{DW_AT_high_pc $_start_end DW_FORM_addr}
+	    }
+	}
+    }
+
+    tu { label tu_label version $dwarf_version } 0x8ece66f4224fddb3 "" {
+	type_unit {} {
+	    declare_labels int_type
+
+	    structure_type {
+		{name struct_with_int_member}
+		{byte_size 4 sdata}
+	    } {
+		member {
+		    {name member}
+		    {type :$int_type}
+		}
+	    }
+	    int_type: base_type {
+		{name int}
+		{encoding @DW_ATE_signed}
+		{byte_size 4 sdata}
+	    }
+	}
+    }
+
+    debug_names {} {
+	cu cu_label
+	tu tu_label
+	name _start subprogram cu_label 0xEDDB6232
+	name struct_with_int_member structure_type tu_label 0x53A2AE86
+    }
+}
+
+if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \
+	[list additional_flags=-nostartfiles]] {
+    return -1
+}
+
+# Verify that .debug_names section is not ignored.
+set index [have_index $binfile]
+gdb_assert { [string equal $index "debug_names"] } ".debug_names used"
+
+# Verify that we can find the type in the type unit.
+set re \
+    [multi_line \
+	 "type = struct struct_with_int_member {" \
+	 "    int member;" \
+	 "}"]
+gdb_test "ptype struct struct_with_int_member" $re
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b0e248c8e2f..6db0ed28deb 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1563,6 +1563,7 @@ namespace eval Dwarf {
 	set label ""
 
 	foreach { name value } $options {
+	    set value [uplevel 1 "subst \"$value\""]
 	    switch -exact -- $name {
 		is_64 { set is_64 $value }
 		version { set _cu_version $value }
@@ -1580,9 +1581,12 @@ namespace eval Dwarf {
 	    }
 	}
 	set _cu_offset_size [expr { $is_64 ? 8 : 4 }]
+	if { $_cu_version == 5 } {
+	    set section ".debug_info"
+	}
 	if { $_cu_is_fission } {
-	    set section ".debug_types.dwo"
-	    set _abbrev_section ".debug_abbrev.dwo"
+	    set section "$section.dwo"
+	    set _abbrev_section "$section.dwo"
 	}
 
 	_section $section
@@ -1609,8 +1613,17 @@ namespace eval Dwarf {
 	}
 	define_label $start_label
 	_op .2byte $_cu_version Version
-	_op_offset $_cu_offset_size $my_abbrevs Abbrevs
-	_op .byte $_cu_addr_size "Pointer size"
+
+	# The CU header for DWARF 4 and 5 are slightly different.
+	if { $_cu_version == 5 } {
+	    _op .byte 0x2 "DW_UT_type"
+	    _op .byte $_cu_addr_size "Pointer size"
+	    _op_offset $_cu_offset_size $my_abbrevs Abbrevs
+	} else {
+	    _op_offset $_cu_offset_size $my_abbrevs Abbrevs
+	    _op .byte $_cu_addr_size "Pointer size"
+	}
+
 	_op .8byte $signature Signature
 	if { $type_label != "" } {
 	    uplevel declare_labels $type_label