On 1/28/19 6:30 AM, Richard Biener wrote:
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.
Looks good, apart from a typo:
+ /* Record in DECL_TABLE that TARG has been alreay copied
"already"
Jason