From patchwork Tue Sep 30 18:43:27 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 395068 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 5AF7F1400A3 for ; Wed, 1 Oct 2014 04:43:43 +1000 (EST) 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:subject:content-type; q= dns; s=default; b=waiGa8FKfVyh6UVhJ/epAw6KsG1hqL5xuG8jsLlwSz8TMu RXBNfvBpUt2DRdge5DM/1rl0WZobSzgWdYOvTefHsJmd5lrTWkuadsOghyzFWYWz gZlVKE18Fj8/jdACwLDbBoRMRjrjxc/2uaaxDdDxouvc1vPzd7yDKhmEQ2ivw= 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:subject:content-type; s= default; bh=iEAOOIBQOBLoLYtNQZKYHRNQ9Gc=; b=JyHeyyfb+yVa1Ew4XD9k be0mnLVO4kpikm2BoqNS5nrzPaYUidPUgcoFom1A65qtB/lBdV+tRIBWGj9AEADL 4VtjoZ95TTcObxX0SME5Y+Dm4hVP5h6+tbg3cgY/xyFyHtrn7+4+p7ZOCzaqNlD2 E4of/qs/CW0FFFj/hY5qtcc= Received: (qmail 31226 invoked by alias); 30 Sep 2014 18:43:36 -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 31207 invoked by uid 89); 30 Sep 2014 18:43:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS 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; Tue, 30 Sep 2014 18:43:33 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8UIhVjq007625 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 30 Sep 2014 14:43:31 -0400 Received: from reynosa.quesejoda.com (vpn-61-24.rdu2.redhat.com [10.10.61.24]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8UIhTmr018694; Tue, 30 Sep 2014 14:43:29 -0400 Message-ID: <542AF9CF.2070208@redhat.com> Date: Tue, 30 Sep 2014 11:43:27 -0700 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: jason merrill , gcc-patches Subject: [debug-early] do not add location info/etc to abstract instances Hi Jason. As discussed on IRC, DIEs of abstract instances of functions (those tagged with DW_AT_inline), cannot include information that would be different between an abstract inline and an out-of-line copy. This, as well as (seeming) gdb snafus regarding abstract origins, was the reason I was seeing less guality regressions on my branch. With the current patch, not only do we fix this oversight, but are now at feature *and* bug parity with mainline. I guess that's good :(. There were a few problems that needed fixing. First, gen_formal_parameter_die() was reusing the abstract instance DIE's parameter, instead of creating a new die with abstract origin set. Also, gen_formal_parameter_die() had some old Michael code, working around decl_ultimate_origin hacks. I've fixed all of these problems. I also fixed gen_subprogram_die(), so it's more aware of a previously generated die. I would appreciate if you could take a look at this patch, particularly at the gen_formal_parameter_die change, since I'm not sure what the proper way is of checking that a parameter belongs to an abstract instance. Below is what I'm using: > bool reusing_die; > - if (parm_die) > + if (parm_die > + /* Make sure the function to which this parameter belongs to is > + not an abstract instance. If it is, we can't reuse anything. > + We must create a new DW_TAG_formal_parameter with a > + corresponding DW_AT_abstract_origin. */ > + && !get_AT (context_die, DW_AT_abstract_origin)) > reusing_die = true; Also, seeing as my changes could potentially render invalid DIEs, I thought it best to add, at the very least, a check for the inline problem described above (see check_die_inline). For that matter, I wonder if we could add more checks to check_die() to make it a general dwarf DIE sanity checker. It still amazes me that you dwarf hackers do all this magic, without any sort of checks. And finally, making changes to _when_ we generate DIEs can sometimes lead to NOT generating DIEs early, and silently behaving like mainline (that is, generating everything at the end of compilation). To solve this problem, which I'm sure I'll stumble into, I've added -fdump-early-debug-stats which will set the ->dumped_early bit in every DIE generated after parsing, and then dumping the DIEs after the late dwarf generation has run. This way I can see if we have too many DIEs that were NOT generated early. It is likely this will only be a temporary hacking tool to check I didn't do anything stupid along :). How does this look? Aldy commit d9b215d7c3aeea2c601e0d983cbe424990c1beab Author: Aldy Hernandez Date: Tue Sep 30 08:20:44 2014 -0700 * common.opt (fdump-early-debug-stats): New. * debug.h (dwarf2out_mark_early_dies): New prototype. (dwarf2out_dump_early_debug_stats): New prototype. * toplev.c (compile_file): Dump early debug stats if requested. * dwarf2out.c (check_die): Check that DIEs containing a DW_AT_inline doe not contain any invalid modifiers. (gen_formal_parameter_die): Do not reuse parameters that belong to an abstract instance. Do not care that an abstract origin is itself. (gen_subprogram_die): Handle old_die's better. (print_die): Print dumped_early bit. (mark_early_dies_helper): New. (dwarf2out_mark_early_dies): New. (dwarf2out_dump_early_debug_stats): New. (check_die_inline): New. (check_die): Call check_die_inline. diff --git a/gcc/common.opt b/gcc/common.opt index 634a72b..c01f935 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1132,6 +1132,10 @@ fdump-unnumbered-links Common Report Var(flag_dump_unnumbered_links) Suppress output of previous and next insn numbers in debugging dumps +fdump-early-debug-stats +Common Report Var(flag_dump_early_debug_stats) +Dump all dwarf DIEs, specifying if they were generated during the early debug stage + fdwarf2-cfi-asm Common Report Var(flag_dwarf2_cfi_asm) Init(HAVE_GAS_CFI_DIRECTIVE) Enable CFI tables via GAS assembler directives. diff --git a/gcc/debug.h b/gcc/debug.h index ec387ca..7158a48 100644 --- a/gcc/debug.h +++ b/gcc/debug.h @@ -190,11 +190,12 @@ extern bool dwarf2out_do_frame (void); extern bool dwarf2out_do_cfi_asm (void); extern void dwarf2out_switch_text_section (void); +extern void dwarf2out_mark_early_dies (void); +extern void dwarf2out_dump_early_debug_stats (void); + const char *remap_debug_filename (const char *); void add_debug_prefix_map (const char *); -extern void dwarf2out_early_decl (tree); - /* For -fdump-go-spec. */ extern const struct gcc_debug_hooks * diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index c92101f..a713435 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -5358,9 +5358,9 @@ print_die (dw_die_ref die, FILE *outfile) unsigned ix; print_spaces (outfile); - fprintf (outfile, "DIE %4ld: %s (%p)\n", + fprintf (outfile, "DIE %4ld: %s (%p)%s\n", die->die_offset, dwarf_tag_name (die->die_tag), - (void*) die); + (void*) die, die->dumped_early ? " (DUMPED EARLY)" : ""); print_spaces (outfile); fprintf (outfile, " abbrev id: %lu", die->die_abbrev); fprintf (outfile, " offset: %ld", die->die_offset); @@ -5528,6 +5528,71 @@ debug_dwarf (void) print_die (comp_unit_die (), stderr); } +/* Callback for htab_traverse_noresize. Set the dumped_early bit for + a given DIE. */ + +static int +mark_early_dies_helper (void **slot, void *info ATTRIBUTE_UNUSED) +{ + dw_die_ref ref = (dw_die_ref) *slot; + + /* Unilaterally enabling this bit can potentially change the + behavior of dwarf2out_decl for DECLs that were discovered through + recursion of dwarf2out_decl(), and may not have the dumped_early + bit set. Since this is only used for debugging the behavior of + dwarf2out, we should be ok-- but it's something to keep in + mind. */ + ref->dumped_early = true; + return 1; +} + +/* Set the dumped_early bit for all DIEs. */ + +void +dwarf2out_mark_early_dies (void) +{ + if (decl_die_table) + htab_traverse_noresize (decl_die_table, mark_early_dies_helper, NULL); +} + +/* Dump the DIE table if available. */ + +void +dwarf2out_dump_early_debug_stats (void) +{ + if (decl_die_table) + debug_dwarf (); +} + +/* Sanity checks on DW_AT_inline DIEs. */ + +static void +check_die_inline (dw_die_ref die) +{ + /* A debugging information entry that is a member of an abstract + instance tree [that has DW_AT_inline] should not contain any + attributes which describe aspects of the subroutine which vary + between distinct inlined expansions or distinct out-of-line + expansions. */ + unsigned ix; + dw_attr_ref a; + bool inline_found = false; + FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a) + if (a->dw_attr == DW_AT_inline) + inline_found = true; + if (inline_found) + { + /* Catch the most common mistakes. */ + FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a) + gcc_assert (a->dw_attr != DW_AT_low_pc + && a->dw_attr != DW_AT_high_pc + && a->dw_attr != DW_AT_location + && a->dw_attr != DW_AT_frame_base + && a->dw_attr != DW_AT_GNU_all_call_sites); + } + +} + /* Perform some sanity checks on DIEs after they have been generated earlier in the compilation process. */ @@ -5536,7 +5601,10 @@ check_die (dw_die_ref die, unsigned level) { static unsigned long mark = 1; dw_die_ref c, p; - /* Check that all our childs have their parent set to us. */ + + check_die_inline (die); + + /* Check that all of our children have their parent set to us. */ c = die->die_child; if (c) do { c = c->die_sib; @@ -17721,7 +17789,12 @@ gen_formal_parameter_die (tree node, tree origin, bool emit_name_p, } bool reusing_die; - if (parm_die) + if (parm_die + /* Make sure the function to which this parameter belongs to is + not an abstract instance. If it is, we can't reuse anything. + We must create a new DW_TAG_formal_parameter with a + corresponding DW_AT_abstract_origin. */ + && !get_AT (context_die, DW_AT_abstract_origin)) reusing_die = true; else { @@ -17739,7 +17812,7 @@ gen_formal_parameter_die (tree node, tree origin, bool emit_name_p, if (reusing_die) goto add_location; - if (origin != NULL && node != origin) + if (origin != NULL) add_abstract_origin_attribute (parm_die, origin); else if (emit_name_p) add_name_and_src_coords_attributes (parm_die, node); @@ -18278,7 +18351,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die) && debug_info_level > DINFO_LEVEL_TERSE) old_die = force_decl_die (decl); - if (origin != NULL && origin != decl) + if (origin != NULL && (origin != decl || old_die)) { gcc_assert (!declaration || local_scope_p (context_die)); diff --git a/gcc/toplev.c b/gcc/toplev.c index 9f29dac..7dca017 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -573,6 +573,10 @@ compile_file (void) /* Run the actual compilation process. */ if (!in_lto_p) { + /* Mark all DIEs generated as dumped early. */ + if (flag_dump_early_debug_stats) + dwarf2out_mark_early_dies (); + timevar_start (TV_PHASE_OPT_GEN); symtab->finalize_compilation_unit (); timevar_stop (TV_PHASE_OPT_GEN); @@ -597,6 +601,9 @@ compile_file (void) debug_hooks->late_global_decl (node->decl); timevar_stop (TV_PHASE_DBGINFO); + if (!in_lto_p && flag_dump_early_debug_stats) + dwarf2out_dump_early_debug_stats (); + timevar_start (TV_PHASE_LATE_ASM); /* Compilation unit is finalized. When producing non-fat LTO object, we are