public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/users/aoliva/heads/testme)] Revisit non-one-only common/weakening of initialized variables
@ 2022-11-25  6:34 Alexandre Oliva
  0 siblings, 0 replies; 3+ messages in thread
From: Alexandre Oliva @ 2022-11-25  6:34 UTC (permalink / raw)
  To: gcc-cvs

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

commit ef29494547346e6a4682df78fb6d05b5d4bd5b7f
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed Nov 23 12:27:05 2022 -0300

    Revisit non-one-only common/weakening of initialized variables
    
    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 introduces a 'revisit' parameter to tell various functions
    involved in the implementation of vague linkage (marking variables
    one-only, common or weak), and adds calls after setting DECL_INITIAL
    that enables decisions based on DECL_INITIAL's being previously clear
    to be revisited.
    
    for  gcc/ChangeLog
    
            * varasm.h (make_decl_one_only): Add revisit parameter,
            defaulting to false.
            * varasm.cc (make_decl_one_only): When revisiting, skip
            MAKE_DECL_ONE_ONLY or clear DECL_COMMON when changing to
            DECL_WEAK.
    
    for  gcc/cp/ChangeLog
    
            * cp-tree.h (maybe_commonize_var): Add revisit parameter,
            defaulting to false.
            (maybe_make_one_only, comdat_linkage, import_export_decl):
            Likewise.
            * decl.cc (maybe_commonize_var): Pass revisit on to
            comdat_linkage.
            (cp_finish_decl): Call maybe_commonize_var with revisit after
            handling initializers, even when flag_weak is set.
            * decl2.cc (comdat_linkage): Pass revisit to
            make_decl_one_only.
            (maybe_make_one_only): Likewise.  Check settings that
            shouldn't change when revisiting.
            (import_export_decl): Likewise.  Allow revisiting of public
            symbols even when DECL_INTERFACE_KNOWN is already set.
            * rtti.cc (tinfo_base_init): Revisit import_export_decl after
            setting DECL_INITIAL.
            (emit_tinfo_decl): Likewise.

Diff:
---
 gcc/cp/cp-tree.h |  8 +++---
 gcc/cp/decl.cc   | 13 +++++----
 gcc/cp/decl2.cc  | 86 +++++++++++++++++++++++++++++++++++++++-----------------
 gcc/cp/rtti.cc   |  2 ++
 gcc/varasm.cc    | 13 +++++++--
 gcc/varasm.h     |  2 +-
 6 files changed, 85 insertions(+), 39 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 548b533266a..aa40418d816 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6907,7 +6907,7 @@ extern tree outer_curly_brace_block		(tree);
 extern tree finish_function			(bool);
 extern tree grokmethod				(cp_decl_specifier_seq *, const cp_declarator *, tree);
 extern void maybe_register_incomplete_var	(tree);
-extern void maybe_commonize_var			(tree);
+extern void maybe_commonize_var			(tree, bool = false);
 extern void complete_vars			(tree);
 extern tree static_fn_type			(tree);
 extern void revert_static_member_fn		(tree);
@@ -6955,7 +6955,7 @@ extern tree build_memfn_type			(tree, tree, cp_cv_quals, cp_ref_qualifier);
 extern tree build_pointer_ptrmemfn_type	(tree);
 extern tree change_return_type			(tree, tree);
 extern void maybe_retrofit_in_chrg		(tree);
-extern void maybe_make_one_only			(tree);
+extern void maybe_make_one_only			(tree, bool = false);
 extern bool vague_linkage_p			(tree);
 extern void grokclassfn				(tree, tree,
 						 enum overload_flags);
@@ -6978,12 +6978,12 @@ extern void finish_anon_union			(tree);
 extern void cxx_post_compilation_parsing_cleanups (void);
 extern tree coerce_new_type			(tree, location_t);
 extern void coerce_delete_type			(tree, location_t);
-extern void comdat_linkage			(tree);
+extern void comdat_linkage			(tree, bool = false);
 extern void determine_visibility		(tree);
 extern void constrain_class_visibility		(tree);
 extern void reset_type_linkage			(tree);
 extern void tentative_decl_linkage		(tree);
-extern void import_export_decl			(tree);
+extern void import_export_decl			(tree, bool = false);
 extern tree build_cleanup			(tree);
 extern tree build_offset_ref_call_from_tree	(tree, vec<tree, va_gc> **,
 						 tsubst_flags_t);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 544efdc9914..266e4d7306e 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6383,10 +6383,12 @@ layout_var_decl (tree decl)
 
 /* If a local static variable is declared in an inline function, or if
    we have a weak definition, we must endeavor to create only one
-   instance of the variable at link-time.  */
+   instance of the variable at link-time.  Sometimes we make a
+   tentative decision before setting DECL_INITIAL; after DECL_INITIAL
+   is set, this should be called again with REVISIT.  */
 
 void
-maybe_commonize_var (tree decl)
+maybe_commonize_var (tree decl, bool revisit)
 {
   /* Don't mess with __FUNCTION__ and similar.  */
   if (DECL_ARTIFICIAL (decl))
@@ -6404,7 +6406,7 @@ maybe_commonize_var (tree decl)
 	  /* With weak symbols, we simply make the variable COMDAT;
 	     that will cause copies in multiple translations units to
 	     be merged.  */
-	  comdat_linkage (decl);
+	  comdat_linkage (decl, revisit);
 	}
       else
 	{
@@ -8665,9 +8667,8 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       if (VAR_P (decl))
 	{
 	  layout_var_decl (decl);
-	  if (!flag_weak)
-	    /* Check again now that we have an initializer.  */
-	    maybe_commonize_var (decl);
+	  /* Check again now that we have an initializer.  */
+	  maybe_commonize_var (decl, /* revisit = */true);
 	  /* A class-scope constexpr variable with an out-of-class declaration.
 	     C++17 makes them implicitly inline, but still force it out.  */
 	  if (DECL_INLINE_VAR_P (decl)
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index f95529a5c9a..c7581922e4e 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2102,16 +2102,19 @@ adjust_var_decl_tls_model (tree decl)
     set_decl_tls_model (decl, decl_default_tls_model (decl));
 }
 
-/* Set DECL up to have the closest approximation of "initialized common"
-   linkage available.  */
+/* Set DECL up to have the closest approximation of "initialized
+   common" linkage available.  Sometimes we make a tentative decision
+   before setting DECL_INITIAL.  Once it's set, this should be called
+   with REVISIT.  */
 
 void
-comdat_linkage (tree decl)
+comdat_linkage (tree decl, bool revisit)
 {
   if (flag_weak)
     {
-      make_decl_one_only (decl, cxx_comdat_group (decl));
-      if (HAVE_COMDAT_GROUP && flag_contracts && DECL_CONTRACTS (decl))
+      make_decl_one_only (decl, cxx_comdat_group (decl), revisit);
+      if (HAVE_COMDAT_GROUP && flag_contracts && DECL_CONTRACTS (decl)
+	  && !revisit)
 	{
 	  symtab_node *n = symtab_node::get (decl);
 	  if (tree pre = DECL_PRE_FN (decl))
@@ -2174,7 +2177,7 @@ comdat_linkage (tree decl)
    linkonce.  */
 
 void
-maybe_make_one_only (tree decl)
+maybe_make_one_only (tree decl, bool revisit)
 {
   /* We used to say that this was not necessary on targets that support weak
      symbols, because the implicit instantiations will defer to the explicit
@@ -2185,6 +2188,16 @@ maybe_make_one_only (tree decl)
   if (! flag_weak)
     return;
 
+#define SET_OR_RECHECK(REVISIT, LVALUE, RVALUE)		\
+  do							\
+    {							\
+      if (REVISIT)					\
+	gcc_checking_assert ((LVALUE) == (RVALUE));	\
+      else						\
+	(LVALUE) = (RVALUE);				\
+    }							\
+  while (0)
+
   /* We can't set DECL_COMDAT on functions, or cp_finish_file will think
      we can get away with not emitting them if they aren't used.  We need
      to for variables so that cp_finish_decl will update their linkage,
@@ -2194,19 +2207,21 @@ maybe_make_one_only (tree decl)
       || (! DECL_EXPLICIT_INSTANTIATION (decl)
 	  && ! DECL_TEMPLATE_SPECIALIZATION (decl)))
     {
-      make_decl_one_only (decl, cxx_comdat_group (decl));
+      make_decl_one_only (decl, cxx_comdat_group (decl), revisit);
 
       if (VAR_P (decl))
 	{
 	  varpool_node *node = varpool_node::get_create (decl);
-	  DECL_COMDAT (decl) = 1;
+	  SET_OR_RECHECK (revisit, DECL_COMDAT (decl), 1);
 	  /* Mark it needed so we don't forget to emit it.  */
-          node->forced_by_abi = true;
-	  TREE_USED (decl) = 1;
+	  SET_OR_RECHECK (revisit, node->forced_by_abi, true);
+	  SET_OR_RECHECK (revisit, TREE_USED (decl), 1);
 
 	  adjust_var_decl_tls_model (decl);
 	}
     }
+
+#undef SET_OR_RECHECK
 }
 
 /* Returns true iff DECL, a FUNCTION_DECL or VAR_DECL, has vague linkage.
@@ -3186,13 +3201,19 @@ tentative_decl_linkage (tree decl)
    required.  */
 
 void
-import_export_decl (tree decl)
+import_export_decl (tree decl, bool revisit)
 {
   bool comdat_p;
   bool import_p;
   tree class_type = NULL_TREE;
 
-  if (DECL_INTERFACE_KNOWN (decl))
+  if (revisit)
+    {
+      gcc_checking_assert (DECL_INTERFACE_KNOWN (decl));
+      if (!TREE_PUBLIC (decl))
+	return;
+    }
+  else if (DECL_INTERFACE_KNOWN (decl))
     return;
 
   /* We cannot determine what linkage to give to an entity with vague
@@ -3244,6 +3265,16 @@ import_export_decl (tree decl)
      unit.  */
   import_p = false;
 
+#define SET_OR_RECHECK(REVISIT, LVALUE, RVALUE)		\
+  do							\
+    {							\
+      if (REVISIT)					\
+	gcc_checking_assert ((LVALUE) == (RVALUE));	\
+      else						\
+	(LVALUE) = (RVALUE);				\
+    }							\
+  while (0)
+
   if (VAR_P (decl) && DECL_VTABLE_OR_VTT_P (decl))
     {
       class_type = DECL_CONTEXT (decl);
@@ -3265,7 +3296,7 @@ import_export_decl (tree decl)
 	   in the case that we know that the virtual table will be
 	   emitted in only one translation unit, we make the virtual
 	   table an ordinary definition with external linkage.  */
-	DECL_EXTERNAL (decl) = 0;
+	SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
       else if (CLASSTYPE_INTERFACE_KNOWN (class_type))
 	{
 	  /* CLASS_TYPE is being exported from this translation unit,
@@ -3278,7 +3309,7 @@ import_export_decl (tree decl)
 	       one translation unit.  Therefore, the explicit
 	       instantiation must be made visible to other translation
 	       units.  */
-	    DECL_EXTERNAL (decl) = 0;
+	    SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
 	  else
 	    {
 	      /* The generic C++ ABI says that class data is always
@@ -3336,7 +3367,7 @@ import_export_decl (tree decl)
 		  if (!flag_weak)
 		    {
 		      comdat_p = false;
-		      DECL_EXTERNAL (decl) = 0;
+		      SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
 		    }
 		}
 	      else
@@ -3368,20 +3399,21 @@ import_export_decl (tree decl)
 	  import_export_class (ctype);
 	  if (CLASSTYPE_INTERFACE_KNOWN (ctype))
 	    {
-	      DECL_NOT_REALLY_EXTERN (decl)
-		= ! (CLASSTYPE_INTERFACE_ONLY (ctype)
-		     || (DECL_DECLARED_INLINE_P (decl)
-			 && ! flag_implement_inlines
-			 && !DECL_VINDEX (decl)));
+	      SET_OR_RECHECK (revisit,
+			    DECL_NOT_REALLY_EXTERN (decl),
+			    ! (CLASSTYPE_INTERFACE_ONLY (ctype)
+			       || (DECL_DECLARED_INLINE_P (decl)
+				   && ! flag_implement_inlines
+				   && !DECL_VINDEX (decl))));
 
 	      if (!DECL_NOT_REALLY_EXTERN (decl))
-		DECL_EXTERNAL (decl) = 1;
+		SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 1);
 
 	      /* Always make artificials weak.  */
 	      if (DECL_ARTIFICIAL (decl) && flag_weak)
 		comdat_p = true;
 	      else
-		maybe_make_one_only (decl);
+		maybe_make_one_only (decl, revisit);
 	    }
 	}
       else
@@ -3394,17 +3426,19 @@ import_export_decl (tree decl)
     {
       /* If we are importing DECL into this translation unit, mark is
 	 an undefined here.  */
-      DECL_EXTERNAL (decl) = 1;
-      DECL_NOT_REALLY_EXTERN (decl) = 0;
+      SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 1);
+      SET_OR_RECHECK (revisit, DECL_NOT_REALLY_EXTERN (decl), 0);
     }
   else if (comdat_p)
     {
       /* If we decided to put DECL in COMDAT, mark it accordingly at
 	 this point.  */
-      comdat_linkage (decl);
+      comdat_linkage (decl, revisit);
     }
 
-  DECL_INTERFACE_KNOWN (decl) = 1;
+  SET_OR_RECHECK (revisit, DECL_INTERFACE_KNOWN (decl), 1);
+
+#undef SET_OR_RECHECK
 }
 
 /* Return an expression that performs the destruction of DECL, which
diff --git a/gcc/cp/rtti.cc b/gcc/cp/rtti.cc
index a85c7b56409..4527fe971f2 100644
--- a/gcc/cp/rtti.cc
+++ b/gcc/cp/rtti.cc
@@ -958,6 +958,7 @@ tinfo_base_init (tinfo_s *ti, tree target)
     import_export_decl (name_decl);
     name_string = tinfo_name (target, !TREE_PUBLIC (name_decl));
     DECL_INITIAL (name_decl) = name_string;
+    import_export_decl (name_decl, /* revisit = */true);
     mark_used (name_decl);
     pushdecl_top_level_and_finish (name_decl, name_string);
   }
@@ -1706,6 +1707,7 @@ emit_tinfo_decl (tree decl)
 	}
       init = get_pseudo_ti_init (type, pseudo_ix);
       DECL_INITIAL (decl) = init;
+      import_export_decl (decl, /* revisit = */true);
       mark_used (decl);
       cp_finish_decl (decl, init, false, NULL_TREE, 0);
       /* Avoid targets optionally bumping up the alignment to improve
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index d0beac8f8e3..c28e76886ea 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6552,10 +6552,13 @@ supports_one_only (void)
 }
 
 /* Set up DECL as a public symbol that can be defined in multiple
-   translation units without generating a linker error.  */
+   translation units without generating a linker error.  Sometimes
+   this function is called before setting DECL_INITIAL, and that may
+   cause the variable to get DECL_COMMON set if !SUPPORTS_ONE_ONLY.
+   After setting DECL_INITIAL, this should be called with REVISIT.  */
 
 void
-make_decl_one_only (tree decl, tree comdat_group)
+make_decl_one_only (tree decl, tree comdat_group, bool revisit)
 {
   struct symtab_node *symbol;
   gcc_assert (VAR_OR_FUNCTION_DECL_P (decl));
@@ -6569,6 +6572,10 @@ make_decl_one_only (tree decl, tree comdat_group)
 
   if (SUPPORTS_ONE_ONLY)
     {
+      if (revisit)
+	/* ??? Do we need a MAKE_DECL_ONE_ONLY_REVISIT?  */
+	return;
+
 #ifdef MAKE_DECL_ONE_ONLY
       MAKE_DECL_ONE_ONLY (decl);
 #endif
@@ -6582,6 +6589,8 @@ make_decl_one_only (tree decl, tree comdat_group)
     {
       gcc_assert (TARGET_SUPPORTS_WEAK);
       DECL_WEAK (decl) = 1;
+      if (revisit)
+	DECL_COMMON (decl) = 0;
     }
 }
 
diff --git a/gcc/varasm.h b/gcc/varasm.h
index acbd9fab7f6..8c83fd6974d 100644
--- a/gcc/varasm.h
+++ b/gcc/varasm.h
@@ -30,7 +30,7 @@ extern tree cold_function_name;
 extern tree tree_output_constant_def (tree);
 extern void make_decl_rtl (tree);
 extern rtx make_decl_rtl_for_debug (tree);
-extern void make_decl_one_only (tree, tree);
+extern void make_decl_one_only (tree, tree, bool = false);
 extern int supports_one_only (void);
 extern void resolve_unique_section (tree, int, int);
 extern void mark_referenced (tree);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] Revisit non-one-only common/weakening of initialized variables
@ 2022-11-25  3:56 Alexandre Oliva
  0 siblings, 0 replies; 3+ messages in thread
From: Alexandre Oliva @ 2022-11-25  3:56 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:10e8743d60035323e4124afdf9e4fb64c46e86bf

commit 10e8743d60035323e4124afdf9e4fb64c46e86bf
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed Nov 23 12:27:05 2022 -0300

    Revisit non-one-only common/weakening of initialized variables
    
    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 introduces a 'revisit' parameter to tell various functions
    involved in the implementation of vague linkage (marking variables
    one-only, common or weak), and adds calls after setting DECL_INITIAL
    that enables decisions based on DECL_INITIAL's being previously clear
    to be revisited.
    
    for  gcc/ChangeLog
    
            * varasm.h (make_decl_one_only): Add revisit parameter,
            defaulting to false.
            * varasm.cc (make_decl_one_only): When revisiting, skip
            MAKE_DECL_ONE_ONLY or clear DECL_COMMON when changing to
            DECL_WEAK.
    
    for  gcc/cp/ChangeLog
    
            * cp-tree.h (maybe_commonize_var): Add revisit parameter,
            defaulting to false.
            (maybe_make_one_only, comdat_linkage, import_export_decl):
            Likewise.
            * decl.cc (maybe_commonize_var): Pass revisit on to
            comdat_linkage.
            (cp_finish_decl): Call maybe_commonize_var with revisit after
            handling initializers, even when flag_weak is set.
            * decl2.cc (comdat_linkage): Pass revisit to
            make_decl_one_only.
            (maybe_make_one_only): Likewise.  Check settings that
            shouldn't change when revisiting.
            (import_export_decl): Likewise.  Allow revisiting of public
            symbols even when DECL_INTERFACE_KNOWN is already set.
            * rtti.cc (tinfo_base_init): Revisit import_export_decl after
            setting DECL_INITIAL.
            (emit_tinfo_decl): Likewise.

Diff:
---
 gcc/cp/cp-tree.h |  8 +++---
 gcc/cp/decl.cc   | 13 +++++----
 gcc/cp/decl2.cc  | 83 +++++++++++++++++++++++++++++++++++++++-----------------
 gcc/cp/rtti.cc   |  2 ++
 gcc/varasm.cc    | 13 +++++++--
 gcc/varasm.h     |  2 +-
 6 files changed, 83 insertions(+), 38 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 548b533266a..aa40418d816 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6907,7 +6907,7 @@ extern tree outer_curly_brace_block		(tree);
 extern tree finish_function			(bool);
 extern tree grokmethod				(cp_decl_specifier_seq *, const cp_declarator *, tree);
 extern void maybe_register_incomplete_var	(tree);
-extern void maybe_commonize_var			(tree);
+extern void maybe_commonize_var			(tree, bool = false);
 extern void complete_vars			(tree);
 extern tree static_fn_type			(tree);
 extern void revert_static_member_fn		(tree);
@@ -6955,7 +6955,7 @@ extern tree build_memfn_type			(tree, tree, cp_cv_quals, cp_ref_qualifier);
 extern tree build_pointer_ptrmemfn_type	(tree);
 extern tree change_return_type			(tree, tree);
 extern void maybe_retrofit_in_chrg		(tree);
-extern void maybe_make_one_only			(tree);
+extern void maybe_make_one_only			(tree, bool = false);
 extern bool vague_linkage_p			(tree);
 extern void grokclassfn				(tree, tree,
 						 enum overload_flags);
@@ -6978,12 +6978,12 @@ extern void finish_anon_union			(tree);
 extern void cxx_post_compilation_parsing_cleanups (void);
 extern tree coerce_new_type			(tree, location_t);
 extern void coerce_delete_type			(tree, location_t);
-extern void comdat_linkage			(tree);
+extern void comdat_linkage			(tree, bool = false);
 extern void determine_visibility		(tree);
 extern void constrain_class_visibility		(tree);
 extern void reset_type_linkage			(tree);
 extern void tentative_decl_linkage		(tree);
-extern void import_export_decl			(tree);
+extern void import_export_decl			(tree, bool = false);
 extern tree build_cleanup			(tree);
 extern tree build_offset_ref_call_from_tree	(tree, vec<tree, va_gc> **,
 						 tsubst_flags_t);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 544efdc9914..266e4d7306e 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6383,10 +6383,12 @@ layout_var_decl (tree decl)
 
 /* If a local static variable is declared in an inline function, or if
    we have a weak definition, we must endeavor to create only one
-   instance of the variable at link-time.  */
+   instance of the variable at link-time.  Sometimes we make a
+   tentative decision before setting DECL_INITIAL; after DECL_INITIAL
+   is set, this should be called again with REVISIT.  */
 
 void
-maybe_commonize_var (tree decl)
+maybe_commonize_var (tree decl, bool revisit)
 {
   /* Don't mess with __FUNCTION__ and similar.  */
   if (DECL_ARTIFICIAL (decl))
@@ -6404,7 +6406,7 @@ maybe_commonize_var (tree decl)
 	  /* With weak symbols, we simply make the variable COMDAT;
 	     that will cause copies in multiple translations units to
 	     be merged.  */
-	  comdat_linkage (decl);
+	  comdat_linkage (decl, revisit);
 	}
       else
 	{
@@ -8665,9 +8667,8 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       if (VAR_P (decl))
 	{
 	  layout_var_decl (decl);
-	  if (!flag_weak)
-	    /* Check again now that we have an initializer.  */
-	    maybe_commonize_var (decl);
+	  /* Check again now that we have an initializer.  */
+	  maybe_commonize_var (decl, /* revisit = */true);
 	  /* A class-scope constexpr variable with an out-of-class declaration.
 	     C++17 makes them implicitly inline, but still force it out.  */
 	  if (DECL_INLINE_VAR_P (decl)
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index f95529a5c9a..37fb06e0992 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2102,15 +2102,17 @@ adjust_var_decl_tls_model (tree decl)
     set_decl_tls_model (decl, decl_default_tls_model (decl));
 }
 
-/* Set DECL up to have the closest approximation of "initialized common"
-   linkage available.  */
+/* Set DECL up to have the closest approximation of "initialized
+   common" linkage available.  Sometimes we make a tentative decision
+   before setting DECL_INITIAL.  Once it's set, this should be called
+   with REVISIT.  */
 
 void
-comdat_linkage (tree decl)
+comdat_linkage (tree decl, bool revisit)
 {
   if (flag_weak)
     {
-      make_decl_one_only (decl, cxx_comdat_group (decl));
+      make_decl_one_only (decl, cxx_comdat_group (decl), revisit);
       if (HAVE_COMDAT_GROUP && flag_contracts && DECL_CONTRACTS (decl))
 	{
 	  symtab_node *n = symtab_node::get (decl);
@@ -2174,7 +2176,7 @@ comdat_linkage (tree decl)
    linkonce.  */
 
 void
-maybe_make_one_only (tree decl)
+maybe_make_one_only (tree decl, bool revisit)
 {
   /* We used to say that this was not necessary on targets that support weak
      symbols, because the implicit instantiations will defer to the explicit
@@ -2185,6 +2187,16 @@ maybe_make_one_only (tree decl)
   if (! flag_weak)
     return;
 
+#define SET_OR_RECHECK(REVISIT, LVALUE, RVALUE)		\
+  do							\
+    {							\
+      if (REVISIT)					\
+	gcc_checking_assert ((LVALUE) == (RVALUE));	\
+      else						\
+	(LVALUE) = (RVALUE);				\
+    }							\
+  while (0)
+
   /* We can't set DECL_COMDAT on functions, or cp_finish_file will think
      we can get away with not emitting them if they aren't used.  We need
      to for variables so that cp_finish_decl will update their linkage,
@@ -2194,19 +2206,21 @@ maybe_make_one_only (tree decl)
       || (! DECL_EXPLICIT_INSTANTIATION (decl)
 	  && ! DECL_TEMPLATE_SPECIALIZATION (decl)))
     {
-      make_decl_one_only (decl, cxx_comdat_group (decl));
+      make_decl_one_only (decl, cxx_comdat_group (decl), revisit);
 
       if (VAR_P (decl))
 	{
 	  varpool_node *node = varpool_node::get_create (decl);
-	  DECL_COMDAT (decl) = 1;
+	  SET_OR_RECHECK (revisit, DECL_COMDAT (decl), 1);
 	  /* Mark it needed so we don't forget to emit it.  */
-          node->forced_by_abi = true;
-	  TREE_USED (decl) = 1;
+	  SET_OR_RECHECK (revisit, node->forced_by_abi, true);
+	  SET_OR_RECHECK (revisit, TREE_USED (decl), 1);
 
 	  adjust_var_decl_tls_model (decl);
 	}
     }
+
+#undef SET_OR_RECHECK
 }
 
 /* Returns true iff DECL, a FUNCTION_DECL or VAR_DECL, has vague linkage.
@@ -3186,13 +3200,19 @@ tentative_decl_linkage (tree decl)
    required.  */
 
 void
-import_export_decl (tree decl)
+import_export_decl (tree decl, bool revisit)
 {
   bool comdat_p;
   bool import_p;
   tree class_type = NULL_TREE;
 
-  if (DECL_INTERFACE_KNOWN (decl))
+  if (revisit)
+    {
+      gcc_checking_assert (DECL_INTERFACE_KNOWN (decl));
+      if (!TREE_PUBLIC (decl))
+	return;
+    }
+  else if (DECL_INTERFACE_KNOWN (decl))
     return;
 
   /* We cannot determine what linkage to give to an entity with vague
@@ -3244,6 +3264,16 @@ import_export_decl (tree decl)
      unit.  */
   import_p = false;
 
+#define SET_OR_RECHECK(REVISIT, LVALUE, RVALUE)		\
+  do							\
+    {							\
+      if (REVISIT)					\
+	gcc_checking_assert ((LVALUE) == (RVALUE));	\
+      else						\
+	(LVALUE) = (RVALUE);				\
+    }							\
+  while (0)
+
   if (VAR_P (decl) && DECL_VTABLE_OR_VTT_P (decl))
     {
       class_type = DECL_CONTEXT (decl);
@@ -3265,7 +3295,7 @@ import_export_decl (tree decl)
 	   in the case that we know that the virtual table will be
 	   emitted in only one translation unit, we make the virtual
 	   table an ordinary definition with external linkage.  */
-	DECL_EXTERNAL (decl) = 0;
+	SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
       else if (CLASSTYPE_INTERFACE_KNOWN (class_type))
 	{
 	  /* CLASS_TYPE is being exported from this translation unit,
@@ -3278,7 +3308,7 @@ import_export_decl (tree decl)
 	       one translation unit.  Therefore, the explicit
 	       instantiation must be made visible to other translation
 	       units.  */
-	    DECL_EXTERNAL (decl) = 0;
+	    SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
 	  else
 	    {
 	      /* The generic C++ ABI says that class data is always
@@ -3336,7 +3366,7 @@ import_export_decl (tree decl)
 		  if (!flag_weak)
 		    {
 		      comdat_p = false;
-		      DECL_EXTERNAL (decl) = 0;
+		      SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
 		    }
 		}
 	      else
@@ -3368,20 +3398,21 @@ import_export_decl (tree decl)
 	  import_export_class (ctype);
 	  if (CLASSTYPE_INTERFACE_KNOWN (ctype))
 	    {
-	      DECL_NOT_REALLY_EXTERN (decl)
-		= ! (CLASSTYPE_INTERFACE_ONLY (ctype)
-		     || (DECL_DECLARED_INLINE_P (decl)
-			 && ! flag_implement_inlines
-			 && !DECL_VINDEX (decl)));
+	      SET_OR_RECHECK (revisit,
+			    DECL_NOT_REALLY_EXTERN (decl),
+			    ! (CLASSTYPE_INTERFACE_ONLY (ctype)
+			       || (DECL_DECLARED_INLINE_P (decl)
+				   && ! flag_implement_inlines
+				   && !DECL_VINDEX (decl))));
 
 	      if (!DECL_NOT_REALLY_EXTERN (decl))
-		DECL_EXTERNAL (decl) = 1;
+		SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 1);
 
 	      /* Always make artificials weak.  */
 	      if (DECL_ARTIFICIAL (decl) && flag_weak)
 		comdat_p = true;
 	      else
-		maybe_make_one_only (decl);
+		maybe_make_one_only (decl, revisit);
 	    }
 	}
       else
@@ -3394,17 +3425,19 @@ import_export_decl (tree decl)
     {
       /* If we are importing DECL into this translation unit, mark is
 	 an undefined here.  */
-      DECL_EXTERNAL (decl) = 1;
-      DECL_NOT_REALLY_EXTERN (decl) = 0;
+      SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 1);
+      SET_OR_RECHECK (revisit, DECL_NOT_REALLY_EXTERN (decl), 0);
     }
   else if (comdat_p)
     {
       /* If we decided to put DECL in COMDAT, mark it accordingly at
 	 this point.  */
-      comdat_linkage (decl);
+      comdat_linkage (decl, revisit);
     }
 
-  DECL_INTERFACE_KNOWN (decl) = 1;
+  SET_OR_RECHECK (revisit, DECL_INTERFACE_KNOWN (decl), 1);
+
+#undef SET_OR_RECHECK
 }
 
 /* Return an expression that performs the destruction of DECL, which
diff --git a/gcc/cp/rtti.cc b/gcc/cp/rtti.cc
index a85c7b56409..4527fe971f2 100644
--- a/gcc/cp/rtti.cc
+++ b/gcc/cp/rtti.cc
@@ -958,6 +958,7 @@ tinfo_base_init (tinfo_s *ti, tree target)
     import_export_decl (name_decl);
     name_string = tinfo_name (target, !TREE_PUBLIC (name_decl));
     DECL_INITIAL (name_decl) = name_string;
+    import_export_decl (name_decl, /* revisit = */true);
     mark_used (name_decl);
     pushdecl_top_level_and_finish (name_decl, name_string);
   }
@@ -1706,6 +1707,7 @@ emit_tinfo_decl (tree decl)
 	}
       init = get_pseudo_ti_init (type, pseudo_ix);
       DECL_INITIAL (decl) = init;
+      import_export_decl (decl, /* revisit = */true);
       mark_used (decl);
       cp_finish_decl (decl, init, false, NULL_TREE, 0);
       /* Avoid targets optionally bumping up the alignment to improve
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index d0beac8f8e3..c28e76886ea 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6552,10 +6552,13 @@ supports_one_only (void)
 }
 
 /* Set up DECL as a public symbol that can be defined in multiple
-   translation units without generating a linker error.  */
+   translation units without generating a linker error.  Sometimes
+   this function is called before setting DECL_INITIAL, and that may
+   cause the variable to get DECL_COMMON set if !SUPPORTS_ONE_ONLY.
+   After setting DECL_INITIAL, this should be called with REVISIT.  */
 
 void
-make_decl_one_only (tree decl, tree comdat_group)
+make_decl_one_only (tree decl, tree comdat_group, bool revisit)
 {
   struct symtab_node *symbol;
   gcc_assert (VAR_OR_FUNCTION_DECL_P (decl));
@@ -6569,6 +6572,10 @@ make_decl_one_only (tree decl, tree comdat_group)
 
   if (SUPPORTS_ONE_ONLY)
     {
+      if (revisit)
+	/* ??? Do we need a MAKE_DECL_ONE_ONLY_REVISIT?  */
+	return;
+
 #ifdef MAKE_DECL_ONE_ONLY
       MAKE_DECL_ONE_ONLY (decl);
 #endif
@@ -6582,6 +6589,8 @@ make_decl_one_only (tree decl, tree comdat_group)
     {
       gcc_assert (TARGET_SUPPORTS_WEAK);
       DECL_WEAK (decl) = 1;
+      if (revisit)
+	DECL_COMMON (decl) = 0;
     }
 }
 
diff --git a/gcc/varasm.h b/gcc/varasm.h
index acbd9fab7f6..8c83fd6974d 100644
--- a/gcc/varasm.h
+++ b/gcc/varasm.h
@@ -30,7 +30,7 @@ extern tree cold_function_name;
 extern tree tree_output_constant_def (tree);
 extern void make_decl_rtl (tree);
 extern rtx make_decl_rtl_for_debug (tree);
-extern void make_decl_one_only (tree, tree);
+extern void make_decl_one_only (tree, tree, bool = false);
 extern int supports_one_only (void);
 extern void resolve_unique_section (tree, int, int);
 extern void mark_referenced (tree);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [gcc(refs/users/aoliva/heads/testme)] Revisit non-one-only common/weakening of initialized variables
@ 2022-11-23 19:52 Alexandre Oliva
  0 siblings, 0 replies; 3+ messages in thread
From: Alexandre Oliva @ 2022-11-23 19:52 UTC (permalink / raw)
  To: gcc-cvs

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

commit 5fc5413985a09c26d2213143a7f68dd7723983e3
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed Nov 23 12:27:05 2022 -0300

    Revisit non-one-only common/weakening of initialized variables
    
    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 introduces a 'revisit' parameter to tell various functions
    involved in the implementation of vague linkage (marking variables
    one-only, common or weak), and adds calls after setting DECL_INITIAL
    that enables decisions based on DECL_INITIAL's being previously clear
    to be revisited.
    
    for  gcc/ChangeLog
    
            * varasm.h (make_decl_one_only): Add revisit parameter,
            defaulting to false.
            * varasm.cc (make_decl_one_only): When revisiting, skip
            MAKE_DECL_ONE_ONLY or clear DECL_COMMON when changing to
            DECL_WEAK.
    
    for  gcc/cp/ChangeLog
    
            * cp-tree.h (maybe_commonize_var): Add revisit parameter,
            defaulting to false.
            (maybe_make_one_only, comdat_linkage, import_export_decl):
            Likewise.
            * decl.cc (maybe_commonize_var): Pass revisit on to
            comdat_linkage.
            (cp_finish_decl): Call maybe_commonize_var with revisit after
            handling initializers, even when flag_weak is set.
            * decl2.cc (comdat_linkage): Pass revisit to
            make_decl_one_only.
            (maybe_make_one_only): Likewise.  Check settings that
            shouldn't change when revisiting.
            (import_export_decl): Likewise.  Allow revisiting of public
            symbols even when DECL_INTERFACE_KNOWN is already set.
            * rtti.cc (tinfo_base_init): Revisit import_export_decl after
            setting DECL_INITIAL.
            (emit_tinfo_decl): Likewise.

Diff:
---
 gcc/cp/cp-tree.h |  8 +++---
 gcc/cp/decl.cc   | 13 +++++----
 gcc/cp/decl2.cc  | 83 +++++++++++++++++++++++++++++++++++++++-----------------
 gcc/cp/rtti.cc   |  2 ++
 gcc/varasm.cc    | 13 +++++++--
 gcc/varasm.h     |  2 +-
 6 files changed, 83 insertions(+), 38 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 548b533266a..aa40418d816 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6907,7 +6907,7 @@ extern tree outer_curly_brace_block		(tree);
 extern tree finish_function			(bool);
 extern tree grokmethod				(cp_decl_specifier_seq *, const cp_declarator *, tree);
 extern void maybe_register_incomplete_var	(tree);
-extern void maybe_commonize_var			(tree);
+extern void maybe_commonize_var			(tree, bool = false);
 extern void complete_vars			(tree);
 extern tree static_fn_type			(tree);
 extern void revert_static_member_fn		(tree);
@@ -6955,7 +6955,7 @@ extern tree build_memfn_type			(tree, tree, cp_cv_quals, cp_ref_qualifier);
 extern tree build_pointer_ptrmemfn_type	(tree);
 extern tree change_return_type			(tree, tree);
 extern void maybe_retrofit_in_chrg		(tree);
-extern void maybe_make_one_only			(tree);
+extern void maybe_make_one_only			(tree, bool = false);
 extern bool vague_linkage_p			(tree);
 extern void grokclassfn				(tree, tree,
 						 enum overload_flags);
@@ -6978,12 +6978,12 @@ extern void finish_anon_union			(tree);
 extern void cxx_post_compilation_parsing_cleanups (void);
 extern tree coerce_new_type			(tree, location_t);
 extern void coerce_delete_type			(tree, location_t);
-extern void comdat_linkage			(tree);
+extern void comdat_linkage			(tree, bool = false);
 extern void determine_visibility		(tree);
 extern void constrain_class_visibility		(tree);
 extern void reset_type_linkage			(tree);
 extern void tentative_decl_linkage		(tree);
-extern void import_export_decl			(tree);
+extern void import_export_decl			(tree, bool = false);
 extern tree build_cleanup			(tree);
 extern tree build_offset_ref_call_from_tree	(tree, vec<tree, va_gc> **,
 						 tsubst_flags_t);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 544efdc9914..266e4d7306e 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6383,10 +6383,12 @@ layout_var_decl (tree decl)
 
 /* If a local static variable is declared in an inline function, or if
    we have a weak definition, we must endeavor to create only one
-   instance of the variable at link-time.  */
+   instance of the variable at link-time.  Sometimes we make a
+   tentative decision before setting DECL_INITIAL; after DECL_INITIAL
+   is set, this should be called again with REVISIT.  */
 
 void
-maybe_commonize_var (tree decl)
+maybe_commonize_var (tree decl, bool revisit)
 {
   /* Don't mess with __FUNCTION__ and similar.  */
   if (DECL_ARTIFICIAL (decl))
@@ -6404,7 +6406,7 @@ maybe_commonize_var (tree decl)
 	  /* With weak symbols, we simply make the variable COMDAT;
 	     that will cause copies in multiple translations units to
 	     be merged.  */
-	  comdat_linkage (decl);
+	  comdat_linkage (decl, revisit);
 	}
       else
 	{
@@ -8665,9 +8667,8 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       if (VAR_P (decl))
 	{
 	  layout_var_decl (decl);
-	  if (!flag_weak)
-	    /* Check again now that we have an initializer.  */
-	    maybe_commonize_var (decl);
+	  /* Check again now that we have an initializer.  */
+	  maybe_commonize_var (decl, /* revisit = */true);
 	  /* A class-scope constexpr variable with an out-of-class declaration.
 	     C++17 makes them implicitly inline, but still force it out.  */
 	  if (DECL_INLINE_VAR_P (decl)
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index f95529a5c9a..37fb06e0992 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2102,15 +2102,17 @@ adjust_var_decl_tls_model (tree decl)
     set_decl_tls_model (decl, decl_default_tls_model (decl));
 }
 
-/* Set DECL up to have the closest approximation of "initialized common"
-   linkage available.  */
+/* Set DECL up to have the closest approximation of "initialized
+   common" linkage available.  Sometimes we make a tentative decision
+   before setting DECL_INITIAL.  Once it's set, this should be called
+   with REVISIT.  */
 
 void
-comdat_linkage (tree decl)
+comdat_linkage (tree decl, bool revisit)
 {
   if (flag_weak)
     {
-      make_decl_one_only (decl, cxx_comdat_group (decl));
+      make_decl_one_only (decl, cxx_comdat_group (decl), revisit);
       if (HAVE_COMDAT_GROUP && flag_contracts && DECL_CONTRACTS (decl))
 	{
 	  symtab_node *n = symtab_node::get (decl);
@@ -2174,7 +2176,7 @@ comdat_linkage (tree decl)
    linkonce.  */
 
 void
-maybe_make_one_only (tree decl)
+maybe_make_one_only (tree decl, bool revisit)
 {
   /* We used to say that this was not necessary on targets that support weak
      symbols, because the implicit instantiations will defer to the explicit
@@ -2185,6 +2187,16 @@ maybe_make_one_only (tree decl)
   if (! flag_weak)
     return;
 
+#define SET_OR_RECHECK(REVISIT, LVALUE, RVALUE)		\
+  do							\
+    {							\
+      if (REVISIT)					\
+	gcc_checking_assert ((LVALUE) == (RVALUE));	\
+      else						\
+	(LVALUE) = (RVALUE);				\
+    }							\
+  while (0)
+
   /* We can't set DECL_COMDAT on functions, or cp_finish_file will think
      we can get away with not emitting them if they aren't used.  We need
      to for variables so that cp_finish_decl will update their linkage,
@@ -2194,19 +2206,21 @@ maybe_make_one_only (tree decl)
       || (! DECL_EXPLICIT_INSTANTIATION (decl)
 	  && ! DECL_TEMPLATE_SPECIALIZATION (decl)))
     {
-      make_decl_one_only (decl, cxx_comdat_group (decl));
+      make_decl_one_only (decl, cxx_comdat_group (decl), revisit);
 
       if (VAR_P (decl))
 	{
 	  varpool_node *node = varpool_node::get_create (decl);
-	  DECL_COMDAT (decl) = 1;
+	  SET_OR_RECHECK (revisit, DECL_COMDAT (decl), 1);
 	  /* Mark it needed so we don't forget to emit it.  */
-          node->forced_by_abi = true;
-	  TREE_USED (decl) = 1;
+	  SET_OR_RECHECK (revisit, node->forced_by_abi, true);
+	  SET_OR_RECHECK (revisit, TREE_USED (decl), 1);
 
 	  adjust_var_decl_tls_model (decl);
 	}
     }
+
+#undef SET_OR_RECHECK
 }
 
 /* Returns true iff DECL, a FUNCTION_DECL or VAR_DECL, has vague linkage.
@@ -3186,13 +3200,19 @@ tentative_decl_linkage (tree decl)
    required.  */
 
 void
-import_export_decl (tree decl)
+import_export_decl (tree decl, bool revisit)
 {
   bool comdat_p;
   bool import_p;
   tree class_type = NULL_TREE;
 
-  if (DECL_INTERFACE_KNOWN (decl))
+  if (revisit)
+    {
+      gcc_checking_assert (DECL_INTERFACE_KNOWN (decl));
+      if (!TREE_PUBLIC (decl))
+	return;
+    }
+  else if (DECL_INTERFACE_KNOWN (decl))
     return;
 
   /* We cannot determine what linkage to give to an entity with vague
@@ -3244,6 +3264,16 @@ import_export_decl (tree decl)
      unit.  */
   import_p = false;
 
+#define SET_OR_RECHECK(REVISIT, LVALUE, RVALUE)		\
+  do							\
+    {							\
+      if (REVISIT)					\
+	gcc_checking_assert ((LVALUE) == (RVALUE));	\
+      else						\
+	(LVALUE) = (RVALUE);				\
+    }							\
+  while (0)
+
   if (VAR_P (decl) && DECL_VTABLE_OR_VTT_P (decl))
     {
       class_type = DECL_CONTEXT (decl);
@@ -3265,7 +3295,7 @@ import_export_decl (tree decl)
 	   in the case that we know that the virtual table will be
 	   emitted in only one translation unit, we make the virtual
 	   table an ordinary definition with external linkage.  */
-	DECL_EXTERNAL (decl) = 0;
+	SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
       else if (CLASSTYPE_INTERFACE_KNOWN (class_type))
 	{
 	  /* CLASS_TYPE is being exported from this translation unit,
@@ -3278,7 +3308,7 @@ import_export_decl (tree decl)
 	       one translation unit.  Therefore, the explicit
 	       instantiation must be made visible to other translation
 	       units.  */
-	    DECL_EXTERNAL (decl) = 0;
+	    SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
 	  else
 	    {
 	      /* The generic C++ ABI says that class data is always
@@ -3336,7 +3366,7 @@ import_export_decl (tree decl)
 		  if (!flag_weak)
 		    {
 		      comdat_p = false;
-		      DECL_EXTERNAL (decl) = 0;
+		      SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 0);
 		    }
 		}
 	      else
@@ -3368,20 +3398,21 @@ import_export_decl (tree decl)
 	  import_export_class (ctype);
 	  if (CLASSTYPE_INTERFACE_KNOWN (ctype))
 	    {
-	      DECL_NOT_REALLY_EXTERN (decl)
-		= ! (CLASSTYPE_INTERFACE_ONLY (ctype)
-		     || (DECL_DECLARED_INLINE_P (decl)
-			 && ! flag_implement_inlines
-			 && !DECL_VINDEX (decl)));
+	      SET_OR_RECHECK (revisit,
+			    DECL_NOT_REALLY_EXTERN (decl),
+			    ! (CLASSTYPE_INTERFACE_ONLY (ctype)
+			       || (DECL_DECLARED_INLINE_P (decl)
+				   && ! flag_implement_inlines
+				   && !DECL_VINDEX (decl))));
 
 	      if (!DECL_NOT_REALLY_EXTERN (decl))
-		DECL_EXTERNAL (decl) = 1;
+		SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 1);
 
 	      /* Always make artificials weak.  */
 	      if (DECL_ARTIFICIAL (decl) && flag_weak)
 		comdat_p = true;
 	      else
-		maybe_make_one_only (decl);
+		maybe_make_one_only (decl, revisit);
 	    }
 	}
       else
@@ -3394,17 +3425,19 @@ import_export_decl (tree decl)
     {
       /* If we are importing DECL into this translation unit, mark is
 	 an undefined here.  */
-      DECL_EXTERNAL (decl) = 1;
-      DECL_NOT_REALLY_EXTERN (decl) = 0;
+      SET_OR_RECHECK (revisit, DECL_EXTERNAL (decl), 1);
+      SET_OR_RECHECK (revisit, DECL_NOT_REALLY_EXTERN (decl), 0);
     }
   else if (comdat_p)
     {
       /* If we decided to put DECL in COMDAT, mark it accordingly at
 	 this point.  */
-      comdat_linkage (decl);
+      comdat_linkage (decl, revisit);
     }
 
-  DECL_INTERFACE_KNOWN (decl) = 1;
+  SET_OR_RECHECK (revisit, DECL_INTERFACE_KNOWN (decl), 1);
+
+#undef SET_OR_RECHECK
 }
 
 /* Return an expression that performs the destruction of DECL, which
diff --git a/gcc/cp/rtti.cc b/gcc/cp/rtti.cc
index a85c7b56409..4527fe971f2 100644
--- a/gcc/cp/rtti.cc
+++ b/gcc/cp/rtti.cc
@@ -958,6 +958,7 @@ tinfo_base_init (tinfo_s *ti, tree target)
     import_export_decl (name_decl);
     name_string = tinfo_name (target, !TREE_PUBLIC (name_decl));
     DECL_INITIAL (name_decl) = name_string;
+    import_export_decl (name_decl, /* revisit = */true);
     mark_used (name_decl);
     pushdecl_top_level_and_finish (name_decl, name_string);
   }
@@ -1706,6 +1707,7 @@ emit_tinfo_decl (tree decl)
 	}
       init = get_pseudo_ti_init (type, pseudo_ix);
       DECL_INITIAL (decl) = init;
+      import_export_decl (decl, /* revisit = */true);
       mark_used (decl);
       cp_finish_decl (decl, init, false, NULL_TREE, 0);
       /* Avoid targets optionally bumping up the alignment to improve
diff --git a/gcc/varasm.cc b/gcc/varasm.cc
index d0beac8f8e3..c28e76886ea 100644
--- a/gcc/varasm.cc
+++ b/gcc/varasm.cc
@@ -6552,10 +6552,13 @@ supports_one_only (void)
 }
 
 /* Set up DECL as a public symbol that can be defined in multiple
-   translation units without generating a linker error.  */
+   translation units without generating a linker error.  Sometimes
+   this function is called before setting DECL_INITIAL, and that may
+   cause the variable to get DECL_COMMON set if !SUPPORTS_ONE_ONLY.
+   After setting DECL_INITIAL, this should be called with REVISIT.  */
 
 void
-make_decl_one_only (tree decl, tree comdat_group)
+make_decl_one_only (tree decl, tree comdat_group, bool revisit)
 {
   struct symtab_node *symbol;
   gcc_assert (VAR_OR_FUNCTION_DECL_P (decl));
@@ -6569,6 +6572,10 @@ make_decl_one_only (tree decl, tree comdat_group)
 
   if (SUPPORTS_ONE_ONLY)
     {
+      if (revisit)
+	/* ??? Do we need a MAKE_DECL_ONE_ONLY_REVISIT?  */
+	return;
+
 #ifdef MAKE_DECL_ONE_ONLY
       MAKE_DECL_ONE_ONLY (decl);
 #endif
@@ -6582,6 +6589,8 @@ make_decl_one_only (tree decl, tree comdat_group)
     {
       gcc_assert (TARGET_SUPPORTS_WEAK);
       DECL_WEAK (decl) = 1;
+      if (revisit)
+	DECL_COMMON (decl) = 0;
     }
 }
 
diff --git a/gcc/varasm.h b/gcc/varasm.h
index acbd9fab7f6..8c83fd6974d 100644
--- a/gcc/varasm.h
+++ b/gcc/varasm.h
@@ -30,7 +30,7 @@ extern tree cold_function_name;
 extern tree tree_output_constant_def (tree);
 extern void make_decl_rtl (tree);
 extern rtx make_decl_rtl_for_debug (tree);
-extern void make_decl_one_only (tree, tree);
+extern void make_decl_one_only (tree, tree, bool = false);
 extern int supports_one_only (void);
 extern void resolve_unique_section (tree, int, int);
 extern void mark_referenced (tree);

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-11-25  6:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-25  6:34 [gcc(refs/users/aoliva/heads/testme)] Revisit non-one-only common/weakening of initialized variables Alexandre Oliva
  -- strict thread matches above, loose matches on Subject: below --
2022-11-25  3:56 Alexandre Oliva
2022-11-23 19:52 Alexandre Oliva

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