On Mon, 28 Jan 2019, Richard Biener wrote:
> On Sat, 26 Jan 2019, Jason Merrill wrote:
>
> > On Fri, Jan 25, 2019 at 7:57 AM Richard Biener <rguent...@suse.de> wrote:
> > >
> > > The following fixes an ICE with -flto -ffat-lto-objects
> > > -fdebug-types-section -g where optimize_external_refs does not
> > > expect to see DW_AT_signature as made "local" by build_abbrev_table(sic!).
> > >
> > > This is because we run optimize_external_refs twice in this setup.
> > >
> > > The fix is to make the second run not pick up "local" DW_AT_signature
> > > as external. Alternatively build_abbrev_table could be adjusted
> > > to not keep those as DW_AT_signature. It's not even clear to me
> > > whether a DW_AT_signature as we output it is valid DWARF ...
> > > to quote non-LTO -g:
> > >
> > > <1><1d>: Abbrev Number: 13 (DW_TAG_structure_type)
> > > <1e> DW_AT_name : (indirect string, offset: 0x8b):
> > > integral_constant<bool, false>
> > > <22> DW_AT_signature : <0x5d>
> >
> > Yes, that seems pretty clearly wrong. It doesn't make sense for
> > DW_AT_signature to be a local reference; it should only have
> > DW_FORM_ref_sig8.
>
> OK, thus the fix below is then obvious.
Apart from not working because I skip marking the refs external.
Adjusted patch in testing...
>
> > > <26> DW_AT_declaration : 1
> > > <26> DW_AT_sibling : <0x48>
> > > ...
> > > <1><5d>: Abbrev Number: 16 (DW_TAG_structure_type)
> > > <5e> DW_AT_name : (indirect string, offset: 0x8b):
> > > integral_constant<bool, false>
> > > <62> DW_AT_signature : signature: 0x1f6a4ae7cc5a362e
> > > <6a> DW_AT_declaration : 1
> > > <6a> DW_AT_sibling : <0x7b>
> >
> > And there's no reason to have two skeleton DIEs for the same type.
> >
> > > Of course the main "issue" is that copy_unworthy_types
> > > duplicated this parent into 1d and 5d in the first place
> > > (but that's a pure optimization issue?).
> >
> > Indeed.
> >
> > > Not doing that
> > > would have avoided the situation in PR87295. But then
> > > why should optimize_external_refs_1 have this special
> > > code handling DW_AT_signature in the first place if this
> > > was not intended... (so even better, kill that and the
> > > build_abbrev_table optimization and fix copy_unworthy_types?)
> >
> > The point of optimize_external_refs_1 is to remember when we've seen a
> > skeleton DIE so we can refer to it from other DIEs in the same unit
> > rather than use DW_FORM_ref_sig8. The problem is just that we get two
> > skeleton DIEs so we wrongly change one to refer to the other. One
> > possibility would be to suppress the local reference optimization for
> > DW_AT_signature, but avoiding the redundant skeleton should fix the
> > problem and also save space.
>
> I'll avoid the bogus DW_AT_signature change for now and see if I
> can come up with a fix for the duplicate as followup. I'm going
> to backport the fix to the 8 branch and on trunk if I manage to
> avoid the duplicate change the fix to assert we do not try to
> replace a DW_AT_signature refrence instead.
... but this one isn't too bad either, see below. I've tried
to merge this into copy_decls_for_unworthy_types but this becomes
somewhat messy, the pre-scan of the CU is easier to follow
thus I've settled with that.
Bootstrap & regtest running on x86_64-unknown-linux-gnu. A backport
would instead of the assert have the a->dw_attr != DW_AT_signature
check in place.
Richard.
2019-01-28 Richard Biener <rguent...@suse.de>
PR debug/87295
* dwarf2out.c (collect_skeleton_dies): New helper.
(copy_decls_for_unworthy_types): Call it.
(build_abbrev_table): Assert we do not try to replace
DW_AT_signature refs with local refs.
* g++.dg/lto/pr87295_0.C: New testcase.
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c (revision 268334)
+++ gcc/dwarf2out.c (working copy)
@@ -8722,6 +8722,33 @@ copy_decls_walk (dw_die_ref unit, dw_die
FOR_EACH_CHILD (die, c, copy_decls_walk (unit, c, decl_table));
}
+/* Collect skeleton dies in DIE created by break_out_comdat_types already
+ and record them in DECL_TABLE. */
+
+static void
+collect_skeleton_dies (dw_die_ref die, decl_hash_type *decl_table)
+{
+ dw_die_ref c;
+
+ if (dw_attr_node *a = get_AT (die, DW_AT_signature))
+ {
+ dw_die_ref targ = AT_ref (a);
+ gcc_assert (targ->die_mark == 0 && targ->comdat_type_p);
+ decl_table_entry **slot
+ = decl_table->find_slot_with_hash (targ,
+ htab_hash_pointer (targ),
+ INSERT);
+ gcc_assert (*slot == HTAB_EMPTY_ENTRY);
+ /* Record in DECL_TABLE that TARG has been alreay copied
+ by remove_child_or_replace_with_skeleton. */
+ decl_table_entry *entry = XCNEW (struct decl_table_entry);
+ entry->orig = targ;
+ entry->copy = die;
+ *slot = entry;
+ }
+ FOR_EACH_CHILD (die, c, collect_skeleton_dies (c, decl_table));
+}
+
/* Copy declarations for "unworthy" types into the new comdat section.
Incomplete types, modified types, and certain other types aren't broken
out into comdat sections of their own, so they don't have a signature,
@@ -8733,6 +8760,7 @@ copy_decls_for_unworthy_types (dw_die_re
{
mark_dies (unit);
decl_hash_type decl_table (10);
+ collect_skeleton_dies (unit, &decl_table);
copy_decls_walk (unit, unit, &decl_table);
unmark_dies (unit);
}
@@ -9029,7 +9057,10 @@ build_abbrev_table (dw_die_ref die, exte
if (is_type_die (c)
&& (ref_p = lookup_external_ref (extern_map, c))
&& ref_p->stub && ref_p->stub != die)
- change_AT_die_ref (a, ref_p->stub);
+ {
+ gcc_assert (a->dw_attr != DW_AT_signature);
+ change_AT_die_ref (a, ref_p->stub);
+ }
else
/* We aren't changing this reference, so mark it external. */
set_AT_ref_external (a, 1);
Index: gcc/testsuite/g++.dg/lto/pr87295_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/pr87295_0.C (nonexistent)
+++ gcc/testsuite/g++.dg/lto/pr87295_0.C (working copy)
@@ -0,0 +1,20 @@
+// { dg-lto-do assemble }
+// { dg-lto-options { { -flto -ffat-lto-objects -fdebug-types-section -g
-std=gnu++17 } } }
+
+template<typename _Tp, _Tp __v>
+struct integral_constant
+{
+ static constexpr _Tp value = __v;
+ typedef _Tp value_type;
+ constexpr operator value_type() const noexcept { return value; }
+};
+
+typedef integral_constant<bool, false> false_type;
+
+template<typename...>
+struct __or_;
+
+template<>
+struct __or_<>
+ : public false_type
+{ };
