On Wed, 21 Aug 2024, Bernd Edlinger wrote:
> On 8/21/24 10:45, Richard Biener wrote:
> > 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
> >>>> <[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.
> >
> > 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?
> >
>
> Ah, okay,
>
> so in terms of my current patch, you mean:
>
> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
> index 79f97b5a55e..c30dad1513a 100644
> --- a/gcc/dwarf2out.cc
> +++ b/gcc/dwarf2out.cc
> @@ -25183,6 +25183,10 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref
> context_die)
> && TREE_CODE (origin) == BLOCK
> && BLOCK_SUPERCONTEXT (origin) == decl)
> unwrap_one = true;
> + for (tree next = BLOCK_CHAIN (sub); unwrap_one && next;
> + next = BLOCK_CHAIN (next))
> + if (BLOCK_FRAGMENT_ORIGIN (next) != sub)
> + unwrap_one = false;
> }
> decls_for_scope (stmt, subr_die, !unwrap_one);
> if (unwrap_one)
Yep, that's what I had in mind.
> I will bootstrap this and post a new patch tomorrow.
Can you try adding a testcase like the one I added for
debug/37801 (gcc.dg/debug/dwarf2/inline4.c)?
Thanks,
Richard.
>
> Thanks
> Bernd.
>
> > 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 <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
