public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Alexandre Oliva <aoliva@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc(refs/users/aoliva/heads/testme)] [C++] non-comdat-weakening of initialized data symbols
Date: Sun, 20 Nov 2022 03:49:23 +0000 (GMT)	[thread overview]
Message-ID: <20221120034923.828D93858C2C@sourceware.org> (raw)

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

commit bee77d3a72debc84b2340485a75fedc6187441b6
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Sun Nov 20 00:47:09 2022 -0300

    [C++] non-comdat-weakening of initialized data symbols
    
    When !SUPPORTS_ONE_ONLY, we implement vague linkage with weak and
    common symbols, but there are a few problems that make symbols that
    should get vague linkage end up as strong definitions, related with
    make_decl_one_only's checking DECL_INITIAL before it is set.
    
    * for typeinfo objects and strings, import_export_decl calls
      comdat_linkage while DECL_INITIAL is still NULL, so we set
      DECL_COMMON instead of DECL_WEAK.  Calling import_export_decl again
      after DECL_INITIAL is no use, for it remembers it's already run.
    
    * cp_finish_decl calls maybe_commonize_var before DECL_INITIAL is set,
      which may call comdat_linkage if flag_weak (why not otherwise?).
      maybe_commonize_var is called again after setting DECL_INITIAL, but
      only if !flag_weak, so we don't get a chance to revisit the decision
      not to set DECL_WEAK, and then _Sp_make_shared_tag::_S_ti's __tag
      static variable ends up multiply-defined.
    
    This patch temporarily sets DECL_INITIAL to a dummy value (NULL and
    error_mark_node won't do, all of maybe_commonize_var, comdat_linkage
    and make_one_decl_only test for both) when there is a known
    initializer, so that we decide against DECL_COMMON and for DECL_WEAK.
    I suspect this may move variables with dynamic initialization out of
    common section, but I'm having trouble seeing how to do better.
    
    ISTM that ideally we should have means to revisit
    maybe_commonize_var's, comdat_linkage's and make_decl_one_only's
    choices once DECL_INITIAL is set, but I'm not happy with the notion of
    duplicating so much very fragile code to that end.
    
    for  gcc/ChangeLog
    
            * rtti.cc (get_tinfo_decl_direct): Set DECL_INITIAL to a dummy
            value.
            (tinfo_base_init): Likewise.
            * decl.cc (cp_finish_decl): Set DECL_INITIAL to a dummy value
            before maybe_commonize_var.

Diff:
---
 gcc/cp/decl.cc | 11 ++++++++++-
 gcc/cp/rtti.cc |  6 ++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 544efdc9914..058bad9fc38 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8532,7 +8532,16 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
   if (VAR_OR_FUNCTION_DECL_P (decl))
     {
       if (VAR_P (decl))
-	maybe_commonize_var (decl);
+	{
+	  bool temp_init = !DECL_INITIAL (decl) && init;
+	  /* If DECL_INITIAL is set after import_export_decl, a symbol
+	     that should be weak won't be if !SUPPORTS_ONE_ONLY.  */
+	  if (temp_init)
+	    DECL_INITIAL (decl) = void_node;
+	  maybe_commonize_var (decl);
+	  if (temp_init)
+	    DECL_INITIAL (decl) = NULL_TREE;
+	}
       determine_visibility (decl);
     }
 
diff --git a/gcc/cp/rtti.cc b/gcc/cp/rtti.cc
index a85c7b56409..530027cc021 100644
--- a/gcc/cp/rtti.cc
+++ b/gcc/cp/rtti.cc
@@ -472,6 +472,9 @@ get_tinfo_decl_direct (tree type, tree name, int pseudo_ix)
       DECL_IGNORED_P (d) = 1;
       TREE_READONLY (d) = 1;
       TREE_STATIC (d) = 1;
+      /* Ensure DECL_INITIAL is set before deciding linkage.
+	 make_decl_one_only may fail to set DECL_WEAK otherwise.  */
+      DECL_INITIAL (d) = void_node;
       /* Tell equal_address_to that different tinfo decls never
 	 overlap.  */
       if (vec_safe_is_empty (unemitted_tinfo_decls))
@@ -954,6 +957,9 @@ tinfo_base_init (tinfo_s *ti, tree target)
     TREE_STATIC (name_decl) = 1;
     DECL_EXTERNAL (name_decl) = 0;
     DECL_TINFO_P (name_decl) = 1;
+      /* Ensure DECL_INITIAL is set before deciding linkage.
+	 make_decl_one_only may fail to set DECL_WEAK otherwise.  */
+    DECL_INITIAL (name_decl) = void_node;
     set_linkage_according_to_type (target, name_decl);
     import_export_decl (name_decl);
     name_string = tinfo_name (target, !TREE_PUBLIC (name_decl));

                 reply	other threads:[~2022-11-20  3:49 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221120034923.828D93858C2C@sourceware.org \
    --to=aoliva@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).