Message ID | 20240131022136.572689-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v2] Handle private COMDAT function symbol reference in readonly data section | expand |
On Tue, Jan 30, 2024 at 06:21:36PM -0800, H.J. Lu wrote: > Changes in v2: > > 1. Check decl non-null before dereferencing it. > 2. Update PR rtl-optimization/113617 from Thanks for updating the testcase. > --- a/gcc/varasm.cc > +++ b/gcc/varasm.cc > @@ -7459,16 +7459,46 @@ default_elf_select_rtx_section (machine_mode mode, rtx x, > { > int reloc = compute_reloc_for_rtx (x); > > + tree decl = nullptr; > + > + /* If it is a private COMDAT function symbol reference, call > + function_rodata_section for the read-only or relocated read-only > + data section associated with function DECL so that the COMDAT > + section will be used for the private COMDAT function symbol. */ > + if (HAVE_COMDAT_GROUP) > + { > + if (GET_CODE (x) == CONST > + && GET_CODE (XEXP (x, 0)) == PLUS > + && CONST_INT_P (XEXP (XEXP (x, 0), 1))) > + x = XEXP (XEXP (x, 0), 0); > + > + if (GET_CODE (x) == SYMBOL_REF) > + { > + decl = SYMBOL_REF_DECL (x); > + if (decl > + && (TREE_CODE (decl) != FUNCTION_DECL > + || !DECL_COMDAT_GROUP (decl) > + || TREE_PUBLIC (decl))) > + decl = nullptr; > + } > + } > + > /* ??? Handle small data here somehow. */ > > if (reloc & targetm.asm_out.reloc_rw_mask ()) > { > + if (decl) > + return targetm.asm_out.function_rodata_section (decl, true); As I wrote before, I still very much dislike this. We want to refer to the _ZN1R1BIFvvENS_1IIFPFvPvxxxEPN2N12N22N31CIN12_GLOBAL__N_11DIxEELb0EEExxxEEEE1FERNS_1HERKSI_NS_1GE private symbol defined in .text._ZN1R1BIFvvENS_1IIFPFvPvxxxEPN2N12N22N31CIN12_GLOBAL__N_11DIxEELb0EEExxxEEEE1FERNS_1HERKSI_NS_1GE section in _ZN1AIxE3fooExx comdat group from some readonly data memory, and read that from _ZN2N12N22N31XILi1EE3booINS1_1CIN12_GLOBAL__N_11DIxEELb0EEEEEvxxxRT_ function defined in .text._ZN2N12N22N31XILi1EE3booINS1_1CIN12_GLOBAL__N_11DIxEELb0EEEEEvxxxRT_ section in the same comdat group. The patch puts that into .data.rel.ro.local._ZN1R1BIFvvENS_1IIFPFvPvxxxEPN2N12N22N31CIN12_GLOBAL__N_11DIxEELb0EEExxxEEEE1FERNS_1HERKSI_NS_1GE section in the same comdat group, but that just looks weird and for targets which use section anchors also inefficient. If we have a shared constant pool (otherwise the constants would be emitted into a per-function constant pool of that _ZN2N12N22N31XILi1EE3booINS1_1CIN12_GLOBAL__N_11DIxEELb0EEEEEvxxxRT_ function and would live in something based on that function name. But in case it is shared, it is normally just .data.rel.ro.local or .data.rel.ro section, shared by whatever refers to it. These comdat private symbols are kind of exception, they can still be shared, but have to be shared only within the containing comdat group because it isn't valid to refer to them from other comdat groups. So, it is ok if say two different functions in the same comdat group actually share those MEM constants. Thus, I think for the DECL_COMDAT_GROUP (decl) && HAVE_COMDAT_GROUP case it would be best to make it clear in the section name that it is a .data.rel.ro.local or .data.rel.ro section shared by everything in the comdat group. So, shouldn't it be just .section .data.rel.ro.local,"awG",@progbits,_ZN1AIxE3fooExx,comdat and emit that directly in this function rather than using targetm.asm_out.function_rodata_section? Looking at targetm.asm_out.function_rodata_section, it is default_function_rodata_section on most targets, then on darwin, cygwin, AIX and mcore default_no_function_rodata_section which just returns the shared readonly_data_section (I hope those targets don't DECL_COMDAT_GROUP (decl) && HAVE_COMDAT_GROUP, otherwise it will simply not work) and then loongarch does some ugly magic (which is related to jumptables and so nothing we need to care about here hopefully). Another question is if we need to do anything about the DECL_COMDAT_GROUP (decl) && DECL_SECTION_NAME (decl) && startswith (DECL_SECTION_NAME (decl), ".gnu.linkonce.t.") case (older linkers) (i.e. when using years old GNU linkers). Jakub
On Wed, Jan 31, 2024 at 8:30 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Jan 30, 2024 at 06:21:36PM -0800, H.J. Lu wrote: > > Changes in v2: > > > > 1. Check decl non-null before dereferencing it. > > 2. Update PR rtl-optimization/113617 from > > Thanks for updating the testcase. > > > --- a/gcc/varasm.cc > > +++ b/gcc/varasm.cc > > @@ -7459,16 +7459,46 @@ default_elf_select_rtx_section (machine_mode mode, rtx x, > > { > > int reloc = compute_reloc_for_rtx (x); > > > > + tree decl = nullptr; > > + > > + /* If it is a private COMDAT function symbol reference, call > > + function_rodata_section for the read-only or relocated read-only > > + data section associated with function DECL so that the COMDAT > > + section will be used for the private COMDAT function symbol. */ > > + if (HAVE_COMDAT_GROUP) > > + { > > + if (GET_CODE (x) == CONST > > + && GET_CODE (XEXP (x, 0)) == PLUS > > + && CONST_INT_P (XEXP (XEXP (x, 0), 1))) > > + x = XEXP (XEXP (x, 0), 0); > > + > > + if (GET_CODE (x) == SYMBOL_REF) > > + { > > + decl = SYMBOL_REF_DECL (x); > > + if (decl > > + && (TREE_CODE (decl) != FUNCTION_DECL > > + || !DECL_COMDAT_GROUP (decl) > > + || TREE_PUBLIC (decl))) > > + decl = nullptr; > > + } > > + } > > + > > /* ??? Handle small data here somehow. */ > > > > if (reloc & targetm.asm_out.reloc_rw_mask ()) > > { > > + if (decl) > > + return targetm.asm_out.function_rodata_section (decl, true); > > As I wrote before, I still very much dislike this. > We want to refer to the > _ZN1R1BIFvvENS_1IIFPFvPvxxxEPN2N12N22N31CIN12_GLOBAL__N_11DIxEELb0EEExxxEEEE1FERNS_1HERKSI_NS_1GE > private symbol defined in > .text._ZN1R1BIFvvENS_1IIFPFvPvxxxEPN2N12N22N31CIN12_GLOBAL__N_11DIxEELb0EEExxxEEEE1FERNS_1HERKSI_NS_1GE > section in _ZN1AIxE3fooExx comdat group from some readonly data > memory, and read that from > _ZN2N12N22N31XILi1EE3booINS1_1CIN12_GLOBAL__N_11DIxEELb0EEEEEvxxxRT_ function > defined in .text._ZN2N12N22N31XILi1EE3booINS1_1CIN12_GLOBAL__N_11DIxEELb0EEEEEvxxxRT_ > section in the same comdat group. > > The patch puts that into > .data.rel.ro.local._ZN1R1BIFvvENS_1IIFPFvPvxxxEPN2N12N22N31CIN12_GLOBAL__N_11DIxEELb0EEExxxEEEE1FERNS_1HERKSI_NS_1GE > section in the same comdat group, but that just looks weird and for > targets which use section anchors also inefficient. > > If we have a shared constant pool (otherwise the constants would be emitted > into a per-function constant pool of that > _ZN2N12N22N31XILi1EE3booINS1_1CIN12_GLOBAL__N_11DIxEELb0EEEEEvxxxRT_ > function and would live in something based on that function name. > But in case it is shared, it is normally just .data.rel.ro.local or > .data.rel.ro section, shared by whatever refers to it. > These comdat private symbols are kind of exception, they can still be > shared, but have to be shared only within the containing comdat group > because it isn't valid to refer to them from other comdat groups. > So, it is ok if say two different functions in the same comdat group > actually share those MEM constants. > Thus, I think for the DECL_COMDAT_GROUP (decl) && HAVE_COMDAT_GROUP > case it would be best to make it clear in the section name that it > is a .data.rel.ro.local or .data.rel.ro section shared by everything > in the comdat group. So, shouldn't it be just > .section .data.rel.ro.local,"awG",@progbits,_ZN1AIxE3fooExx,comdat > and emit that directly in this function rather than using > targetm.asm_out.function_rodata_section? Which function (target hook) can I use to generate .section .data.rel.ro.local,"awG",@progbits,_ZN1AIxE3fooExx,comdat > Looking at targetm.asm_out.function_rodata_section, it is > default_function_rodata_section on most targets, then on darwin, > cygwin, AIX and mcore default_no_function_rodata_section which just > returns the shared readonly_data_section (I hope those targets don't > DECL_COMDAT_GROUP (decl) && HAVE_COMDAT_GROUP, otherwise it will simply not > work) and then loongarch does some ugly magic (which is related to > jumptables and so nothing we need to care about here hopefully). > > Another question is if we need to do anything about the > DECL_COMDAT_GROUP (decl) && DECL_SECTION_NAME (decl) > && startswith (DECL_SECTION_NAME (decl), ".gnu.linkonce.t.") > case (older linkers) (i.e. when using years old GNU linkers). > Should we support such targets? It is not easy for me to test it. Thanks.
On Wed, Jan 31, 2024 at 08:48:33AM -0800, H.J. Lu wrote: > Which function (target hook) can I use to generate > > .section .data.rel.ro.local,"awG",@progbits,_ZN1AIxE3fooExx,comdat Just if (decl) return get_section (reloc == 1 ? ".data.rel.ro.local" : ".data.rel.ro", SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE, decl); for the first hunk and else if (decl) return get_section (".rodata", SECTION_LINKONCE, decl); in the second case? Haven't tried it though, maybe the get_section section conflict stuff isn't able to handle it (and perhaps that is the reason why we simply don't emit functions into .section .text,"axG",@progbits,whatever,comdat sections instead of .section .text.function_name,"axG",@progbits,whatever,comdat In such case, we could append something to those section names, like const.pool (or whatever else that couldn't clash with function names, so needs probably a dot somewhere). Could be .data.rel.ro.local.const.pool .data.rel.ro.const.pool or .data.rel.ro.local..shared .data.rel.ro..shared or something similar, but .data.rel.ro.local.shared would not be ok, because it could clash with .data.rel.ro.local section for shared function. > > Another question is if we need to do anything about the > > DECL_COMDAT_GROUP (decl) && DECL_SECTION_NAME (decl) > > && startswith (DECL_SECTION_NAME (decl), ".gnu.linkonce.t.") > > case (older linkers) (i.e. when using years old GNU linkers). > > > > Should we support such targets? It is not easy for me to test it. Perhaps let's wait if somebody files an issue with such configuration. Jakub
On Wed, Jan 31, 2024 at 9:10 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Jan 31, 2024 at 08:48:33AM -0800, H.J. Lu wrote: > > Which function (target hook) can I use to generate > > > > .section .data.rel.ro.local,"awG",@progbits,_ZN1AIxE3fooExx,comdat > > Just > if (decl) > return get_section (reloc == 1 > ? ".data.rel.ro.local" : ".data.rel.ro", > SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE, > decl); > for the first hunk and > else if (decl) > return get_section (".rodata", SECTION_LINKONCE, decl); > in the second case? > > Haven't tried it though, maybe the get_section section conflict stuff > isn't able to handle it (and perhaps that is the reason why we simply don't > emit functions into > .section .text,"axG",@progbits,whatever,comdat > sections instead of > .section .text.function_name,"axG",@progbits,whatever,comdat GNU binutils has no issues with it: [hjl@gnu-cfl-3 tmp]$ cat x.s .section .text,"axG",@progbits,whatever,comdat nop .text nop [hjl@gnu-cfl-3 tmp]$ gcc -c x.s [hjl@gnu-cfl-3 tmp]$ readelf -SW x.o | grep text [ 2] .text PROGBITS 0000000000000000 000048 000001 00 AX 0 0 1 [ 5] .text PROGBITS 0000000000000000 000049 000001 00 AXG 0 0 1 [hjl@gnu-cfl-3 tmp]$ If it doesn't work for some targets, we can use targetm.asm_out.function_rodata_section. > In such case, we could append something to those section names, > like const.pool (or whatever else that couldn't clash with function names, > so needs probably a dot somewhere). Could be > .data.rel.ro.local.const.pool > .data.rel.ro.const.pool > or > .data.rel.ro.local..shared > .data.rel.ro..shared > or something similar, but .data.rel.ro.local.shared would not be ok, > because it could clash with .data.rel.ro.local section for shared function. > > > > Another question is if we need to do anything about the > > > DECL_COMDAT_GROUP (decl) && DECL_SECTION_NAME (decl) > > > && startswith (DECL_SECTION_NAME (decl), ".gnu.linkonce.t.") > > > case (older linkers) (i.e. when using years old GNU linkers). > > > > > > > Should we support such targets? It is not easy for me to test it. > > Perhaps let's wait if somebody files an issue with such configuration. > > Jakub >
On Wed, Jan 31, 2024 at 09:39:12AM -0800, H.J. Lu wrote:
> GNU binutils has no issues with it:
I know, I meant gcc.
If I try the proposed:
--- gcc/varasm.cc.jj 2024-01-30 08:44:43.304175273 +0100
+++ gcc/varasm.cc 2024-01-31 18:45:57.271087170 +0100
@@ -7459,15 +7459,46 @@ default_elf_select_rtx_section (machine_
{
int reloc = compute_reloc_for_rtx (x);
+ tree decl = nullptr;
+
+ /* If it is a private COMDAT function symbol reference, call
+ function_rodata_section for the read-only or relocated read-only
+ data section associated with function DECL so that the COMDAT
+ section will be used for the private COMDAT function symbol. */
+ if (HAVE_COMDAT_GROUP)
+ {
+ if (GET_CODE (x) == CONST
+ && GET_CODE (XEXP (x, 0)) == PLUS
+ && CONST_INT_P (XEXP (XEXP (x, 0), 1)))
+ x = XEXP (XEXP (x, 0), 0);
+
+ if (GET_CODE (x) == SYMBOL_REF)
+ {
+ decl = SYMBOL_REF_DECL (x);
+ if (decl
+ && (TREE_CODE (decl) != FUNCTION_DECL
+ || !DECL_COMDAT_GROUP (decl)
+ || TREE_PUBLIC (decl)))
+ decl = nullptr;
+ }
+ }
+
/* ??? Handle small data here somehow. */
if (reloc & targetm.asm_out.reloc_rw_mask ())
{
+ if (decl)
+ return get_section (reloc == 1
+ ? ".data.rel.ro.local" : ".data.rel.ro",
+ SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE,
+ decl);
if (reloc == 1)
return get_named_section (NULL, ".data.rel.ro.local", 1);
else
return get_named_section (NULL, ".data.rel.ro", 3);
}
+ else if (decl)
+ return get_section (".rodata", SECTION_LINKONCE, decl);
return mergeable_constant_section (mode, align, 0);
}
and append
typedef unsigned long int VV __attribute__((vector_size (2 * sizeof (long))));
VV vv;
__attribute__((noipa)) static void fn1 (void) {}
__attribute__((noipa)) static void fn2 (void) {}
void
fn3 ()
{
VV a = { (unsigned long) &fn1, (unsigned long) &fn2 };
vv = a;
}
to the first testcase (this is just to get a normal non-comdat
.data.rel.ro.local section referencing non-comdat non-public syumbol),
then I get the
pr113617.C:19:1: error: section type conflict with ‘static bool R::B<_R(_A ...), _F>::F(R::H&, const R::H&, R::G) [with _R = void; _F = R::I<void (*(N1::N2::N3::C<{anonymous}::D<long long int>, false>*, long long int, long long int, long long int))(void*, long long int, long long int, long long int)>; _A = {}]’
19 | }
| ^
In file included from pr113617.C:1:
pr113617.h:21:15: note: ‘static bool R::B<_R(_A ...), _F>::F(R::H&, const R::H&, R::G) [with _R = void; _F = R::I<void (*(N1::N2::N3::C<{anonymous}::D<long long int>, false>*, long long int, long long int, long long int))(void*, long long int, long long int, long long int)>; _A = {}]’ was declared here
21 | static bool F(H &, const H &, G) { return false; }
| ^
I feared.
So, it seems get_section handles section purely by name lookup
and isn't prepared to deal with multiple different sections
of the same name, but different comdat group.
Thus, maybe at least temporarily we need to use unique
section names here, say
.data.rel.ro.local.pool.<comdat_name>
.data.rel.ro.pool.<comdat_name>
.rodata.pool.<comdat_name>
where <comdat_name> would be the name of the comdat group, i.e.
IDENTIFIER_POINTER (DECL_COMDAT_GROUP (decl))
Jakub
On Wed, Jan 31, 2024 at 10:11 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, Jan 31, 2024 at 09:39:12AM -0800, H.J. Lu wrote: > > GNU binutils has no issues with it: > > I know, I meant gcc. > If I try the proposed: > --- gcc/varasm.cc.jj 2024-01-30 08:44:43.304175273 +0100 > +++ gcc/varasm.cc 2024-01-31 18:45:57.271087170 +0100 > @@ -7459,15 +7459,46 @@ default_elf_select_rtx_section (machine_ > { > int reloc = compute_reloc_for_rtx (x); > > + tree decl = nullptr; > + > + /* If it is a private COMDAT function symbol reference, call > + function_rodata_section for the read-only or relocated read-only > + data section associated with function DECL so that the COMDAT > + section will be used for the private COMDAT function symbol. */ > + if (HAVE_COMDAT_GROUP) > + { > + if (GET_CODE (x) == CONST > + && GET_CODE (XEXP (x, 0)) == PLUS > + && CONST_INT_P (XEXP (XEXP (x, 0), 1))) > + x = XEXP (XEXP (x, 0), 0); > + > + if (GET_CODE (x) == SYMBOL_REF) > + { > + decl = SYMBOL_REF_DECL (x); > + if (decl > + && (TREE_CODE (decl) != FUNCTION_DECL > + || !DECL_COMDAT_GROUP (decl) > + || TREE_PUBLIC (decl))) > + decl = nullptr; > + } > + } > + > /* ??? Handle small data here somehow. */ > > if (reloc & targetm.asm_out.reloc_rw_mask ()) > { > + if (decl) > + return get_section (reloc == 1 > + ? ".data.rel.ro.local" : ".data.rel.ro", > + SECTION_WRITE | SECTION_RELRO | SECTION_LINKONCE, > + decl); > if (reloc == 1) > return get_named_section (NULL, ".data.rel.ro.local", 1); > else > return get_named_section (NULL, ".data.rel.ro", 3); > } > + else if (decl) > + return get_section (".rodata", SECTION_LINKONCE, decl); > > return mergeable_constant_section (mode, align, 0); > } > > and append > typedef unsigned long int VV __attribute__((vector_size (2 * sizeof (long)))); > VV vv; > __attribute__((noipa)) static void fn1 (void) {} > __attribute__((noipa)) static void fn2 (void) {} > > void > fn3 () > { > VV a = { (unsigned long) &fn1, (unsigned long) &fn2 }; > vv = a; > } > to the first testcase (this is just to get a normal non-comdat > .data.rel.ro.local section referencing non-comdat non-public syumbol), > then I get the > pr113617.C:19:1: error: section type conflict with ‘static bool R::B<_R(_A ...), _F>::F(R::H&, const R::H&, R::G) [with _R = void; _F = R::I<void (*(N1::N2::N3::C<{anonymous}::D<long long int>, false>*, long long int, long long int, long long int))(void*, long long int, long long int, long long int)>; _A = {}]’ > 19 | } > | ^ > In file included from pr113617.C:1: > pr113617.h:21:15: note: ‘static bool R::B<_R(_A ...), _F>::F(R::H&, const R::H&, R::G) [with _R = void; _F = R::I<void (*(N1::N2::N3::C<{anonymous}::D<long long int>, false>*, long long int, long long int, long long int))(void*, long long int, long long int, long long int)>; _A = {}]’ was declared here > 21 | static bool F(H &, const H &, G) { return false; } > | ^ > I feared. > So, it seems get_section handles section purely by name lookup > and isn't prepared to deal with multiple different sections > of the same name, but different comdat group. > > Thus, maybe at least temporarily we need to use unique > section names here, say > .data.rel.ro.local.pool.<comdat_name> > .data.rel.ro.pool.<comdat_name> > .rodata.pool.<comdat_name> > where <comdat_name> would be the name of the comdat group, i.e. > IDENTIFIER_POINTER (DECL_COMDAT_GROUP (decl)) > > Jakub > I am testing this patch.
diff --git a/gcc/testsuite/g++.dg/pr113617-1a.C b/gcc/testsuite/g++.dg/pr113617-1a.C new file mode 100644 index 00000000000..c93f08b5068 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr113617-1a.C @@ -0,0 +1,145 @@ +// { dg-do compile { target fpic } } +// { dg-require-visibility "" } +// { dg-options "-O2 -std=c++11 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden" } + +namespace { +template <int V> struct J { static constexpr int value = V; }; +template <bool V> using K = J<V>; +using M = K<true>; +template <int> struct L { template <typename _Tp, typename> using type = _Tp; }; +template <bool _Cond, typename _If, typename _Else> using N = typename L<_Cond>::type<_If, _Else>; +M k; +template <typename _Tp> struct O { using type = _Tp; }; +template <typename _Up> +struct P : N<M ::value, O<_Up>, _Up> {}; +template <typename _Tp> struct Q { using type = typename P<_Tp>::type; }; +} +namespace R { +struct H; +enum G {}; +template <typename> class S; +struct T { using U = bool (*) (H &, const H &, G); U F; }; +template <typename, typename> class B; +template <typename _R, typename _F, typename... _A> +struct B<_R(_A...), _F> { + static bool F(H &, const H &, G) { return false; } + __attribute__((noipa)) static _R bar(const H &) {} +}; +template <typename _R, typename... _A> +struct S<_R(_A...)> : T { + template <typename _F> using AH = B<_R(), _F>; + template <typename _F> S(_F) { + using AG = AH<_F>; + barr = AG::bar; + F = AG::F; + } + using AF = _R (*)(const H &); + AF barr; +}; +template <typename> class I; +template <typename _F, typename... _B> +struct I<_F(_B...)> {}; +template <typename> using W = decltype(k); +template <int, typename _F, typename... _B> struct V { + typedef I<typename Q<_F>::type(typename Q<_B>::type...)> type; +}; +template <typename _F, typename... _B> +__attribute__((noipa)) typename V<W<_F>::value, _F, _B...>::type +baz(_F, _B...) { return typename V<W<_F>::value, _F, _B...>::type (); } +template <typename _Tp> struct AJ { + template <typename _Up> struct _Ptr { using type = _Up *; }; + using AI = typename _Ptr<_Tp>::type; +}; +template <typename _Tp> struct Y { + using AI = typename AJ<_Tp>::AI; + AI operator->(); +}; +} +extern int z; +namespace N1 { +namespace N2 { +namespace N3 { +enum Z { Z1, Z2 }; +template <int> struct X { + template <typename _F> + __attribute__((noipa)) void boo(long long, long long, long long, _F &) {} +}; +struct AC { + AC(int); + void m1(R::S<void()>); +}; +template <typename> +__attribute__((noipa)) void garply(void *, long long, long long, long long) {} +template <> +template <typename _F> +void X<Z2>::boo(long long, long long x, long long y, _F &fi) { + AC pool(z); + for (;;) { + auto job = R::baz(garply<_F>, &fi, y, y, x); + pool.m1(job); + } +} +struct AB { + static AB &bleh(); + template <typename _F> + void boo(long first, long x, long y, _F fi) { + switch (ab1) { + case Z1: + ab2->boo(first, x, y, fi); + case Z2: + ab3->boo(first, x, y, fi); + } + } + Z ab1; + R::Y<X<Z1>> ab2; + R::Y<X<Z2>> ab3; +}; +template <typename, bool> struct C; +template <typename _F> struct C<_F, false> { + __attribute__((noipa)) C(_F) {} + void boo(long first, long x, long y) { + auto u = AB::bleh(); + u.boo(first, x, y, *this); + } +}; +template <typename _F> struct AA { typedef C<_F, 0> type; }; +} +} +} +struct AD { + template <typename _F> + static void boo(long first, long x, long y, _F f) { + typename N1::N2::N3::AA<_F>::type fi(f); + fi.boo(first, x, y); + } + template <typename _F> + static void boo(long first, long x, _F f) { + boo(first, x, 0, f); + } +}; +template <typename> struct A { + void foo(long long, long long); + int *c; +}; +namespace { +template <typename> struct D { __attribute__((noipa)) D(int *) {} }; +} +template <typename T> +void A<T>::foo(long long x, long long y) +{ + int e; + D<T> d(&e); + AD::boo(0, y, d); + long p; + for (p = 0; p < x; p++) + c[p] = c[p - 1]; +} +int z; +long xx1; +void corge() { + A<long long> a; + a.foo(xx1, 0); +} + +// { dg-final { scan-assembler-not ".section\t\.data\.rel\.ro\.local,\"aw\"" { target { { i?86-*-linux* x86_64-*-linux* } && { ! ia32 } } } } } +// { dg-final { scan-assembler ".section\t.data\.rel\.ro\.local\..*,\"awG\"" { target { { i?86-*-linux* x86_64-*-linux* } && { ! ia32 } } } } } diff --git a/gcc/testsuite/g++.dg/pr113617-1b.C b/gcc/testsuite/g++.dg/pr113617-1b.C new file mode 100644 index 00000000000..4a02bf44e72 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr113617-1b.C @@ -0,0 +1,8 @@ +// { dg-do compile { target fpic } } +// { dg-require-visibility "" } +// { dg-options "-O2 -std=c++11 -fno-pic -fvisibility=hidden -fvisibility-inlines-hidden" } + +#include "pr113617-1a.C" + +// { dg-final { scan-assembler-not ".section\t\.rodata\.cst(4|8),\"aM\"" { target { { i?86-*-linux* x86_64-*-linux* } && { ! ia32 } } } } } +// { dg-final { scan-assembler ".section\t\.rodata\..*,\"aG\"" { target { { i?86-*-linux* x86_64-*-linux* } && { ! ia32 } } } } } diff --git a/gcc/varasm.cc b/gcc/varasm.cc index fa17eff551e..c26d95b7178 100644 --- a/gcc/varasm.cc +++ b/gcc/varasm.cc @@ -7459,16 +7459,46 @@ default_elf_select_rtx_section (machine_mode mode, rtx x, { int reloc = compute_reloc_for_rtx (x); + tree decl = nullptr; + + /* If it is a private COMDAT function symbol reference, call + function_rodata_section for the read-only or relocated read-only + data section associated with function DECL so that the COMDAT + section will be used for the private COMDAT function symbol. */ + if (HAVE_COMDAT_GROUP) + { + if (GET_CODE (x) == CONST + && GET_CODE (XEXP (x, 0)) == PLUS + && CONST_INT_P (XEXP (XEXP (x, 0), 1))) + x = XEXP (XEXP (x, 0), 0); + + if (GET_CODE (x) == SYMBOL_REF) + { + decl = SYMBOL_REF_DECL (x); + if (decl + && (TREE_CODE (decl) != FUNCTION_DECL + || !DECL_COMDAT_GROUP (decl) + || TREE_PUBLIC (decl))) + decl = nullptr; + } + } + /* ??? Handle small data here somehow. */ if (reloc & targetm.asm_out.reloc_rw_mask ()) { + if (decl) + return targetm.asm_out.function_rodata_section (decl, true); + if (reloc == 1) return get_named_section (NULL, ".data.rel.ro.local", 1); else return get_named_section (NULL, ".data.rel.ro", 3); } + if (decl) + return targetm.asm_out.function_rodata_section (decl, false); + return mergeable_constant_section (mode, align, 0); }