| Message ID | 20170119153203.GN1867@tucnak |
|---|---|
| State | New |
| Headers | show |
OK. On Thu, Jan 19, 2017 at 10:32 AM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > This is on top of the PR78835 patch I've posted yesterday. > We ICE on the following testcase reduced from libstdc++ ios_failure.cc. > If there are nested comdat types, as B (outer type) and A (inner type) in the > testcase, we first move A to a type unit and add there copies of its > children, and preserve a skeleton copy of A in the main CU with the > originals of their children (i.e. something other DIEs may refer to). > Then we try to move B into another type unit, but mishandle the children, > the DIE for foo subprogram ends up in the B type unit rather than in the > main CU where it belongs. Of course nothing can refer to that there from > outside of that type unit. > > Either hunk in dwarf2out.c is actually enough to fix the ICE. The second > hunk fixes the bug that after the moving of the children etc. we end up with > node.old_die == node.new_die, which is obviously wrong. We always want > old_die to be the DIE that will be in the type unit while new_die one that > will remain in the main CU. The first hunk is an optimization, my > understanding is that the creation of the skeleton DIEs is so that uses > outside of the type unit (i.e. from the main CU) can actually refer to those > DIEs (when we lookup_decl_die etc.; from main CU to type units we can only > refer using DW_FORM_ref_sig8 to the type unit's single chosen type die). > As break_out_comdat_types and its helpers don't actually adjust random > references, for nested types I actually think it is enough to have in the > type unit of the outer type just a skeleton unit of the inner type with > DW_AT_signature of the type residing in yet another type unit, I think there > is no way to refer to its children from that type unit. So there is no need > to duplicate the children and we can just move them to the main CU. > > Bootstrapped/regtested on i686-linux with -fdebug-types-section hacked in > common.opt to be the default, ok for trunk? > > Note Ada bootstrap on x86_64-linux with -fdebug-types-section is still > broken, but that is preexisting, not introduced by this patch: > .../gcc/gnat1 -gnatwa -quiet -nostdinc -dumpbase g-pehage.adb -auxbase-strip g-pehage.o -O2 -Wextra -Wall -g -fpic -gnatpg -mtune=generic -march=x86-64 -gnatO g-pehage.o g-pehage.adb -o /tmp/ccrAYuNB.s -fdebug-types-section > raised STORAGE_ERROR : stack overflow or erroneous memory access > > 2017-01-19 Jakub Jelinek <jakub@redhat.com> > > PR debug/79129 > * dwarf2out.c (generate_skeleton_bottom_up): For children with > comdat_type_p set, just clone them, but keep the children in the > original DIE. > > * g++.dg/debug/dwarf2/pr79129.C: New test. > > --- gcc/dwarf2out.c.jj 2017-01-18 13:40:09.000000000 +0100 > +++ gcc/dwarf2out.c 2017-01-19 10:46:36.297776725 +0100 > @@ -7918,6 +7918,19 @@ generate_skeleton_bottom_up (skeleton_ch > add_child_die (parent->new_die, c); > c = prev; > } > + else if (c->comdat_type_p) > + { > + /* This is the skeleton of earlier break_out_comdat_types > + type. Clone the existing DIE, but keep the children > + under the original (which is in the main CU). */ > + dw_die_ref clone = clone_die (c); > + > + replace_child (c, clone, prev); > + generate_skeleton_ancestor_tree (parent); > + add_child_die (parent->new_die, c); > + c = clone; > + continue; > + } > else > { > /* Clone the existing DIE, move the original to the skeleton > @@ -7936,6 +7949,7 @@ generate_skeleton_bottom_up (skeleton_ch > replace_child (c, clone, prev); > generate_skeleton_ancestor_tree (parent); > add_child_die (parent->new_die, c); > + node.old_die = clone; > node.new_die = c; > c = clone; > } > --- gcc/testsuite/g++.dg/debug/dwarf2/pr79129.C.jj 2017-01-19 11:15:01.876727938 +0100 > +++ gcc/testsuite/g++.dg/debug/dwarf2/pr79129.C 2017-01-19 11:14:25.000000000 +0100 > @@ -0,0 +1,12 @@ > +/* PR debug/79129 */ > +/* { dg-do compile } */ > +/* { dg-options "-gdwarf-4 -O2 -fdebug-types-section" } */ > + > +struct B > +{ > + struct A { void foo (int &); }; > + A *bar (); > + ~B () { int a = 1; bar ()->foo (a); } > +}; > +struct C { ~C (); B c; }; > +C::~C () {} > > Jakub
--- gcc/dwarf2out.c.jj 2017-01-18 13:40:09.000000000 +0100 +++ gcc/dwarf2out.c 2017-01-19 10:46:36.297776725 +0100 @@ -7918,6 +7918,19 @@ generate_skeleton_bottom_up (skeleton_ch add_child_die (parent->new_die, c); c = prev; } + else if (c->comdat_type_p) + { + /* This is the skeleton of earlier break_out_comdat_types + type. Clone the existing DIE, but keep the children + under the original (which is in the main CU). */ + dw_die_ref clone = clone_die (c); + + replace_child (c, clone, prev); + generate_skeleton_ancestor_tree (parent); + add_child_die (parent->new_die, c); + c = clone; + continue; + } else { /* Clone the existing DIE, move the original to the skeleton @@ -7936,6 +7949,7 @@ generate_skeleton_bottom_up (skeleton_ch replace_child (c, clone, prev); generate_skeleton_ancestor_tree (parent); add_child_die (parent->new_die, c); + node.old_die = clone; node.new_die = c; c = clone; } --- gcc/testsuite/g++.dg/debug/dwarf2/pr79129.C.jj 2017-01-19 11:15:01.876727938 +0100 +++ gcc/testsuite/g++.dg/debug/dwarf2/pr79129.C 2017-01-19 11:14:25.000000000 +0100 @@ -0,0 +1,12 @@ +/* PR debug/79129 */ +/* { dg-do compile } */ +/* { dg-options "-gdwarf-4 -O2 -fdebug-types-section" } */ + +struct B +{ + struct A { void foo (int &); }; + A *bar (); + ~B () { int a = 1; bar ()->foo (a); } +}; +struct C { ~C (); B c; }; +C::~C () {}