[PATCH] Fix attribute((section)) for templates

[PATCH] Fix attribute((section)) for templates

[Strager Neds]

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.nds@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)

Re: [PATCH] Fix attribute((section)) for templates

[Strager Neds]

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.nds@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.nds@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)

Re: [PATCH] Fix attribute((section)) for templates

[Strager Neds]

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.nds@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

Re: [PATCH] Fix attribute((section)) for templates

[Strager Neds]

I discovered an issue with my patch. I need help resolving it.

Take the following code for example:

    template<class>
    struct s
    {
      static inline int __attribute__((section(".testsection"))) var = 1;
    };

    struct public_symbol {};

    namespace {
    struct internal_symbol {};
    }

    int *
    f(bool which)
    {
      if (which)
        return &s<public_symbol>::var;
      else
        return &s<internal_symbol>::var;
    }

With my patch, compiling this code fails with the following error:

    example.C:4:62: error: 's<{anonymous}::internal_symbol>::var'
causes a section type conflict with 's<public_symbol>::var'
    example.C:4:62: note: 's<public_symbol>::var' was declared here

The error is reported by gcc/varasm.c (get_section) because
s<internal_symbol>::var has the following section flags:

    SECTION_NAMED | SECTION_NOTYPE | SECTION_WRITE
    (flags == 0x280200)

but s<public_symbol>::var has the following section flags:

    SECTION_NAMED | SECTION_LINKONCE | SECTION_WRITE
    (sect->common.flags == 0x200a00)

and a section can't have both of these flag at the same time. In
particular, SECTION_LINKONCE conflicts with not-SECTION_LINKONCE.

How can we solve this problem? Some ideas (none of which I like):

* Disallow this code, possibly with an improved diagnostic.
* Silently make the section SECTION_LINKONCE if there is a conflict.
* Silently make the section not-SECTION_LINKONCE if there is a conflict.
* Silently make the section not-SECTION_LINKONCE unconditionally (even
  if there is no conflict).
* Make two sections with the same name, one with SECTION_LINKONCE and
  one with not-SECTION_LINKONCE. This is what Clang does. Clang seems to
  Do What I Mean for ELF; the .o file has one COMDAT section and another
  non-COMDAT section.
* Extend attribute((section())) to allow specifying different section
  names for different section flags.

Thanks in advance for your feedback!

On Fri, Nov 22, 2019 at 12:09 PM Strager Neds <strager.nds@gmail.com> wrote:
>
> 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.nds@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

Re: [PATCH] Fix attribute((section)) for templates

[Jason Merrill]

On 11/29/19 6:23 PM, Strager Neds wrote:
> I discovered an issue with my patch. I need help resolving it.
> 
> Take the following code for example:
> 
>      template<class>
>      struct s
>      {
>        static inline int __attribute__((section(".testsection"))) var = 1;
>      };
> 
>      struct public_symbol {};
> 
>      namespace {
>      struct internal_symbol {};
>      }
> 
>      int *
>      f(bool which)
>      {
>        if (which)
>          return &s<public_symbol>::var;
>        else
>          return &s<internal_symbol>::var;
>      }
> 
> With my patch, compiling this code fails with the following error:
> 
>      example.C:4:62: error: 's<{anonymous}::internal_symbol>::var'
> causes a section type conflict with 's<public_symbol>::var'
>      example.C:4:62: note: 's<public_symbol>::var' was declared here
> 
> The error is reported by gcc/varasm.c (get_section) because
> s<internal_symbol>::var has the following section flags:
> 
>      SECTION_NAMED | SECTION_NOTYPE | SECTION_WRITE
>      (flags == 0x280200)
> 
> but s<public_symbol>::var has the following section flags:
> 
>      SECTION_NAMED | SECTION_LINKONCE | SECTION_WRITE
>      (sect->common.flags == 0x200a00)
> 
> and a section can't have both of these flag at the same time. In
> particular, SECTION_LINKONCE conflicts with not-SECTION_LINKONCE.
> 
> How can we solve this problem? Some ideas (none of which I like):
> 
> * Disallow this code, possibly with an improved diagnostic.
> * Silently make the section SECTION_LINKONCE if there is a conflict.
> * Silently make the section not-SECTION_LINKONCE if there is a conflict.
> * Silently make the section not-SECTION_LINKONCE unconditionally (even
>    if there is no conflict).
> * Make two sections with the same name, one with SECTION_LINKONCE and
>    one with not-SECTION_LINKONCE. This is what Clang does. Clang seems to
>    Do What I Mean for ELF; the .o file has one COMDAT section and another
>    non-COMDAT section.
> * Extend attribute((section())) to allow specifying different section
>    names for different section flags.
> 
> Thanks in advance for your feedback!

s<public_symbol>::var should probably go into its own COMDAT section 
named something like .gnu.linkonce.testsection.<mangled name>. 
Currently resolve_unique_section does nothing if DECL_SECTION_NAME is set.

Jason

Re: [PATCH] Fix attribute((section)) for templates

[Nathan Sidwell]

On 12/16/19 6:20 PM, Jason Merrill wrote:
> On 11/29/19 6:23 PM, Strager Neds wrote:

>> How can we solve this problem? Some ideas (none of which I like):
>>
>> * Disallow this code, possibly with an improved diagnostic.
>> * Silently make the section SECTION_LINKONCE if there is a conflict.
>> * Silently make the section not-SECTION_LINKONCE if there is a conflict.
>> * Silently make the section not-SECTION_LINKONCE unconditionally (even
>>    if there is no conflict).
>> * Make two sections with the same name, one with SECTION_LINKONCE and
>>    one with not-SECTION_LINKONCE. This is what Clang does. Clang seems to
>>    Do What I Mean for ELF; the .o file has one COMDAT section and another
>>    non-COMDAT section.
>> * Extend attribute((section())) to allow specifying different section
>>    names for different section flags.
>>
>> Thanks in advance for your feedback!
> 
> s<public_symbol>::var should probably go into its own COMDAT section named 
> something like .gnu.linkonce.testsection.<mangled name>. Currently 
> resolve_unique_section does nothing if DECL_SECTION_NAME is set.

coincidentally I fell over this (again) recently.  I agree, [[gnu::section 
("prefix")]] on a template should use that as a prefix to the comdat section for 
the instantiations -- overriding the default of .rodata or .data for variable 
instantiations for example.

nathan

-- 
Nathan Sidwell