public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/testsuite] Add KFAILs for PR symtab/24549
@ 2021-01-29 15:47 Tom de Vries
  2021-02-05 15:25 ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2021-01-29 15:47 UTC (permalink / raw)
  To: gdb-patches

Hi,

When an executable contains an index such as a .gdb_index or .debug_names
section, gdb ignores the DW_AT_subprogram attribute.

This problem has been filed as PR symtab/24549.

Add KFAILs for this PR in test-cases gdb.dwarf2/main-subprogram.exp and
gdb.fortran/mixed-lang-stack.exp.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Add KFAILs for PR symtab/24549

gdb/testsuite/ChangeLog:

2021-01-29  Tom de Vries  <tdevries@suse.de>

	* gdb.dwarf2/main-subprogram.exp: Add KFAIL for PR symtab/24549.
	* gdb.fortran/mixed-lang-stack.exp: Same.

---
 gdb/testsuite/gdb.dwarf2/main-subprogram.exp   | 14 +++++++++++++-
 gdb/testsuite/gdb.fortran/mixed-lang-stack.exp | 21 +++++++++++++++++++--
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
index ad0113e404d..c618a40f2fb 100644
--- a/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
+++ b/gdb/testsuite/gdb.dwarf2/main-subprogram.exp
@@ -55,6 +55,8 @@ if {[prepare_for_testing "failed to prepare" ${testfile} \
     return -1
 }
 
+set have_index [exec_has_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
 # that this was the real "main".
@@ -64,4 +66,14 @@ if {[gdb_start_cmd] < 0} {
     return -1
 }
 
-gdb_test "" "Temporary breakpoint .* mymain.*" "stopped at mymain"
+gdb_test_multiple "" "stopped at mymain" {
+    -re -wrap "Temporary breakpoint .* mymain.*" {
+	pass $gdb_test_name
+    }
+    -re -wrap "Temporary breakpoint .* main.*" {
+	if { $have_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 411fd784dc0..6a249e60f2f 100644
--- a/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
+++ b/gdb/testsuite/gdb.fortran/mixed-lang-stack.exp
@@ -35,11 +35,13 @@ if {[prepare_for_testing_full "failed to prepare" \
     return -1
 }
 
+set have_index [exec_has_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
+	global binfile hex have_index
 
 	clean_restart ${binfile}
 
@@ -74,7 +76,22 @@ proc run_tests { lang } {
 		 "#7\\s+$hex in mixed_func_1b \\($1b_args\\) at \[^\r\n\]+" \
 		 "#8\\s+$hex in mixed_func_1a \\(\\) at \[^\r\n\]+" \
 		 "#9\\s+$hex in mixed_stack_main \\(\\) at \[^\r\n\]+" ]
-	gdb_test "bt -frame-arguments all" $bt_stack
+	set main_args "argc=1, argv=${hex}( \[^\r\n\]+)?"
+	set bt_stack_kfail \
+	    [multi_line \
+		 $bt_stack \
+		 "#10\\s+$hex in main \\($main_args\\) at \[^\r\n\]+"]
+	gdb_test_multiple "bt -frame-arguments all" "" {
+	    -re -wrap $bt_stack {
+		pass $gdb_test_name
+	    }
+	    -re -wrap $bt_stack_kfail {
+		if { $have_index } {
+		    setup_kfail "gdb/24549" *-*-*
+		}
+		fail $gdb_test_name
+	    }
+	}
 
 	# Check the language for frame #0.
 	gdb_test "info frame" "source language fortran\..*" \

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb/testsuite] Add KFAILs for PR symtab/24549
  2021-01-29 15:47 [PATCH][gdb/testsuite] Add KFAILs for PR symtab/24549 Tom de Vries
@ 2021-02-05 15:25 ` Tom Tromey
  2021-02-05 17:28   ` Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2021-02-05 15:25 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

>>>>> "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.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb/testsuite] Add KFAILs for PR symtab/24549
  2021-02-05 15:25 ` Tom Tromey
@ 2021-02-05 17:28   ` Tom de Vries
  2021-02-08 11:46     ` [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2021-02-05 17:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

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?

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram
  2021-02-05 17:28   ` Tom de Vries
@ 2021-02-08 11:46     ` Tom de Vries
  2021-02-24 20:46       ` Tom Tromey
  2021-03-02  1:44       ` Tom Tromey
  0 siblings, 2 replies; 9+ messages in thread
From: Tom de Vries @ 2021-02-08 11:46 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Simon Marchi

[-- 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  */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram
  2021-02-08 11:46     ` [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram Tom de Vries
@ 2021-02-24 20:46       ` Tom Tromey
  2021-02-24 20:51         ` Tom Tromey
  2021-03-02  1:44       ` Tom Tromey
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2021-02-24 20:46 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Tom Tromey, gdb-patches

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

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

Nice idea.

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

It seems unfortunate to have to read most of the index entries in order
to find this at startup.  How much slowdown does it cause?

Tom> +      /* Set main name and language.  */
Tom> +      for (int i = 0; i < per_bfd->debug_names_table->name_count; ++i)
Tom> +	{
Tom> +	  const auto &map = *per_bfd->debug_names_table;

It seems like this could be hoisted out of the loop and then also used
in the loop header.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram
  2021-02-24 20:46       ` Tom Tromey
@ 2021-02-24 20:51         ` Tom Tromey
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2021-02-24 20:51 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom de Vries, gdb-patches

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

I forgot to mention, the new constant probably has to go to gcc first.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram
  2021-02-08 11:46     ` [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram Tom de Vries
  2021-02-24 20:46       ` Tom Tromey
@ 2021-03-02  1:44       ` Tom Tromey
  2021-03-02  6:49         ` Tom de Vries
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2021-03-02  1:44 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Tom Tromey, gdb-patches, Simon Marchi

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

I had a hilarious idea for this tonight, which is to stuff the main name
into the augmentation string.  Right now the augmentation string is just
"GDB", but we could do something like "GDB:name_of_main"; then extract
that when reading.

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram
  2021-03-02  1:44       ` Tom Tromey
@ 2021-03-02  6:49         ` Tom de Vries
  2021-03-02 15:01           ` Tom Tromey
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2021-03-02  6:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Simon Marchi

On 3/2/21 2:44 AM, Tom Tromey wrote:
> Tom> Fix this by adding a GNU extension DW_IDX_GNU_main_subprogram to the name
> Tom> index attributes, encoded using a number in the DW_IDX_lo_user-DW_IDX_hi_user
> Tom> range.
> 
> I had a hilarious idea for this tonight, which is to stuff the main name
> into the augmentation string.  Right now the augmentation string is just
> "GDB", but we could do something like "GDB:name_of_main"; then extract
> that when reading.

That's a fun idea indeed.

I wonder though about backward compatibility.

We have:
...
/* DWARF-5 augmentation string for GDB's DW_IDX_GNU_* extension.  */
static const gdb_byte dwarf5_augmentation[] = { 'G', 'D', 'B', 0 };

...

  /* augmentation_string_size - The size in bytes of the augmentation

     string.  This value is rounded up to a multiple of 4.  */
  uint32_t augmentation_string_size = read_4_bytes (abfd, addr);
  addr += 4;
  map.augmentation_is_gdb
    = ((augmentation_string_size
        == sizeof (dwarf5_augmentation))
       && memcmp (addr, dwarf5_augmentation,
                  sizeof (dwarf5_augmentation)) == 0);
...

So as soon as we use this idea, the augmentation_string_size will be
bigger, and the index will no longer be recognized by older gdb as being
a gdb index, so DW_IDX_GNU_internal/external will be ignored. [ There's
also a use in create_cus_from_debug_names_list, but that doesn't look
like any functionality will be broken. ]

One might say though that this is an actual gdb bug.  The dwarf standard
says that the "string _begins_ with a 4-character vendor ID".  So the
code should not check for augmentation_string_size == sizeof
(dwarf5_augmentation).

Anyway, to support any older reader that does the right thing, we'd need
to use "GDB\0name_of_main" or some such.

For future extensibility, we might consider encoding the type of data
using a char, like so "GDB\0M:name_of_main".

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram
  2021-03-02  6:49         ` Tom de Vries
@ 2021-03-02 15:01           ` Tom Tromey
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Tromey @ 2021-03-02 15:01 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Tom Tromey, gdb-patches, Simon Marchi

Tom> One might say though that this is an actual gdb bug.  The dwarf standard
Tom> says that the "string _begins_ with a 4-character vendor ID".  So the
Tom> code should not check for augmentation_string_size == sizeof
Tom> (dwarf5_augmentation).

Yes.  Also "GDB" isn't 4 characters.  However, GDB also puts the wrong
names into .debug_names, so the problems go much further than that.

Tom> Anyway, to support any older reader that does the right thing, we'd need
Tom> to use "GDB\0name_of_main" or some such.

Tom> For future extensibility, we might consider encoding the type of data
Tom> using a char, like so "GDB\0M:name_of_main".

Makes sense to me!

Tom

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-02 15:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29 15:47 [PATCH][gdb/testsuite] Add KFAILs for PR symtab/24549 Tom de Vries
2021-02-05 15:25 ` Tom Tromey
2021-02-05 17:28   ` Tom de Vries
2021-02-08 11:46     ` [RFC][gdb/symtab] Add DW_IDX_GNU_main_subprogram Tom de Vries
2021-02-24 20:46       ` Tom Tromey
2021-02-24 20:51         ` Tom Tromey
2021-03-02  1:44       ` Tom Tromey
2021-03-02  6:49         ` Tom de Vries
2021-03-02 15:01           ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).