public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
@ 2010-11-18 20:56 Jakub Jelinek
  2010-11-25  1:59 ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2010-11-18 20:56 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

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.

--- 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;
+}

	Jakub

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-18 20:56 [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123) Jakub Jelinek
@ 2010-11-25  1:59 ` Jason Merrill
  2010-11-26  8:57   ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-11-25  1:59 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-25  1:59 ` Jason Merrill
@ 2010-11-26  8:57   ` Jason Merrill
  2010-11-26 15:15     ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-11-26  8:57 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-26  8:57   ` Jason Merrill
@ 2010-11-26 15:15     ` Jakub Jelinek
  2010-11-26 15:23       ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2010-11-26 15:15 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-26 15:15     ` Jakub Jelinek
@ 2010-11-26 15:23       ` Jakub Jelinek
  2010-11-26 16:12         ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2010-11-26 15:23 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-26 15:23       ` Jakub Jelinek
@ 2010-11-26 16:12         ` Jason Merrill
  2010-11-26 20:04           ` Jakub Jelinek
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2010-11-26 16:12 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-26 16:12         ` Jason Merrill
@ 2010-11-26 20:04           ` Jakub Jelinek
  2010-11-27 10:47             ` Jason Merrill
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2010-11-26 20:04 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Fri, Nov 26, 2010 at 09:56:17AM -0500, Jason Merrill wrote:
> 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.

--- dwarf2out.c.jj	2010-11-19 20:56:54.000000000 +0100
+++ dwarf2out.c	2010-11-26 17:26:25.000000000 +0100
@@ -20837,7 +20837,8 @@ gen_decl_die (tree decl, tree origin, dw
 
       /* 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)
+      else if ((cgraph_function_possibly_inlined_p (decl)
+		|| DECL_DECLARED_INLINE_P (decl))
 	       && ! DECL_ABSTRACT (decl)
 	       && ! class_or_namespace_scope_p (context_die)
 	       /* dwarf2out_abstract_function won't emit a die if this is
 	        * just

doesn't help though, it still ICEs.  Without -feliminate-dwarf2-dups or
-gdwarf-4 it shows that abstract and concrete DIEs are separate, but
both are children of the same DW_TAG_structure_type "baz".

	Jakub

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-26 20:04           ` Jakub Jelinek
@ 2010-11-27 10:47             ` Jason Merrill
  2010-11-29 13:24               ` Jakub Jelinek
       [not found]               ` <4CF5839F.1080608@redhat.com>
  0 siblings, 2 replies; 10+ messages in thread
From: Jason Merrill @ 2010-11-27 10:47 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 11/26/2010 11:37 AM, Jakub Jelinek wrote:
> +      else if ((cgraph_function_possibly_inlined_p (decl)
> +		|| DECL_DECLARED_INLINE_P (decl))
>
> doesn't help though, it still ICEs.  Without -feliminate-dwarf2-dups or
> -gdwarf-4 it shows that abstract and concrete DIEs are separate, but
> both are children of the same DW_TAG_structure_type "baz".

Right, so the concrete instance needs to move to be under comp_unit_die. 
  A comment in gen_subprogram_die says "We always want the DIE for this 
function that has the *_pc attributes to be under comp_unit_die so the 
debugger can find it."  But it's not clear to me right now what code is 
trying to make that happen.

Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
  2010-11-27 10:47             ` Jason Merrill
@ 2010-11-29 13:24               ` Jakub Jelinek
       [not found]               ` <4CF5839F.1080608@redhat.com>
  1 sibling, 0 replies; 10+ messages in thread
From: Jakub Jelinek @ 2010-11-29 13:24 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Sat, Nov 27, 2010 at 12:51:07AM -0500, Jason Merrill wrote:
> On 11/26/2010 11:37 AM, Jakub Jelinek wrote:
> >+      else if ((cgraph_function_possibly_inlined_p (decl)
> >+		|| DECL_DECLARED_INLINE_P (decl))
> >
> >doesn't help though, it still ICEs.  Without -feliminate-dwarf2-dups or
> >-gdwarf-4 it shows that abstract and concrete DIEs are separate, but
> >both are children of the same DW_TAG_structure_type "baz".
> 
> Right, so the concrete instance needs to move to be under
> comp_unit_die.  A comment in gen_subprogram_die says "We always want
> the DIE for this function that has the *_pc attributes to be under
> comp_unit_die so the debugger can find it."  But it's not clear to
> me right now what code is trying to make that happen.

It is dwarf2out_finish:
  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)
            add_child_die (origin->die_parent, die);
...
which places the limbo nodes at origin's parent.  BTW, the ICE
happens with -O2 -gdwarf-4 -fkeep-inline-function mangle3.C or
-O2 -feliminate-dwarf2-dups -fkeep-inline-functions mangle3.C
as well.

	Jakub

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123)
       [not found]               ` <4CF5839F.1080608@redhat.com>
@ 2010-12-01  7:35                 ` Jakub Jelinek
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Jelinek @ 2010-12-01  7:35 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Tue, Nov 30, 2010 at 06:07:11PM -0500, Jason Merrill wrote:
> On 11/27/2010 12:51 AM, Jason Merrill wrote:
> I poked around at this for a while myself; I was puzzled as to why
> we were trying to put the local type in a comdat unit in the first
> place, since it's internal to a function.  It turned out that we
> were wrongly attaching the local type to the declaration die for the
> containing function; fixing that makes your testcases pass.  Does
> this patch make sense to you?
> 

> commit 61e1c85d04c7f2e57b5e4da45c904479f82c240e
> Author: Jason Merrill <jason@redhat.com>
> Date:   Mon Nov 29 20:30:04 2010 -0500
> 
>     	PR debug/46123
>     	* dwarf2out.c (gen_tagged_type_die): Don't put local types in
>     	a declaration DIE.
> 

Yeah, this makes sense.  Thanks for looking into it.

	Jakub

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-12-01  7:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-18 20:56 [PATCH] Fix ICE with -feliminate-dwarf2-dups or -gdwarf-4 (PR debug/46123) Jakub Jelinek
2010-11-25  1:59 ` Jason Merrill
2010-11-26  8:57   ` Jason Merrill
2010-11-26 15:15     ` Jakub Jelinek
2010-11-26 15:23       ` Jakub Jelinek
2010-11-26 16:12         ` Jason Merrill
2010-11-26 20:04           ` Jakub Jelinek
2010-11-27 10:47             ` Jason Merrill
2010-11-29 13:24               ` Jakub Jelinek
     [not found]               ` <4CF5839F.1080608@redhat.com>
2010-12-01  7:35                 ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).