OK.
On Wed, Jan 18, 2017 at 7:32 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> The following patch fixes ICE with -fdebug-types-section on the following
> testcase. We prune DIEs in the main CU for methods in a class that is
> moved into .debug_types section, because we don't see any uses of those
> in the debug info. But those uses are only added later when adding
> DW_TAG_call_site DIEs. The patch just goes through all direct call edges
> at the time of the early debug finish and marks the callees as used, so that
> we do not prune them. Of course, some edges might be still indirect and
> turned into direct edges only later on, so as fallback the patch for now
> just drops DW_AT_call_origin attributes on the floor if they would need to
> point into a DIE that would need to be (re-)created inside of .debug_types
> section (which is not valid and one can't refer to such DIEs anyway).
> The right thing to do is create skeleton DIEs in the main CU as mentioned
> in the PR, but that is lots of work.
>
> The patch grows .debug_info on bootstrapped x86_64-linux cc1plus by 0.009%
> and on libstdc++.so.6 by 0.29%, so I think that is still acceptable.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> I've also tried bootstrap with -fdebug-types-section on by default, but that
> failed miserably during building of libstdc++, seems there are other bugs
> unrelated to what this patch addresses (the ICE I've looked at has been
> that std::string class had _Rep class inside of it and that contained some
> method and during break_out_comdat_type GCC actually didn't keep that
> method's DIE inside of the main CU, which is what it ought to be doing
> (i.e. have std::string in one .debug_types unit (that DIE moved, all
> children duplicated into the .debug_types unit), then std::string::_Rep
> in another .debug_types unit (again, that DIE moved, all children
> duplicated) and in the main CU std::string and std::string::_Rep be copies
> (skeleton), and the rest kept and optionally pruned as unused later on.
>
> I'll try to reduce a testcase for that and file another PR.
>
> 2017-01-18 Jakub Jelinek <ja...@redhat.com>
>
> 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.
>
> --- gcc/dwarf2out.c.jj 2017-01-16 22:31:10.779526222 +0100
> +++ gcc/dwarf2out.c 2017-01-17 10:12:11.853440897 +0100
> @@ -27768,6 +27768,25 @@ prune_unused_types (void)
> for (i = 0; base_types.iterate (i, &base_type); i++)
> prune_unused_types_mark (base_type, 1);
>
> + /* For -fvar-tracking-assignments, also set the mark on nodes that could be
> + referenced by DW_TAG_call_site DW_AT_call_origin (i.e. direct call
> + callees). */
> + cgraph_node *cnode;
> + FOR_EACH_FUNCTION (cnode)
> + if (cnode->referred_to_p (false))
> + {
> + dw_die_ref die = lookup_decl_die (cnode->decl);
> + if (die == NULL || die->die_mark)
> + continue;
> + for (cgraph_edge *e = cnode->callers; e; e = e->next_caller)
> + if (e->caller != cnode
> + && opt_for_fn (e->caller->decl, flag_var_tracking_assignments))
> + {
> + prune_unused_types_mark (die, 1);
> + break;
> + }
> + }
> +
> if (debug_str_hash)
> debug_str_hash->empty ();
> if (skeleton_debug_str_hash)
> @@ -28669,16 +28688,27 @@ 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.
> + We could emit skeleton DIEs for context (namespaces,
> + outer structs/classes) and a skeleton DIE for the
> + innermost context with DW_AT_signature pointing to the
> + type unit. See PR78835. */
> + 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)
> {
> --- gcc/testsuite/g++.dg/debug/dwarf2/pr78835.C.jj 2017-01-17
> 10:16:34.301004406 +0100
> +++ gcc/testsuite/g++.dg/debug/dwarf2/pr78835.C 2017-01-17 10:17:45.034078227
> +0100
> @@ -0,0 +1,11 @@
> +/* PR debug/78835 */
> +/* { dg-do compile } */
> +/* { dg-options "-gdwarf-4 -O2 -fdebug-types-section" } */
> +
> +struct A { void foo (); };
> +
> +void
> +bar (A &x)
> +{
> + x.foo ();
> +}
>
> Jakub
