Re: [PATCH] debug/126355 - preserve debug info for IPA ICF wrappers

Thanks for the approval, and apologies for the delayed follow-up.

I have rechecked the original patch against current trunk; it still applies
unchanged.  I also bootstrapped and regression-tested it on
x86_64-pc-linux-gnu (C and C++ only), with no new failures.  The new test
fails without the patch and passes with it.

I do not currently have commit access.  Could someone please commit this
approved patch for me?

I am continuing to work on LTO-related debug issues and have several related
problems under investigation.  If you think it would be appropriate at this
stage, Richard, would you be willing to sponsor me for write-after-approval
access?

Richard Biener <[email protected]> 于2026年7月29日周三 20:17写道：

> On Sat, 25 Jul 2026, Longjun Luo wrote:
>
> > I tested the global-removal variant based on trunk commit
> > 5453f858146 on x86_64-pc-linux-gnu.
> >
> > The bootstrap completed successfully, including the stage2/stage3
> > comparison, and the new PR126355 testcase passed. However, the full
> > regression test exposed ICEs in the following existing tests with
> > -O3 -g:
> >
> > g++.dg/torture/covariant-1.C
> > g++.dg/torture/pr45699.C
> > g++.dg/torture/pr46287.C
> > g++.dg/torture/pr48661.C
> > g++.dg/torture/pr58585.C
> > g++.dg/torture/pr59226.C
> > g++.dg/torture/pr94202.C
> >
> > They all fail at the same assertion:
> >
> > old_die = lookup_decl_die (decl);
> > gcc_assert (old_die != NULL);
> >
> > in dwarf2out_abstract_function. A representative backtrace is:
> >
> > dwarf2out_abstract_function
> > tree_function_versioning
> > save_inline_function_body
> > inline_transform
> >
> > These cases involve C++ thunks expanded through the inline path. Without
> > the DECL_IGNORED_P setting, final debug processing reaches declarations
> > which do not have an early DIE, and lookup_decl_die returns NULL.
> >
> > Thus, although g++.dg/other/pr87574.C itself no longer reproduces its
> > original ICE on current trunk, it is not a sufficient guard for the
> > underlying condition. At least some forced GIMPLE thunks expanded through
> > the inline path still rely on DECL_IGNORED_P.
>
> Thanks for investigating.
>
> > I therefore will not post the global-removal variant as v2. The
> > caller-specific save/restore in cgraph_node::create_wrapper remains the
> > narrower fix: it bypasses the protection only for an ICF wrapper which
> > reuses the original source declaration and its early DIE, while leaving
> > late-created thunks in the other callers unchanged.
> >
> > Unless there is a better way to distinguish forced thunks which already
> > have a usable early DIE, I suggest keeping the ICF-local approach.
>
> I agree, so the original patch is OK.
>
> Thanks,
> Richard.
>
> > Richard Biener <[email protected]> 于2026年7月24日周五 15:56写道：
> >
> > > On Thu, 23 Jul 2026, Longjun Luo wrote:
> > >
> > > > Thanks. I traced the setting back to PR middle-end/87574.
> > > >
> > > > r264943 removed the previously unconditional DECL_IGNORED_P setting
> from
> > > > expand_thunk, which exposed the DWARF ICE reported as PR87574.
> r265028
> > > > (Eric Botcazou) then restored it conditionally for forced GIMPLE
> thunks.
> > > >
> > > > For the record, the PR87574 reproducer itself is not LTO-related: it
> is a
> > > > plain "-O2 -g" ICE in add_data_member_location_attribute
> > > > (dwarf2out.c:19226), and its testcase g++.dg/other/pr87574.C is a
> > > > { dg-do compile }, so compiling it exercises the original failure
> mode
> > > > directly.
> > > >
> > > > There are in fact three callers of
> > > > expand_thunk (..., /*force_gimple_thunk=*/true): inline_call
> > > > (ipa-inline-transform.cc), the ICF wrapper
> > > > (cgraph_node::create_wrapper), and coverage instrumentation
> > > > (tree-profile.cc).
> > > >
> > > > I built an x86_64-pc-linux-gnu compiler from trunk commit 5453f858146
> > > > with only the conditional DECL_IGNORED_P setting removed, and ran
> these
> > > > targeted compile checks:
> > > >
> > > > - g++.dg/other/pr87574.C compiled with -O2 -g and no ICE; an
> additional
> > > > IPA inline dump showed the thunk expanded through inline_call;
> > > > - the new PR126355 test source compiled with the test options; the
> ICF
> > > > dump showed that the wrapper was created, and the assembly showed an
> > > > address-bearing DIE for pr_icf_wrapper_b;
> > > > - g++.dg/tree-prof/devirt.C compiled to assembly with
> > > > -O3 -g -fprofile-generate -fno-profile-values
> > > > -fdump-ipa-cgraph-details -S -dA, and the generated thunks contained
> > > > their __gcov counters, exercising the tree-profile.cc path.
> > > >
> > > > Thus, the original PR87574 failure no longer reproduces on this trunk
> > > > checkout with the setting removed, and g++.dg/other/pr87574.C
> remains as
> > > > a guard should it ever return.
> > > >
> > > > Given that, removing the setting globally looks worth testing as the
> > > > simpler fix. It addresses the common setting in expand_thunk rather
> than
> > > > undoing it only in the ICF caller. It also changes debug emission
> for the
> > > > inline_call and coverage paths, however, so I will do a full
> bootstrap
> > > and
> > > > regtest, with particular attention to the LTO, debug, guality,
> dwarf2,
> > > and
> > > > tree-prof suites, before posting such a revision.
> > > >
> > > > Would you prefer that global removal, or the more conservative
> > > > caller-specific patch as posted? I can go either way; the ICF-only
> > > version
> > > > remains a fallback if the removal turns up any fallout.
> > >
> > > I prefer the global removal, but I CCed Eric in case he remembers
> > > details and can argue the actual issue still exists.
> > >
> > > Thanks,
> > > Richard.
> > >
> > > > Richard Biener <[email protected]> 于2026年7月23日周四 16:38写道：
> > > >
> > > > > On Thu, 23 Jul 2026, Longjun Luo wrote:
> > > > >
> > > > > > When IPA ICF keeps a distinct symbol for an address-taken
> function by
> > > > > > replacing its body with a wrapper, cgraph_node::create_wrapper
> > > reuses the
> > > > > > original function declaration and its early debug DIE.
> > > > > >
> > > > > > create_wrapper then calls expand_thunk with force_gimple_thunk
> set.
> > > > > Forced
> > > > > > GIMPLE thunks are normally created after early debug, so
> expand_thunk
> > > > > marks
> > > > > > their declarations ignored.  For an ICF wrapper this also
> prevents
> > > final
> > > > > > debug emission from attaching the wrapper address to the existing
> > > DIE.
> > > > > > The wrapper consequently has code and an STT_FUNC symbol but no
> > > > > > address-bearing DW_TAG_subprogram DIE.
> > > > > >
> > > > > > Preserve the declaration's original DECL_IGNORED_P value across
> > > forced
> > > > > > GIMPLE thunk expansion in create_wrapper.  Declarations that were
> > > already
> > > > > > ignored remain ignored, while an original source declaration can
> > > receive
> > > > > > its final address information.
> > > > > >
> > > > > > Tested on x86_64-pc-linux-gnu with the new gcc.dg/debug/dwarf2
> > > test.  The
> > > > > > test fails before the change and passes afterwards.
> > > > >
> > > > > There's two calls to expand_thunk (..., true), one from ICF as you
> > > > > figured and one from inline_call ().  I wonder if it's better to
> > > > > remove
> > > > >
> > > > >       /* We need to force DECL_IGNORED_P when the thunk is created
> > > > >          after early debug was run.  */
> > > > >       if (force_gimple_thunk)
> > > > >         DECL_IGNORED_P (thunk_fndecl) = 1;
> > > > >
> > > > > or figure why it was put there and see if the reason still holds
> > > > > (sounds LTO-ish).
> > > > >
> > > > > Richard.
> > > > >
> > > > > >       PR debug/126355
> > > > > >
> > > > > > gcc/ChangeLog:
> > > > > >
> > > > > >       * cgraphunit.cc (cgraph_node::create_wrapper): Preserve
> > > > > >       DECL_IGNORED_P across forced GIMPLE thunk expansion.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > >
> > > > > >       * gcc.dg/debug/dwarf2/pr126355.c: New test.
> > > > > >
> > > > > > Signed-off-by: Longjun Luo <[email protected]>
> > > > > > ---
> > > > > >  gcc/cgraphunit.cc                            |  5 +++
> > > > > >  gcc/testsuite/gcc.dg/debug/dwarf2/pr126355.c | 35
> > > ++++++++++++++++++++
> > > > > >  2 files changed, 40 insertions(+)
> > > > > >  create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2/pr126355.c
> > > > > >
> > > > > > diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
> > > > > > index 67aaebf108b..d397c9ae4cc 100644
> > > > > > --- a/gcc/cgraphunit.cc
> > > > > > +++ b/gcc/cgraphunit.cc
> > > > > > @@ -2683,7 +2683,12 @@ cgraph_node::create_wrapper (cgraph_node
> > > *target)
> > > > > >        arguments = TREE_CHAIN (arguments);
> > > > > >      }
> > > > > >
> > > > > > +  /* Forced GIMPLE thunks are normally ignored because they are
> > > created
> > > > > > +     after early debug.  ICF wrappers retain the original
> function
> > > decl
> > > > > and
> > > > > > +     its early DIE, so preserve its original debug state.  */
> > > > > > +  bool ignored_p = DECL_IGNORED_P (decl);
> > > > > >    expand_thunk (this, false, true);
> > > > > > +  DECL_IGNORED_P (decl) = ignored_p;
> > > > > >    thunk_info::remove (this);
> > > > > >
> > > > > >    /* Inline summary set-up.  */
> > > > > > diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr126355.c
> > > > > b/gcc/testsuite/gcc.dg/debug/dwarf2/pr126355.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..35e321008aa
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr126355.c
> > > > > > @@ -0,0 +1,35 @@
> > > > > > +/* PR debug/126355 */
> > > > > > +/* Verify that an IPA ICF wrapper for an address-taken function
> > > keeps an
> > > > > > +   address-bearing subprogram DIE.  */
> > > > > > +/* { dg-do compile } */
> > > > > > +/* { dg-options "-O2 -g -gdwarf -dA -fdump-ipa-icf-details" } */
> > > > > > +
> > > > > > +int
> > > > > > +pr_icf_wrapper_a (const char *host)
> > > > > > +{
> > > > > > +  (void) host;
> > > > > > +  return 0;
> > > > > > +}
> > > > > > +
> > > > > > +int
> > > > > > +pr_icf_wrapper_b (const char *host)
> > > > > > +{
> > > > > > +  (void) host;
> > > > > > +  return 0;
> > > > > > +}
> > > > > > +
> > > > > > +int (*keep_a) (const char *) = pr_icf_wrapper_a;
> > > > > > +int (*keep_b) (const char *) = pr_icf_wrapper_b;
> > > > > > +
> > > > > > +int
> > > > > > +main (int argc, char **argv)
> > > > > > +{
> > > > > > +  const char *arg = argc > 1 ? argv[1] : "x";
> > > > > > +  return keep_a (arg) + keep_b (arg);
> > > > > > +}
> > > > > > +
> > > > > > +/* Check the ICF direction explicitly because the DWARF scan
> below
> > > > > inspects
> > > > > > +   pr_icf_wrapper_b, the wrapper.  */
> > > > > > +/* { dg-final { scan-ipa-dump "Semantic equality
> > > > > hit:pr_icf_wrapper_a/\[0-9+\]+->pr_icf_wrapper_b/\[0-9+\]+" "icf"
> } }
> > > */
> > > > > > +/* { dg-final { scan-ipa-dump "Wrapper has been created" "icf"
> } }
> > > */
> > > > > > +/* { dg-final { scan-assembler "\\(DIE \\(0x\[0-9a-f\]+\\)
> > > > >
> > >
> DW_TAG_subprogram\\)\[\r\n\]+(\[^\r\n\]*\[\r\n\]+){1,12}\[^\r\n\]*DW_AT_name:
> > > > >
> > >
> \"pr_icf_wrapper_b\"\[\r\n\]+(\[^\r\n\]*\[\r\n\]+){1,12}\[^\r\n\]*DW_AT_low_pc"
> > > > > } } */
> > > > > >
> > > > >
> > > > > --
> > > > > Richard Biener <[email protected]>
> > > > > SUSE Software Solutions Germany GmbH,
> > > > > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > > > > GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG
> > > Nuernberg)
> > > > >
> > > >
> > >
> > > --
> > > Richard Biener <[email protected]>
> > > SUSE Software Solutions Germany GmbH,
> > > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > > GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG
> Nuernberg)
> >
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)