[debug-early] do not add location info/etc to abstract instances
Aldy Hernandez
aldyh@redhat.com
Tue Sep 30 18:43:00 GMT 2014
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
-------------- next part --------------
commit d9b215d7c3aeea2c601e0d983cbe424990c1beab
Author: Aldy Hernandez <aldyh@redhat.com>
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
More information about the Gcc-patches
mailing list