Here's a revised version of the patch. This revised version is ready for review.
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. Originally, I tried copying section names in copy_node. This caused problems for reasons I do not understand. This patch in this email avoids those problems by copying section names only in the callers of copy_node relevant to template instantation. Known unknowns (questions for the audience): * 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 and with Mach-O on macOS (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. Is this something I should worry much about? * Do we need to check or copy implicit_section or alias? I don't know what these properties mean, but they look related to section_name. Note: This patch depends on the following unmerged patches (but could be changed to not depend on them): * Simplify testing symbol sections: https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02062.html * Fix attribute((section)) with -flto: https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02063.html * Refactor copying decl section names: https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00979.html Testing: * Bootstrap on x86_64-linux-gnu with --disable-multilib --enable-checking=release --enable-languages=c,c++. Observe no change in test results (aside from the added tests). * Bootstrap on macOS x86_64-apple-darwin16.7.0 with --disable-multilib --enable-checking=release --enable-languages=c,c++. Observe no change in test results (aside from the added tests). 2019-11-20 Matthew Glazar <strager....@gmail.com> * gcc/cp/pt.c (tsubst_function_decl): Copy the section name from the original function. (tsubst_decl): Copy the section name from the original variable (if the variable is global). --- gcc/cp/pt.c | 5 +++ ...section-class-template-function-template.C | 25 +++++++++++ ...ass-template-specialized-static-variable.C | 29 +++++++++++++ ...template-static-inline-variable-template.C | 19 ++++++++ ...on-class-template-static-inline-variable.C | 19 ++++++++ .../section-class-template-static-variable.C | 20 +++++++++ ...on-function-template-static-variable-lto.C | 19 ++++++++ ...ection-function-template-static-variable.C | 26 +++++++++++ .../g++.dg/ext/section-function-template.C | 20 +++++++++ .../ext/section-multi-tu-template-main.C | 43 +++++++++++++++++++ .../ext/section-multi-tu-template-other.C | 24 +++++++++++ .../g++.dg/ext/section-multi-tu-template.h | 33 ++++++++++++++ .../g++.dg/ext/section-variable-template.C | 16 +++++++ 13 files changed, 298 insertions(+) create mode 100644 gcc/testsuite/g++.dg/ext/section-class-template-function-template.C create mode 100644 gcc/testsuite/g++.dg/ext/section-class-template-specialized-static-variable.C create mode 100644 gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable-template.C create mode 100644 gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable.C create mode 100644 gcc/testsuite/g++.dg/ext/section-class-template-static-variable.C create mode 100644 gcc/testsuite/g++.dg/ext/section-function-template-static-variable-lto.C create mode 100644 gcc/testsuite/g++.dg/ext/section-function-template-static-variable.C create mode 100644 gcc/testsuite/g++.dg/ext/section-function-template.C create mode 100644 gcc/testsuite/g++.dg/ext/section-multi-tu-template-main.C create mode 100644 gcc/testsuite/g++.dg/ext/section-multi-tu-template-other.C create mode 100644 gcc/testsuite/g++.dg/ext/section-multi-tu-template.h create mode 100644 gcc/testsuite/g++.dg/ext/section-variable-template.C diff --git gcc/cp/pt.c gcc/cp/pt.c index 8bacb3952ff..2593cf67a20 100644 --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not see #include "gimplify.h" #include "gcc-rich-location.h" #include "selftest.h" +#include "cgraph.h" /* The type of functions taking a tree, and some additional data, and returning an int. */ @@ -13526,6 +13527,7 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, if (!DECL_DELETED_FN (r)) DECL_INITIAL (r) = NULL_TREE; DECL_CONTEXT (r) = ctx; + set_decl_section_name (r, t); /* Handle explicit(dependent-expr). */ if (DECL_HAS_DEPENDENT_EXPLICIT_SPEC_P (t)) @@ -14432,6 +14434,9 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) register_local_specialization (r, t); } + if (VAR_P (t) && is_global_var (t)) + set_decl_section_name (r, t); + DECL_CHAIN (r) = NULL_TREE; apply_late_template_attributes (&r, DECL_ATTRIBUTES (r), diff --git gcc/testsuite/g++.dg/ext/section-class-template-function-template.C gcc/testsuite/g++.dg/ext/section-class-template-function-template.C new file mode 100644 index 00000000000..b890cda770e --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-class-template-function-template.C @@ -0,0 +1,25 @@ +// attribute((section)) should affect function templates +// within class templates. + +// { dg-do compile } +// { dg-require-named-sections "" } +// { dg-final { scan-assembler-symbol-section {callee} {^\.testsection$} } } + +template<class> +struct s +{ + template<class> + static __attribute__((section(".testsection"))) void + callee() + { + } +}; + +void +f(bool which) +{ + if (which) + s<int>::callee<int>(); + else + s<float>::callee<float>(); +} 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..d518da858ab --- /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-symbol-section {my_var} {^\.(char|float)section$} } } + +template<class> +struct s +{ + static int my_var; +}; + +// { dg-final { scan-assembler-symbol-section {ZN1sIcE6my_varE} {^\.charsection$} } } +template<> +int +s<char>::my_var __attribute__((section(".charsection"))) = 1; + +// { dg-final { scan-assembler-symbol-section {ZN1sIfE6my_varE} {^\.floatsection$} } } +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-template.C gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable-template.C new file mode 100644 index 00000000000..bef0f79064a --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable-template.C @@ -0,0 +1,19 @@ +// attribute((section)) should affect static inline variable templates in class +// templates. + +// { dg-options "-std=c++17" } +// { dg-require-named-sections "" } +// { dg-final { scan-assembler-symbol-section {my_var} {^\.testsection} } } + +template<class> +struct s +{ + template<class> + inline static int my_var __attribute__((section(".testsection"))) = 1; +}; + +int * +f(bool which) +{ + return which ? &s<char>::my_var<char> : &s<float>::my_var<float>; +} 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..359b79b6247 --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-class-template-static-inline-variable.C @@ -0,0 +1,19 @@ +// 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-symbol-section {my_var} {^\.testsection} } } + +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..77d742cc746 --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-class-template-static-variable.C @@ -0,0 +1,20 @@ +// attribute((section)) should affect static variables in class templates. + +// { dg-require-named-sections "" } +// { dg-final { scan-assembler-symbol-section {my_var} {^\.testsection$} } } + +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-lto.C gcc/testsuite/g++.dg/ext/section-function-template-static-variable-lto.C new file mode 100644 index 00000000000..8b960a3cda3 --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-function-template-static-variable-lto.C @@ -0,0 +1,19 @@ +// PR c++/70435 +// attribute((section)) should affect static variables in function templates +// even with -flto. + +// { dg-do link } +// { dg-require-effective-target lto } +// { dg-require-named-sections "" } +// { dg-options "-flto --save-temps" } + +// { dg-final { scan-lto-assembler-symbol-section {my_var} {^(\.testsection|__DATA,__testsection)$} } } +#include "section-function-template-static-variable.C" + +// { dg-final { cleanup-saved-temps } } + +int +main() +{ + return *f(true); +} 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..fc039950a3f --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-function-template-static-variable.C @@ -0,0 +1,26 @@ +// PR c++/70435 +// attribute((section)) should affect static variables in function templates. + +// { dg-do compile } +// { dg-require-named-sections "" } +// { dg-final { scan-assembler-symbol-section {my_var} {^(\.testsection|__DATA,__testsection)$} } } + +#if defined(__APPLE__) +#define TESTSECTION "__DATA,__testsection" +#else +#define TESTSECTION ".testsection" +#endif + +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..623f88b2d8e --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-function-template.C @@ -0,0 +1,20 @@ +// attribute((section)) should affect function templates. + +// { dg-do compile } +// { dg-require-named-sections "" } +// { dg-final { scan-assembler-symbol-section {callee} {^\.testsection$} } } + +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..58ddcba49da --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-multi-tu-template-main.C @@ -0,0 +1,43 @@ +// attribute((section)) variables in templates should be +// shared across translation units. + +// { dg-do run } +// { dg-require-named-sections "" } +// { dg-additional-sources "section-multi-tu-template-other.C" } +// { dg-options "--save-temps -std=c++17" } + +// { dg-final { scan-symbol-section "section-multi-tu-template-main.s" {[^_]f_var} {^(\.myfsection|__DATA,__myfsection)$} } } +// { dg-final { scan-symbol-section "section-multi-tu-template-main.s" {[^_]s_var} {^(\.myssection|__DATA,__myssection)$} } } + +// { dg-final { cleanup-saved-temps } } + +#include "section-multi-tu-template.h" + +int * +get_main_f_var_int() +{ + return f<int>(); +} + +int * +get_main_s_var_int() +{ + return &s<int>::s_var; +} + +float * +get_main_s_var_float() +{ + return &s<float>::s_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..be4e7c19665 --- /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>::s_var; +} + +float * +get_other_s_var_float() +{ + return &s<float>::s_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..8bfcbc36f46 --- /dev/null +++ gcc/testsuite/g++.dg/ext/section-multi-tu-template.h @@ -0,0 +1,33 @@ +// This file is part of the section-multi-tu-template-main.C test. + +#if defined(__APPLE__) +#define MYFSECTION "__DATA,__myfsection" +#define MYSSECTION "__DATA,__myssection" +#else +#define MYFSECTION ".myfsection" +#define MYSSECTION ".myssection" +#endif + +template<class T> +struct s +{ + static inline T s_var __attribute__((section(MYSSECTION))) = 42; +}; + +template<class T> +T * +f() +{ + static T f_var __attribute__((section(MYFSECTION))) = 42; + return &f_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..8a1577b8880 --- /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-symbol-section {my_var} {^\.testsection$} } } + +template<class> +inline int +my_var __attribute__((section(".testsection"))) = 1; + +int * +f(bool which) +{ + return which ? &my_var<char> : &my_var<float>; +} -- 2.22.0