Patchwork Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 18, 2010, 8:42 p.m.
Message ID <20101118204216.GB29412@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/72145/
State New
Headers show

Comments

Jakub Jelinek - Nov. 18, 2010, 8:42 p.m.
Hi!

As mentioned in the PR, reusing a DW_TAG_subprogram DW_AT_declaration DIE
for the definition doesn't work with -feliminate-dwarf2-dups or -gdwarf-4,
there we really want to avoid DW_TAG_subprograms for actual routine
definitions, otherwise we ICE because .debug_aranges isn't split to point to
those units (but the comdat CUs don't seem to be meant to contain debug info
for code).

Fixed thusly, bootstrapped/regtested on x86_64-linux, ok for trunk?

2010-11-18  Jakub Jelinek  <jakub@redhat.com>

	PR debug/46123
	* dwarf2out.c (gen_subprogram_die): Don't reuse the old
	die if -feliminate-dwarf2-dups or -gdwarf-4.

	* g++.dg/debug/pr46123.C: New test.
	* g++.dg/debug/dwarf2/pr46123.C: New test.


	Jakub
Jason Merrill - Nov. 24, 2010, 10:49 p.m.
On 11/18/2010 03:42 PM, Jakub Jelinek wrote:
> As mentioned in the PR, reusing a DW_TAG_subprogram DW_AT_declaration DIE
> for the definition doesn't work with -feliminate-dwarf2-dups or -gdwarf-4,
> there we really want to avoid DW_TAG_subprograms for actual routine
> definitions, otherwise we ICE because .debug_aranges isn't split to point to
> those units (but the comdat CUs don't seem to be meant to contain debug info
> for code).

It seems like the logic already there is trying to do the right thing. 
It only shares the old DIE if the old one is either at file scope or 
inside a function; in the testcase the old one is inside a function.

I think it's correct to re-use the DIE in the local class for the 
abstract instance of the member function; it's just any concrete 
instance that we want to go somewhere else.

Jason
Jason Merrill - Nov. 26, 2010, 3:18 a.m.
On 11/24/2010 05:49 PM, Jason Merrill wrote:
> I think it's correct to re-use the DIE in the local class for the
> abstract instance of the member function; it's just any concrete
> instance that we want to go somewhere else.

So, I'm not sure why you would need to change the if (old_die) branch at 
all; concrete instances should fall under the first branch, if (origin 
!= NULL).

It seems that perhaps the problem is that we are failing to split the 
abstract and concrete instances here: gen_decl_die has

>         /* If we're emitting an out-of-line copy of an inline function,
>            emit info for the abstract instance and set up to refer to it.  */
>         else if (cgraph_function_possibly_inlined_p (decl)
>                  && ! DECL_ABSTRACT (decl)
>                  && ! class_or_namespace_scope_p (context_die)
>                  /* dwarf2out_abstract_function won't emit a die if this is just
>                     a declaration.  We must avoid setting DECL_ABSTRACT_ORIGIN in
>                     that case, because that works only if we have a die.  */
>                  && DECL_INITIAL (decl) != NULL_TREE)
>           {
>             dwarf2out_abstract_function (decl);
>             set_decl_origin_self (decl);
>           }

but cgraph_function_possibly_inlined_p is false in this case.  I guess 
that isn't the right test.

Jason
Jakub Jelinek - Nov. 26, 2010, 11:26 a.m.
On Thu, Nov 25, 2010 at 10:18:12PM -0500, Jason Merrill wrote:
> It seems that perhaps the problem is that we are failing to split
> the abstract and concrete instances here: gen_decl_die has
> 
> >        /* If we're emitting an out-of-line copy of an inline function,
> >           emit info for the abstract instance and set up to refer to it.  */
> >        else if (cgraph_function_possibly_inlined_p (decl)
> >                 && ! DECL_ABSTRACT (decl)
> >                 && ! class_or_namespace_scope_p (context_die)
> >                 /* dwarf2out_abstract_function won't emit a die if this is just
> >                    a declaration.  We must avoid setting DECL_ABSTRACT_ORIGIN in
> >                    that case, because that works only if we have a die.  */
> >                 && DECL_INITIAL (decl) != NULL_TREE)
> >          {
> >            dwarf2out_abstract_function (decl);
> >            set_decl_origin_self (decl);
> >          }
> 
> but cgraph_function_possibly_inlined_p is false in this case.  I
> guess that isn't the right test.

You're right, I'll look at it.

	Jakub
Jakub Jelinek - Nov. 26, 2010, 1:41 p.m.
On Fri, Nov 26, 2010 at 12:26:22PM +0100, Jakub Jelinek wrote:
> On Thu, Nov 25, 2010 at 10:18:12PM -0500, Jason Merrill wrote:
> > It seems that perhaps the problem is that we are failing to split
> > the abstract and concrete instances here: gen_decl_die has
> > 
> > >        /* If we're emitting an out-of-line copy of an inline function,
> > >           emit info for the abstract instance and set up to refer to it.  */
> > >        else if (cgraph_function_possibly_inlined_p (decl)
> > >                 && ! DECL_ABSTRACT (decl)
> > >                 && ! class_or_namespace_scope_p (context_die)
> > >                 /* dwarf2out_abstract_function won't emit a die if this is just
> > >                    a declaration.  We must avoid setting DECL_ABSTRACT_ORIGIN in
> > >                    that case, because that works only if we have a die.  */
> > >                 && DECL_INITIAL (decl) != NULL_TREE)
> > >          {
> > >            dwarf2out_abstract_function (decl);
> > >            set_decl_origin_self (decl);
> > >          }
> > 
> > but cgraph_function_possibly_inlined_p is false in this case.  I
> > guess that isn't the right test.
> 
> You're right, I'll look at it.

Actually no, the reason we aren't splitting is -O0 and the function
not having always_inline attribute.
We don't split any inlines into abstract/concrete DIEs when no inlining
is performed.

	Jakub
Jason Merrill - Nov. 26, 2010, 2:56 p.m.
On 11/26/2010 08:41 AM, Jakub Jelinek wrote:
> Actually no, the reason we aren't splitting is -O0 and the function
> not having always_inline attribute.
> We don't split any inlines into abstract/concrete DIEs when no inlining
> is performed.

Right, but this PR seems to indicate that this should be changed.

Jason

Patch

--- gcc/dwarf2out.c.jj	2010-11-15 23:28:02.000000000 +0100
+++ gcc/dwarf2out.c	2010-11-18 13:45:10.189654791 +0100
@@ -18840,6 +18840,21 @@  gen_subprogram_die (tree decl, dw_die_re
 		  && (get_AT_unsigned (old_die, DW_AT_decl_line)
 		      == (unsigned) s.line))))
 	{
+	 /* Don't use the old DIE if eliminating DWARF dups using
+	    break_out_includes or DWARF4 .debug_types, as that could
+	    leave subprogram DIEs with *_pc attributes in the comdat CUs
+	    or .debug_types.  */
+	  if ((flag_eliminate_dwarf2_dups || dwarf_version >= 4)
+	      && !is_cu_die (old_die->die_parent))
+	    {
+	      if (decl_function_context (decl)
+		  && DECL_CONTEXT (decl)
+		  && TREE_CODE (DECL_CONTEXT (decl)) != FUNCTION_DECL
+		  && TREE_CODE (DECL_CONTEXT (decl)) != NAMESPACE_DECL)
+		context_die = comp_unit_die ();
+	      goto new_subr_die;
+	    }
+	    
 	  subr_die = old_die;
 
 	  /* Clear out the declaration attribute and the formal parameters.
@@ -18853,6 +18868,7 @@  gen_subprogram_die (tree decl, dw_die_re
 	}
       else
 	{
+	new_subr_die:
 	  subr_die = new_die (DW_TAG_subprogram, context_die, decl);
 	  add_AT_specification (subr_die, old_die);
 	  if (get_AT_file (old_die, DW_AT_decl_file) != file_index)
--- gcc/testsuite/g++.dg/debug/dwarf2/pr46123.C.jj	2010-11-18 13:31:40.000000000 +0100
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr46123.C	2010-11-18 13:46:09.889261025 +0100
@@ -0,0 +1,47 @@ 
+// PR debug/46123
+// { dg-do compile }
+// { dg-options "-gdwarf-4" }
+
+struct foo
+{
+  static int bar ()
+  {
+    int i;
+    static int baz = 1;
+    {
+      static int baz = 2;
+      i = baz++;
+    }
+    {
+      struct baz
+      {
+	static int m ()
+	{
+	  static int n;
+	  return n += 10;
+	}
+      };
+      baz a;
+      i += a.m ();
+    }
+    {
+      static int baz = 3;
+      i += baz;
+      baz += 30;
+    }
+    i += baz;
+    baz += 60;
+    return i;
+  }
+};
+
+int main ()
+{
+  foo x;
+
+  if (x.bar () != 16)
+    return 1;
+  if (x.bar() != 117)
+    return 1;
+  return 0;
+}
--- gcc/testsuite/g++.dg/debug/pr46123.C.jj	2010-11-18 13:31:40.078247818 +0100
+++ gcc/testsuite/g++.dg/debug/pr46123.C	2010-11-18 13:34:06.486261122 +0100
@@ -0,0 +1,47 @@ 
+// PR debug/46123
+// { dg-do compile }
+// { dg-options "-g -feliminate-dwarf2-dups" }
+
+struct foo
+{
+  static int bar ()
+  {
+    int i;
+    static int baz = 1;
+    {
+      static int baz = 2;
+      i = baz++;
+    }
+    {
+      struct baz
+      {
+	static int m ()
+	{
+	  static int n;
+	  return n += 10;
+	}
+      };
+      baz a;
+      i += a.m ();
+    }
+    {
+      static int baz = 3;
+      i += baz;
+      baz += 30;
+    }
+    i += baz;
+    baz += 60;
+    return i;
+  }
+};
+
+int main ()
+{
+  foo x;
+
+  if (x.bar () != 16)
+    return 1;
+  if (x.bar() != 117)
+    return 1;
+  return 0;
+}