On Wed, 21 Aug 2024, Richard Biener wrote:
> 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
> > > <bernd.edlin...@hotmail.de> 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.
ICEs for example c-c++-common/torture/complex-sign-mixed-div.c
and gcc.dg/torture/pr50823.c. The latter has
{ Scope block #24
/space/rguenther/src/gcc/gcc/testsuite/gcc.dg/torture/pr50823.c:25
Originating from : static void emit_pattern_after_noloc (int (*<T3dc>)
(void)); Fragment chain : #31
int (*<T3dc>) (void) make_raw;
{ Scope block #25
/space/rguenther/src/gcc/gcc/testsuite/gcc.dg/torture/pr50823.c:29
Originating from : static int make_insn_raw (void); Fragment chain : #28
{ Scope block #26 Originating from :#0 Fragment chain : #27
}
{ Scope block #27 Originating from :#0 Fragment of : #26
}
}
{ Scope block #28
/space/rguenther/src/gcc/gcc/testsuite/gcc.dg/torture/pr50823.c:29
Originating from : static int make_insn_raw (void); Fragment of : #25
}
{ Scope block #29
/space/rguenther/src/gcc/gcc/testsuite/gcc.dg/torture/pr50823.c:30
Originating from : static void add_insn_after (void);
{ Scope block #30 Originating from :#0
}
}
}
so that's for a function without any local vars, it does seem we
have to check explicitly for the case with "proper" fragments and
otherwise not unwrap.
Can you update your patch accordingly?
Richard.
> 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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
