From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35458 invoked by alias); 12 May 2015 14:50:26 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 35440 invoked by uid 89); 12 May 2015 14:50:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.3 required=5.0 tests=AWL,BAYES_50,KAM_ASCII_DIVIDERS,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 12 May 2015 14:50:24 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id C4EE7543EE5; Tue, 12 May 2015 16:50:20 +0200 (CEST) Date: Tue, 12 May 2015 14:54:00 -0000 From: Jan Hubicka To: Jan Hubicka Cc: Jason Merrill , gcc-patches@gcc.gnu.org Subject: Re: False ODR violation positives on anonymous namespace types Message-ID: <20150512145020.GF35526@kam.mff.cuni.cz> References: <20150511142810.GA6584@kam.mff.cuni.cz> <5550E3D2.2080408@redhat.com> <20150511174607.GB59663@kam.mff.cuni.cz> <5550ECB5.8000607@redhat.com> <20150511180509.GB22960@kam.mff.cuni.cz> <55511D40.1010808@redhat.com> <20150511213949.GB35526@kam.mff.cuni.cz> <55520517.1020507@redhat.com> <20150512140313.GA26537@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150512140313.GA26537@kam.mff.cuni.cz> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2015-05/txt/msg01092.txt.bz2 > /* Record, union and enumeration type have linkage that allows use > to check type_in_anonymous_namespace_p. Compound types > can be always compared structurally. > To save some work we compare builtin types properties of its main > variant. A special case are integer types where mangling do make > differences between char/signed char/unsigned char etc. Storing name > for these makes e.g. -fno-signed-char/-fsigned-char mismatches to be > handled well. > > See cp/mangle.c:write_builtin_type for details. */ > && ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)) > || TREE_CODE (TREE_TYPE (decl)) == ENUMERATION_TIME > && !type_in_anonymous_namespace_p (TREE_TYPE (decl))) > || TREE_CODE (TREE_TYPE (decl)) == INTEGER_TYPE) I had quite some typos in the conditional. Here is variant of patch I am testing. I added predicate type_with_linkage_p to ipa-devirt and made type_in_anonymous_namespace_p to ICE on anything else - it is very easy to run into a trap with false negatives by calling it on something that does not have linkage defined. This is patch I am testing. Honza Index: tree.c =================================================================== --- tree.c (revision 223072) +++ tree.c (working copy) @@ -5160,27 +5160,33 @@ free_lang_data_in_type (tree type) static inline bool need_assembler_name_p (tree decl) { - /* We use DECL_ASSEMBLER_NAME to hold mangled type names for One Definition Rule - merging. */ + /* We use DECL_ASSEMBLER_NAME to hold mangled type names for One Definition + Rule merging. This makes type_odr_p to return true on those types during + LTO and by comparing the mangled name, we can say what types are intended + to be equivalent across compilation unit. + + We do not store names of type_in_anonymous_namespace_p. + + Record, union and enumeration type have linkage that allows use + to check type_in_anonymous_namespace_p. We do not mangle compound types + that always can be compared structurally. + + Similarly for builtin types, we compare properties of their main variant. + A special case are integer types where mangling do make differences + between char/signed char/unsigned char etc. Storing name for these makes + e.g. -fno-signed-char/-fsigned-char mismatches to be handled well. + See cp/mangle.c:write_builtin_type for details. */ + if (flag_lto_odr_type_mering && TREE_CODE (decl) == TYPE_DECL && DECL_NAME (decl) && decl == TYPE_NAME (TREE_TYPE (decl)) - && !is_lang_specific (TREE_TYPE (decl)) - /* Save some work. Names of builtin types are always derived from - properties of its main variant. A special case are integer types - where mangling do make differences between char/signed char/unsigned - char etc. Storing name for these makes e.g. - -fno-signed-char/-fsigned-char mismatches to be handled well. - - See cp/mangle.c:write_builtin_type for details. */ - && (TREE_CODE (TREE_TYPE (decl)) != VOID_TYPE - && TREE_CODE (TREE_TYPE (decl)) != BOOLEAN_TYPE - && TREE_CODE (TREE_TYPE (decl)) != REAL_TYPE - && TREE_CODE (TREE_TYPE (decl)) != FIXED_POINT_TYPE) && !TYPE_ARTIFICIAL (TREE_TYPE (decl)) - && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE) - && !type_in_anonymous_namespace_p (TREE_TYPE (decl))) + && (((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)) + || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE) + && !type_in_anonymous_namespace_p (TREE_TYPE (decl))) + || TREE_CODE (TREE_TYPE (decl)) == INTEGER_TYPE) + && !variably_modified_type_p (TREE_TYPE (decl), NULL_TREE)) return !DECL_ASSEMBLER_NAME_SET_P (decl); /* Only FUNCTION_DECLs and VAR_DECLs are considered. */ if (TREE_CODE (decl) != FUNCTION_DECL @@ -12037,18 +12043,6 @@ obj_type_ref_class (tree ref) return TREE_TYPE (ref); } -/* Return true if T is in anonymous namespace. */ - -bool -type_in_anonymous_namespace_p (const_tree t) -{ - /* TREE_PUBLIC of TYPE_STUB_DECL may not be properly set for - bulitin types; those have CONTEXT NULL. */ - if (!TYPE_CONTEXT (t)) - return false; - return (TYPE_STUB_DECL (t) && !TREE_PUBLIC (TYPE_STUB_DECL (t))); -} - /* Lookup sub-BINFO of BINFO of TYPE at offset POS. */ static tree Index: ipa-utils.h =================================================================== --- ipa-utils.h (revision 223072) +++ ipa-utils.h (working copy) @@ -63,6 +63,7 @@ possible_polymorphic_call_targets (tree, void **cache_token = NULL, bool speuclative = false); odr_type get_odr_type (tree, bool insert = false); +bool odr_type_p (const_tree t); bool possible_polymorphic_call_target_p (tree ref, gimple stmt, struct cgraph_node *n); void dump_possible_polymorphic_call_targets (FILE *, tree, HOST_WIDE_INT, const ipa_polymorphic_call_context &); @@ -153,23 +157,6 @@ possible_polymorphic_call_target_p (stru context, n); } -/* Return true of T is type with One Definition Rule info attached. - It means that either it is anonymous type or it has assembler name - set. */ - -static inline bool -odr_type_p (const_tree t) -{ - if (type_in_anonymous_namespace_p (t)) - return true; - /* We do not have this information when not in LTO, but we do not need - to care, since it is used only for type merging. */ - gcc_assert (in_lto_p || flag_lto); - - return (TYPE_NAME (t) - && (DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t)))); -} - /* Return true if BINFO corresponds to a type with virtual methods. Every type has several BINFOs. One is the BINFO associated by the type Index: ipa-devirt.c =================================================================== --- ipa-devirt.c (revision 223072) +++ ipa-devirt.c (working copy) @@ -245,6 +245,47 @@ struct GTY(()) odr_type_d bool rtti_broken; }; +/* Return true if T is a type with linkage defined. */ + +static bool +type_with_linkage_p (const_tree t) +{ + return (RECORD_OR_UNION_TYPE_P (t) + || TREE_CODE (t) == ENUMERAL_TYPE); +} + +/* Return true if T is in anonymous namespace. + This works only on those C++ types with linkage defined. */ + +bool +type_in_anonymous_namespace_p (const_tree t) +{ + gcc_assert (type_with_linkage_p (t)); + /* TREE_PUBLIC of TYPE_STUB_DECL may not be properly set for + backend produced types (such as va_arg_type); those have CONTEXT NULL + and never are considered anonymoius. */ + if (!TYPE_CONTEXT (t)) + return false; + return (TYPE_STUB_DECL (t) && !TREE_PUBLIC (TYPE_STUB_DECL (t))); +} + +/* Return true of T is type with One Definition Rule info attached. + It means that either it is anonymous type or it has assembler name + set. */ + +bool +odr_type_p (const_tree t) +{ + if (type_with_linkage_p (t) && type_in_anonymous_namespace_p (t)) + return true; + /* We do not have this information when not in LTO, but we do not need + to care, since it is used only for type merging. */ + gcc_checking_assert (in_lto_p || flag_lto); + + return (TYPE_NAME (t) + && (DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t)))); +} + /* Return TRUE if all derived types of T are known and thus we may consider the walk of derived type complete. @@ -341,8 +382,7 @@ main_odr_variant (const_tree t) static bool can_be_name_hashed_p (tree t) { - return (!in_lto_p || type_in_anonymous_namespace_p (t) - || (TYPE_NAME (t) && DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t)))); + return (!in_lto_p || odr_type_p (t)); } /* Hash type by its ODR name. */ @@ -358,7 +398,7 @@ hash_odr_name (const_tree t) return htab_hash_pointer (t); /* Anonymous types are unique. */ - if (type_in_anonymous_namespace_p (t)) + if (type_with_linkage_p (t) && type_in_anonymous_namespace_p (t)) return htab_hash_pointer (t); gcc_checking_assert (TYPE_NAME (t) @@ -381,7 +421,7 @@ can_be_vtable_hashed_p (tree t) if (TYPE_MAIN_VARIANT (t) != t) return false; /* Anonymous namespace types are always handled by name hash. */ - if (type_in_anonymous_namespace_p (t)) + if (type_with_linkage_p (t) && type_in_anonymous_namespace_p (t)) return false; return (TREE_CODE (t) == RECORD_TYPE && TYPE_BINFO (t) && BINFO_VTABLE (TYPE_BINFO (t))); @@ -455,8 +495,8 @@ types_same_for_odr (const_tree type1, co /* Check for anonymous namespaces. Those have !TREE_PUBLIC on the corresponding TYPE_STUB_DECL. */ - if (type_in_anonymous_namespace_p (type1) - || type_in_anonymous_namespace_p (type2)) + if ((type_with_linkage_p (type1) && type_in_anonymous_namespace_p (type1)) + || (type_with_linkage_p (type2) && type_in_anonymous_namespace_p (type2))) return false; @@ -565,8 +658,8 @@ odr_name_hasher::equal (const odr_type_d return false; /* Check for anonymous namespaces. Those have !TREE_PUBLIC on the corresponding TYPE_STUB_DECL. */ - if (type_in_anonymous_namespace_p (t1) - || type_in_anonymous_namespace_p (t2)) + if ((type_with_linkage_p (t1) && type_in_anonymous_namespace_p (t1)) + || (type_with_linkage_p (t2) && type_in_anonymous_namespace_p (t2))) return false; gcc_checking_assert (DECL_ASSEMBLER_NAME (TYPE_NAME (t1))); gcc_checking_assert (DECL_ASSEMBLER_NAME (TYPE_NAME (t2))); @@ -642,7 +735,6 @@ static bool odr_subtypes_equivalent_p (tree t1, tree t2, hash_set *visited) { - bool an1, an2; /* This can happen in incomplete types that should be handled earlier. */ gcc_assert (t1 && t2); @@ -653,9 +745,8 @@ odr_subtypes_equivalent_p (tree t1, tree return true; /* Anonymous namespace types must match exactly. */ - an1 = type_in_anonymous_namespace_p (t1); - an2 = type_in_anonymous_namespace_p (t2); - if (an1 != an2 || an1) + if ((type_with_linkage_p (t1) && type_in_anonymous_namespace_p (t1)) + || (type_with_linkage_p (t2) && type_in_anonymous_namespace_p (t2))) return false; /* For ODR types be sure to compare their names. @@ -1019,10 +1122,10 @@ warn_types_mismatch (tree t1, tree t2) } /* It is a quite common bug to reference anonymous namespace type in non-anonymous namespace class. */ - if (type_in_anonymous_namespace_p (t1) - || type_in_anonymous_namespace_p (t2)) + if ((type_with_linkage_p (t1) && type_in_anonymous_namespace_p (t1)) + || (type_with_linkage_p (t2) && type_in_anonymous_namespace_p (t2))) { - if (!type_in_anonymous_namespace_p (t1)) + if (type_with_linkage_p (t1) && !type_in_anonymous_namespace_p (t1)) { tree tmp = t1;; t1 = t2; @@ -1166,8 +1269,8 @@ odr_types_equivalent_p (tree t1, tree t2 /* Check first for the obvious case of pointer identity. */ if (t1 == t2) return true; - gcc_assert (!type_in_anonymous_namespace_p (t1)); - gcc_assert (!type_in_anonymous_namespace_p (t2)); + gcc_assert (!type_with_linkage_p (t1) || !type_in_anonymous_namespace_p (t1)); + gcc_assert (!type_with_linkage_p (t2) || !type_in_anonymous_namespace_p (t2)); /* Can't be the same type if the types don't have the same code. */ if (TREE_CODE (t1) != TREE_CODE (t2)) @@ -1976,7 +2089,10 @@ get_odr_type (tree type, bool insert) val->type = type; val->bases = vNULL; val->derived_types = vNULL; - val->anonymous_namespace = type_in_anonymous_namespace_p (type); + if (type_with_linkage_p (type)) + val->anonymous_namespace = type_in_anonymous_namespace_p (type); + else + val->anonymous_namespace = 0; build_bases = COMPLETE_TYPE_P (val->type); insert_to_odr_array = true; if (slot)