Re: [PATCH][1/n] dwarf2out refactoring for early (LTO) debug

[Richard Biener <rguenther@suse.de>]

On Tue, 18 Aug 2015, Aldy Hernandez wrote:

> On 08/18/2015 07:20 AM, Richard Biener wrote:
> > 
> > This starts a series of patches (still in development) to refactor
> > dwarf2out.c to better cope with early debug (and LTO debug).
> 
> Awesome!  Thanks.
> 
> > Aldyh, what other testing did you usually do for changes?  Run
> > the gdb testsuite against the new compiler?  Anything else?
> 
> gdb testsuite, and make sure you test GCC with --enable-languages=all,go,ada,
> though the latter is mostly useful while you iron out bugs initially.  I found
> that ultimately, the best test was C++.

I see.

> Pre merge I also bootstrapped the compiler and compared .debug* section sizes
> in object files to make sure things were within reason.
> 
> > +
> > +static void
> > +vmsdbgout_early_finish (const char *filename ATTRIBUTE_UNUSED)
> > +{
> > +  if (write_symbols == VMS_AND_DWARF2_DEBUG)
> > +    (*dwarf2_debug_hooks.early_finish) (filename);
> > +}
> 
> You can get rid of ATTRIBUTE_UNUSED now.

Done.  I've also refrained from moving

  gen_scheduled_generic_parms_dies ();
  gen_remaining_tmpl_value_param_die_attribute ();

for now as that causes regressions I have to investigate.

The patch below has passed bootstrap & regtest on x86_64-unknown-linux-gnu
as well as gdb testing.  Twice unpatched, twice patched - results seem
to be somewhat unstable!?  I even refrained from using any -j with
make check-gdb...  maybe it's just contrib/test_summary not coping well
with gdb?  any hints?  Difference between unpatched run 1 & 2 is
for example

--- results.unpatched   2015-08-19 15:08:36.152899926 +0200
+++ results.unpatched2  2015-08-19 15:29:46.902060797 +0200
@@ -209,7 +209,6 @@
 WARNING: remote_expect statement without a default case?!
 WARNING: remote_expect statement without a default case?!
 WARNING: remote_expect statement without a default case?!
-FAIL: gdb.base/varargs.exp: print find_max_float_real(4, fc1, fc2, fc3, 
fc4)
 FAIL: gdb.cp/inherit.exp: print g_vD
 FAIL: gdb.cp/inherit.exp: print g_vE
 FAIL: gdb.cp/no-dmgl-verbose.exp: setting breakpoint at 'f(std::string)'
@@ -238,6 +237,7 @@
 UNRESOLVED: gdb.fortran/types.exp: set print sevenbit-strings
 FAIL: gdb.fortran/whatis_type.exp: run to MAIN__
 WARNING: remote_expect statement without a default case?!
+FAIL: gdb.gdb/complaints.exp: print symfile_complaints->root->fmt
 WARNING: remote_expect statement without a default case?!
 WARNING: remote_expect statement without a default case?!
 WARNING: remote_expect statement without a default case?!
@@ -362,12 +362,12 @@
                === gdb Summary ===
 
-# of expected passes           30881
+# of expected passes           30884
 # of unexpected failures       284
 # of unexpected successes      2
-# of expected failures         85
+# of expected failures         83
 # of unknown successes         2
-# of known failures            60
+# of known failures            59
 # of unresolved testcases      6
 # of untested testcases                32
 # of unsupported tests         165

the sames changes randomly appear/disappear in the patched case.  
Otherwise patched/unpatched agree.

Ok?

Thanks,
Richard.

2015-08-18  Richard Biener  <rguenther@suse.de>

	* debug.h (gcc_debug_hooks::early_finish): Add filename argument.
	* dbxout.c (dbx_debug_hooks): Adjust.
	* debug.c (do_nothing_hooks): Likewise.
	* sdbout.c (sdb_debug_hooks): Likewise.
	* vmsdbgout.c (vmsdbgout_early_finish): New function dispatching
	to dwarf2out variant if needed.
	(vmsdbg_debug_hooks): Adjust.
	* dwarf2out.c (dwarf2_line_hooks): Adjust.
	(flush_limbo_die_list): New function.
	(dwarf2out_finish): Call flush_limbo_die_list instead of
	dwarf2out_early_finish.  Assert there are no deferred asm-names.
	Move early stuff ...
	(dwarf2out_early_finish): ... here.
	* cgraphunit.c (symbol_table::finalize_compilation_unit):
	Call early_finish with main_input_filename argument.


Index: gcc/cgraphunit.c
===================================================================
--- gcc/cgraphunit.c	(revision 226966)
+++ gcc/cgraphunit.c	(working copy)
@@ -2490,7 +2490,7 @@ symbol_table::finalize_compilation_unit
 
   /* Clean up anything that needs cleaning up after initial debug
      generation.  */
-  (*debug_hooks->early_finish) ();
+  (*debug_hooks->early_finish) (main_input_filename);
 
   /* Finally drive the pass manager.  */
   compile ();
Index: gcc/dbxout.c
===================================================================
--- gcc/dbxout.c	(revision 226966)
+++ gcc/dbxout.c	(working copy)
@@ -354,7 +354,7 @@ const struct gcc_debug_hooks dbx_debug_h
 {
   dbxout_init,
   dbxout_finish,
-  debug_nothing_void,
+  debug_nothing_charstar,
   debug_nothing_void,
   debug_nothing_int_charstar,
   debug_nothing_int_charstar,
Index: gcc/debug.c
===================================================================
--- gcc/debug.c	(revision 226966)
+++ gcc/debug.c	(working copy)
@@ -28,7 +28,7 @@ const struct gcc_debug_hooks do_nothing_
 {
   debug_nothing_charstar,
   debug_nothing_charstar,
-  debug_nothing_void,			/* early_finish */
+  debug_nothing_charstar,			/* early_finish */
   debug_nothing_void,
   debug_nothing_int_charstar,
   debug_nothing_int_charstar,
Index: gcc/debug.h
===================================================================
--- gcc/debug.h	(revision 226966)
+++ gcc/debug.h	(working copy)
@@ -31,7 +31,7 @@ struct gcc_debug_hooks
   void (* finish) (const char *main_filename);
 
   /* Run cleanups necessary after early debug generation.  */
-  void (* early_finish) (void);
+  void (* early_finish) (const char *main_filename);
 
   /* Called from cgraph_optimize before starting to assemble
      functions/variables/toplevel asms.  */
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 226966)
+++ gcc/dwarf2out.c	(working copy)
@@ -2420,7 +2420,7 @@ build_cfa_aligned_loc (dw_cfa_location *
 
 static void dwarf2out_init (const char *);
 static void dwarf2out_finish (const char *);
-static void dwarf2out_early_finish (void);
+static void dwarf2out_early_finish (const char *);
 static void dwarf2out_assembly_start (void);
 static void dwarf2out_define (unsigned int, const char *);
 static void dwarf2out_undef (unsigned int, const char *);
@@ -2494,7 +2494,7 @@ const struct gcc_debug_hooks dwarf2_line
 {
   dwarf2out_init,
   debug_nothing_charstar,
-  debug_nothing_void,
+  debug_nothing_charstar,
   debug_nothing_void,
   debug_nothing_int_charstar,
   debug_nothing_int_charstar,
@@ -25128,47 +25128,81 @@ optimize_location_lists (dw_die_ref die)
   optimize_location_lists_1 (die, &htab);
 }
 \f
+/* Traverse the limbo die list, and add parent/child links.  The only
+   dies without parents that should be here are concrete instances of
+   inline functions, and the comp_unit_die.  We can ignore the comp_unit_die.
+   For concrete instances, we can get the parent die from the abstract
+   instance.  */
+
+static void
+flush_limbo_die_list (void)
+{
+  limbo_die_node *node, *next_node;
+
+  for (node = limbo_die_list; node; node = next_node)
+    {
+      dw_die_ref die = node->die;
+      next_node = node->next;
+
+      if (die->die_parent == NULL)
+	{
+	  dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin);
+
+	  if (origin && origin->die_parent)
+	    add_child_die (origin->die_parent, die);
+	  else if (is_cu_die (die))
+	    ;
+	  else if (seen_error ())
+	    /* It's OK to be confused by errors in the input.  */
+	    add_child_die (comp_unit_die (), die);
+	  else
+	    {
+	      /* In certain situations, the lexical block containing a
+		 nested function can be optimized away, which results
+		 in the nested function die being orphaned.  Likewise
+		 with the return type of that nested function.  Force
+		 this to be a child of the containing function.
+
+		 It may happen that even the containing function got fully
+		 inlined and optimized out.  In that case we are lost and
+		 assign the empty child.  This should not be big issue as
+		 the function is likely unreachable too.  */
+	      gcc_assert (node->created_for);
+
+	      if (DECL_P (node->created_for))
+		origin = get_context_die (DECL_CONTEXT (node->created_for));
+	      else if (TYPE_P (node->created_for))
+		origin = scope_die_for (node->created_for, comp_unit_die ());
+	      else
+		origin = comp_unit_die ();
+
+	      add_child_die (origin, die);
+	    }
+	}
+    }
+
+  limbo_die_list = NULL;
+}
+
 /* Output stuff that dwarf requires at the end of every file,
    and generate the DWARF-2 debugging info.  */
 
 static void
-dwarf2out_finish (const char *filename)
+dwarf2out_finish (const char *)
 {
   comdat_type_node *ctnode;
   dw_die_ref main_comp_unit_die;
 
   /* Flush out any latecomers to the limbo party.  */
-  dwarf2out_early_finish ();
+  flush_limbo_die_list ();
 
-  /* PCH might result in DW_AT_producer string being restored from the
-     header compilation, so always fill it with empty string initially
-     and overwrite only here.  */
-  dw_attr_ref producer = get_AT (comp_unit_die (), DW_AT_producer);
-  producer_string = gen_producer_string ();
-  producer->dw_attr_val.v.val_str->refcount--;
-  producer->dw_attr_val.v.val_str = find_AT_string (producer_string);
+  /* We shouldn't have any symbols with delayed asm names for
+     DIEs generated after early finish.  */
+  gcc_assert (deferred_asm_name == NULL);
 
   gen_scheduled_generic_parms_dies ();
   gen_remaining_tmpl_value_param_die_attribute ();
 
-  /* Add the name for the main input file now.  We delayed this from
-     dwarf2out_init to avoid complications with PCH.
-     For LTO produced units use a fixed artificial name to avoid
-     leaking tempfile names into the dwarf.  */
-  if (!in_lto_p)
-    add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
-  else
-    add_name_attribute (comp_unit_die (), "<artificial>");
-  if (!IS_ABSOLUTE_PATH (filename) || targetm.force_at_comp_dir)
-    add_comp_dir_attribute (comp_unit_die ());
-  else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
-    {
-      bool p = false;
-      file_table->traverse<bool *, file_table_relative_p> (&p);
-      if (p)
-	add_comp_dir_attribute (comp_unit_die ());
-    }
-
 #if ENABLE_ASSERT_CHECKING
   {
     dw_die_ref die = comp_unit_die (), c;
@@ -25482,9 +25516,9 @@ dwarf2out_finish (const char *filename)
    has run.  */
 
 static void
-dwarf2out_early_finish (void)
+dwarf2out_early_finish (const char *filename)
 {
-  limbo_die_node *node, *next_node;
+  limbo_die_node *node;
 
   /* Add DW_AT_linkage_name for all deferred DIEs.  */
   for (node = deferred_asm_name; node; node = node->next)
@@ -25502,57 +25536,35 @@ dwarf2out_early_finish (void)
     }
   deferred_asm_name = NULL;
 
-  /* Traverse the limbo die list, and add parent/child links.  The only
-     dies without parents that should be here are concrete instances of
-     inline functions, and the comp_unit_die.  We can ignore the comp_unit_die.
-     For concrete instances, we can get the parent die from the abstract
-     instance.
-
-     The point here is to flush out the limbo list so that it is empty
+  /* The point here is to flush out the limbo list so that it is empty
      and we don't need to stream it for LTO.  */
-  for (node = limbo_die_list; node; node = next_node)
-    {
-      dw_die_ref die = node->die;
-      next_node = node->next;
-
-      if (die->die_parent == NULL)
-	{
-	  dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin);
-
-	  if (origin && origin->die_parent)
-	    add_child_die (origin->die_parent, die);
-	  else if (is_cu_die (die))
-	    ;
-	  else if (seen_error ())
-	    /* It's OK to be confused by errors in the input.  */
-	    add_child_die (comp_unit_die (), die);
-	  else
-	    {
-	      /* In certain situations, the lexical block containing a
-		 nested function can be optimized away, which results
-		 in the nested function die being orphaned.  Likewise
-		 with the return type of that nested function.  Force
-		 this to be a child of the containing function.
+  flush_limbo_die_list ();
 
-		 It may happen that even the containing function got fully
-		 inlined and optimized out.  In that case we are lost and
-		 assign the empty child.  This should not be big issue as
-		 the function is likely unreachable too.  */
-	      gcc_assert (node->created_for);
-
-	      if (DECL_P (node->created_for))
-		origin = get_context_die (DECL_CONTEXT (node->created_for));
-	      else if (TYPE_P (node->created_for))
-		origin = scope_die_for (node->created_for, comp_unit_die ());
-	      else
-		origin = comp_unit_die ();
+  /* PCH might result in DW_AT_producer string being restored from the
+     header compilation, so always fill it with empty string initially
+     and overwrite only here.  */
+  dw_attr_ref producer = get_AT (comp_unit_die (), DW_AT_producer);
+  producer_string = gen_producer_string ();
+  producer->dw_attr_val.v.val_str->refcount--;
+  producer->dw_attr_val.v.val_str = find_AT_string (producer_string);
 
-	      add_child_die (origin, die);
-	    }
-	}
+  /* Add the name for the main input file now.  We delayed this from
+     dwarf2out_init to avoid complications with PCH.
+     For LTO produced units use a fixed artificial name to avoid
+     leaking tempfile names into the dwarf.  */
+  if (!in_lto_p)
+    add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
+  else
+    add_name_attribute (comp_unit_die (), "<artificial>");
+  if (!IS_ABSOLUTE_PATH (filename) || targetm.force_at_comp_dir)
+    add_comp_dir_attribute (comp_unit_die ());
+  else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
+    {
+      bool p = false;
+      file_table->traverse<bool *, file_table_relative_p> (&p);
+      if (p)
+	add_comp_dir_attribute (comp_unit_die ());
     }
-
-  limbo_die_list = NULL;
 }
 
 /* Reset all state within dwarf2out.c so that we can rerun the compiler
Index: gcc/sdbout.c
===================================================================
--- gcc/sdbout.c	(revision 226966)
+++ gcc/sdbout.c	(working copy)
@@ -278,7 +278,7 @@ const struct gcc_debug_hooks sdb_debug_h
 {
   sdbout_init,			         /* init */
   sdbout_finish,		         /* finish */
-  debug_nothing_void,			 /* early_finish */
+  debug_nothing_charstar,		 /* early_finish */
   debug_nothing_void,			 /* assembly_start */
   debug_nothing_int_charstar,	         /* define */
   debug_nothing_int_charstar,	         /* undef */
Index: gcc/vmsdbgout.c
===================================================================
--- gcc/vmsdbgout.c	(revision 226966)
+++ gcc/vmsdbgout.c	(working copy)
@@ -147,6 +147,7 @@ static int write_srccorrs (int);
 
 static void vmsdbgout_init (const char *);
 static void vmsdbgout_finish (const char *);
+static void vmsdbgout_early_finish (const char *);
 static void vmsdbgout_assembly_start (void);
 static void vmsdbgout_define (unsigned int, const char *);
 static void vmsdbgout_undef (unsigned int, const char *);
@@ -174,7 +175,7 @@ static void vmsdbgout_abstract_function
 const struct gcc_debug_hooks vmsdbg_debug_hooks
 = {vmsdbgout_init,
    vmsdbgout_finish,
-   debug_nothing_void,
+   vmsdbgout_early_finish,
    vmsdbgout_assembly_start,
    vmsdbgout_define,
    vmsdbgout_undef,
@@ -1552,7 +1553,17 @@ vmsdbgout_abstract_function (tree decl)
    VMS Debug debugging info.  */
 
 static void
-vmsdbgout_finish (const char *filename ATTRIBUTE_UNUSED)
+vmsdbgout_early_finish (const char *filename)
+{
+  if (write_symbols == VMS_AND_DWARF2_DEBUG)
+    (*dwarf2_debug_hooks.early_finish) (filename);
+}
+
+/* Output stuff that Debug requires at the end of every file and generate the
+   VMS Debug debugging info.  */
+
+static void
+vmsdbgout_finish (const char *filename)
 {
   unsigned int i, ifunc;
   int totsize;