* [PATCH] PR105169 Fix references to discarded sections
@ 2022-04-14 20:42 Giuliano Belinassi
2022-04-19 8:11 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Giuliano Belinassi @ 2022-04-14 20:42 UTC (permalink / raw)
To: gcc-patches; +Cc: rguenther, matz, mliska, Giuliano Belinassi
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);
+ 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);
\f
/* 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);
--
2.35.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PR105169 Fix references to discarded sections
2022-04-14 20:42 [PATCH] PR105169 Fix references to discarded sections Giuliano Belinassi
@ 2022-04-19 8:11 ` Richard Biener
2022-05-07 2:21 ` Giuliano Belinassi
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2022-04-19 8:11 UTC (permalink / raw)
To: Giuliano Belinassi; +Cc: gcc-patches, matz, mliska
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);
> \f
> /* 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);
>
--
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] 3+ messages in thread
* Re: [PATCH] PR105169 Fix references to discarded sections
2022-04-19 8:11 ` Richard Biener
@ 2022-05-07 2:21 ` Giuliano Belinassi
0 siblings, 0 replies; 3+ messages in thread
From: Giuliano Belinassi @ 2022-05-07 2:21 UTC (permalink / raw)
To: gbelinassi, rguenther; +Cc: gcc-patches, mliska, matz
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);
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-07 2:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-14 20:42 [PATCH] PR105169 Fix references to discarded sections Giuliano Belinassi
2022-04-19 8:11 ` Richard Biener
2022-05-07 2:21 ` 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).