On Mon, 2022-05-09 at 13:39 +0200, Richard Biener wrote:
> On Sat, 7 May 2022, Giuliano Belinassi wrote:
>
> > When -fpatchable-function-entry= is enabled, certain C++ codes
> > fails to
> > link because of generated references to discarded sections in
> > __patchable_function_entry section. This commit fixes this problem
> > by
> > puting those references in a COMDAT section.
> >
> > On the previous patch, GCC fails to compile with --enable-vtable-
> > verify
> > enabled. This version compiles fine with it.
>
> OK for trunk.
>
Pushed to trunk. Is it okay if I backport it to older versions as well?
Giuliano.
> Thanks,
> Richard.
>
> > 2022-05-06 Giuliano Belinassi <gbelina...@suse.de>
> >
> > PR c++/105169
> > * targhooks.cc (default_print_patchable_function_entry_1):
> > Handle COMDAT case.
> > * varasm.cc (switch_to_comdat_section): New
> > (handle_vtv_comdat_section): Call switch_to_comdat_section.
> > * varasm.h: Declare switch_to_comdat_section.
> >
> > gcc/testsuite/ChangeLog
> > 2022-05-06 Giuliano Belinassi <gbelina...@suse.de>
> >
> > PR c++/105169
> > * g++.dg/modules/pr105169.h: New file.
> > * g++.dg/modules/pr105169_a.C: New test.
> > * g++.dg/modules/pr105169_b.C: New file.
> > ---
> > gcc/targhooks.cc | 8 ++++--
> > gcc/testsuite/g++.dg/modules/pr105169.h | 22 +++++++++++++++
> > gcc/testsuite/g++.dg/modules/pr105169_a.C | 25 +++++++++++++++++
> > gcc/testsuite/g++.dg/modules/pr105169_b.C | 12 +++++++++
> > gcc/varasm.cc | 33 ++++++++++++++-----
> > ----
> > gcc/varasm.h | 2 ++
> > 6 files changed, 87 insertions(+), 15 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr105169.h
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr105169_a.C
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr105169_b.C
> >
> > diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> > index 399d6f874dc..b15ae19bcb6 100644
> > --- a/gcc/targhooks.cc
> > +++ b/gcc/targhooks.cc
> > @@ -2009,8 +2009,12 @@ default_print_patchable_function_entry_1
> > (FILE *file,
> > patch_area_number++;
> > ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE",
> > patch_area_number);
> >
> > - switch_to_section (get_section
> > ("__patchable_function_entries",
> > - flags, current_function_decl));
> > + section *sect = get_section ("__patchable_function_entries",
> > + flags, current_function_decl);
> > + if (HAVE_COMDAT_GROUP && DECL_COMDAT_GROUP
> > (current_function_decl))
> > + switch_to_comdat_section (sect, current_function_decl);
> > + else
> > + switch_to_section (sect);
> > assemble_align (POINTER_SIZE);
> > fputs (asm_op, file);
> > assemble_name_raw (file, buf);
> > diff --git a/gcc/testsuite/g++.dg/modules/pr105169.h
> > b/gcc/testsuite/g++.dg/modules/pr105169.h
> > new file mode 100644
> > index 00000000000..a7e76270531
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr105169.h
> > @@ -0,0 +1,22 @@
> > +class IPXAddressClass
> > +{
> > +public:
> > + IPXAddressClass(void);
> > +};
> > +
> > +class WinsockInterfaceClass
> > +{
> > +
> > +public:
> > + WinsockInterfaceClass(void);
> > +
> > + virtual void Set_Broadcast_Address(void*){};
> > +
> > + virtual int Get_Protocol(void)
> > + {
> > + return 0;
> > + };
> > +
> > +protected:
> > +};
> > +
> > diff --git a/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > b/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > new file mode 100644
> > index 00000000000..66dc4b7901f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr105169_a.C
> > @@ -0,0 +1,25 @@
> > +/* { dg-module-do link } */
> > +/* { dg-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> > +/* { dg-additional-options "-std=c++11 -fpatchable-function-
> > entry=1 -O2" } */
> > +
> > +/* This test is in the "modules" package because it supports
> > multiple files
> > + linkage. */
> > +
> > +#include "pr105169.h"
> > +
> > +WinsockInterfaceClass* PacketTransport;
> > +
> > +IPXAddressClass::IPXAddressClass(void)
> > +{
> > +}
> > +
> > +int function()
> > +{
> > + return PacketTransport->Get_Protocol();
> > +}
> > +
> > +int main()
> > +{
> > + IPXAddressClass ipxaddr;
> > + return 0;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > b/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > new file mode 100644
> > index 00000000000..5f8b00dfe51
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr105169_b.C
> > @@ -0,0 +1,12 @@
> > +/* { dg-module-do link } */
> > +/* { dg-options "-std=c++11 -fpatchable-function-entry=1 -O2" } */
> > +/* { dg-additional-options "-std=c++11 -fpatchable-function-
> > entry=1 -O2" } */
> > +
> > +/* This test is in the "modules" package because it supports
> > multiple files
> > + linkage. */
> > +
> > +#include "pr105169.h"
> > +
> > +WinsockInterfaceClass::WinsockInterfaceClass(void)
> > +{
> > +}
> > diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> > index c41f17d64f7..6454f1ce519 100644
> > --- a/gcc/varasm.cc
> > +++ b/gcc/varasm.cc
> > @@ -8457,25 +8457,21 @@ default_asm_output_ident_directive (const
> > char *ident_str)
> > fprintf (asm_out_file, "%s\"%s\"\n", ident_asm_op, ident_str);
> > }
> >
> > -
> > -/* This function ensures that vtable_map variables are not only
> > - in the comdat section, but that each variable has its own
> > unique
> > - comdat name. Without this the variables end up in the same
> > section
> > - with a single comdat name.
> > -
> > +/* Switch to a COMDAT section with COMDAT name of decl.
> > +
> > FIXME: resolve_unique_section needs to deal better with
> > decls with both DECL_SECTION_NAME and DECL_ONE_ONLY. Once
> > that is fixed, this if-else statement can be replaced with
> > a single call to "switch_to_section (sect)". */
> >
> > -static void
> > -handle_vtv_comdat_section (section *sect, const_tree decl
> > ATTRIBUTE_UNUSED)
> > +void
> > +switch_to_comdat_section (section *sect, tree decl)
> > {
> > #if defined (OBJECT_FORMAT_ELF)
> > targetm.asm_out.named_section (sect->named.name,
> > sect->named.common.flags
> > | SECTION_LINKONCE,
> > - DECL_NAME (decl));
> > + decl);
> > in_section = sect;
> > #else
> > /* Neither OBJECT_FORMAT_PE, nor OBJECT_FORMAT_COFF is set here.
> > @@ -8490,18 +8486,18 @@ handle_vtv_comdat_section (section *sect,
> > const_tree decl ATTRIBUTE_UNUSED)
> > {
> > char *name;
> >
> > - if (TREE_CODE (DECL_NAME (decl)) == IDENTIFIER_NODE)
> > + if (TREE_CODE (decl) == IDENTIFIER_NODE)
> > name = ACONCAT ((sect->named.name, "$",
> > - IDENTIFIER_POINTER (DECL_NAME (decl)), NULL));
> > + IDENTIFIER_POINTER (decl), NULL));
> > else
> > name = ACONCAT ((sect->named.name, "$",
> > - IDENTIFIER_POINTER (DECL_COMDAT_GROUP
> > (DECL_NAME (decl))),
> > + IDENTIFIER_POINTER (DECL_COMDAT_GROUP (decl)),
> > NULL));
> >
> > targetm.asm_out.named_section (name,
> > sect->named.common.flags
> > | SECTION_LINKONCE,
> > - DECL_NAME (decl));
> > + decl);
> > in_section = sect;
> > }
> > else
> > @@ -8509,4 +8505,15 @@ handle_vtv_comdat_section (section *sect,
> > const_tree decl ATTRIBUTE_UNUSED)
> > #endif
> > }
> >
> > +/* This function ensures that vtable_map variables are not only
> > + in the comdat section, but that each variable has its own
> > unique
> > + comdat name. Without this the variables end up in the same
> > section
> > + with a single comdat name. */
> > +
> > +static void
> > +handle_vtv_comdat_section (section *sect, const_tree decl
> > ATTRIBUTE_UNUSED)
> > +{
> > + switch_to_comdat_section(sect, DECL_NAME (decl));
> > +}
> > +
> > #include "gt-varasm.h"
> > diff --git a/gcc/varasm.h b/gcc/varasm.h
> > index d5d8c4e5578..8ba8374e779 100644
> > --- a/gcc/varasm.h
> > +++ b/gcc/varasm.h
> > @@ -79,4 +79,6 @@ extern rtx assemble_static_space (unsigned
> > HOST_WIDE_INT);
> >
> > extern rtx assemble_trampoline_template (void);
> >
> > +extern void switch_to_comdat_section (section *, tree);
> > +
> > #endif // GCC_VARASM_H
> >
