> Am 17.05.2022 um 19:37 schrieb Giuliano Belinassi via Gcc-patches
> <gcc-patches@gcc.gnu.org>:
>
> 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?
Ok for the GCC 12 branch after a week with no reported issues. Since it’s not a
regression further backporting requires good reasons.
Richard.
> 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
>>>
Re: [PATCH v2] PR105169 Fix references to discarded sections
Richard Biener via Gcc-patches Tue, 17 May 2022 10:42:17 -0700
- Re: [PATCH v2] PR105169 Fix references ... Richard Biener via Gcc-patches
- Re: [PATCH v2] PR105169 Fix refere... Giuliano Belinassi via Gcc-patches
- Re: [PATCH v2] PR105169 Fix re... Richard Biener via Gcc-patches
- [PATCH v2] PR105169 Fix references... Giuliano Belinassi via Gcc-patches
