From patchwork Mon May 11 11:38:55 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 470721 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id F0CD314016A for ; Mon, 11 May 2015 21:39:08 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=XvVBmL62; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=H5VSxVfprlTBhxj5XGl3bQwu0isdAAV0hADiq7a8oHEDZvY00CJDY etsvmegmZpCgOVxpo2o/qQI6aOSy+vHw6idmoyxi1lmnlAWjx+ekVR6mCJ7DzGWm os7PUmxVj+0KVQbWbKZIlvKLIhS2Z5Hmhij4p05MfYDFNqjaPzaYcQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=mnkHhi/78lewvivWFMgexRQEZHg=; b=XvVBmL62jF4cwfX8vfWm RYc8+TO+WZxRvmqAGAJTGKfulxfy9i+Owgucr7lNYYh/61ReF2Nv1Wtcl5ly+BD4 3RNd+6I8yjzj9wjEBpMEEQ7UujZXhq5envUYLhcOTSe5MQ34DTY78i2ho9tTxgZ1 m4H0mVGXhyS/PSstfOZpBU8= Received: (qmail 106677 invoked by alias); 11 May 2015 11:39:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 106666 invoked by uid 89); 11 May 2015 11:39:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00, 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; Mon, 11 May 2015 11:38:59 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 5CC7F543A36; Mon, 11 May 2015 13:38:55 +0200 (CEST) Date: Mon, 11 May 2015 13:38:55 +0200 From: Jan Hubicka To: gcc-patches@gcc.gnu.org, jason@redhat.com Subject: Fix verify_type ICE on TYPE_METHOS Message-ID: <20150511113853.GC22971@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Jason, this patch is trying to fix the following verify_type_ice that check that TYPE_METHODS is same for main variant and all its complete variants. - /* FIXME: this check triggers during libstdc++ build that is a bug. - It affects non-LTO debug output only, because free_lang_data clears - this anyway. */ - if (RECORD_OR_UNION_TYPE_P (t) && COMPLETE_TYPE_P (t) && 0 - && TYPE_METHODS (t) != TYPE_METHODS (tv)) I tried to go in a way of making TYPE_METHODS to be defined only on main vairants as it seems impractical to update variants each time we change TYPE_METHODS. While doing so, I noticed that lazily_declare_fn sometimes adds new method to variant only (that is probably source of ICE as fixup_type_variants copies the TYPE_METHODS). The attach patch however removes the copying and verifies that TYPE_MEHTODS is NULL at all variants. It also updates middle-end uses to always check for main variant. Bootstrapped/regtested ppc64-linux, does this seem to make sense? I also noticed that TYPE_METHODS != NULL matters in some cases: it makes function.c to drop register keyword and it makes us to speak of class instead of struct in diagnostics, so I changed free_lang_data to zap the list to error_mark_node instead of NULL (in past I tried to stream it for ODR warnings and I remember it added measurable extra overhead that I considered too large for mainline). Honza * class.c (fixup_type_variants): Do not copy TYPE_METHODS (one_inheriting_sig): Assert tat we always set TYPE_METHODS of main variant. * semantics.c (finish_member_declaration): Likewise. * method.c (lazily_declare_fn): Allways add method to main variant list. * dwarf2out.c (gen_member_die): Sanity check that we access TYPE_MAIN_VARIANT for TYPE_METHODS. * function.c (use_register_for_decl): Look for TYPE_MAIN_VARIANT when checking TYPE_METHODS. * tree.c (free_lang_data_in_type): See TYPE_METHODS to error_mark_node if non-null. (build_distinct_type_copy): Clear TYPE_METHODS. (verify_type_variant): Verify that TYPE_METHODS is NULL for variants. (verify_type): Allow TYPE_METHODS to be error_mark_node. * tree.def: Update docs of YTPE_STUB_DECL and TYPE_METHODS. Index: cp/class.c =================================================================== --- cp/class.c (revision 222991) +++ cp/class.c (working copy) @@ -1972,7 +1972,6 @@ fixup_type_variants (tree t) /* Copy whatever these are holding today. */ TYPE_VFIELD (variants) = TYPE_VFIELD (t); - TYPE_METHODS (variants) = TYPE_METHODS (t); TYPE_FIELDS (variants) = TYPE_FIELDS (t); } } @@ -3238,6 +3237,7 @@ one_inheriting_sig (tree t, tree ctor, t parmlist = tree_cons (NULL_TREE, parms[i], parmlist); tree fn = implicitly_declare_fn (sfk_inheriting_constructor, t, false, ctor, parmlist); + gcc_assert (TYPE_MAIN_VARIANT (t) == t); if (add_method (t, fn, NULL_TREE)) { DECL_CHAIN (fn) = TYPE_METHODS (t); Index: cp/method.c =================================================================== --- cp/method.c (revision 222991) +++ cp/method.c (working copy) @@ -2189,11 +2189,11 @@ lazily_declare_fn (special_function_kind && DECL_VIRTUAL_P (fn)) /* The ABI requires that a virtual destructor go at the end of the vtable. */ - TYPE_METHODS (type) = chainon (TYPE_METHODS (type), fn); + TYPE_METHODS (type) = chainon (TYPE_METHODS (TYPE_MAIN_VARIANT (type)), fn); else { - DECL_CHAIN (fn) = TYPE_METHODS (type); - TYPE_METHODS (type) = fn; + DECL_CHAIN (fn) = TYPE_METHODS (TYPE_MAIN_VARIANT (type)); + TYPE_METHODS (TYPE_MAIN_VARIANT (type)) = fn; } maybe_add_class_template_decl_list (type, fn, /*friend_p=*/0); if (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (fn) Index: cp/semantics.c =================================================================== --- cp/semantics.c (revision 222991) +++ cp/semantics.c (working copy) @@ -2913,6 +2913,7 @@ finish_member_declaration (tree decl) CLASSTYPE_METHOD_VEC. */ if (add_method (current_class_type, decl, NULL_TREE)) { + gcc_assert (TYPE_MAIN_VARIANT (current_class_type) == current_class_type); DECL_CHAIN (decl) = TYPE_METHODS (current_class_type); TYPE_METHODS (current_class_type) = decl; Index: dwarf2out.c =================================================================== --- dwarf2out.c (revision 222991) +++ dwarf2out.c (working copy) @@ -19945,6 +19945,8 @@ gen_member_die (tree type, dw_die_ref co gen_decl_die (member, NULL, context_die); } + /* We do not keep type methods in type variants. */ + gcc_assert (TYPE_MAIN_VARIANT (type) == type); /* Now output info about the function members (if any). */ for (member = TYPE_METHODS (type); member; member = DECL_CHAIN (member)) { Index: function.c =================================================================== --- function.c (revision 222991) +++ function.c (working copy) @@ -2170,7 +2170,7 @@ use_register_for_decl (const_tree decl) /* When not optimizing, disregard register keyword for variables with types containing methods, otherwise the methods won't be callable from the debugger. */ - if (TYPE_METHODS (TREE_TYPE (decl))) + if (TYPE_METHODS (TYPE_MAIN_VARIANT (TREE_TYPE (decl)))) return false; break; default: Index: tree.c =================================================================== --- tree.c (revision 222991) +++ tree.c (working copy) @@ -5100,7 +5100,13 @@ free_lang_data_in_type (tree type) if (TYPE_VFIELD (type) && TREE_CODE (TYPE_VFIELD (type)) != FIELD_DECL) TYPE_VFIELD (type) = NULL_TREE; - TYPE_METHODS (type) = NULL_TREE; + /* Remove TYPE_METHODS list. While it would be nice to keep it + to enable ODR warnings about different method lists, doing so + seems to impractically increase size of LTO data streamed. + Keep the information if TYPE_METHODS was non-NULL. This is used + by function.c and pretty printers. */ + if (TYPE_METHODS (type)) + TYPE_METHODS (type) = error_mark_node; if (TYPE_BINFO (type)) { free_lang_data_in_binfo (TYPE_BINFO (type)); @@ -6574,6 +6580,12 @@ build_distinct_type_copy (tree type) TYPE_MAIN_VARIANT (t) = t; TYPE_NEXT_VARIANT (t) = 0; + /* We do not record methods in type copies nor variants + so we do not need to keep them up to date when new method + is inserted. */ + if (RECORD_OR_UNION_TYPE_P (t)) + TYPE_METHODS (t) = NULL_TREE; + /* Note that it is now possible for TYPE_MIN_VALUE to be a value whose TREE_TYPE is not t. This can also happen in the Ada frontend when using subtypes. */ @@ -12528,13 +12540,9 @@ verify_type_variant (const_tree t, tree debug_tree (tv); return false; } - /* FIXME: this check triggers during libstdc++ build that is a bug. - It affects non-LTO debug output only, because free_lang_data clears - this anyway. */ - if (RECORD_OR_UNION_TYPE_P (t) && COMPLETE_TYPE_P (t) && 0 - && TYPE_METHODS (t) != TYPE_METHODS (tv)) + if (RECORD_OR_UNION_TYPE_P (t) && TYPE_METHODS (t)) { - error ("type variant has different TYPE_METHODS"); + error ("type variant has TYPE_METHODS"); debug_tree (tv); return false; } @@ -12749,9 +12757,10 @@ verify_type (const_tree t) if (RECORD_OR_UNION_TYPE_P (t)) { if (TYPE_METHODS (t) && TREE_CODE (TYPE_METHODS (t)) != FUNCTION_DECL - && TREE_CODE (TYPE_METHODS (t)) != TEMPLATE_DECL) + && TREE_CODE (TYPE_METHODS (t)) != TEMPLATE_DECL + && TYPE_METHODS (t) != error_mark_node) { - error ("TYPE_METHODS is not FUNCTION_DECL nor TEMPLATE_DECL"); + error ("TYPE_METHODS is not FUNCTION_DECL, TEMPLATE_DECL nor error_mark_node"); debug_tree (TYPE_METHODS (t)); error_found = true; } Index: tree.def =================================================================== --- tree.def (revision 222991) +++ tree.def (working copy) @@ -110,9 +110,12 @@ DEFTREECODE (BLOCK, "block", tcc_excepti particular, since any type which is of some type category (e.g. an array type or a function type) which cannot either have a name itself or have named members doesn't really have a "scope" per se. - The TREE_CHAIN field is used as a forward-references to names for + The TYPE_STUB_DECL field is used as a forward-references to names for ENUMERAL_TYPE, RECORD_TYPE, UNION_TYPE, and QUAL_UNION_TYPE nodes; - see below. */ + see below. + The TYPE_METHODS points to list of all methods associated with the type. + It is non-NULL only at main variant of the type and after free_lang_data + it may be set to error_mark_node instead of actual list to save memory. */ /* The ordering of the following codes is optimized for the checking macros in tree.h. Changing the order will degrade the speed of the