diff mbox series

[committed] d: Use weak linkage for template symbols instead of gnu.linkonce (PR99914)

Message ID 20210405114319.2483840-1-ibuclaw@gdcproject.org
State New
Headers show
Series [committed] d: Use weak linkage for template symbols instead of gnu.linkonce (PR99914) | expand

Commit Message

Iain Buclaw April 5, 2021, 11:43 a.m. UTC
Hi,

This patch changes the default linkage of templates in the D language to
be DECL_WEAK instead of DECL_ONE_ONLY, if supported.  This better
matches the expected override semantics of template symbols compiled to
object code.  For example:

 module rt.config;
 template rt_flag()
 {
   pragma(mangle, "rt_flag") __gshared bool rt_flag = true;
 }

 module main;
 extern(C) __gshared bool rt_flag = false;

The above currently does not succeed in linking due to there being
multiple definitions of `rt_flag' in different sections that aren't
considered mergeable.

The compiler flag enabling toggling of this has been given a clearer
name `-fweak-templates', distinguishing itself from G++ `-fweak', which
is intended for testing only.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32/-mx32, and
committed to mainline.

Regards,
Iain.

---
gcc/d/ChangeLog:

	PR d/99914
	* d-lang.cc (d_init): Disable flag_weak_templates if no support for
	weak or one-only symbols.
	* d-tree.h (VAR_OR_FUNCTION_DECL_CHECK): New macro.
	(DECL_INSTANTIATED): New macro.
	(d_comdat_linkage): Remove declaration.
	(d_linkonce_linkage): Remove declaration.
	(set_linkage_for_decl): New declaration.
	* decl.cc (DeclVisitor::visit (StructDeclaration *)): Replace call to
	d_linkonce_linkage with setting DECL_INSTANTIATED.
	(DeclVisitor::visit (ClassDeclaration *)): Likewise.
	(DeclVisitor::visit (EnumDeclaration *)): Likewise.
	(DeclVisitor::visit (InterfaceDeclaration *)): Remove call to
	d_linkonce_linkage.
	(get_symbol_decl): Call set_linkage_for_decl instead of
	d_linkonce_linkage.
	(d_finish_decl): Call set_linkage_for_decl.
	(d_comdat_linkage): Made function static.  Only set DECL_COMDAT for
	DECL_INSTANTIATED decls.
	(d_linkonce_linkage): Remove function.
	(d_weak_linkage): New function.
	(set_linkage_for_decl): New function.
	* gdc.texi (Runtime Options): Rename -fno-weak to -fno-weak-templates,
	update documentation of option.
	* lang.opt (fweak): Rename option to ...
	(fweak-templates): ... this.  Update help string.
	* modules.cc (get_internal_fn): Add Prot parameter.  Set generated
	function flag.
	(build_internal_fn): Update call to get_internal_fn.
	(build_dso_cdtor_fn): Likewise.
	(register_moduleinfo): Call d_finish_decl on dso_slot_node and
	dso_initialized_node.
	* typeinfo.cc (TypeInfoVisitor::internal_reference): Call
	set_linkage_for_decl instead of d_comdat_linkage.
	(TypeInfoDeclVisitor::visit (TypeInfoDeclaration *)): Remove calls to
	d_linkonce_linkage and d_comdat_linkage.
	(get_cpp_typeinfo_decl): Likewise.

gcc/testsuite/ChangeLog:

	PR d/99914
	* gdc.dg/pr99914.d: New test.
---
 gcc/d/d-lang.cc                |  2 +-
 gcc/d/d-tree.h                 | 15 ++++--
 gcc/d/decl.cc                  | 92 +++++++++++++++++++++-------------
 gcc/d/gdc.texi                 | 14 +++---
 gcc/d/lang.opt                 |  6 +--
 gcc/d/modules.cc               | 20 +++-----
 gcc/d/typeinfo.cc              | 12 +----
 gcc/testsuite/gdc.dg/pr99914.d |  5 ++
 8 files changed, 94 insertions(+), 72 deletions(-)
 create mode 100644 gcc/testsuite/gdc.dg/pr99914.d
diff mbox series

Patch

diff --git a/gcc/d/d-lang.cc b/gcc/d/d-lang.cc
index 5028698a5bc..a65af290cb8 100644
--- a/gcc/d/d-lang.cc
+++ b/gcc/d/d-lang.cc
@@ -386,7 +386,7 @@  d_init (void)
     using_eh_for_cleanups ();
 
   if (!supports_one_only ())
-    flag_weak = 0;
+    flag_weak_templates = 0;
 
   /* This is the C main, not the D main.  */
   main_identifier_node = get_identifier ("main");
diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h
index 724a0bfd87b..c1b6f275149 100644
--- a/gcc/d/d-tree.h
+++ b/gcc/d/d-tree.h
@@ -59,7 +59,13 @@  typedef Array <Expression *> Expressions;
    Usage of DECL_LANG_FLAG_?:
    0: LABEL_VARIABLE_CASE (in LABEL_DECL).
       DECL_BUILT_IN_CTFE (in FUNCTION_DECL).
-   1: DECL_IN_UNITTEST_CONDITION_P (in FUNCTION_DECL).  */
+   1: DECL_IN_UNITTEST_CONDITION_P (in FUNCTION_DECL).
+   2: DECL_INSTANTIATED (in FUNCTION_DECL, VAR_DECL).  */
+
+/* Language-specific tree checkers.  */
+
+#define VAR_OR_FUNCTION_DECL_CHECK(NODE) \
+  TREE_CHECK2 (NODE, VAR_DECL, FUNCTION_DECL)
 
 /* The kinds of scopes we recognize.  */
 
@@ -388,6 +394,10 @@  lang_tree_node
 #define DECL_IN_UNITTEST_CONDITION_P(NODE) \
   (DECL_LANG_FLAG_1 (FUNCTION_DECL_CHECK (NODE)))
 
+/* True if the decl comes from a template instance.  */
+#define DECL_INSTANTIATED(NODE) \
+  (DECL_LANG_FLAG_1 (VAR_OR_FUNCTION_DECL_CHECK (NODE)))
+
 enum d_tree_index
 {
   DTI_VTABLE_ENTRY_TYPE,
@@ -631,8 +641,7 @@  extern tree enum_initializer_decl (EnumDeclaration *);
 extern tree build_artificial_decl (tree, tree, const char * = NULL);
 extern tree create_field_decl (tree, const char *, int, int);
 extern void build_type_decl (tree, Dsymbol *);
-extern void d_comdat_linkage (tree);
-extern void d_linkonce_linkage (tree);
+extern void set_linkage_for_decl (tree);
 
 /* In expr.cc.  */
 extern tree build_expr (Expression *, bool = false, bool = false);
diff --git a/gcc/d/decl.cc b/gcc/d/decl.cc
index 0ec19345272..a59f00d4f2a 100644
--- a/gcc/d/decl.cc
+++ b/gcc/d/decl.cc
@@ -387,10 +387,7 @@  public:
     /* Generate static initializer.  */
     d->sinit = aggregate_initializer_decl (d);
     DECL_INITIAL (d->sinit) = layout_struct_initializer (d);
-
-    if (d->isInstantiated ())
-      d_linkonce_linkage (d->sinit);
-
+    DECL_INSTANTIATED (d->sinit) = (d->isInstantiated () != NULL);
     d_finish_decl (d->sinit);
 
     /* Put out the members.  There might be static constructors in the members
@@ -503,7 +500,7 @@  public:
 
     /* Generate static initializer.  */
     DECL_INITIAL (d->sinit) = layout_class_initializer (d);
-    d_linkonce_linkage (d->sinit);
+    DECL_INSTANTIATED (d->sinit) = (d->isInstantiated () != NULL);
     d_finish_decl (d->sinit);
 
     /* Put out the TypeInfo.  */
@@ -511,7 +508,6 @@  public:
       create_typeinfo (d->type, NULL);
 
     DECL_INITIAL (d->csym) = layout_classinfo (d);
-    d_linkonce_linkage (d->csym);
     d_finish_decl (d->csym);
 
     /* Put out the vtbl[].  */
@@ -534,7 +530,6 @@  public:
 
     DECL_INITIAL (d->vtblsym)
       = build_constructor (TREE_TYPE (d->vtblsym), elms);
-    d_comdat_linkage (d->vtblsym);
     d_finish_decl (d->vtblsym);
 
     /* Add this decl to the current binding level.  */
@@ -578,7 +573,6 @@  public:
       }
 
     DECL_INITIAL (d->csym) = layout_classinfo (d);
-    d_linkonce_linkage (d->csym);
     d_finish_decl (d->csym);
 
     /* Add this decl to the current binding level.  */
@@ -617,10 +611,7 @@  public:
 	/* Generate static initializer.  */
 	d->sinit = enum_initializer_decl (d);
 	DECL_INITIAL (d->sinit) = build_expr (tc->sym->defaultval, true);
-
-	if (d->isInstantiated ())
-	  d_linkonce_linkage (d->sinit);
-
+	DECL_INSTANTIATED (d->sinit) = (d->isInstantiated () != NULL);
 	d_finish_decl (d->sinit);
       }
 
@@ -1257,22 +1248,22 @@  get_symbol_decl (Declaration *decl)
 	  DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl->csym) = 1;
 	}
 
+      /* Mark compiler generated functions as artificial.  */
+      if (fd->generated)
+	DECL_ARTIFICIAL (decl->csym) = 1;
+
       /* Vector array operations are always compiler generated.  */
       if (fd->isArrayOp)
 	{
-	  TREE_PUBLIC (decl->csym) = 1;
 	  DECL_ARTIFICIAL (decl->csym) = 1;
 	  DECL_DECLARED_INLINE_P (decl->csym) = 1;
-	  d_comdat_linkage (decl->csym);
 	}
 
-      /* And so are ensure and require contracts.  */
+      /* Ensure and require contracts are lexically nested in the function they
+	 part of, but are always publicly callable.  */
       if (fd->ident == Identifier::idPool ("ensure")
 	  || fd->ident == Identifier::idPool ("require"))
-	{
-	  DECL_ARTIFICIAL (decl->csym) = 1;
-	  TREE_PUBLIC (decl->csym) = 1;
-	}
+	TREE_PUBLIC (decl->csym) = 1;
 
       if (decl->storage_class & STCfinal)
 	DECL_FINAL_P (decl->csym) = 1;
@@ -1336,8 +1327,8 @@  get_symbol_decl (Declaration *decl)
       /* The decl has not been defined -- yet.  */
       DECL_EXTERNAL (decl->csym) = 1;
 
-      if (decl->isInstantiated ())
-	d_linkonce_linkage (decl->csym);
+      DECL_INSTANTIATED (decl->csym) = (decl->isInstantiated () != NULL);
+      set_linkage_for_decl (decl->csym);
     }
 
   /* Symbol is going in thread local storage.  */
@@ -1545,6 +1536,7 @@  d_finish_decl (tree decl)
     set_decl_tls_model (decl, decl_default_tls_model (decl));
 
   relayout_decl (decl);
+  set_linkage_for_decl (decl);
 
   if (flag_checking && DECL_INITIAL (decl))
     {
@@ -2327,12 +2319,15 @@  d_comdat_group (tree decl)
 /* Set DECL up to have the closest approximation of "initialized common"
    linkage available.  */
 
-void
+static void
 d_comdat_linkage (tree decl)
 {
-  if (flag_weak)
+  /* COMDAT definitions have to be public.  */
+  gcc_assert (TREE_PUBLIC (decl));
+
+  if (supports_one_only ())
     make_decl_one_only (decl, d_comdat_group (decl));
-  else if (TREE_CODE (decl) == FUNCTION_DECL
+  else if ((TREE_CODE (decl) == FUNCTION_DECL && DECL_INSTANTIATED (decl))
 	   || (VAR_P (decl) && DECL_ARTIFICIAL (decl)))
     /* We can just emit function and compiler-generated variables statically;
        having multiple copies is (for the most part) only a waste of space.  */
@@ -2342,26 +2337,53 @@  d_comdat_linkage (tree decl)
     /* Fallback, cannot have multiple copies.  */
     DECL_COMMON (decl) = 1;
 
-  if (TREE_PUBLIC (decl))
+  if (TREE_PUBLIC (decl) && DECL_INSTANTIATED (decl))
     DECL_COMDAT (decl) = 1;
 }
 
-/* Set DECL up to have the closest approximation of "linkonce" linkage.  */
+/* Set DECL up to have the closest approximation of "weak" linkage.  */
 
-void
-d_linkonce_linkage (tree decl)
+static void
+d_weak_linkage (tree decl)
 {
   /* Weak definitions have to be public.  */
+  gcc_assert (TREE_PUBLIC (decl));
+
+  /* Allow comdat linkage to be forced with the flag `-fno-weak-templates'.  */
+  if (!flag_weak_templates || !TARGET_SUPPORTS_WEAK)
+    return d_comdat_linkage (decl);
+
+  declare_weak (decl);
+}
+
+/* DECL is a FUNCTION_DECL or a VAR_DECL with static storage.  Set flags to
+   reflect the linkage that DECL will receive in the object file.  */
+
+void
+set_linkage_for_decl (tree decl)
+{
+  gcc_assert (VAR_OR_FUNCTION_DECL_P (decl) && TREE_STATIC (decl));
+
+  /* Non-public decls keep their internal linkage. */
   if (!TREE_PUBLIC (decl))
     return;
 
-  /* Necessary to allow DECL_ONE_ONLY or DECL_WEAK functions to be inlined.  */
-  if (TREE_CODE (decl) == FUNCTION_DECL)
-    DECL_DECLARED_INLINE_P (decl) = 1;
+  /* Don't need to give private or protected symbols a special linkage.  */
+  if ((TREE_PRIVATE (decl) || TREE_PROTECTED (decl))
+      && !DECL_INSTANTIATED (decl))
+    return;
+
+  /* Functions declared as `pragma(inline, true)' can appear in multiple
+     translation units.  */
+  if (TREE_CODE (decl) == FUNCTION_DECL && DECL_DECLARED_INLINE_P (decl))
+    return d_comdat_linkage (decl);
 
-  /* No weak support, fallback to COMDAT linkage.  */
-  if (!flag_weak)
-   return d_comdat_linkage (decl);
+  /* Instantiated variables and functions need to be overridable by any other
+     symbol with the same name, so give them weak linkage.  */
+  if (DECL_INSTANTIATED (decl))
+    return d_weak_linkage (decl);
 
-  make_decl_one_only (decl, d_comdat_group (decl));
+  /* Compiler generated public symbols can appear in multiple contexts.  */
+  if (DECL_ARTIFICIAL (decl))
+    return d_weak_linkage (decl);
 }
diff --git a/gcc/d/gdc.texi b/gcc/d/gdc.texi
index e727848895d..095f7ecca41 100644
--- a/gcc/d/gdc.texi
+++ b/gcc/d/gdc.texi
@@ -325,13 +325,13 @@  is compiled into the program.
 Turns on compilation of @code{version} code identified by @var{ident}.
 @end table
 
-@item -fno-weak
-@cindex @option{-fweak}
-@cindex @option{-fno-weak}
-Turns off emission of instantiated declarations that can be defined in multiple
-objects as weak or one-only symbols.  The default is to emit all public symbols
-as weak, unless the target lacks support for weak symbols.  Disabling this
-option means that common symbols are instead put in COMDAT or become private.
+@item -fno-weak-templates
+@cindex @option{-fweak-templates}
+@cindex @option{-fno-weak-templates}
+Turns off emission of declarations that can be defined in multiple objects as
+weak symbols.  The default is to emit all public symbols as weak, unless the
+target lacks support for weak symbols.  Disabling this option means that common
+symbols are instead put in COMDAT or become private.
 
 @end table
 
diff --git a/gcc/d/lang.opt b/gcc/d/lang.opt
index 62e9f8ecfd2..ded218fc5e3 100644
--- a/gcc/d/lang.opt
+++ b/gcc/d/lang.opt
@@ -321,9 +321,9 @@  fversion=
 D Joined RejectNegative
 -fversion=<level|ident>	Compile in version code >= <level> or identified by <ident>.
 
-fweak
-D Var(flag_weak) Init(1)
-Emit common-like symbols as weak symbols.
+fweak-templates
+D Var(flag_weak_templates) Init(1)
+Emit template instantiations as weak symbols.
 
 imultilib
 D Joined Separate
diff --git a/gcc/d/modules.cc b/gcc/d/modules.cc
index af69815deb9..8786344d954 100644
--- a/gcc/d/modules.cc
+++ b/gcc/d/modules.cc
@@ -125,7 +125,7 @@  static Module *current_module_decl;
    by both module initialization and dso handlers.  */
 
 static FuncDeclaration *
-get_internal_fn (tree ident)
+get_internal_fn (tree ident, const Prot &prot)
 {
   Module *mod = current_module_decl;
   const char *name = IDENTIFIER_POINTER (ident);
@@ -141,9 +141,10 @@  get_internal_fn (tree ident)
 
   FuncDeclaration *fd = FuncDeclaration::genCfunc (NULL, Type::tvoid,
 						   Identifier::idPool (name));
+  fd->generated = true;
   fd->loc = Loc (mod->srcfile->toChars (), 1, 0);
   fd->parent = mod;
-  fd->protection.kind = Prot::private_;
+  fd->protection = prot;
   fd->semanticRun = PASSsemantic3done;
 
   return fd;
@@ -155,7 +156,7 @@  get_internal_fn (tree ident)
 static tree
 build_internal_fn (tree ident, tree expr)
 {
-  FuncDeclaration *fd = get_internal_fn (ident);
+  FuncDeclaration *fd = get_internal_fn (ident, Prot (Prot::private_));
   tree decl = get_symbol_decl (fd);
 
   tree old_context = start_function (fd);
@@ -337,7 +338,8 @@  build_dso_cdtor_fn (bool ctor_p)
 	}
     }
    */
-  FuncDeclaration *fd = get_internal_fn (get_identifier (name));
+  FuncDeclaration *fd = get_internal_fn (get_identifier (name),
+					 Prot (Prot::public_));
   tree decl = get_symbol_decl (fd);
 
   TREE_PUBLIC (decl) = 1;
@@ -345,8 +347,6 @@  build_dso_cdtor_fn (bool ctor_p)
   DECL_VISIBILITY (decl) = VISIBILITY_HIDDEN;
   DECL_VISIBILITY_SPECIFIED (decl) = 1;
 
-  d_comdat_linkage (decl);
-
   /* Start laying out the body.  */
   tree old_context = start_function (fd);
   rest_of_decl_compilation (decl, 1, 0);
@@ -443,15 +443,11 @@  register_moduleinfo (Module *decl, tree minfo)
   /* Declare dso_slot and dso_initialized.  */
   dso_slot_node = build_dso_registry_var (GDC_PREFIX ("dso_slot"),
 					  ptr_type_node);
-  DECL_EXTERNAL (dso_slot_node) = 0;
-  d_comdat_linkage (dso_slot_node);
-  rest_of_decl_compilation (dso_slot_node, 1, 0);
+  d_finish_decl (dso_slot_node);
 
   dso_initialized_node = build_dso_registry_var (GDC_PREFIX ("dso_initialized"),
 						 boolean_type_node);
-  DECL_EXTERNAL (dso_initialized_node) = 0;
-  d_comdat_linkage (dso_initialized_node);
-  rest_of_decl_compilation (dso_initialized_node, 1, 0);
+  d_finish_decl (dso_initialized_node);
 
   /* Declare dso_ctor() and dso_dtor().  */
   tree dso_ctor = build_dso_cdtor_fn (true);
diff --git a/gcc/d/typeinfo.cc b/gcc/d/typeinfo.cc
index 698e7669884..f8ffcbfff25 100644
--- a/gcc/d/typeinfo.cc
+++ b/gcc/d/typeinfo.cc
@@ -358,7 +358,7 @@  class TypeInfoVisitor : public Visitor
     DECL_EXTERNAL (decl) = 0;
     TREE_PUBLIC (decl) = 1;
     DECL_VISIBILITY (decl) = VISIBILITY_INTERNAL;
-    d_comdat_linkage (decl);
+    set_linkage_for_decl (decl);
     d_pushdecl (decl);
 
     return decl;
@@ -1320,14 +1320,6 @@  public:
 
     DECL_CONTEXT (tid->csym) = d_decl_context (tid);
     TREE_READONLY (tid->csym) = 1;
-
-    /* Built-in typeinfo will be referenced as one-only.  */
-    gcc_assert (!tid->isInstantiated ());
-
-    if (builtin_typeinfo_p (tid->tinfo))
-      d_linkonce_linkage (tid->csym);
-    else
-      d_comdat_linkage (tid->csym);
   }
 
   void visit (TypeInfoClassDeclaration *tid)
@@ -1467,8 +1459,6 @@  get_cpp_typeinfo_decl (ClassDeclaration *decl)
     = TREE_TYPE (build_ctype (decl->type));
   TREE_READONLY (decl->cpp_type_info_ptr_sym) = 1;
 
-  d_comdat_linkage (decl->cpp_type_info_ptr_sym);
-
   /* Layout the initializer and emit the symbol.  */
   layout_cpp_typeinfo (decl);
 
diff --git a/gcc/testsuite/gdc.dg/pr99914.d b/gcc/testsuite/gdc.dg/pr99914.d
new file mode 100644
index 00000000000..689eae02136
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr99914.d
@@ -0,0 +1,5 @@ 
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914
+// { dg-additional-options "-fmain" }
+// { dg-do link { target d_runtime } }
+
+extern(C) __gshared bool rt_cmdline_enabled = false;