[PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Jakub Jelinek]

Hi!

i686-linux LTO bootstrap currently fails, because in one partition
we emit .Ldebug_info0 label twice.  The problem is that
resolve_addr for call_site support attempts to force_decl_die external
function decls, and at least with LTO that in turn can attempt
to create new type DIEs, in this case an enumeration with context_die
being NULL.  Unfortunately the code to add proper parents to limbo nodes
is done right before resolve_addr (and should be done there, so that
resolve_addr reaches all the needed DIEs).

This patch fixes it by running over limbo_die_list nodes again if
resolve_addr created any.

LTO bootstrapped/regtested on i686-linux, ok for trunk?

2011-04-05  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/48148
	* dwarf2out.c (add_parents_for_limbo_dies): New function.
	(dwarf2out_finish): Call it, and call it again if any new
	limbo_die_list nodes appeared after resolve_addr.

--- gcc/dwarf2out.c.jj	2011-04-05 12:34:11.000000000 +0200
+++ gcc/dwarf2out.c	2011-04-05 13:34:04.628420737 +0200
@@ -23495,47 +23495,17 @@ optimize_location_lists (dw_die_ref die)
   htab_delete (htab);
 }
 \f
-/* Output stuff that dwarf requires at the end of every file,
-   and generate the DWARF-2 debugging info.  */
+/* 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
-dwarf2out_finish (const char *filename)
+add_parents_for_limbo_dies (void)
 {
   limbo_die_node *node, *next_node;
-  comdat_type_node *ctnode;
-  htab_t comdat_type_table;
-  unsigned int i;
-
-  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.  */
-  add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
-  if (!IS_ABSOLUTE_PATH (filename))
-    add_comp_dir_attribute (comp_unit_die ());
-  else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
-    {
-      bool p = false;
-      htab_traverse (file_table, file_table_relative_p, &p);
-      if (p)
-	add_comp_dir_attribute (comp_unit_die ());
-    }
-
-  for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++)
-    {
-      add_location_or_const_value_attribute (
-        VEC_index (deferred_locations, deferred_locations_list, i)->die,
-        VEC_index (deferred_locations, deferred_locations_list, i)->variable,
-	false,
-	DW_AT_location);
-    }
-
-  /* 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.  */
   for (node = limbo_die_list; node; node = next_node)
     {
       dw_die_ref die = node->die;
@@ -23587,9 +23557,52 @@ dwarf2out_finish (const char *filename)
     }
 
   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)
+{
+  limbo_die_node *node;
+  comdat_type_node *ctnode;
+  htab_t comdat_type_table;
+  unsigned int i;
+
+  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.  */
+  add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
+  if (!IS_ABSOLUTE_PATH (filename))
+    add_comp_dir_attribute (comp_unit_die ());
+  else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
+    {
+      bool p = false;
+      htab_traverse (file_table, file_table_relative_p, &p);
+      if (p)
+	add_comp_dir_attribute (comp_unit_die ());
+    }
+
+  for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++)
+    {
+      add_location_or_const_value_attribute (
+	VEC_index (deferred_locations, deferred_locations_list, i)->die,
+	VEC_index (deferred_locations, deferred_locations_list, i)->variable,
+	false,
+	DW_AT_location);
+    }
+
+  add_parents_for_limbo_dies ();
 
   resolve_addr (comp_unit_die ());
 
+  /* resolve_addr might have created new DIEs.  */
+  if (limbo_die_list)
+    add_parents_for_limbo_dies ();
+
   for (node = deferred_asm_name; node; node = node->next)
     {
       tree decl = node->created_for;

	Jakub

Re: [PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Nathan Froyd]

On Tue, Apr 05, 2011 at 04:19:39PM +0200, Jakub Jelinek wrote:
> +  for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++)
> +    {
> +      add_location_or_const_value_attribute (
> +	VEC_index (deferred_locations, deferred_locations_list, i)->die,
> +	VEC_index (deferred_locations, deferred_locations_list, i)->variable,
> +	false,
> +	DW_AT_location);
> +    }

Tiny, non-binding suggestion: use FOR_EACH_VEC_ELT here?

-Nathan

Re: [PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Jakub Jelinek]

On Tue, Apr 05, 2011 at 02:06:14PM -0700, Nathan Froyd wrote:
> On Tue, Apr 05, 2011 at 04:19:39PM +0200, Jakub Jelinek wrote:
> > +  for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++)
> > +    {
> > +      add_location_or_const_value_attribute (
> > +	VEC_index (deferred_locations, deferred_locations_list, i)->die,
> > +	VEC_index (deferred_locations, deferred_locations_list, i)->variable,
> > +	false,
> > +	DW_AT_location);
> > +    }
> 
> Tiny, non-binding suggestion: use FOR_EACH_VEC_ELT here?

Feel free to do that afterwards, while diff decided to include that
part of code in the patch, it wasn't actually changing at all, what changed
was that the code afterwards was moved into a separate function
and the length of the code being moved probably was bigger than
length from dwarf2out_finish start to that point.

	Jakub

Re: [PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Jason Merrill]

On 04/05/2011 10:19 AM, Jakub Jelinek wrote:
> i686-linux LTO bootstrap currently fails, because in one partition
> we emit .Ldebug_info0 label twice.  The problem is that
> resolve_addr for call_site support attempts to force_decl_die external
> function decls, and at least with LTO that in turn can attempt
> to create new type DIEs, in this case an enumeration with context_die
> being NULL.  Unfortunately the code to add proper parents to limbo nodes
> is done right before resolve_addr (and should be done there, so that
> resolve_addr reaches all the needed DIEs).
>
> +/* 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.  */

Sounds like this comment needs to be updated if there can be types on 
the list as well.

Jason

Re: [PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Jakub Jelinek]

On Fri, Apr 08, 2011 at 01:58:14PM -0400, Jason Merrill wrote:
> On 04/05/2011 10:19 AM, Jakub Jelinek wrote:
> >i686-linux LTO bootstrap currently fails, because in one partition
> >we emit .Ldebug_info0 label twice.  The problem is that
> >resolve_addr for call_site support attempts to force_decl_die external
> >function decls, and at least with LTO that in turn can attempt
> >to create new type DIEs, in this case an enumeration with context_die
> >being NULL.  Unfortunately the code to add proper parents to limbo nodes
> >is done right before resolve_addr (and should be done there, so that
> >resolve_addr reaches all the needed DIEs).
> >
> >+/* 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.  */
> 
> Sounds like this comment needs to be updated if there can be types
> on the list as well.

On a closer look, this seems to be because LTO messes up types terribly,
struct cpp_options's lang field doesn't have enum c_lang type, but
enum prec whose TYPE_CONTEXT is c_parser_binary_expression
function from c_parser.c.  So when trying to create DIE for cpp_options
and stuff in it we end up with the surprising limbo die.
Therefore, I'm withdrawing my patch and will look into this mess on Monday.

	Jakub

Re: [PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Richard Guenther]

On Fri, 8 Apr 2011, Jakub Jelinek wrote:

> On Fri, Apr 08, 2011 at 01:58:14PM -0400, Jason Merrill wrote:
> > On 04/05/2011 10:19 AM, Jakub Jelinek wrote:
> > >i686-linux LTO bootstrap currently fails, because in one partition
> > >we emit .Ldebug_info0 label twice.  The problem is that
> > >resolve_addr for call_site support attempts to force_decl_die external
> > >function decls, and at least with LTO that in turn can attempt
> > >to create new type DIEs, in this case an enumeration with context_die
> > >being NULL.  Unfortunately the code to add proper parents to limbo nodes
> > >is done right before resolve_addr (and should be done there, so that
> > >resolve_addr reaches all the needed DIEs).
> > >
> > >+/* 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.  */
> > 
> > Sounds like this comment needs to be updated if there can be types
> > on the list as well.
> 
> On a closer look, this seems to be because LTO messes up types terribly,
> struct cpp_options's lang field doesn't have enum c_lang type, but
> enum prec whose TYPE_CONTEXT is c_parser_binary_expression
> function from c_parser.c.  So when trying to create DIE for cpp_options
> and stuff in it we end up with the surprising limbo die.
> Therefore, I'm withdrawing my patch and will look into this mess on Monday.

We are definitely unifying enum types too eagerly.  It's on my TODO
to fix that, but it had low priority sofar.

Richard.

Re: [PATCH] Fix LTO bootstrap on i686-linux (problem with two Ldebug_info0 labels; PR bootstrap/48148)

[Michael Matz]

Hi,

On Fri, 8 Apr 2011, Richard Guenther wrote:

> > > Sounds like this comment needs to be updated if there can be types 
> > > on the list as well.
> > 
> > On a closer look, this seems to be because LTO messes up types 
> > terribly, struct cpp_options's lang field doesn't have enum c_lang 
> > type, but enum prec whose TYPE_CONTEXT is c_parser_binary_expression 
> > function from c_parser.c.  So when trying to create DIE for 
> > cpp_options and stuff in it we end up with the surprising limbo die. 
> > Therefore, I'm withdrawing my patch and will look into this mess on 
> > Monday.
> 
> We are definitely unifying enum types too eagerly.  It's on my TODO to 
> fix that, but it had low priority sofar.

It's too eager "only" for debug info, and that is in a suboptimal state 
for LTO anyway.  early-debug-info will fix all of our problems.  ahem :-)


Ciao,
Michael.