[Bug d/99914] New: d: Template symbols not overridable by normal symbols

[Bug d/99914] New: d: Template symbols not overridable by normal symbols

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914

            Bug ID: 99914
           Summary: d: Template symbols not overridable by normal symbols
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: d
          Assignee: ibuclaw at gdcproject dot org
          Reporter: ibuclaw at gdcproject dot org
  Target Milestone: ---

Currently, the following does not link:

 extern(C) __gshared bool rt_cmdline_enabled = false;

Because the symbol conflicts with a template symbol of the same name in the D
runtime library.

 template rt_cmdline_enabled()
 {
     pragma(mangle, "rt_cmdline_enabled")
     __gshared bool rt_cmdline_enabled = true;
 }

Template symbols are made DECL_ONE_ONLY, which ends up in the gnu.linkonce
section as a unique global symbol.  However, the linker only considers other
symbols in gnu.linkonce as being candidates for discarding duplicates.

The expected and correct behaviour is for all instantiations to be declared
weak.

[Bug d/99914] d: Template symbols not overridable by normal symbols

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914

--- Comment #1 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain Buclaw <ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:76a7e7e706ac4c01cead3c6514322aaad88f9a63

commit r11-7983-g76a7e7e706ac4c01cead3c6514322aaad88f9a63
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Sun Mar 14 22:51:56 2021 +0100

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

    The default linkage of templates in the D language is now 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
    named `-fweak-templates', which distinguishes itself from G++ `-fweak',
    which is intended only for testing.

    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.

[Bug d/99914] d: Template symbols not overridable by normal symbols

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914

Iain Buclaw <ibuclaw at gdcproject dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Fix committed.

[Bug d/99914] d: Template symbols not overridable by normal symbols

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain Buclaw <ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:5b63eb17d863ac080cf3c7df08233054b09d3747

commit r11-8173-g5b63eb17d863ac080cf3c7df08233054b09d3747
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Tue Apr 13 16:19:03 2021 +0200

    d: Move call to set_linkage_for_decl to declare_extern_var.

    This both prevents against it being called twice for declarations that
    are defined, and fixes an issue where variables defined in the
    compilation get one kind of linkage (weak), and the same variables
    declared via declare_extern_var get another (extern).

    gcc/d/ChangeLog:

            PR d/99914
            * decl.cc (DeclVisitor::visit (StructDeclaration *)): Don't set
            DECL_INSTANTIATED on static initializer declarations.
            (DeclVisitor::visit (ClassDeclaration *)): Likewise.
            (DeclVisitor::visit (EnumDeclaration *)): Likewise.
            (d_finish_decl): Move call to set_linkage_for_decl to...
            (declare_extern_var): ...here.

[Bug d/99914] d: Template symbols not overridable by normal symbols

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914

--- Comment #4 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Weak declarations (both functions and variables) were found not to be working
at all on MinGW targets.  The only way that there desired behaviour can be
achieved there then is to mark *all* declarations with external linkage as
being DECL_ONE_ONLY, so they can override each other.

[Bug d/99914] d: Template symbols not overridable by normal symbols

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99914

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain Buclaw <ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:bda519596543e49f77914b5677693e86be5d01d0

commit r11-8234-gbda519596543e49f77914b5677693e86be5d01d0
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Tue Apr 13 22:28:55 2021 +0200

    d: Add TARGET_D_TEMPLATES_ALWAYS_COMDAT

    Following up on the fix for PR99914, when testing on MinGW, it was found
    not to support weak in the same way as on ELF or Mach-O targets.

    So the linkage has been reverted back to COMDAT for that target, however
    in order to properly support overriding functions and variables, all
    declarations with external linkage must be put on COMDAT.  For this a
    new target hook has been added to control the behavior.

    gcc/ChangeLog:

            PR d/99914
            * config/i386/winnt-d.c (TARGET_D_TEMPLATES_ALWAYS_COMDAT): Define.
            * doc/tm.texi: Regenerate.
            * doc/tm.texi.in (D language and ABI): Add @hook for
            TARGET_D_TEMPLATES_ALWAYS_COMDAT.

    gcc/d/ChangeLog:

            PR d/99914
            * d-target.def (d_templates_always_comdat): New hook.
            * d-tree.h (mark_needed): Remove prototype.
            * decl.cc: Include d-target.h.
            (mark_needed): Rename to...
            (d_mark_needed): ...this.  Make static.
            (set_linkage_for_decl): Put variables in comdat if
            d_templates_always_comdat.