public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [PATCH 13/13] [gdb/symtab] Add DW_TAG_inlined_subroutine entries in the cooked index for c++
Date: Mon,  2 Oct 2023 14:50:51 +0200	[thread overview]
Message-ID: <20231002125051.29911-14-tdevries@suse.de> (raw)
In-Reply-To: <20231002125051.29911-1-tdevries@suse.de>

When running test-case gdb.cp/breakpoint-locs.exp with target board
cc-with-dwz, we occasionally run into:
...
FAIL: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
...
due to the breakpoint having one instead of two locations.

By using "maint set worker-threads 0" we can make the test-case fail reliably.

In the failing case the inlined_subroutine DIE at 0x1cf has an incorrect
parent, and the one at 0x148 has the correct parent:
...
    [7] ((cooked_index_entry *) 0x7fdf800030b0)
    name:       baz
    canonical:  baz
    qualified:  N1::C1::baz
    DWARF tag:  DW_TAG_inlined_subroutine
    flags:      0x0 []
    DIE offset: 0x148
    parent:     ((cooked_index_entry *) 0x7fdf80002ed0) [C1]
  ...
    [20] ((cooked_index_entry *) 0x7fdf84005340)
    name:       baz
    canonical:  baz
    qualified:  baz
    DWARF tag:  DW_TAG_inlined_subroutine
    flags:      0x0 []
    DIE offset: 0x1cf
    parent:     ((cooked_index_entry *) 0)
...
and in the passing case it's the other way around.

The corresponding dwarf for 0x148 is this:
...
 <0><b>: Abbrev Number: 34 (DW_TAG_partial_unit)
 <1><14>: Abbrev Number: 37 (DW_TAG_namespace)
    <15>   DW_AT_name        : N1
 <2><19>: Abbrev Number: 36 (DW_TAG_class_type)
    <1a>   DW_AT_name        : C1
 <3><20>: Abbrev Number: 32 (DW_TAG_subprogram)
    <21>   DW_AT_external    : 1
    <21>   DW_AT_name        : baz
    <27>   DW_AT_linkage_name: _ZN2N12C13bazEv
    <2b>   DW_AT_accessibility: 1       (public)
    <2c>   DW_AT_declaration : 1
 <1><2e>: Abbrev Number: 35 (DW_TAG_subprogram)
    <2f>   DW_AT_specification: <0x20>
    <30>   DW_AT_inline      : 3        (declared as inline and inlined)
 <0><ee>: Abbrev Number: 16 (DW_TAG_compile_unit)
    <f3>   DW_AT_language    : 4        (C++)
    <f4>   DW_AT_name        : gdb.cp/breakpoint-locs.cc
 <1><13b>: Abbrev Number: 25 (DW_TAG_subprogram)
    <13c>   DW_AT_specification: <0x115>
    <13d>   DW_AT_low_pc      : 0x4004d7
    <145>   DW_AT_high_pc     : 16
    <146>   DW_AT_frame_base  : 1 byte block: 9c        (DW_OP_call_frame_cfa)
    <148>   DW_AT_GNU_all_call_sites: 1
 <2><148>: Abbrev Number: 24 (DW_TAG_inlined_subroutine)
    <149>   DW_AT_abstract_origin: <0x2e>
    <14d>   DW_AT_low_pc      : 0x4004db
    <155>   DW_AT_high_pc     : 9
    <156>   DW_AT_call_file   : 1
    <157>   DW_AT_call_line   : 22
...
and for 0x1cf it's similar but in a CU with name gdb.cp/breakpoint-locs-2.cc.

In the target board unix case, the test-case passes because both CUs are
expanded, because each CU contains the equivalent of 0x2e.

But in the dwz case, dwz factored out that DIE into the PU, so that mechanism
no longer works.

Note that when we used partial symtabs for dwarf we relied on the
DW_TAG_inlined_subroutine DIEs, see commit f9b5d5ea18a ("[gdb/symtab] Fix
missing breakpoint location for inlined function").  So lets fix this in the
same way, by adding DW_TAG_inlined_subroutine entries in the cooked index.

Currently the cooked index doesn't add an entry for it because in
cooked_indexer::index_dies we don't recurse into the children of a
DW_TAG_subprogram for c++:
...
	    case DW_TAG_subprogram:
	      if ((m_language == language_fortran
		   || m_language == language_ada)
		  && this_entry != nullptr)
		{
		  info_ptr = recurse (reader, info_ptr, this_entry, true);
...

Doing so yields entries with incorrect qualified names N1::foo::baz and
N1::bar::baz, due to an incorrect parent field.  Indeed in DIE terms the
parent is foo/bar, but we use the parent field to track encapsulating scopes,
so fix this by ignoring the actual DIE parent.

That gives us the qualified name baz in both cases.  Adding some debugging in
handle_deferred_dies shows us why:
...
  0x0000000000000015 0x7f9400002eb0 (0x14)^M
  0x000000000000001a 0x7f9400002ee0 (0x19)^M
  0x000000000000002d 0x7f9400002eb0 (0x14)^M
  0x000000000000002e 0x0^M
  0x000000000000002f 0x7f9400002f10 (0x2e)^M
  0x0000000000000039 0x0^M
  0x000000000000010f 0x7f9400002fd0 (0x10e)^M
  0x0000000000000121 0x0^M
  0x000000000000013c 0x7f9400003060 (0x13b)^M
  0x0000000000000148 0x2934520 (deferred)^M
  0x0000000000000149 0x7f9400003060 (0x13b)^M
  0x000000000000016d 0x0^M
  0x000000000000019a 0x7f94080052e0 (0x199)^M
  0x00000000000001ac 0x0^M
  0x00000000000001c3 0x7f9408005370 (0x1c2)^M
  0x00000000000001cf 0x2934520 (deferred)^M
  0x00000000000001d0 0x7f9408005370 (0x1c2)^M
  0x00000000000001f4 0x0^M
Resolve deferred: 0x148 -> 0x2e: no parent^M
Resolve deferred: 0x1cf -> 0x2e: no parent^M
...

We try to find the find the parent of 0x148, by looking up the parent of 0x2e
in m_die_range_map, which is nullptr, in other words no parent.

Fix this by recording more parent relations in m_die_range_map, such that we
have:
...
  0x0000000000000015 0x7fc980002eb0 (0x14)^M
  0x000000000000001a 0x7fc980002ee0 (0x19)^M
  0x000000000000002d 0x7fc980002eb0 (0x14)^M
  0x000000000000002e 0x7fc980002ee0 (0x19)^M
  0x000000000000002f 0x7fc980002f10 (0x2e)^M
  0x0000000000000039 0x0^M
  0x000000000000010f 0x7fc980002fd0 (0x10e)^M
  0x0000000000000121 0x0^M
  0x000000000000013b 0x7fc980002fd0 (0x10e)^M
  0x000000000000013c 0x7fc980003060 (0x13b)^M
  0x0000000000000148 0x2934520 (deferred)^M
  0x0000000000000149 0x7fc980003060 (0x13b)^M
  0x000000000000016d 0x0^M
  0x000000000000019a 0x7fc978002eb0 (0x199)^M
  0x00000000000001ac 0x0^M
  0x00000000000001c2 0x7fc978002eb0 (0x199)^M
  0x00000000000001c3 0x7fc978002f40 (0x1c2)^M
  0x00000000000001cf 0x2934520 (deferred)^M
  0x00000000000001d0 0x7fc978002f40 (0x1c2)^M
  0x00000000000001f4 0x0^M
Resolve deferred: 0x148 -> 0x2e: 0x19^M
Resolve deferred: 0x1cf -> 0x2e: 0x19^M
...

This gives us the desired outcome of N1::C1::baz for both DIEs, and makes the
test-case pass.

Tested on x86_64-linux.
---
 gdb/dwarf2/read.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6f218f172a9..a770282c6ae 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16606,6 +16606,8 @@ cooked_indexer::index_dies (cutu_reader *reader,
       cooked_index_flag flags = IS_STATIC;
       sect_offset sibling {};
       const cooked_index_entry *this_parent_entry = parent_entry;
+      if (abbrev->tag == DW_TAG_inlined_subroutine)
+	this_parent_entry = nullptr;
       info_ptr = scan_attributes (reader->cu->per_cu, reader, info_ptr,
 				  info_ptr, abbrev, &name, &linkage_name,
 				  &flags, &sibling, &this_parent_entry,
@@ -16635,9 +16637,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
 		});
 	    }
 	  else
-	    this_entry = m_index_storage->add (this_die, abbrev->tag, flags,
-					       name, this_parent_entry,
-					       m_per_cu);
+	    {
+	      CORE_ADDR addr = form_addr (this_die, reader->cu->per_cu->is_dwz,
+					  reader->cu->per_cu->is_debug_types);
+	      set_parent (addr, addr, this_parent_entry);
+	      this_entry = m_index_storage->add (this_die, abbrev->tag, flags,
+						 name, this_parent_entry,
+						 m_per_cu);
+	    }
 	}
 
       if (linkage_name != nullptr)
@@ -16698,7 +16705,8 @@ cooked_indexer::index_dies (cutu_reader *reader,
 
 	    case DW_TAG_subprogram:
 	      if ((m_language == language_fortran
-		   || m_language == language_ada)
+		   || m_language == language_ada
+		   || m_language == language_cplus)
 		  && this_entry != nullptr)
 		{
 		  info_ptr = recurse (reader, info_ptr, this_entry, true);
-- 
2.35.3


  parent reply	other threads:[~2023-10-02 12:50 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-02 12:50 [PATCH 00/13] [gdb/symtab, cc-with-dwz] Fix gdb.cp/breakpoint-locs.exp Tom de Vries
2023-10-02 12:50 ` [PATCH 01/13] [gdb/symtab] Factor out m_die_range_map usage Tom de Vries
2023-10-09 20:33   ` Alexandra Petlanova Hajkova
2023-10-09 21:43     ` Tom de Vries
2023-10-20 19:50   ` Tom Tromey
2023-11-30 12:55     ` Tom de Vries
2023-10-02 12:50 ` [PATCH 02/13] [gdb/symtab] Check effect in parent_map::set_parent Tom de Vries
2023-10-20 19:51   ` Tom Tromey
2023-11-30 14:05     ` Tom de Vries
2023-10-02 12:50 ` [PATCH 03/13] [gdb/symtab] Handle nullptr parent " Tom de Vries
2023-10-02 12:50 ` [PATCH 04/13] [gdb/symtab] Add parent_map::dump Tom de Vries
2023-10-10  9:05   ` Alexandra Petlanova Hajkova
2023-10-20 19:51   ` Tom Tromey
2023-10-02 12:50 ` [PATCH 05/13] [gdb/symtab] Factor out m_deferred_entries usage Tom de Vries
2023-10-02 12:50 ` [PATCH 06/13] [gdb/symtab] Add debug_handle_deferred_entries Tom de Vries
2023-10-02 12:50 ` [PATCH 07/13] [gdb/symtab] Resolve deferred entries, inter-shard case Tom de Vries
2023-10-20 20:11   ` Tom Tromey
2023-12-10  8:10   ` Tom de Vries
2023-10-02 12:50 ` [PATCH 08/13] [gdb/testsuite] Add gdb.dwarf2/forward-spec-inter-cu.exp Tom de Vries
2023-10-02 12:50 ` [PATCH 09/13] [gdb/symtab] Keep track of processed DIEs in shard Tom de Vries
2023-10-22 11:00   ` Alexandra Petlanova Hajkova
2023-12-08 12:09   ` Tom de Vries
2023-10-02 12:50 ` [PATCH 10/13] [gdb/symtab] Resolve deferred entries, intra-shard case Tom de Vries
2023-10-02 12:50 ` [PATCH 11/13] [gdb/symtab] Don't defer backward refs, inter-cu " Tom de Vries
2023-10-02 12:50 ` [PATCH 12/13] [gdb/testsuite] Add gdb.dwarf2/backward-spec-inter-cu.exp Tom de Vries
2023-10-02 12:50 ` Tom de Vries [this message]
2023-10-16 11:40 ` [PING][PATCH 00/13] [gdb/symtab, cc-with-dwz] Fix gdb.cp/breakpoint-locs.exp Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231002125051.29911-14-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).