From patchwork Thu Dec 11 17:44:58 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 420193 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 D48DD1400DD for ; Fri, 12 Dec 2014 04:47:55 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=e/CVeFvb0X6RKsbrkn9bWMwRSxoFTZNXUTN1wIaFiH3 4NkWs5yli6GUNVmZzFEdyZuxjdBDo+67X86kvpYAlgszGHn92mG/zHgDziIgjVRy kAsCc/F+K0SsPYQvighwfm2clWwe/wSBvnkqusdzqZj6tpaHBWLQNdmRyyavQZVA = 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 :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=4YnWG5M3NNvOwakvaEaF462ubxM=; b=WQvw9tU795NGUtDR8 kt1yn46v56yZr2t7gz2KfelSRwqb3F1A/0b1W1WR/3PM3uMCGSQzqcuR79RDr+EJ BCVpvAn5Rr1+5B7bJueJg0Me2Ui3i4MMr/imZfurJKmfzU4WMkhHp1UC2Y88+A1p hC0XQEP0qG6k/KuX5QoQf2l7z8= Received: (qmail 16144 invoked by alias); 11 Dec 2014 17:47:48 -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 16133 invoked by uid 89); 11 Dec 2014 17:47:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 11 Dec 2014 17:47:46 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBBHle7N009576 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 11 Dec 2014 12:47:44 -0500 Received: from reynosa.quesejoda.com (vpn-54-227.rdu2.redhat.com [10.10.54.227]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sBBHiwPa005465; Thu, 11 Dec 2014 12:44:58 -0500 Message-ID: <5489D81A.60801@redhat.com> Date: Thu, 11 Dec 2014 09:44:58 -0800 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: jason merrill CC: gcc-patches Subject: RFC: handle cached local static DIEs Hi Jason. After my last set of dwarf changes for locals, I found some target library building failures which I am now fixing. The problem at hand is that, by design, the caching code in gen_variable_die() refuses to use a previously cached DIE if the current context and the cached context are different: else if (old_die->die_parent != context_die) { /* If the contexts differ, it means we're not talking about the same thing. Clear things so we can get a new DIE. This can happen when creating an inlined instance, in which case we need to create a new DIE that will get annotated with DW_AT_abstract_origin. */ old_die = NULL; gcc_assert (!DECL_ABSTRACT_P (decl)); } This is causing problems with local statics which are handled at dwarf2out_late_global_decl, and which originally have a context of the compilation unit (by virtue of the dwarf2out_decl call). This context then gets changed here: /* For local statics lookup proper context die. */ if (TREE_STATIC (decl) && DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) context_die = lookup_decl_die (DECL_CONTEXT (decl)); This new context may be correct for front/middle-end purposes, but is not the DIE context I am expecting in gen_variable_die. For example, in the following example, the DECL_CONTEXT for the static is funky's DW_TAG_subprogram, whereas the caching code is expecting the DW_TAG_lexical_block: void funky() { { static const char *nested_static_const = "testing123"; } } My proposed way of handling it (attached) is by tightening the check in gen_variable_die(), and special casing this scenario (assuming, there is no other way to get a differing context). This works, and fixes all the failures, without introducing any regressions. Another approach would be to use whatever context is already cached with just "context_die = lookup_decl_die (decl)", but that feels like cheating. Are you OK with the attached approach, or do you have something else in mind? Thanks. Aldy commit 515a20666d0ea73f2380bae6d9b8ec1d5bb2f001 Author: Aldy Hernandez Date: Thu Dec 11 09:26:25 2014 -0800 * dwarf2out.c (gen_subprogram_die): Handle as cached die if dumped_early bit is set. (dwarf2out_decl): Abstract local static check... (local_function_static): ...into here. (gen_variable_die): Handle different contexts in a cached die gracefully for the non inline case. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index e4a7973..5d55d1f 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -18511,7 +18511,9 @@ gen_subprogram_die (tree decl, dw_die_ref context_die) apply; we just use the old DIE. */ expanded_location s = expand_location (DECL_SOURCE_LOCATION (decl)); struct dwarf_file_data * file_index = lookup_filename (s.file); - if (((is_cu_die (old_die->die_parent) || context_die == NULL) + if (((is_cu_die (old_die->die_parent) + || context_die == NULL + || dumped_early) && (DECL_ARTIFICIAL (decl) || (get_AT_file (old_die, DW_AT_decl_file) == file_index && (get_AT_unsigned (old_die, DW_AT_decl_line) @@ -19132,6 +19134,17 @@ decl_will_get_specification_p (dw_die_ref old_die, tree decl, bool declaration) && get_AT_flag (old_die, DW_AT_declaration) == 1); } +/* Return true if DECL is a local static. */ + +static inline bool +local_function_static (tree decl) +{ + gcc_assert (TREE_CODE (decl) == VAR_DECL); + return TREE_STATIC (decl) + && DECL_CONTEXT (decl) + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL; +} + /* Generate a DIE to represent a declared data object. Either DECL or ORIGIN must be non-null. */ @@ -19283,13 +19296,35 @@ gen_variable_die (tree decl, tree origin, dw_die_ref context_die) } else if (old_die->die_parent != context_die) { - /* If the contexts differ, it means we're not talking about - the same thing. Clear things so we can get a new DIE. - This can happen when creating an inlined instance, in - which case we need to create a new DIE that will get - annotated with DW_AT_abstract_origin. */ - old_die = NULL; - gcc_assert (!DECL_ABSTRACT_P (decl)); + /* If the contexts differ, it means we _MAY_ not be talking + about the same thing. */ + if (origin) + { + /* If we will be creating an inlined instance, we need a + new DIE that will get annotated with + DW_AT_abstract_origin. Clear things so we can get a + new DIE. */ + gcc_assert (!DECL_ABSTRACT_P (decl)); + old_die = NULL; + } + else + { + /* Otherwise, the only reasonable alternate way of + getting here is during the final dwarf pass, when + being called on a local static. We can end up with + different contexts because the context_die is set to + the context of the containing function, whereas the + cached die (old_die) is correctly set to the + (possible) enclosing lexical scope + (DW_TAG_lexical_block). In which case, special + handle it (hack). + + See dwarf2out_decl and its use of + local_function_static to see how this happened. */ + gcc_assert (local_function_static (decl)); + var_die = old_die; + goto gen_variable_die_location; + } } else { @@ -21563,9 +21598,7 @@ dwarf2out_decl (tree decl) return NULL; /* For local statics lookup proper context die. */ - if (TREE_STATIC (decl) - && DECL_CONTEXT (decl) - && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) + if (local_function_static (decl)) context_die = lookup_decl_die (DECL_CONTEXT (decl)); /* If we are in terse mode, don't generate any DIEs to represent any