[RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram

[Tom de Vries <tdevries@suse.de>]

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

[ was: Re: [PATCH][gdb/testsuite] Add KFAILs for PR symtab/24549 ]

On 2/5/21 6:28 PM, Tom de Vries wrote:
> On 2/5/21 4:25 PM, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> gdb/testsuite/ChangeLog:
>>
>> Tom> 2021-01-29  Tom de Vries  <tdevries@suse.de>
>>
>> Tom> 	* gdb.dwarf2/main-subprogram.exp: Add KFAIL for PR symtab/24549.
>> Tom> 	* gdb.fortran/mixed-lang-stack.exp: Same.
>>
>> This seems fine to me.  Thanks for doing this.
>>
>> This is definitely a hole in .gdb_index (which I wouldn't bother
>> fixing), but I wonder if the official DWARF index has support for this
>> situation.
> 
> AFAICT, no.  Maybe we should bring this up on the DWARF std ml?

I tried fixing this by adding a GNU extension DW_IDX_GNU_main_subprogram
to .debug_names.

WDYT?

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-symtab-Add-DW_IDX_GNU_main_subprogram.patch --]
[-- Type: text/x-patch, Size: 14144 bytes --]

[gdb/symtab] Add DW_IDX_GNU_main_subprogram

A debugging session idiom is to set a breakpoint on the program entry point,
f.i. main for a C program.  GDB has a shorthand for this, the start command.

The start command needs to know the name of the program entry point.  This can
be indicated by the language, but also using the DW_AT_main_subprogram
attribute.

This flag attribute can be set on a DW_TAG_subprogram DIE, as well as on the
containing CU DIE.  Currently gcc only emits the former, not the latter
(gcc PR debug/90422).

When loading an executable with a .debug_names index, the symbols are not
parsed, and consequently the DW_AT_main_subprogram flag is ignored
(gdb PR symtab/24549).

Fix this by adding a GNU extension DW_IDX_GNU_main_subprogram to the name
index attributes, encoded using a number in the DW_IDX_lo_user-DW_IDX_hi_user
range.

Tested on x86_64-linux, with native and target board cc-with-debug-names.

gdb/ChangeLog:

2021-02-08  Tom de Vries  <tdevries@suse.de>

	PR symtab/24549
	* dwarf2/index-write.c (debug_names::insert): Add and handle is_main
	and objfile parameters.
	(debug_names::build): Write DW_IDX_GNU_main_subprogram.
	(debug_names::recursively_write_psymbols): Add and handle objfile
	parameter.
	(debug_names::symbol_value): Add is_main member.
	(debug_names::write_psymbols): Add and handle objfile parameter.
	(debug_names::write_one_signatured_type): Pass objfile argument to
	write_psymbols.
	* dwarf2/read.c (dw2_debug_names_iterator::dw2_debug_names_iterator):
	Make block_index parameter a gdb::optional.  Add is_main parameter.
	(dw2_debug_names_iterator): Add m_main member.
	(dw2_debug_names_iterator::next): Handle m_main.
	(dw2_debug_names_map_matching_symbols): Update
	dw2_debug_names_iterator constructor call.
	(dwarf2_initialize_objfile): Expand CU containing main for
	dw_index_kind::DEBUG_NAMES case.

gdb/testsuite/ChangeLog:

2021-02-08  Tom de Vries  <tdevries@suse.de>

	PR symtab/24549
	* lib/gdb.exp (exec_has_section): New proc, factored out of ...
	(exec_has_index_section): ... here.
	(exec_has_gdb_index_section): New proc.
	* gdb.dwarf2/main-subprogram.exp: Use exec_has_gdb_index_section
	instead of exec_has_index_section for kfail.
	* gdb.fortran/mixed-lang-stack.exp: Same.

include/ChangeLog:

2021-02-08  Tom de Vries  <tdevries@suse.de>

	PR symtab/24549
	* dwarf2.def (DW_IDX_GNU_main_subprogram): New DW_IDX.

---
 gdb/dwarf2/index-write.c                       | 45 +++++++++++++++++++-------
 gdb/dwarf2/read.c                              | 38 ++++++++++++++++++++--
 gdb/testsuite/gdb.dwarf2/main-subprogram.exp   |  4 +--
 gdb/testsuite/gdb.fortran/mixed-lang-stack.exp |  6 ++--
 gdb/testsuite/lib/gdb.exp                      | 18 ++++++++---
 include/dwarf2.def                             |  1 +
 6 files changed, 88 insertions(+), 24 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index a7b9ae66cae..1e5c3bbd6cb 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -711,8 +711,9 @@ class debug_names
   enum class unit_kind { cu, tu };
 
   /* Insert one symbol.  */
-  void insert (const partial_symbol *psym, int cu_index, bool is_static,
-	       unit_kind kind)
+  void insert (struct objfile *objfile,
+	       const partial_symbol *psym, int cu_index, bool is_static,
+	       unit_kind kind, bool is_main)
   {
     const int dwarf_tag = psymbol_tag (psym);
     if (dwarf_tag == 0)
@@ -733,7 +734,7 @@ class debug_names
 					     std::set<symbol_value> ());
 	    std::set<symbol_value> &value_set = insertpair.first->second;
 	    value_set.emplace (symbol_value (dwarf_tag, cu_index, is_static,
-					     kind));
+					     kind, is_main));
 	  }
 
 	/* In order for the index to work when read back into gdb, it
@@ -759,7 +760,8 @@ class debug_names
       = m_name_to_value_set.emplace (c_str_view (name),
 				     std::set<symbol_value> ());
     std::set<symbol_value> &value_set = insertpair.first->second;
-    value_set.emplace (symbol_value (dwarf_tag, cu_index, is_static, kind));
+    value_set.emplace (symbol_value (dwarf_tag, cu_index, is_static, kind,
+				     is_main));
   }
 
   /* Build all the tables.  All symbols must be already inserted.
@@ -839,6 +841,13 @@ class debug_names
 							   ? DW_IDX_GNU_internal
 							   : DW_IDX_GNU_external);
 		    m_abbrev_table.append_unsigned_leb128 (DW_FORM_flag_present);
+		    if (value.is_main)
+		      {
+			m_abbrev_table.append_unsigned_leb128
+			  (DW_IDX_GNU_main_subprogram);
+			m_abbrev_table.append_unsigned_leb128
+			  (DW_FORM_flag_present);
+		      }
 
 		    /* Terminate attributes list.  */
 		    m_abbrev_table.append_unsigned_leb128 (0);
@@ -906,9 +915,9 @@ class debug_names
 	recursively_write_psymbols
 	  (objfile, psymtab->dependencies[i], psyms_seen, cu_index);
 
-    write_psymbols (psyms_seen, psymtab->global_psymbols,
+    write_psymbols (objfile, psyms_seen, psymtab->global_psymbols,
 		    cu_index, false, unit_kind::cu);
-    write_psymbols (psyms_seen, psymtab->static_psymbols,
+    write_psymbols (objfile, psyms_seen, psymtab->static_psymbols,
 		    cu_index, true, unit_kind::cu);
   }
 
@@ -1073,11 +1082,12 @@ class debug_names
     const int dwarf_tag, cu_index;
     const bool is_static;
     const unit_kind kind;
+    const bool is_main;
 
     symbol_value (int dwarf_tag_, int cu_index_, bool is_static_,
-		  unit_kind kind_)
+		  unit_kind kind_, bool is_main_)
       : dwarf_tag (dwarf_tag_), cu_index (cu_index_), is_static (is_static_),
-	kind (kind_)
+	kind (kind_), is_main (is_main_)
     {}
 
     bool
@@ -1096,6 +1106,7 @@ class debug_names
       X (is_static);
       X (kind);
       X (cu_index);
+      X (is_main);
 #undef X
       return false;
     }
@@ -1240,7 +1251,8 @@ class debug_names
   }
 
   /* Call insert for all partial symbols and mark them in PSYMS_SEEN.  */
-  void write_psymbols (std::unordered_set<partial_symbol *> &psyms_seen,
+  void write_psymbols (struct objfile *objfile,
+		       std::unordered_set<partial_symbol *> &psyms_seen,
 		       const std::vector<partial_symbol *> &symbols,
 		       int cu_index, bool is_static, unit_kind kind)
   {
@@ -1248,7 +1260,16 @@ class debug_names
       {
 	/* Only add a given psymbol once.  */
 	if (psyms_seen.insert (psym).second)
-	  insert (psym, cu_index, is_static, kind);
+	  {
+	    bool is_main = false;
+	    if (objfile != nullptr
+		&& objfile->per_bfd->name_of_main != nullptr
+		&& strcmp (objfile->per_bfd->name_of_main,
+			   psym->ginfo.search_name()) == 0)
+	      is_main = true;
+
+	    insert (objfile, psym, cu_index, is_static, kind, is_main);
+	  }
       }
   }
 
@@ -1260,9 +1281,9 @@ class debug_names
   {
     partial_symtab *psymtab = entry->per_cu.v.psymtab;
 
-    write_psymbols (info->psyms_seen, psymtab->global_psymbols,
+    write_psymbols (NULL, info->psyms_seen, psymtab->global_psymbols,
 		    info->cu_index, false, unit_kind::tu);
-    write_psymbols (info->psyms_seen, psymtab->static_psymbols,
+    write_psymbols (NULL, info->psyms_seen, psymtab->static_psymbols,
 		    info->cu_index, true, unit_kind::tu);
 
     info->types_list.append_uint (dwarf5_offset_size (), m_dwarf5_byte_order,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3f60ce6a4f1..92851adc1c2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5489,9 +5489,11 @@ class dw2_debug_names_iterator
   {}
 
   dw2_debug_names_iterator (const mapped_debug_names &map,
-			    block_enum block_index, domain_enum domain,
-			    uint32_t namei, dwarf2_per_objfile *per_objfile)
+			    gdb::optional<block_enum> block_index,
+			    domain_enum domain, uint32_t namei,
+			    dwarf2_per_objfile *per_objfile, bool is_main)
     : m_map (map), m_block_index (block_index), m_domain (domain),
+      m_main (is_main),
       m_addr (find_vec_in_debug_names (map, namei, per_objfile)),
       m_per_objfile (per_objfile)
   {}
@@ -5517,6 +5519,7 @@ class dw2_debug_names_iterator
   /* The kind of symbol we're looking for.  */
   const domain_enum m_domain = UNDEF_DOMAIN;
   const search_domain m_search = ALL_DOMAIN;
+  const bool m_main = false;
 
   /* The list of CUs from the index entry of the symbol, or NULL if
      not found.  */
@@ -5677,6 +5680,7 @@ dw2_debug_names_iterator::next ()
     static_,
     extern_,
   } symbol_linkage_ = symbol_linkage::unknown;
+  bool is_main = false;
   dwarf2_per_cu_data *per_cu = NULL;
   for (const mapped_debug_names::index_val::attr &attr : indexval.attr_vec)
     {
@@ -5753,6 +5757,11 @@ dw2_debug_names_iterator::next ()
 	    break;
 	  symbol_linkage_ = symbol_linkage::extern_;
 	  break;
+	case DW_IDX_GNU_main_subprogram:
+	  if (!m_map.augmentation_is_gdb)
+	    break;
+	  is_main = true;
+	  break;
 	}
     }
 
@@ -5760,6 +5769,9 @@ dw2_debug_names_iterator::next ()
   if (m_per_objfile->symtab_set_p (per_cu))
     goto again;
 
+  if (m_main && !is_main)
+    goto again;
+
   /* Check static vs global.  */
   if (symbol_linkage_ != symbol_linkage::unknown && m_block_index.has_value ())
     {
@@ -5983,7 +5995,7 @@ dw2_debug_names_map_matching_symbols
       /* The name was matched, now expand corresponding CUs that were
 	 marked.  */
       dw2_debug_names_iterator iter (map, block_kind, domain, namei,
-				     per_objfile);
+				     per_objfile, false);
 
       struct dwarf2_per_cu_data *per_cu;
       while ((per_cu = iter.next ()) != NULL)
@@ -6209,6 +6221,26 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
     {
       *index_kind = dw_index_kind::DEBUG_NAMES;
       per_objfile->resize_symtabs ();
+
+      /* Set main name and language.  */
+      for (int i = 0; i < per_bfd->debug_names_table->name_count; ++i)
+	{
+	  const auto &map = *per_bfd->debug_names_table;
+
+	  dw2_debug_names_iterator iter (map, {}, VAR_DOMAIN, i, per_objfile,
+					 true);
+
+	  struct dwarf2_per_cu_data *per_cu;
+	  while ((per_cu = iter.next ()) != NULL)
+	    {
+	      /* Expand the whole CU, to trigger set_objfile_main_name.
+		 Ideally we'd only read the CU toplevel DIE here to get
+		 the language, and then call set_objfile_main_name
+		 directly.  */
+	      dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
+					       nullptr);
+	    }
+	}
       return true;
     }
 
diff --git a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
index c618a40f2fb..0651d483013 100644
--- a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
+++ b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
@@ -55,7 +55,7 @@ if {[prepare_for_testing "failed to prepare" ${testfile} \
     return -1
 }
 
-set have_index [exec_has_index_section $binfile]
+set have_gdb_index [exec_has_gdb_index_section $binfile]
 
 # Test that the "start" command stops in the "mymain" function.
 # This should happen because we used DW_AT_main_subprogram to tell gdb
@@ -71,7 +71,7 @@ gdb_test_multiple "" "stopped at mymain" {
 	pass $gdb_test_name
     }
     -re -wrap "Temporary breakpoint .* main.*" {
-	if { $have_index } {
+	if { $have_gdb_index } {
 	    setup_kfail "gdb/24549" *-*-*
 	}
 	fail $gdb_test_name
diff --git a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
index 6a249e60f2f..86ed4dac143 100644
--- a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
+++ b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
@@ -35,13 +35,13 @@ if {[prepare_for_testing_full "failed to prepare" \
     return -1
 }
 
-set have_index [exec_has_index_section $binfile]
+set have_gdb_index [exec_has_gdb_index_section $binfile]
 
 # Runs the test program and examins the stack.  LANG is a string, the
 # value to pass to GDB's 'set language ...' command.
 proc run_tests { lang } {
     with_test_prefix "lang=${lang}" {
-	global binfile hex have_index
+	global binfile hex have_gdb_index
 
 	clean_restart ${binfile}
 
@@ -86,7 +86,7 @@ proc run_tests { lang } {
 		pass $gdb_test_name
 	    }
 	    -re -wrap $bt_stack_kfail {
-		if { $have_index } {
+		if { $have_gdb_index } {
 		    setup_kfail "gdb/24549" *-*-*
 		}
 		fail $gdb_test_name
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 53ac9f1408c..15429420dcf 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5871,18 +5871,28 @@ proc rerun_to_main {} {
   }
 }
 
-# Return true if EXECUTABLE contains a .gdb_index or .debug_names index section.
-
-proc exec_has_index_section { executable } {
+# Return true if EXECUTABLE contains a section matching RE.
+proc exec_has_section { executable re } {
     set readelf_program [gdb_find_readelf]
     set res [catch {exec $readelf_program -S $executable \
-			| grep -E "\.gdb_index|\.debug_names" }]
+			| grep -E $re }]
     if { $res == 0 } {
 	return 1
     }
     return 0
 }
 
+# Return true if EXECUTABLE contains a .gdb_index index section.
+proc exec_has_gdb_index_section { executable } {
+    return [exec_has_section $executable "\.gdb_index"]
+}
+
+# Return true if EXECUTABLE contains a .gdb_index or .debug_names
+# index section.
+proc exec_has_index_section { executable } {
+    return [exec_has_section $executable "\.gdb_index|\.debug_names"]
+}
+
 # Return list with major and minor version of readelf, or an empty list.
 gdb_caching_proc readelf_version {
     set readelf_program [gdb_find_readelf]
diff --git a/include/dwarf2.def b/include/dwarf2.def
index 1ae6e1df298..36060e3fa73 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -804,6 +804,7 @@ DW_IDX_DUP (DW_IDX_lo_user, 0x2000)
 DW_IDX (DW_IDX_hi_user, 0x3fff)
 DW_IDX (DW_IDX_GNU_internal, 0x2000)
 DW_IDX (DW_IDX_GNU_external, 0x2001)
+DW_IDX (DW_IDX_GNU_main_subprogram, 0x2002)
 DW_END_IDX
 
 /* DWARF5 Unit type header encodings  */