Hi Jason, > On 27 Sep 2026, at 17:59, Jason Merrill <[email protected]> wrote: > > On 9/27/26 11:01 AM, Iain Sandoe wrote: >> While chasing two testsuite fails for -Os this wrong-code bug surfaced. >> Fix tested on x86_64-linux/darwin. OK for trunk / backports? >> thanks >> Iain >> --- 8< --- >> For Darwin, at least, when -Os is selected we attempt to declone cdtors >> since the platform does not support strong symbol aliases the clones >> are emitted verbatim, leading to larger binaries. >> The unified cdtor is a private implementational detail local to a TU >> and so we should not emit a thunk if that will not be emitted. > > Hmm, isn't the unified ctor linkonce when the separate ctors are?
The bug I was seeing was that the thunk got inlined leaving a reference to a C4 that was not emitted; It is quite hard to reproduce since it requires the right combination of sizes - but the libstdc++ testsuite triggers it reliably (was just not able to simplify that). Maybe your comment means that there’s another issue (although the prohibition of the vague-linkage solves it, perhaps that’s a side-effect). >> Likewise, a thunk and the unified body must be selected together at >> link time; for a vague-linkage cdtor COMDAT groups support this but, >> without COMDAT, there is no guarantee and so we have to decline the >> declone and emit ordinary clones instead. > > It's desirable that they are selected together, but is it really necessary? > As long as one is selected, I'd think they should work interchangeably. In principle, I suppose so - they are all supposed to be token-identical (let’s not get into the ‘different TUs with different opts’). So long as the call to the thunkee is properly indirected (either through the GOT [64b] or a machopic indirect symbol [32b]), the static linker should be ok. If the call goes directly we would get a warning. Not sure exactly what avenue to explore on this - it’s a real end-user issue since -Os is a “recommended setting” on the platform. Iain. > >> gcc/cp/ChangeLog: >> * optimize.cc (maybe_thunk_body): Decline declone for targets >> without COMDAT when the item is vague-linkage. >> gcc/testsuite/ChangeLog: >> * g++.dg/opt/declone4.C: New test. >> * g++.dg/opt/declone5.C: New test. >> Signed-off-by: Iain Sandoe <[email protected]> >> --- >> gcc/cp/optimize.cc | 8 +++++++ >> gcc/testsuite/g++.dg/opt/declone4.C | 37 +++++++++++++++++++++++++++++ >> gcc/testsuite/g++.dg/opt/declone5.C | 33 +++++++++++++++++++++++++ >> 3 files changed, 78 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/opt/declone4.C >> create mode 100644 gcc/testsuite/g++.dg/opt/declone5.C >> diff --git a/gcc/cp/optimize.cc b/gcc/cp/optimize.cc >> index 0afeefd5704..d81e8e4ba15 100644 >> --- a/gcc/cp/optimize.cc >> +++ b/gcc/cp/optimize.cc >> @@ -287,6 +287,14 @@ maybe_thunk_body (tree fn, bool force) >> if (ctor_omit_inherited_parms (fns[0])) >> return 0; >> + /* A thunk and the unified body must be selected together at link time; >> + for a vague-linkage cdtor COMDAT groups support this. Without COMDAT >> + there is no guarantee and so we have to decline and emit ordinary >> clones >> + instead. FORCE means the body must not be copied at all, so it is >> still >> + honoured. */ >> + if (!force && DECL_WEAK (fn) && !HAVE_COMDAT_GROUP) >> + return 0; >> + >> /* Don't diagnose deprecated or unavailable cdtors just because they >> have thunks emitted for them. */ >> auto du = make_temp_override (deprecated_state, >> diff --git a/gcc/testsuite/g++.dg/opt/declone4.C >> b/gcc/testsuite/g++.dg/opt/declone4.C >> new file mode 100644 >> index 00000000000..1d3f87d3126 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/opt/declone4.C >> @@ -0,0 +1,37 @@ >> +// { dg-do run } >> +// { dg-options "-fdeclone-ctor-dtor -save-temps" } >> + >> +// A cdtor whose unified body this translation unit does emit should be >> +// decloned. The C1/C2 clones become thunks and the unified C4 body is >> +// emitted either private to the translation unit here, or to the COMDAT >> +// group on targets that support them. >> + >> +struct V { int v; V (); }; >> +V::V () : v (1) {} >> + >> +struct B : virtual V >> +{ >> + int n; >> + B (int); >> +}; >> + >> +B::B (int x) : n (x * 3 + 1) {} >> + >> +struct D : B // uses B::C2 >> +{ >> + D (int x) : B (x) {} >> +}; >> + >> +B *p; >> +D *q; >> + >> +int >> +main () >> +{ >> + p = new B (2); // uses B::C1 >> + q = new D (5); >> + return !(p->n == 7 && q->n == 16 && p->v == 1 && q->v == 1); >> +} >> + >> +// { dg-final { scan-assembler "_ZN1BC4Ei" } } >> +// { dg-final { scan-assembler-not "globl\[^\n\]*_ZN1BC4Ei" } } >> diff --git a/gcc/testsuite/g++.dg/opt/declone5.C >> b/gcc/testsuite/g++.dg/opt/declone5.C >> new file mode 100644 >> index 00000000000..d714f99de40 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/opt/declone5.C >> @@ -0,0 +1,33 @@ >> +// { dg-do run } >> +// { dg-options "-fdeclone-ctor-dtor -save-temps" } >> + >> +// We cannot emit a thunk to a unified cdtor on targets without comdat >> +// support, since the thunk and the target must be selected together for >> +// vague linkage. >> + >> +struct V { int v; V () : v (1) {} }; >> + >> +template <typename T> >> +struct B : virtual V >> +{ >> + T n; >> + B (T x) : n (x * 3 + 1) {} >> +}; >> + >> +struct D : B<int> // uses B<int>::C2 >> +{ >> + D (int x) : B<int> (x) {} >> +}; >> + >> +B<int> *p; >> +D *q; >> + >> +int >> +main () >> +{ >> + p = new B<int> (2); // uses B<int>::C1 >> + q = new D (5); >> + return !(p->n == 7 && q->n == 16 && p->v == 1 && q->v == 1); >> +} >> + >> +// { dg-final { scan-assembler-not "_ZN1BIiEC4Ei" { target { ! comdat_group >> } } } } >
