Summary: Do not merge my patch. It needs more work. I realized a problem with my patch. With -flto, __attribute__((section)) is still broken (i.e. my patch has no effect for LTO builds). I narrowed the problem to localize_node (gcc/ipa-visibility.c, historically part of function_and_variable_visibility) deliberately clearing the section name [1]. If I delete the section-clearing code in localize_node, the bug is fixed with -flto. However, that code probably exists for a reason. =]
While trying to find the meaning of the code in localize_node, I stumbled upon some discussion on copy_node and section names [2]. It looks like I should carefully audit callers of copy_node, and I should consider putting the section name copy logic in a caller instead. [1] https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00692.html [2] https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00965.html On Tue, Nov 5, 2019 at 3:38 PM Strager Neds <strager....@gmail.com> wrote: > > Aside: This is my first time working in GCC's code base. I probably made > some mistakes. Please point them out. =] > > When GCC encounters __attribute__((section("foo"))) on a function or > variable declaration, it adds an entry in the symbol table for the > declaration to remember its desired section. The symbol table is > separate from the declaration's tree node. > > When instantiating a template, GCC copies the tree of the template > recursively. GCC does *not* copy symbol table entries when copying > function and variable declarations. > > Combined, these two details mean that section attributes on function and > variable declarations in a template have no effect. > > Fix this issue by copying the section name (in the symbol table) when > copying a tree node for template instantiation. This addresses PR > c++/70435 and PR c++/88061. > > Known unknowns (these are questions I'm thinking aloud to myself): > > * For all targets which support the section attribute, are functions and > variables deduplicated (comdat) when using a custom section? It seems > to work with GNU ELF on Linux (i.e. I end up with only one copy), but > I'm unsure about other platforms. Richard Biener raised this concern > in PR c++/88061 > * Are there other callers of copy_node which do not want section > attributes to be copied? I did not audit callers of copy_node. > * Did this patch break anything? I had trouble running GCC's full test > suite, so I have not compared test results with and without this > patch. > > 2019-11-05 Matthew Glazar <strager....@gmail.com> > > * gcc/tree.c (copy_node): Copy section name from source SYMTAB_NODE, not > just init priority. > > diff --git > gcc/testsuite/g++.dg/ext/section-class-template-specialized-static-variable.C > gcc/testsuite/g++.dg/ext/section-class-template-specialized-static-variable.C > new file mode 100644 > index 00000000000..20f51fe665d > --- /dev/null > +++ > gcc/testsuite/g++.dg/ext/section-class-template-specialized-static-variable.C > @@ -0,0 +1,29 @@ > +// PR c++/70435 > +// attribute((section)) should affect specialized static > +// variables in class templates. > + > +// { dg-options "-std=c++17" } > +// { dg-require-named-sections "" } > +// { dg-final { scan-assembler-not {\.data.*my_var} } } > +// { dg-final { scan-assembler {\.charsection.*my_var} } } > +// { dg-final { scan-assembler {\.floatsection.*my_var} } } > + > +template<class> > +struct s > +{ > + static int my_var; > +}; > + > +template<> > +int s<char>:: > +my_var __attribute__((section(".charsection"))) = 1; > + > +template<> > +int s<float>:: > +my_var __attribute__((section(".floatsection"))) = 2; > + > +int * > +f(bool which) > +{ > + return which ? &s<char>::my_var : &s<float>::my_var; > +} > diff --git > gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable.C > gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable.C > new file mode 100644 > index 00000000000..e047c90c601 > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable.C > @@ -0,0 +1,20 @@ > +// PR c++/70435 > +// attribute((section)) should affect static inline variables in class > +// templates. > + > +// { dg-options "-std=c++17" } > +// { dg-require-named-sections "" } > +// { dg-final { scan-assembler-not {\.data.*my_var} } } > +// { dg-final { scan-assembler {\.testsection.*my_var} } } > + > +template<class> > +struct s > +{ > + inline static int my_var __attribute__((section(".testsection"))) = 1; > +}; > + > +int * > +f(bool which) > +{ > + return which ? &s<char>::my_var : &s<float>::my_var; > +} > diff --git gcc/testsuite/g++.dg/ext/section-class-template-static-variable.C > gcc/testsuite/g++.dg/ext/section-class-template-static-variable.C > new file mode 100644 > index 00000000000..ccf71e7c5df > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-class-template-static-variable.C > @@ -0,0 +1,22 @@ > +// attribute((section)) should affect static variables in class templates. > + > +// { dg-options "-std=c++17" } > +// { dg-require-named-sections "" } > +// { dg-final { scan-assembler-not {\.data.*my_var} } } > +// { dg-final { scan-assembler {\.testsection.*my_var} } } > + > +template<class> > +struct s > +{ > + static int my_var; > +}; > + > +template<class T> > +int s<T>:: > +my_var __attribute__((section(".testsection"))) = 1; > + > +int * > +f(bool which) > +{ > + return which ? &s<char>::my_var : &s<float>::my_var; > +} > diff --git > gcc/testsuite/g++.dg/ext/section-function-template-static-variable.C > gcc/testsuite/g++.dg/ext/section-function-template-static-variable.C > new file mode 100644 > index 00000000000..7685f255d82 > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-function-template-static-variable.C > @@ -0,0 +1,21 @@ > +// PR c++/70435 > +// attribute((section)) should affect static variables in function templates. > + > +// { dg-do compile } > +// { dg-require-named-sections "" } > +// { dg-final { scan-assembler-not {\.data.*my_var} } } > +// { dg-final { scan-assembler {\.testsection.*my_var} } } > + > +template<class> > +int * > +callee() > +{ > + static int my_var __attribute__((section(".testsection"))) = 1; > + return &my_var; > +} > + > +int * > +f(bool which) > +{ > + return which ? callee<char>() : callee<float>(); > +} > diff --git gcc/testsuite/g++.dg/ext/section-function-template.C > gcc/testsuite/g++.dg/ext/section-function-template.C > new file mode 100644 > index 00000000000..49d367eeafc > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-function-template.C > @@ -0,0 +1,21 @@ > +// attribute((section)) should affect function templates. > + > +// { dg-do compile } > +// { dg-require-named-sections "" } > +// { dg-final { scan-assembler-not {\.text\..*callee} } } > +// { dg-final { scan-assembler {\.testsection.*callee} } } > + > +template<class> > +__attribute__((section(".testsection"))) void > +callee() > +{ > +} > + > +void > +f(bool which) > +{ > + if (which) > + callee<int>(); > + else > + callee<float>(); > +} > diff --git gcc/testsuite/g++.dg/ext/section-multi-tu-template-main.C > gcc/testsuite/g++.dg/ext/section-multi-tu-template-main.C > new file mode 100644 > index 00000000000..0b4d5ee6b78 > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-multi-tu-template-main.C > @@ -0,0 +1,38 @@ > +// attribute((section)) variables in templates should be > +// shared across translation units. > + > +// { dg-do run } > +// { dg-options "-std=c++17" } > +// { dg-require-named-sections "" } > +// { dg-additional-sources "section-multi-tu-template-other.C" } > + > +#include "section-multi-tu-template.h" > + > +int * > +get_main_f_var_int() > +{ > + return f<int>(); > +} > + > +int * > +get_main_s_var_int() > +{ > + return &s<int>::var; > +} > + > +float * > +get_main_s_var_float() > +{ > + return &s<float>::var; > +} > + > +int main() > +{ > + if (get_main_f_var_int() != get_other_f_var_int()) > + return 1; > + if (get_main_s_var_int() != get_other_s_var_int()) > + return 2; > + if (get_main_s_var_float() != get_other_s_var_float()) > + return 3; > + return 0; > +} > diff --git gcc/testsuite/g++.dg/ext/section-multi-tu-template-other.C > gcc/testsuite/g++.dg/ext/section-multi-tu-template-other.C > new file mode 100644 > index 00000000000..b15c1f7ee43 > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-multi-tu-template-other.C > @@ -0,0 +1,24 @@ > +// This file is part of the section-multi-tu-template-main.C test. > + > +// { dg-options "-std=c++17" } > +// { dg-require-named-sections "" } > + > +#include "section-multi-tu-template.h" > + > +int * > +get_other_f_var_int() > +{ > + return f<int>(); > +} > + > +int * > +get_other_s_var_int() > +{ > + return &s<int>::var; > +} > + > +float * > +get_other_s_var_float() > +{ > + return &s<float>::var; > +} > diff --git gcc/testsuite/g++.dg/ext/section-multi-tu-template.h > gcc/testsuite/g++.dg/ext/section-multi-tu-template.h > new file mode 100644 > index 00000000000..41a0ce311ac > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-multi-tu-template.h > @@ -0,0 +1,25 @@ > +// This file is part of the section-multi-tu-template-main.C test. > + > +template<class T> > +struct s > +{ > + static inline T var __attribute__((section(".mysection"))) = 42; > +}; > + > +template<class T> > +T * > +f() > +{ > + static T var __attribute__((section(".mysection"))) = 42; > + return &var; > +} > + > +// section-multi-tu-template-main.C > +int *get_main_f_var_int(); > +int *get_main_s_var_int(); > +float *get_main_s_var_float(); > + > +// section-multi-tu-template-other.C > +int *get_other_f_var_int(); > +int *get_other_s_var_int(); > +float *get_other_s_var_float(); > diff --git gcc/testsuite/g++.dg/ext/section-variable-template.C > gcc/testsuite/g++.dg/ext/section-variable-template.C > new file mode 100644 > index 00000000000..32a83d264c9 > --- /dev/null > +++ gcc/testsuite/g++.dg/ext/section-variable-template.C > @@ -0,0 +1,16 @@ > +// PR c++/88061 > +// attribute((section)) should affect variable templates. > + > +// { dg-options "-std=c++17" } > +// { dg-require-named-sections "" } > +// { dg-final { scan-assembler-not {\.data.*my_var} } } > +// { dg-final { scan-assembler {\.testsection.*my_var} } } > + > +template<class> > +inline int my_var __attribute__((section(".testsection"))) = 1; > + > +int * > +f(bool which) > +{ > + return which ? &my_var<char> : &my_var<float>; > +} > diff --git gcc/tree.c gcc/tree.c > index 3299b344ea6..244f2a958db 100644 > --- gcc/tree.c > +++ gcc/tree.c > @@ -1215,17 +1215,26 @@ copy_node (tree node MEM_STAT_DECL) > if (VAR_P (node)) > { > DECL_HAS_DEBUG_EXPR_P (t) = 0; > - t->decl_with_vis.symtab_node = NULL; > - } > - if (VAR_P (node) && DECL_HAS_INIT_PRIORITY_P (node)) > - { > - SET_DECL_INIT_PRIORITY (t, DECL_INIT_PRIORITY (node)); > - DECL_HAS_INIT_PRIORITY_P (t) = 1; > } > if (TREE_CODE (node) == FUNCTION_DECL) > { > DECL_STRUCT_FUNCTION (t) = NULL; > + } > + > + /* Copy attributes of SYMTAB_NODE. */ > + if (VAR_OR_FUNCTION_DECL_P (node)) > + { > t->decl_with_vis.symtab_node = NULL; > + if (VAR_P (node) && DECL_HAS_INIT_PRIORITY_P (node)) > + { > + SET_DECL_INIT_PRIORITY (t, DECL_INIT_PRIORITY (node)); > + DECL_HAS_INIT_PRIORITY_P (t) = 1; > + } > + if (struct symtab_node *snode = node->decl_with_vis.symtab_node) > + { > + if (const char *section_name = snode->get_section ()) > + set_decl_section_name(t, section_name); > + } > } > } > else if (TREE_CODE_CLASS (code) == tcc_type)
