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

Reply via email to