On Tue, 20 Aug 2024, Bernd Edlinger wrote:
> On 8/20/24 13:00, Richard Biener wrote:
> > On Fri, Aug 16, 2024 at 12:49 PM Bernd Edlinger
> > <[email protected]> wrote:
> >>
> >> While this already works correctly for the case when an inlined
> >> subroutine contains only one subrange, a redundant DW_TAG_lexical_block
> >> is still emitted when the subroutine has multiple blocks.
> >
> > Huh. The point is that the inline context is a single scope block with no
> > siblings - how did that get messed up? The patch unfortunately does not
> > contain a testcase.
> >
>
> Well, I became aware of this because I am working on a gdb patch,
> which improves the debug experience of optimized C code, and to my surprise
> the test case did not work with gcc-8, while gcc-9 and following were fine.
> Initially I did not see what is wrong, therefore I started to bisect when
> this changed, and so I found your patch, which removed some lexical blocks
> in the debug info of this gdb test case:
>
> from binutils-gdb/gdb/testsuite/gdb.cp/step-and-next-inline.cc
> in case you have the binutils-gdb already downloaded you can skip this:
> $ git clone git://sourceware.org/git/binutils-gdb
> $ cd binutils-gdb/gdb/testsuite/gdb.cp
> $ gcc -g -O2 step-and-next-inline.cc
>
> when you look at the debug info with readelf -w a.out
> you will see, that the function "tree_check"
> is inlined three times, one looks like this
> <2><86b>: Abbrev Number: 40 (DW_TAG_inlined_subroutine)
> <86c> DW_AT_abstract_origin: <0x95b>
> <870> DW_AT_entry_pc : 0x1175
> <878> DW_AT_GNU_entry_view: 0
> <879> DW_AT_ranges : 0x21
> <87d> DW_AT_call_file : 1
> <87e> DW_AT_call_line : 52
> <87f> DW_AT_call_column : 10
> <880> DW_AT_sibling : <0x8bf>
> <3><884>: Abbrev Number: 8 (DW_TAG_formal_parameter)
> <885> DW_AT_abstract_origin: <0x974>
> <889> DW_AT_location : 0x37 (location list)
> <88d> DW_AT_GNU_locviews: 0x35
> <3><891>: Abbrev Number: 8 (DW_TAG_formal_parameter)
> <892> DW_AT_abstract_origin: <0x96c>
> <896> DW_AT_location : 0x47 (location list)
> <89a> DW_AT_GNU_locviews: 0x45
> <3><89e>: Abbrev Number: 41 (DW_TAG_lexical_block)
> <89f> DW_AT_ranges : 0x21
>
> see the lexical block has the same DW_AT_ranges, as the
> inlined subroutine, but the other invocations do not
> have this lexical block, since your original fix removed
> those.
> And this lexical block triggered an unexpected issue
> in my gdb patch, which I owe you one, for helping me
> finding it :-)
>
> Before that I have never looked at these lexical blocks,
> but all I can say is that while compiling this test case,
> in the first invocation of gen_inlined_subroutine_die
> there are several SUBBLOCKS linked via BLOCK_CHAIN,
> and only the first one is used to emit the lexical_block,
> while the other siblings must be fully decoded, otherwise
> there is an internal error, that I found by try-and-error.
> I thought that is since the subroutine is split over several
> places, and therefore it appeared natural to me, that the
> subroutine is also using several SUBBLOCKS.
OK, so the case in question looks like
{ Scope block #8 step-and-next-inline.cc:52 Originating from : static
struct tree * tree_check (struct tree *, int); Fragment chain : #16 #17
struct tree * t;
int i;
{ Scope block #9 Originating from :#0 Fragment chain : #10 #11
struct tree * x;
}
{ Scope block #10 Originating from :#0 Fragment of : #9
struct tree * x;
}
{ Scope block #11 Originating from :#0 Fragment of : #9
struct tree * x;
}
}
so we have fragments here which we should ignore, but then fragments
are to collect multiple ranges which, when we do not emit a
lexical block for block #9 above, we will likely fail to emit and
which we instead should associate with block #8, the
DW_TAG_inlined_subroutine.
Somehow it seems to "work" as to associate DW_AT_ranges with the
DW_TAG_inlined_subroutine.
I've used the following - there's no need to process BLOCK_CHAIN
as fragments are ignored by gen_block_die.
diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
index d5144714c6e..4e6ad2ab7e1 100644
--- a/gcc/dwarf2out.cc
+++ b/gcc/dwarf2out.cc
@@ -25194,8 +25194,13 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref
context_die)
Do that by doing the recursion to subblocks on the single subblock
of STMT. */
bool unwrap_one = false;
- if (BLOCK_SUBBLOCKS (stmt) && !BLOCK_CHAIN (BLOCK_SUBBLOCKS (stmt)))
+ if (BLOCK_SUBBLOCKS (stmt))
{
+ tree subblock = BLOCK_SUBBLOCKS (stmt);
+ /* We should never elide that BLOCK, but we may have multiple
fragments.
+ Assert that there's only a single real inline-scope block. */
+ for (tree next = BLOCK_CHAIN (subblock); next; next = BLOCK_CHAIN
(next))
+ gcc_checking_assert (BLOCK_FRAGMENT_ORIGIN (next) == subblock);
tree origin = block_ultimate_origin (BLOCK_SUBBLOCKS (stmt));
if (origin
&& TREE_CODE (origin) == BLOCK
I'm quite sure this will blow up, so the appropriate thing would be
to only unwrap the block if the assertion would hold.
I'm testing the above.
Richard.
>
> Thanks
> Bernd.
>
> > Richard.
> >
> >> Fixes: ac02e5b75451 ("re PR debug/37801 (DWARF output for inlined functions
> >> doesn't always use DW_TAG_inlined_subroutine)")
> >>
> >> gcc/ChangeLog:
> >>
> >> PR debug/87440
> >> * dwarf2out.cc (gen_inlined_subroutine_die): Handle the case
> >> of multiple subranges correctly.
> >> ---
> >> some more context is here:
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87440#c5
> >> Bootstrapped and regression-tested on x86_64-pc-linux-gnu, OK for trunk?
> >>
> >> gcc/dwarf2out.cc | 11 ++++++++---
> >> 1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
> >> index 357efaa5990..346feeb53c8 100644
> >> --- a/gcc/dwarf2out.cc
> >> +++ b/gcc/dwarf2out.cc
> >> @@ -25171,9 +25171,10 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref
> >> context_die)
> >> Do that by doing the recursion to subblocks on the single subblock
> >> of STMT. */
> >> bool unwrap_one = false;
> >> - if (BLOCK_SUBBLOCKS (stmt) && !BLOCK_CHAIN (BLOCK_SUBBLOCKS (stmt)))
> >> + tree sub = BLOCK_SUBBLOCKS (stmt);
> >> + if (sub)
> >> {
> >> - tree origin = block_ultimate_origin (BLOCK_SUBBLOCKS (stmt));
> >> + tree origin = block_ultimate_origin (sub);
> >> if (origin
> >> && TREE_CODE (origin) == BLOCK
> >> && BLOCK_SUPERCONTEXT (origin) == decl)
> >> @@ -25181,7 +25182,11 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref
> >> context_die)
> >> }
> >> decls_for_scope (stmt, subr_die, !unwrap_one);
> >> if (unwrap_one)
> >> - decls_for_scope (BLOCK_SUBBLOCKS (stmt), subr_die);
> >> + {
> >> + decls_for_scope (sub, subr_die);
> >> + for (sub = BLOCK_CHAIN (sub); sub; sub = BLOCK_CHAIN (sub))
> >> + gen_block_die (sub, subr_die);
> >> + }
> >> }
> >>
> >> /* Generate a DIE for a field in a record, or structure. CTX is
> >> required: see
> >> --
> >> 2.39.2
> >>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
