On Thu, 11 Sep 2025, Richard Biener wrote:
> On Wed, 10 Sep 2025, Nathaniel Shead wrote:
>
> > Does this fix seem reasonable, or is there something I've missed?
> >
> > My change to g++.dg/lto/pr101396_0.C also causes it to fail link with
> > some flags on my machine, with:
> >
> > FAIL: g++.dg/lto/pr101396 cp_lto_pr101396_0.o-cp_lto_pr101396_1.o link, -O0
> > -flto -flto-partition=none -fuse-linker-plugin
> > FAIL: g++.dg/lto/pr101396 cp_lto_pr101396_0.o-cp_lto_pr101396_1.o link, -O0
> > -flto -flto-partition=1to1 -fno-use-linker-plugin
> > FAIL: g++.dg/lto/pr101396 cp_lto_pr101396_0.o-cp_lto_pr101396_1.o link, -O0
> > -flto -fuse-linker-plugin -fno-fat-lto-objects
> >
> > But the logs doesn't indicate exactly what failed with the link, and
> > when I try running the commands from the log myself it works fine (exit
> > status 0, executable generated and can be ran, etc.). This seems to
> > happen also on a clean tree without my changes, so not sure if this is a
> > bug or I've misconfigured something or what; any hints?
> >
> > Other than that failure, successfully bootstrapped and regtested on
> > x86_64-pc-linux-gnu.
> >
> > -- >8 --
> >
> > On a DECL, TREE_CHAIN will find any other declarations in the same
> > binding level. This caused an ICE in PR121865 because the next entity
> > in the binding level was the uninstantiated unique friend 'foo', for
> > which after being found the compiler tries to generate a mangled name
> > for it and crashes.
> >
> > This didn't happen in non-modules testcases only because normally the
> > unique friend function would have been chained after its template_decl,
> > and find_decl_types_r bails on lang-specific nodes so it never saw the
> > uninstantiated decl. With modules however the order of chaining
> > changed, causing the error.
>
> So it didn't visit TREE_CHAIN of a lang specific thing? That would
> have been a bug I think. Btw, what context was this chained in?
>
> I suppose we could change things like
>
> fld_worklist_push (TYPE_ATTRIBUTES (t), fld);
>
> to be
>
> fld_worklist_push_{chain,list}
>
> (note there's also TREE_PURPOSE and TREE_VALUE on list nodes). The
> way we exempt field/type decls is definitely a bit vague (I'll note
> we also have LABEL_DECLs at least).
>
> >
> > I don't think it's ever necessary to walk into the TREE_CHAIN, from what
> > I can see; other cases where it might be useful (block vars or type
> > fields) are already handled explicitly elsewhere, and only one test
> > fails because of the change, due to accidentally relying on this "walk
> > into the next in-scope declaration" behaviour.
>
> We have various tree lists for which this is necessary, for example
> DECL/TYPE_ATTRIBUTES. So I think this is wrong.
Btw, it would be nice if we'd have a tree_base bit for debugging
that we can set during free-lang-data when we visit a tree and
assert that each tree we stream during LTO streaming has this bit
set. Of course we add new trees between FLD and streaming so
this is probably not going to work easily.
Just to say, it's quite difficult to run into actual issues when
missing to visit some trees in FLD, so the setup is quite fragile.
Richard.
> > PR c++/121865
> >
> > gcc/ChangeLog:
> >
> > * ipa-free-lang-data.cc (find_decls_types_r): Don't walk into
> > TREE_CHAINs of DECLs.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/lto/pr101396_0.C: Ensure A will be walked into (and
> > isn't constant-folded out of the GIMPLE for the function).
> > * g++.dg/modules/lto-4_a.C: New test.
> > * g++.dg/modules/lto-4_b.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > ---
> > gcc/ipa-free-lang-data.cc | 3 ---
> > gcc/testsuite/g++.dg/lto/pr101396_0.C | 3 ++-
> > gcc/testsuite/g++.dg/modules/lto-4_a.C | 10 ++++++++++
> > gcc/testsuite/g++.dg/modules/lto-4_b.C | 8 ++++++++
> > 4 files changed, 20 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/lto-4_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/lto-4_b.C
> >
> > diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc
> > index 8c4fb3c6b64..41afc6ec82f 100644
> > --- a/gcc/ipa-free-lang-data.cc
> > +++ b/gcc/ipa-free-lang-data.cc
> > @@ -750,9 +750,6 @@ find_decls_types_r (tree *tp, int *ws, void *data)
> > && DECL_HAS_VALUE_EXPR_P (t))
> > fld_worklist_push (DECL_VALUE_EXPR (t), fld);
> >
> > - if (TREE_CODE (t) != FIELD_DECL
> > - && TREE_CODE (t) != TYPE_DECL)
> > - fld_worklist_push (TREE_CHAIN (t), fld);
> > *ws = 0;
> > }
> > else if (TYPE_P (t))
> > diff --git a/gcc/testsuite/g++.dg/lto/pr101396_0.C
> > b/gcc/testsuite/g++.dg/lto/pr101396_0.C
> > index b7a2947a880..d26174df8e0 100644
> > --- a/gcc/testsuite/g++.dg/lto/pr101396_0.C
> > +++ b/gcc/testsuite/g++.dg/lto/pr101396_0.C
> > @@ -8,5 +8,6 @@ enum A : __UINT32_TYPE__ { // { dg-lto-warning "6: type 'A'
> > violates the C\\+\\+
> >
> > int main()
> > {
> > - return (int) A::a;
> > + enum A x = A::a;
> > + return (int) x;
> > }
> > diff --git a/gcc/testsuite/g++.dg/modules/lto-4_a.C
> > b/gcc/testsuite/g++.dg/modules/lto-4_a.C
> > new file mode 100644
> > index 00000000000..16629158dc2
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lto-4_a.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/121865
> > +// { dg-require-effective-target lto }
> > +// { dg-additional-options "-fmodules -flto" }
> > +
> > +export module M;
> > +export template <typename T> struct S;
> > +export template <typename T> void foo(S<T>) {}
> > +template <typename T> struct S {
> > + friend void foo<>(S);
> > +};
> > diff --git a/gcc/testsuite/g++.dg/modules/lto-4_b.C
> > b/gcc/testsuite/g++.dg/modules/lto-4_b.C
> > new file mode 100644
> > index 00000000000..0322855caf4
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lto-4_b.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/121865
> > +// { dg-module-do link }
> > +// { dg-require-effective-target lto }
> > +// { dg-additional-options "-fmodules -flto" }
> > +
> > +import M;
> > +template struct S<int>;
> > +int main() {}
> >
>
>
--
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)
