From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2140) id 9A932386187E; Wed, 23 Nov 2022 19:52:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9A932386187E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669233168; bh=n15PUa5sdznF9mnk1B/xW9tKlDRqIyN2MuF1P0wv0F8=; h=From:To:Subject:Date:From; b=b8yrJDqNyAZIYS1rKfoFymYEy4zsVppE8mZls3qw8qu5D2yMd7feGezxhXIjv/yip 6uF/qzpg9yNF9IlAYcEY2xSbbsne7e5S4X0hoNngAMkV0UVKm7SZs7AJ27slmlb9qp +mA5ovZtZw30tZIiTab0tFgtm4D6tlrkniseP4nQ= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Alexandre Oliva To: gcc-cvs@gcc.gnu.org Subject: [gcc(refs/users/aoliva/heads/testme)] Revisit non-one-only common/weakening of initialized variables X-Act-Checkin: gcc X-Git-Author: Alexandre Oliva X-Git-Refname: refs/users/aoliva/heads/testme X-Git-Oldrev: 6d82e0fea5f988e829912aaa70a9964a81ad4e5e X-Git-Newrev: 5fc5413985a09c26d2213143a7f68dd7723983e3 Message-Id: <20221123195248.9A932386187E@sourceware.org> Date: Wed, 23 Nov 2022 19:52:48 +0000 (GMT) List-Id: https://gcc.gnu.org/g:5fc5413985a09c26d2213143a7f68dd7723983e3 commit 5fc5413985a09c26d2213143a7f68dd7723983e3 Author: Alexandre Oliva 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 **, 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);