[Bug debug/55059] New: DWARF missing concrete class definition

[Bug debug/55059] New: DWARF missing concrete class definition

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

             Bug #: 55059
           Summary: DWARF missing concrete class definition
    Classification: Unclassified
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: tromey@gcc.gnu.org


Created attachment 28519
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28519
test program

This comes from http://sourceware.org/bugzilla/show_bug.cgi?id=14760

I think GCC master from today generates invalid, or at least
questionable, DWARF for the attached test case.
This causes gdb test suite failures.

The bug is in the DWARF for the instantiation of Base.
Current GCC emits:

 <1><f1>: Abbrev Number: 15 (DW_TAG_structure_type)
    <f2>   DW_AT_name        : (indirect string, offset: 0x191): Base<long int,
47, (& a_global), &S::f>    
    <f6>   DW_AT_byte_size   : 1    
    <f7>   DW_AT_decl_file   : 1    
    <f8>   DW_AT_decl_line   : 30    
    <f9>   DW_AT_declaration : 1    
    <f9>   DW_AT_sibling     : <0x155>    

This has DW_AT_declaration -- but there is no concrete instance of the type.

Previous versions of GCC omitted this attribute.  For example, Fedora 16
GCC:

 <1><f1>: Abbrev Number: 10 (DW_TAG_structure_type)
    <f2>   DW_AT_name        : (indirect string, offset: 0x135): Base<long int,
47, (& a_global), &S::f>    
    <f6>   DW_AT_byte_size   : 1    
    <f7>   DW_AT_decl_file   : 1    
    <f8>   DW_AT_decl_line   : 30    
    <f9>   DW_AT_sibling     : <0x155>    

I think the previous approach is preferable.

[Bug debug/55059] DWARF missing concrete class definition

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ccoutant at gcc dot
                   |                            |gnu.org, jakub at gcc dot
                   |                            |gnu.org, jason at gcc dot
                   |                            |gnu.org, pkoning at gcc dot
                   |                            |gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-10-25 06:59:47 UTC ---
See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192739
http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02059.html

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-debug
            Version|unknown                     |4.8.0
           See Also|                            |http://sourceware.org/bugzi
                   |                            |lla/show_bug.cgi?id=14760
   Target Milestone|---                         |4.8.0
            Summary|DWARF missing concrete      |[4.8 Regression] DWARF
                   |class definition            |missing concrete class
                   |                            |definition

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-17 14:47:37 UTC ---
So, where do we stand with this?  Can GDB be changed to cope with this, or do
you think it isn't valid DWARF?

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

--- Comment #3 from Tom Tromey <tromey at gcc dot gnu.org> 2013-01-03 19:29:28 UTC ---
(In reply to comment #2)
> So, where do we stand with this?  Can GDB be changed to cope with this, or do
> you think it isn't valid DWARF?

It seems strange at least.
I don't have chapter and verse against it though.

If we change gdb to cope with this, I think the effect will be to undo what
the patches here were attempting to accomplish.
That would be fine by me -- but in that case, why not just revert
gcc to the status quo ante, preserving compatibility with existing
versions of gdb?

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

--- Comment #4 from Jason Merrill <jason at gcc dot gnu.org> 2013-01-28 01:54:25 UTC ---
(In reply to comment #3)
> If we change gdb to cope with this, I think the effect will be to undo what
> the patches here were attempting to accomplish.

I believe the patch was trying to avoid treating a partial definition as a full
definition when there's a full definition of the class in another CU.  If
there's only the partial definition, the best we can do is to use that.  That
seems better to me than the situation we had before.

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[tromey at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

--- Comment #5 from Tom Tromey <tromey at gcc dot gnu.org> 2013-01-28 20:08:11 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > If we change gdb to cope with this, I think the effect will be to undo what
> > the patches here were attempting to accomplish.
> 
> I believe the patch was trying to avoid treating a partial definition as a full
> definition when there's a full definition of the class in another CU.  If
> there's only the partial definition, the best we can do is to use that.  That
> seems better to me than the situation we had before.

That isn't what gdb does, though.

I've been thinking about this today and have a list of objections -

I couldn't find support in the DWARF standard for this approach.
I realize DWARF is "anything goes" -- but if this approach is taken
then I think at least a bug report to all the known DWARF consumers is
in order.  I imagine several of them will need fixes.

Fixing this in gdb seems a bit expensive.  I think we will have to track
all partial definitions of a type and then replace them with the real
definition, if one is found.  It is unclear how much DWARF we'll have to
read to dig up the real definition when using the .gdb_index.  Basically
this is a new, non-local property of DWARF with zero connections between
the declaration and definition.  It seems like there can be many obscure
bad scenarios.

If the motivation for this feature in GCC is space savings in the DWARF,
then it seems like it should be on one of the various -f options, not
the default.  For .debug_types at least I think it is better to always
emit the full information.  This may be true of dwz as well, though I suppose
dwz could also be taught this convention.

IIUC this approach relies on ODR for C++ (and presumably just can't be
used for other languages).  However, I think this presumes too much.
It may be that the ODR is violated in a program, and the debuginfo can
be used to help determine this.  But this rule makes it harder to
discover differences between CUs.

Finally, the original thread discusses gdb a fair amount and by my reading
was contingent on "gdb doing the right thing".  But this is not the case.

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-01-31
         AssignedTo|unassigned at gcc dot       |jason at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> 2013-01-31 17:48:15 UTC ---
(In reply to comment #4)
> I believe the patch was trying to avoid treating a partial definition as a full
> definition when there's a full definition of the class in another CU.  If
> there's only the partial definition, the best we can do is to use that.

I was wrong; the patch was just trying to avoid pretending that a partial
definition is a full definition.  Which would be fine except that GCC is being
too aggressive about pruning class members.

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

--- Comment #7 from Jason Merrill <jason at gcc dot gnu.org> 2013-01-31 18:29:21 UTC ---
Author: jason
Date: Thu Jan 31 18:29:15 2013
New Revision: 195622

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195622
Log:
    PR debug/55059
    PR debug/54508
    * dwarf2out.c (prune_unused_types_mark): Mark all of parent's
    children if parent is a class.
    (prune_unused_types_prune): Don't add DW_AT_declaration.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dwarf2out.c
    trunk/gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
    trunk/gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C
    trunk/gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C

[Bug debug/55059] [4.8 Regression] DWARF missing concrete class definition

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55059

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> 2013-01-31 18:32:01 UTC ---
Fixed.