* Re: [PATCH v2] PR105169 Fix references to discarded sections
[not found] <20220507031740.6879-1-gbelinassi@suse.de>
@ 2022-05-09 11:39 ` Richard Biener
2022-05-17 17:36 ` Giuliano Belinassi
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2022-05-09 11:39 UTC (permalink / raw)
To: Giuliano Belinassi; +Cc: gcc-patches, matz, mliska, mjambor
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.
Thanks,
Richard.
> 2022-05-06 Giuliano Belinassi <gbelinassi@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 <gbelinassi@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
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] PR105169 Fix references to discarded sections
2022-05-09 11:39 ` [PATCH v2] PR105169 Fix references to discarded sections Richard Biener
@ 2022-05-17 17:36 ` Giuliano Belinassi
2022-05-17 17:42 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Giuliano Belinassi @ 2022-05-17 17:36 UTC (permalink / raw)
To: gbelinassi, rguenther; +Cc: gcc-patches, mliska, matz, mjambor
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 <gbelinassi@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 <gbelinassi@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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] PR105169 Fix references to discarded sections
2022-05-17 17:36 ` Giuliano Belinassi
@ 2022-05-17 17:42 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2022-05-17 17:42 UTC (permalink / raw)
To: Giuliano Belinassi via Gcc-patches; +Cc: gbelinassi, matz
> 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 <gbelinassi@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 <gbelinassi@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
>>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] PR105169 Fix references to discarded sections
@ 2022-05-07 3:28 Giuliano Belinassi
0 siblings, 0 replies; 4+ messages in thread
From: Giuliano Belinassi @ 2022-05-07 3:28 UTC (permalink / raw)
To: gcc-patches; +Cc: rguenther, matz, mliska, mjambor, gbelinassi
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.
2022-05-06 Giuliano Belinassi <gbelinassi@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 <gbelinassi@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
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-17 17:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220507031740.6879-1-gbelinassi@suse.de>
2022-05-09 11:39 ` [PATCH v2] PR105169 Fix references to discarded sections Richard Biener
2022-05-17 17:36 ` Giuliano Belinassi
2022-05-17 17:42 ` Richard Biener
2022-05-07 3:28 Giuliano Belinassi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).