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.
> > <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.
> > Oh, and insert rants of -fdebug-types-section not being
> > used by anybody.
>
> Jakub, what's your thinking on -fdebug-types-section these days?
Bootstrap & regtest running on x86_64-unknown-linux-gnu.
Richard.
2019-01-28 Richard Biener <rguent...@suse.de>
PR debug/87295
* dwarf2out.c (optimize_external_refs_1): Do not pick up
already optimized DW_AT_signature refs.
* g++.dg/lto/pr87295_0.C: New testcase.
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c (revision 268260)
+++ gcc/dwarf2out.c (working copy)
@@ -9018,9 +9018,12 @@ build_abbrev_table (dw_die_ref die, exte
/* Scan the DIE references, and replace any that refer to
DIEs from other CUs (i.e. those which are not marked) with
- the local stubs we built in optimize_external_refs. */
+ the local stubs we built in optimize_external_refs.
+ But avoid redirecting any references to type units from
+ duplicate local stubs we might have inadvertently created. */
FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
- if (AT_class (a) == dw_val_class_die_ref
+ if (a->dw_attr != DW_AT_signature
+ && AT_class (a) == dw_val_class_die_ref
&& (c = AT_ref (a))->die_mark == 0)
{
struct external_ref *ref_p;
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
+{ };
