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

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

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

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

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

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

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