| Message ID | 20220414204241.20073-1-gbelinassi@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | PR105169 Fix references to discarded sections | expand |
On Thu, 14 Apr 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. > > Boostrapped and regtested on x86_64 linux. > > OK for Stage4? > > 2022-04-13 Giuliano Belinassi <gbelinassi@suse.de> > > PR c++/105169 > * targhooks.cc (default_print_patchable_function_entry_1): Handle COMDAT case. > * varasm.cc (handle_vtv_comdat_section): Rename to... > (switch_to_comdat_section): Generalize to also cover > __patchable_function_entry case. > (assemble_variable): Rename call from handle_vtv_comdat_section to > switch_to_comdat_section. > (output_object_block): Same as above. > * varasm.h: Declare switch_to_comdat_section. > > 2022-04-13 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. > > Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de> > --- > gcc/targhooks.cc | 8 ++++++-- > gcc/testsuite/ChangeLog | 7 +++++++ > 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 | 25 +++++++++++++---------- > gcc/varasm.h | 1 + > 7 files changed, 87 insertions(+), 13 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 e22bc66a6c8..540460e7db9 100644 > --- a/gcc/targhooks.cc > +++ b/gcc/targhooks.cc > @@ -1995,8 +1995,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); You are passing a decl here, but ... > + else > + switch_to_section (sect); > assemble_align (POINTER_SIZE); > fputs (asm_op, file); > assemble_name_raw (file, buf); > diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog > index 9ab7a178bf8..524a546a832 100644 > --- a/gcc/testsuite/ChangeLog > +++ b/gcc/testsuite/ChangeLog > @@ -1,3 +1,10 @@ > +2022-04-13 Giuliano Belinassi <giuliano.belinassi@suse.com> > + > + 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. > + > 2022-04-12 Antoni Boucher <bouanto@zoho.com> > > PR jit/104293 > 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..7cd91e2bb56 100644 > --- a/gcc/varasm.cc > +++ b/gcc/varasm.cc > @@ -128,7 +128,6 @@ static void asm_output_aligned_bss (FILE *, tree, const char *, > #endif /* BSS_SECTION_ASM_OP */ > static void mark_weak (tree); > static void output_constant_pool (const char *, tree); > -static void handle_vtv_comdat_section (section *, const_tree); > > /* Well-known sections, each one associated with some sort of *_ASM_OP. */ > section *text_section; > @@ -2406,7 +2405,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, > /* Special-case handling of vtv comdat sections. */ > if (sect->named.name > && (strcmp (sect->named.name, ".vtable_map_vars") == 0)) > - handle_vtv_comdat_section (sect, decl); > + switch_to_comdat_section (sect, DECL_NAME (decl)); ... possibly an IDENTIFIER_NODE here > else > switch_to_section (sect, decl); > if (align > BITS_PER_UNIT) > @@ -8037,7 +8036,7 @@ output_object_block (struct object_block *block) > if (SECTION_STYLE (block->sect) == SECTION_NAMED > && block->sect->named.name > && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0)) > - handle_vtv_comdat_section (block->sect, block->sect->named.decl); > + switch_to_comdat_section (block->sect, DECL_NAME (block->sect->named.decl)); > else > switch_to_section (block->sect, SYMBOL_REF_DECL ((*block->objects)[0])); and here. > @@ -8458,7 +8457,7 @@ default_asm_output_ident_directive (const char *ident_str) > } > > > -/* This function ensures that vtable_map variables are not only > +/* This function ensures that 'section' 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. > @@ -8468,14 +8467,18 @@ default_asm_output_ident_directive (const char *ident_str) > that is fixed, this if-else statement can be replaced with > a single call to "switch_to_section (sect)". */ Please update the functions overall comment. > -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) > + /* In case decl is not in a COMDAT group, we can not switch to the COMDAT > + section. So assert that this condition is always true. */ > + gcc_assert (DECL_COMDAT_GROUP (decl)); > + here you are relying on 'decl' being a decl. > 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 +8493,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) But here it can appearantly be a name. Maybe really always pass in the decl? Did you test on a non-ELF target? Did you configure with --enable-vtable-verify to increase test coverage? There seems to be only two testcases passing -fvtable-verify=std, g++.dg/ubsan/pr59437.C and testsuite/g++.dg/ubsan/pr59415.C > 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 > diff --git a/gcc/varasm.h b/gcc/varasm.h > index d5d8c4e5578..233301d6952 100644 > --- a/gcc/varasm.h > +++ b/gcc/varasm.h > @@ -41,6 +41,7 @@ extern void process_pending_assemble_externals (void); > extern bool decl_replaceable_p (tree, bool); > extern bool decl_binds_to_current_def_p (const_tree); > extern enum tls_model decl_default_tls_model (const_tree); > +extern void switch_to_comdat_section (section *, tree); > > /* Declare DECL to be a weak symbol. */ > extern void declare_weak (tree); >
Hi, On Tue, 2022-04-19 at 10:11 +0200, Richard Biener wrote: > On Thu, 14 Apr 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. > > > > Boostrapped and regtested on x86_64 linux. > > > > OK for Stage4? > > > > 2022-04-13 Giuliano Belinassi <gbelinassi@suse.de> > > > > PR c++/105169 > > * targhooks.cc (default_print_patchable_function_entry_1): > > Handle COMDAT case. > > * varasm.cc (handle_vtv_comdat_section): Rename to... > > (switch_to_comdat_section): Generalize to also cover > > __patchable_function_entry case. > > (assemble_variable): Rename call from handle_vtv_comdat_section > > to > > switch_to_comdat_section. > > (output_object_block): Same as above. > > * varasm.h: Declare switch_to_comdat_section. > > > > 2022-04-13 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. > > > > Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de> > > --- > > gcc/targhooks.cc | 8 ++++++-- > > gcc/testsuite/ChangeLog | 7 +++++++ > > 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 | 25 +++++++++++++---- > > ------ > > gcc/varasm.h | 1 + > > 7 files changed, 87 insertions(+), 13 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 e22bc66a6c8..540460e7db9 100644 > > --- a/gcc/targhooks.cc > > +++ b/gcc/targhooks.cc > > @@ -1995,8 +1995,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); > > You are passing a decl here, but ... > > > + else > > + switch_to_section (sect); > > assemble_align (POINTER_SIZE); > > fputs (asm_op, file); > > assemble_name_raw (file, buf); > > diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog > > index 9ab7a178bf8..524a546a832 100644 > > --- a/gcc/testsuite/ChangeLog > > +++ b/gcc/testsuite/ChangeLog > > @@ -1,3 +1,10 @@ > > +2022-04-13 Giuliano Belinassi <giuliano.belinassi@suse.com> > > + > > + 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. > > + > > 2022-04-12 Antoni Boucher <bouanto@zoho.com> > > > > PR jit/104293 > > 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..7cd91e2bb56 100644 > > --- a/gcc/varasm.cc > > +++ b/gcc/varasm.cc > > @@ -128,7 +128,6 @@ static void asm_output_aligned_bss (FILE *, > > tree, const char *, > > #endif /* BSS_SECTION_ASM_OP */ > > static void mark_weak (tree); > > static void output_constant_pool (const char *, tree); > > -static void handle_vtv_comdat_section (section *, const_tree); > > > > /* Well-known sections, each one associated with some sort of > > *_ASM_OP. */ > > section *text_section; > > @@ -2406,7 +2405,7 @@ assemble_variable (tree decl, int top_level > > ATTRIBUTE_UNUSED, > > /* Special-case handling of vtv comdat sections. */ > > if (sect->named.name > > && (strcmp (sect->named.name, ".vtable_map_vars") == 0)) > > - handle_vtv_comdat_section (sect, decl); > > + switch_to_comdat_section (sect, DECL_NAME (decl)); > > ... possibly an IDENTIFIER_NODE here > > > else > > switch_to_section (sect, decl); > > if (align > BITS_PER_UNIT) > > @@ -8037,7 +8036,7 @@ output_object_block (struct object_block > > *block) > > if (SECTION_STYLE (block->sect) == SECTION_NAMED > > && block->sect->named.name > > && (strcmp (block->sect->named.name, ".vtable_map_vars") == > > 0)) > > - handle_vtv_comdat_section (block->sect, block->sect- > > >named.decl); > > + switch_to_comdat_section (block->sect, DECL_NAME (block->sect- > > >named.decl)); > > else > > switch_to_section (block->sect, SYMBOL_REF_DECL ((*block- > > >objects)[0])); > > and here. > > > @@ -8458,7 +8457,7 @@ default_asm_output_ident_directive (const > > char *ident_str) > > } > > > > > > -/* This function ensures that vtable_map variables are not only > > +/* This function ensures that 'section' 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. > > @@ -8468,14 +8467,18 @@ default_asm_output_ident_directive (const > > char *ident_str) > > that is fixed, this if-else statement can be replaced with > > a single call to "switch_to_section (sect)". */ > > Please update the functions overall comment. > > > -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) > > + /* In case decl is not in a COMDAT group, we can not switch to > > the COMDAT > > + section. So assert that this condition is always true. */ > > + gcc_assert (DECL_COMDAT_GROUP (decl)); > > + > > here you are relying on 'decl' being a decl. > > > 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 +8493,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) > > But here it can appearantly be a name. Maybe really always pass in > the > decl? > > Did you test on a non-ELF target? Did you configure with > --enable-vtable-verify to increase test coverage? There > seems to be only two testcases passing -fvtable-verify=std, > g++.dg/ubsan/pr59437.C and testsuite/g++.dg/ubsan/pr59415.C Indeed, GCC doesn't even compile with this patch when --enable-vtable- verify is enabled. The next patch does compile and I see no fails regarding those tests. As for testing on non-ELF targets, I was unabled to reproduce this problem in Windows (test case attached to bugzilla compiles fine there). I guess we will only see if this is also a problem there when this feature gets widely used there. Giuliano. > > > 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 > > diff --git a/gcc/varasm.h b/gcc/varasm.h > > index d5d8c4e5578..233301d6952 100644 > > --- a/gcc/varasm.h > > +++ b/gcc/varasm.h > > @@ -41,6 +41,7 @@ extern void process_pending_assemble_externals > > (void); > > extern bool decl_replaceable_p (tree, bool); > > extern bool decl_binds_to_current_def_p (const_tree); > > extern enum tls_model decl_default_tls_model (const_tree); > > +extern void switch_to_comdat_section (section *, tree); > > > > /* Declare DECL to be a weak symbol. */ > > extern void declare_weak (tree); > >
diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc index e22bc66a6c8..540460e7db9 100644 --- a/gcc/targhooks.cc +++ b/gcc/targhooks.cc @@ -1995,8 +1995,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/ChangeLog b/gcc/testsuite/ChangeLog index 9ab7a178bf8..524a546a832 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2022-04-13 Giuliano Belinassi <giuliano.belinassi@suse.com> + + 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. + 2022-04-12 Antoni Boucher <bouanto@zoho.com> PR jit/104293 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..7cd91e2bb56 100644 --- a/gcc/varasm.cc +++ b/gcc/varasm.cc @@ -128,7 +128,6 @@ static void asm_output_aligned_bss (FILE *, tree, const char *, #endif /* BSS_SECTION_ASM_OP */ static void mark_weak (tree); static void output_constant_pool (const char *, tree); -static void handle_vtv_comdat_section (section *, const_tree); /* Well-known sections, each one associated with some sort of *_ASM_OP. */ section *text_section; @@ -2406,7 +2405,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, /* Special-case handling of vtv comdat sections. */ if (sect->named.name && (strcmp (sect->named.name, ".vtable_map_vars") == 0)) - handle_vtv_comdat_section (sect, decl); + switch_to_comdat_section (sect, DECL_NAME (decl)); else switch_to_section (sect, decl); if (align > BITS_PER_UNIT) @@ -8037,7 +8036,7 @@ output_object_block (struct object_block *block) if (SECTION_STYLE (block->sect) == SECTION_NAMED && block->sect->named.name && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0)) - handle_vtv_comdat_section (block->sect, block->sect->named.decl); + switch_to_comdat_section (block->sect, DECL_NAME (block->sect->named.decl)); else switch_to_section (block->sect, SYMBOL_REF_DECL ((*block->objects)[0])); @@ -8458,7 +8457,7 @@ default_asm_output_ident_directive (const char *ident_str) } -/* This function ensures that vtable_map variables are not only +/* This function ensures that 'section' 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. @@ -8468,14 +8467,18 @@ default_asm_output_ident_directive (const char *ident_str) 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) + /* In case decl is not in a COMDAT group, we can not switch to the COMDAT + section. So assert that this condition is always true. */ + gcc_assert (DECL_COMDAT_GROUP (decl)); + 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 +8493,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 diff --git a/gcc/varasm.h b/gcc/varasm.h index d5d8c4e5578..233301d6952 100644 --- a/gcc/varasm.h +++ b/gcc/varasm.h @@ -41,6 +41,7 @@ extern void process_pending_assemble_externals (void); extern bool decl_replaceable_p (tree, bool); extern bool decl_binds_to_current_def_p (const_tree); extern enum tls_model decl_default_tls_model (const_tree); +extern void switch_to_comdat_section (section *, tree); /* Declare DECL to be a weak symbol. */ extern void declare_weak (tree);
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. Boostrapped and regtested on x86_64 linux. OK for Stage4? 2022-04-13 Giuliano Belinassi <gbelinassi@suse.de> PR c++/105169 * targhooks.cc (default_print_patchable_function_entry_1): Handle COMDAT case. * varasm.cc (handle_vtv_comdat_section): Rename to... (switch_to_comdat_section): Generalize to also cover __patchable_function_entry case. (assemble_variable): Rename call from handle_vtv_comdat_section to switch_to_comdat_section. (output_object_block): Same as above. * varasm.h: Declare switch_to_comdat_section. 2022-04-13 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. Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de> --- gcc/targhooks.cc | 8 ++++++-- gcc/testsuite/ChangeLog | 7 +++++++ 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 | 25 +++++++++++++---------- gcc/varasm.h | 1 + 7 files changed, 87 insertions(+), 13 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