diff mbox series

[v2,1/2,gdb/symtab] Fix parent of enumerator

Message ID 20240926130509.7960-1-tdevries@suse.de
State New
Headers
Series [v2,1/2,gdb/symtab] Fix parent of enumerator |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom de Vries Sept. 26, 2024, 1:05 p.m. UTC 
  As mentioned in commit 489b82720f5 ('[gdb/symtab] Revert "Change handling of
DW_TAG_enumeration_type in DWARF scanner"'), when doing "maint print objfiles" in
test-case gdb.dwarf2/enum-type.exp, for val1 we get an entry without parent:
...
    [27] ((cooked_index_entry *) 0x7fbbb4002ef0)
    name:       val1
    canonical:  val1
    qualified:  val1
    DWARF tag:  DW_TAG_enumerator
    flags:      0x0 []
    DIE offset: 0x124
    parent:     ((cooked_index_entry *) 0)
...

This happens here in cooked_indexer::index_dies:
...
	      info_ptr = recurse (reader, info_ptr,
				  is_enum_class ? this_entry : parent_entry,
				  fully);
...
when we're passing down a nullptr parent_entry, while the parent of this_entry
is deferred.

Fix this in cooked_indexer::index_dies by passing down a deffered parent
instead, such that we get:
...
    [27] ((cooked_index_entry *) 0x7ff0e4002ef0)^M
    name:       val1^M
    canonical:  val1^M
    qualified:  ns::val1^M
    DWARF tag:  DW_TAG_enumerator^M
    flags:      0x0 []^M
    DIE offset: 0x124^M
    parent:     ((cooked_index_entry *) 0x7ff0e4002f20) [ns]^M
...

Tested on x86_64-linux.
---
 gdb/dwarf2/read.c                      | 46 ++++++++++++++++++++------
 gdb/testsuite/gdb.dwarf2/enum-type.exp | 15 +++++++++
 2 files changed, 51 insertions(+), 10 deletions(-)


base-commit: 174e5e38b92c2cd381feffa644de2b12ce0980a8

Comments

Tom Tromey Sept. 27, 2024, 6:24 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> +  if (parent.index () != 0)
Tom> +    return info_ptr;
Tom> +  const cooked_index_entry *parent_entry = std::get<0> (parent);

I think these spots would be nicer if checking was done with
std::holds_alternative and fetching was done with the type-based
std::get (e.g. std::get<const cooked_index_entry *>)

thanks,
Tom
Tom de Vries Sept. 28, 2024, 6:39 a.m. UTC | #2 
On 9/27/24 20:24, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> +  if (parent.index () != 0)
> Tom> +    return info_ptr;
> Tom> +  const cooked_index_entry *parent_entry = std::get<0> (parent);
> 
> I think these spots would be nicer if checking was done with
> std::holds_alternative and fetching was done with the type-based
> std::get (e.g. std::get<const cooked_index_entry *>)

Hi Tom,

thanks for the reviews.

I'll fix this and push the series after I come back from my vacation in 
a week.

Can I add an approved-by/reviewed-by tag from you, or did you have other 
comments?

Thanks,
- Tom
Tom Tromey Sept. 28, 2024, 3 p.m. UTC | #3 
Tom> Can I add an approved-by/reviewed-by tag from you, or did you have
Tom> other comments?

Yeah, that's fine, sorry I didn't do it the first time around.  Thanks.

Tom
diff mbox series

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 193a52ef6e0..1a386b8d542 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -96,6 +96,7 @@ 
 #include "run-on-main-thread.h"
 #include "dwarf2/parent-map.h"
 #include "dwarf2/error.h"
+#include <variant>
 
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
@@ -4486,7 +4487,7 @@  class cooked_indexer
 				 bool is_dwz,
 				 bool for_scanning);
 
-  /* Index DIEs in the READER starting at INFO_PTR.  PARENT_ENTRY is
+  /* Index DIEs in the READER starting at INFO_PTR.  PARENT is
      the entry for the enclosing scope (nullptr at top level).  FULLY
      is true when a full scan must be done -- in some languages,
      function scopes must be fully explored in order to find nested
@@ -4494,7 +4495,8 @@  class cooked_indexer
      reading stopped.  */
   const gdb_byte *index_dies (cutu_reader *reader,
 			      const gdb_byte *info_ptr,
-			      const cooked_index_entry *parent_entry,
+			      std::variant<const cooked_index_entry *,
+					   parent_map::addr_type> parent,
 			      bool fully);
 
   /* Scan the attributes for a given DIE and update the out
@@ -4524,7 +4526,8 @@  class cooked_indexer
      m_die_range_map and then calling index_dies.  */
   const gdb_byte *recurse (cutu_reader *reader,
 			   const gdb_byte *info_ptr,
-			   const cooked_index_entry *parent_entry,
+			   std::variant<const cooked_index_entry *,
+					parent_map::addr_type> parent_entry,
 			   bool fully);
 
   /* The storage object, where the results are kept.  */
@@ -16417,10 +16420,15 @@  cooked_indexer::index_imported_unit (cutu_reader *reader,
 const gdb_byte *
 cooked_indexer::recurse (cutu_reader *reader,
 			 const gdb_byte *info_ptr,
-			 const cooked_index_entry *parent_entry,
+			 std::variant<const cooked_index_entry *,
+				      parent_map::addr_type> parent,
 			 bool fully)
 {
-  info_ptr = index_dies (reader, info_ptr, parent_entry, fully);
+  info_ptr = index_dies (reader, info_ptr, parent, fully);
+
+  if (parent.index () != 0)
+    return info_ptr;
+  const cooked_index_entry *parent_entry = std::get<0> (parent);
 
   if (parent_entry != nullptr)
     {
@@ -16441,7 +16449,8 @@  cooked_indexer::recurse (cutu_reader *reader,
 const gdb_byte *
 cooked_indexer::index_dies (cutu_reader *reader,
 			    const gdb_byte *info_ptr,
-			    const cooked_index_entry *parent_entry,
+			    std::variant<const cooked_index_entry *,
+					 parent_map::addr_type> parent,
 			    bool fully)
 {
   const gdb_byte *end_ptr = (reader->buffer
@@ -16468,15 +16477,20 @@  cooked_indexer::index_dies (cutu_reader *reader,
 	{
 	  info_ptr = skip_one_die (reader, info_ptr, abbrev, !fully);
 	  if (fully && abbrev->has_children)
-	    info_ptr = index_dies (reader, info_ptr, parent_entry, fully);
+	    info_ptr = index_dies (reader, info_ptr, parent, fully);
 	  continue;
 	}
 
       const char *name = nullptr;
       const char *linkage_name = nullptr;
       parent_map::addr_type defer {};
+      if (parent.index () == 1)
+	defer = std::get<1> (parent);
       cooked_index_flag flags = IS_STATIC;
       sect_offset sibling {};
+      const cooked_index_entry *parent_entry = nullptr;
+      if (parent.index () == 0)
+	parent_entry = std::get<0> (parent);
       const cooked_index_entry *this_parent_entry = parent_entry;
       bool is_enum_class = false;
 
@@ -16560,9 +16574,21 @@  cooked_indexer::index_dies (cutu_reader *reader,
 		 the enum itself as the parent, yielding names like
 		 "enum_class::enumerator"; otherwise we inject the
 		 names into our own parent scope.  */
-	      info_ptr = recurse (reader, info_ptr,
-				  is_enum_class ? this_entry : parent_entry,
-				  fully);
+	      {
+		std::variant<const cooked_index_entry *,
+			     parent_map::addr_type> recurse_parent;
+		if (is_enum_class)
+		  {
+		    gdb_assert (this_entry != nullptr);
+		    recurse_parent = this_entry;
+		  }
+		else if (defer != 0)
+		  recurse_parent = defer;
+		else
+		  recurse_parent = parent_entry;
+
+		info_ptr = recurse (reader, info_ptr, recurse_parent, fully);
+	      }
 	      continue;
 
 	    case DW_TAG_module:
diff --git a/gdb/testsuite/gdb.dwarf2/enum-type.exp b/gdb/testsuite/gdb.dwarf2/enum-type.exp
index b2b3dc6c828..ec12db57c8c 100644
--- a/gdb/testsuite/gdb.dwarf2/enum-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/enum-type.exp
@@ -114,3 +114,18 @@  gdb_test "ptype enum EU" "type = enum EU {TWO = 2}" \
 gdb_test_no_output "set lang c++"
 gdb_test "ptype enum EU" "type = enum EU : unsigned int {TWO = 2}" \
     "ptype EU in C++"
+
+gdb_test "p ns::val1" \
+    " = ns::val1"
+
+require !readnow
+require {string equal [have_index $binfile] ""}
+
+set re_ws "\[ \t\]"
+
+gdb_test_lines "maint print objfiles" \
+    "val1 has a parent" \
+    [multi_line \
+	 "" \
+	 "$re_ws+qualified:$re_ws+ns::val1" \
+	 ".*"]