In reducing 72828 I stumbled onto a different ICE with: struct Bar { void Baz (); }; void Foo (Bar &t) { t.Baz (); } ./cc1plus -fdebug-types-section -O2 -g bug.ii -quiet ../../../72828.ii:8:1: internal compiler error: in build_abbrev_table, at dwarf2out.c:8587 0xc169f4 build_abbrev_table ../../../src/gcc/dwarf2out.c:8587 0xc16c81 build_abbrev_table ../../../src/gcc/dwarf2out.c:8640 0xc16c81 build_abbrev_table ../../../src/gcc/dwarf2out.c:8640 0xc1a94b output_comp_unit ../../../src/gcc/dwarf2out.c:10297 0xc4bec7 dwarf2out_finish ../../../src/gcc/dwarf2out.c:29551 Please submit a full bug report,
I have once written: --- gcc/dwarf2out.c.jj 2016-10-21 18:40:34.000000000 +0200 +++ gcc/dwarf2out.c 2016-10-21 19:36:28.622894699 +0200 @@ -28399,16 +28399,23 @@ resolve_addr (dw_die_ref die) && DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE && (cdie = lookup_context_die (DECL_CONTEXT (tdecl)))) { - /* Creating a full DIE for tdecl is overly expensive and - at this point even wrong when in the LTO phase - as it can end up generating new type DIEs we didn't - output and thus optimize_external_refs will crash. */ - tdie = new_die (DW_TAG_subprogram, cdie, NULL_TREE); - add_AT_flag (tdie, DW_AT_external, 1); - add_AT_flag (tdie, DW_AT_declaration, 1); - add_linkage_attr (tdie, tdecl); - add_name_and_src_coords_attributes (tdie, tdecl); - equate_decl_number_to_die (tdecl, tdie); + dw_die_ref pdie = cdie; + /* Make sure we don't add these DIEs into type units. */ + while (pdie && pdie->die_tag != DW_TAG_type_unit) + pdie = pdie->die_parent; + if (pdie == NULL) + { + /* Creating a full DIE for tdecl is overly expensive and + at this point even wrong when in the LTO phase + as it can end up generating new type DIEs we didn't + output and thus optimize_external_refs will crash. */ + tdie = new_die (DW_TAG_subprogram, cdie, NULL_TREE); + add_AT_flag (tdie, DW_AT_external, 1); + add_AT_flag (tdie, DW_AT_declaration, 1); + add_linkage_attr (tdie, tdecl); + add_name_and_src_coords_attributes (tdie, tdecl); + equate_decl_number_to_die (tdecl, tdie); + } } if (tdie) { for this, but am not really sure what is right. Basically, for DW_TAG_call_site we sometimes want to refer to a DIE that isn't included, creating it on the fly outside of type units works, inside of type units is harder (but I'm afraid I don't know the type units code too much to even know what should be emitted then). So the above patch just pessimizes the call site info in that case rather than ICEing.
Note this regressed with r240578.
started failing with: r240578 | rguenth | 2016-09-28 07:30:19 -0700 (Wed, 28 Sep 2016) | 17 lines 2016-09-28 Richard Biener <rguenther@suse.de> * dwarf2out.c (struct die_struct): Add removed flag. (lookup_type_die): If the DIE is marked as removed, clear TYPE_SYMTAB_DIE and return NULL. (lookup_decl_die): If the DIE is marked as removed, remove it from the hash and return NULL. (mark_removed): New helper. (prune_unused_types_prune): Call it for removed DIEs. (gen_subprogram_die): Move the premark_used_types call to after DIEs for the functions scopes are generated. (process_scope_var): Do not re-create pruned types or type decls. Make sure to also re-parent type decls. (dwarf2out_finish): Move unused type pruning and debug_types handling ... (dwarf2out_early_finish): ... here.
I will look at it but it's probably a latent issue. Before the patch we happily returned a DIE from the hash table we pruned previously. Will look at next year, that is.
So initially we generate DIE 0: DW_TAG_GNU_call_site (0x7ffff68cd690) abbrev id: 0 offset: 0 mark: 0 DW_AT_low_pc: label: *.LVL1 DW_AT_abstract_origin: address but that later gets rewritten invalidly where Jakub points out. The regression is because we prune the DIE for Baz as unused after /* Generate separate COMDAT sections for type DIEs. */ if (use_debug_types) { break_out_comdat_types (comp_unit_die ()); ... now it is in a type unit ... /* In the process of copying declarations from one unit to another, we may have left some declarations behind that are no longer referenced. Prune them. */ prune_unused_types (); } ... and gets pruned. But nothing adjusts what the decl refers to via lookup_decl_die (not sure if we are supposed to adjust sth here). Previously the call-site attribute kept the DIE live in the main CU DIE so we had duplicate DIEs in the type unit plus the main CU. With -gstrict-dwarf it is only on the type unit as expected.
Perhaps we could combine the #c1 patch with some changes to make sure we don't prune those types that appear in callgraph edges callee contexts? Is the early finish debug hook called after some minimal cgraph optimizations (removal of unused functions)?
The important thing is that it is only possible to refer to a single DIE in the type unit (.debug.types.* in DWARF4, .debug_info DW_UT_type unit in DWARF5); in the #c0 case it is the Bar DW_TAG_structure_type, a child of DW_TAG_unit_type. If one needs to refer to anything else, the DIE with selected needed children needs to be copied from the type unit to the compilation unit. So if you e.g. look at what debug info we generate for: struct Bar { void Baz (); int i; }; Bar b; void Foo (Bar &t) { t.Baz (); } (compile with -g -fdebug-types-section -O2 -dA -gstrict-dwarf, the last option so that it doesn't ICE), and struct Bar { void Baz (); static int i; }; int Bar::i = 17; Bar b; void Foo (Bar &t) { t.Baz (); } (compile with -g -fdebug-types-section -O2 -dA), you'll see that in the former case we don't know about any references to Bar::Baz and everything that needs Bar only needs Bar itself and thus there is the type unit, the b variable's DW_AT_type uses DW_FORM_ref_sig8 to refer to the Bar DIE in the type unit and similarly DW_TAG_reference_type's DW_AT_type (used in t's type). Without -gstrict-dwarf there is a need for the reference to Bar::Baz that can't refer to the type unit, but we don't know about that reference during early debug finish. Now compare that to the second case. We need to refer to Bar::i, because we provide a definition for that. So there is a skeleton DW_TAG_structure_type in the compilation unit for Bar type, with DW_AT_signature that says for anything not mentioned here, refer to the type unit's Bar. This testcase doesn't ICE without -gstrict-dwarf.
Now, even going through all cgraph edges and marking the callee DIEs as used (setting die_mark for them) early (but I guess it would be better to do the early finish then after early optimizations, but before free_lang_data) would help in this particular case, but not when we only need devirtualization or later optimizations to turn indirect calls into direct calls. Creating a skeleton DIE tree in resolve_addr is yet another option, but I'm not very familiar with the break_out_comdat_types details (generate_skeleton and the like) to know how difficult it would be exactly. Though, for LTO we'll need to figure out anyway where to create those.
Created attachment 40524 [details] gcc7-pr78835.patch Untested patch that fixes the ICE on this testcase by marking directly called functions as needed. Will see what will it do with the size of the debug info. Of course, it is not sufficient, with -fdebug-types-section we can still ICE unless the earlier patch is also applied 9or some more sophisticated one to construct skeleton DIEs if type inside of type unit is detected).
(In reply to Jakub Jelinek from comment #9) > Created attachment 40524 [details] > gcc7-pr78835.patch > > Untested patch that fixes the ICE on this testcase by marking directly > called functions as needed. Will see what will it do with the size of the > debug info. > Of course, it is not sufficient, with -fdebug-types-section we can still ICE > unless the earlier patch is also applied 9or some more sophisticated one to > construct skeleton DIEs if type inside of type unit is detected). I think this is a reasonable approach (plus the hunk from comment#1 of course). But we can't really create new skeleton DIEs after early-finish because with LTO we do not have access to enough information to build type DIEs.
(In reply to Richard Biener from comment #10) > (In reply to Jakub Jelinek from comment #9) > > Created attachment 40524 [details] > > gcc7-pr78835.patch > > > > Untested patch that fixes the ICE on this testcase by marking directly > > called functions as needed. Will see what will it do with the size of the > > debug info. > > Of course, it is not sufficient, with -fdebug-types-section we can still ICE > > unless the earlier patch is also applied 9or some more sophisticated one to > > construct skeleton DIEs if type inside of type unit is detected). > > I think this is a reasonable approach (plus the hunk from comment#1 of > course). For the above patch (sans #c1) I've bootstrapped/regtested it last night, and compared to gcc without the patch the growth of the debug info is small and IMHO acceptable. On cc1plus on x86_64 comparing build without the patch and with the patch (in the former I've applied the patch and did make in stage3 dir, so that it is the same source) .debug_info size grew by 0.009% and in libstdc++.so.6 by 0.29%. > But we can't really create new skeleton DIEs after early-finish because with > LTO we do not have access to enough information to build type DIEs. I think it is possible, e.g. considering namespace A { inline namespace B { namespace C { struct D { struct Bar { void Baz (); int i; }; }; } } int vvv = 17; } A::C::D::Bar b; void Foo (A::C::D::Bar &t) { t.Baz (); } to create .uleb128 0x2 # (DIE (0x25) DW_TAG_namespace) .ascii "A\0" # DW_AT_name # DW_AT_declaration .uleb128 0x3 # (DIE (0x2c) DW_TAG_namespace) .ascii "B\0" # DW_AT_name # DW_AT_declaration .uleb128 0x3 # (DIE (0x2f) DW_TAG_namespace) .ascii "C\0" # DW_AT_name # DW_AT_declaration .uleb128 0xa # (DIE (0x32) DW_TAG_structure_type) .ascii "D\0" # DW_AT_name # DW_AT_declaration .uleb128 0x4 # (DIE (0x35) DW_TAG_structure_type) .ascii "Bar\0" # DW_AT_name # DW_AT_declaration .byte 0xf0 # DW_AT_signature .byte 0x8 .byte 0x56 .byte 0xd4 .byte 0x2d .byte 0xa7 .byte 0x74 .byte 0x7c .uleb128 0x8 # (DIE (0x5c) DW_TAG_subprogram) # DW_AT_external # DW_AT_declaration .ascii "Baz\0" # DW_AT_name Where the DW_AT_signature would actually point to the .debug_types signature and therefore the full declaration. But I'm not going to work on that right now, so would prefer for now just #c9 + #c1 + testcase.
On Tue, 17 Jan 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78835 > > --- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #10) > > (In reply to Jakub Jelinek from comment #9) > > > Created attachment 40524 [details] > > > gcc7-pr78835.patch > > > > > > Untested patch that fixes the ICE on this testcase by marking directly > > > called functions as needed. Will see what will it do with the size of the > > > debug info. > > > Of course, it is not sufficient, with -fdebug-types-section we can still ICE > > > unless the earlier patch is also applied 9or some more sophisticated one to > > > construct skeleton DIEs if type inside of type unit is detected). > > > > I think this is a reasonable approach (plus the hunk from comment#1 of > > course). > > For the above patch (sans #c1) I've bootstrapped/regtested it last night, and > compared to gcc without the patch the growth of the debug info is small and > IMHO acceptable. On cc1plus on x86_64 comparing build without the patch and > with the patch (in the former I've applied the patch and did make in stage3 > dir, so that it is the same source) .debug_info size grew by 0.009% and in > libstdc++.so.6 by 0.29%. > > > But we can't really create new skeleton DIEs after early-finish because with > > LTO we do not have access to enough information to build type DIEs. > > I think it is possible, e.g. considering > namespace A { inline namespace B { namespace C { struct D { struct Bar { void > Baz (); int i; }; }; > } } int vvv = 17; } > A::C::D::Bar b; > void Foo (A::C::D::Bar &t) { t.Baz (); } > to create > .uleb128 0x2 # (DIE (0x25) DW_TAG_namespace) > .ascii "A\0" # DW_AT_name > # DW_AT_declaration > .uleb128 0x3 # (DIE (0x2c) DW_TAG_namespace) > .ascii "B\0" # DW_AT_name > # DW_AT_declaration > .uleb128 0x3 # (DIE (0x2f) DW_TAG_namespace) > .ascii "C\0" # DW_AT_name > # DW_AT_declaration > .uleb128 0xa # (DIE (0x32) DW_TAG_structure_type) > .ascii "D\0" # DW_AT_name > # DW_AT_declaration > .uleb128 0x4 # (DIE (0x35) DW_TAG_structure_type) > .ascii "Bar\0" # DW_AT_name > # DW_AT_declaration > .byte 0xf0 # DW_AT_signature > .byte 0x8 > .byte 0x56 > .byte 0xd4 > .byte 0x2d > .byte 0xa7 > .byte 0x74 > .byte 0x7c > .uleb128 0x8 # (DIE (0x5c) DW_TAG_subprogram) > # DW_AT_external > # DW_AT_declaration > .ascii "Baz\0" # DW_AT_name In the end the great plan(TM) is to no longer have the TREE_TYPEs/DECLs of the contexts around during GIMPLE optimizations ... I think -fdebug-types-section always creates the skeletons so we may choose to just never remove those as unused? > Where the DW_AT_signature would actually point to the .debug_types signature > and therefore the full declaration. But I'm not going to work on that right > now, so would prefer for now just #c9 + #c1 + testcase. Yes, that's reasonable.
Created attachment 40527 [details] gcc7-pr78835.patch Ok, updated full patch I'll retest.
Author: jakub Date: Thu Jan 26 21:44:49 2017 New Revision: 244954 URL: https://gcc.gnu.org/viewcvs?rev=244954&root=gcc&view=rev Log: PR debug/78835 * dwarf2out.c (prune_unused_types): Mark all functions with DIEs which have direct callers with -fvar-tracking-assignments enabled in the current TU. (resolve_addr): Avoid adding skeleton DIEs for DW_AT_call_origin inside of type units. * g++.dg/debug/dwarf2/pr78835.C: New test. Added: trunk/gcc/testsuite/g++.dg/debug/dwarf2/pr78835.C Modified: trunk/gcc/ChangeLog trunk/gcc/dwarf2out.c trunk/gcc/testsuite/ChangeLog
Fixed.