From: Tom de Vries <tdevries@suse.de>
To: dwz@sourceware.org, jakub@redhat.com, mark@klomp.org
Subject: [PATCH] Fix CK_BAD propagation for --odr
Date: Tue, 16 Feb 2021 10:11:47 +0100 [thread overview]
Message-ID: <20210216091146.GA32276@delia> (raw)
Hi,
With the reproducer from PR26252 we get:
...
$ dwz -m 3 --odr 1 2
dwz: 1: DWARF compression not beneficial - old size 317760 new size 317760
dwz: dwz.c:12431: write_die: \
Assertion `value && refdcu->cu_kind != CU_ALT' failed.
Aborted (core dumped)
...
The assertion fails when writing out the contribution of file 2 to the
multifile.
It's trying to write out a copy of this DIE:
...
<3><9f3e>: Abbrev Number: 149 (DW_TAG_subprogram)
<9f40> DW_AT_name :
_M_access<ydotool::Tool::ToolManager::ScanPath(const string&)::\
<lambda(const string&, dirent*)> >
<9f44> DW_AT_decl_file : 11
<9f45> DW_AT_decl_line : 92
<9f46> DW_AT_decl_column : 7
<9f47> DW_AT_type : <0x1a2a5>
<9f4b> DW_AT_declaration : 1
<9f4b> DW_AT_object_pointer: <0x9f5c>
<9f4f> DW_AT_sibling : <0x9f62>
...
and specifically, for the DW_AT_type attribute it attempts to write out the
reference to the copy of the DIE at 0x1a2a5:
...
<1><1a2a5>: Abbrev Number: 9 (DW_TAG_reference_type)
<1a2a6> DW_AT_byte_size : 8
<1a2a7> DW_AT_type : <0x27cd9>
...
There is no copy of that DIE, because it has a bad checksum:
...
1a2a5 X d27b2f2a d27b2f2a reference_type (type: 27cd9 structure_type)
...
which means that its toplevel DIE 0x9edc:
...
<2><9edc>: Abbrev Number: 182 (DW_TAG_union_type)
<9ede> DW_AT_name : _Any_data
...
should also have a bad checksum, but it doesn't:
...
9edc O 9d6524b4 9d6524b4 _Any_data union_type
...
while without --odr, it does:
...
9edc X d8f947f3 5100c9e1 _Any_data union_type
...
We can catch the problem of the checksum much earlier, with an assert:
...
static void
partition_found_dups (dw_die_ref die, struct obstack *vec)
{
+ assert (die->die_ck_state == CK_KNOWN);
...
which means we can reproduce with just one file:
...
$ dwz 2 --odr
dwz: dwz.c:7372: partition_found_dups: \
Assertion `die->die_ck_state == CK_KNOWN' failed.
Aborted (core dumped)
...
The problem is caused by the odr code in checksum_ref_die, which skips
checksum calculation for the children of odr types, with as unintended
side-effect that it break the CK_BAD propagation to toplevel DIEs.
Fix this by making the skipping of the checksum calculation less intrusive.
Also, add the assert in partition_found_dups to catch this earlier and
easier.
Unfortunately, the fix also means that the earlier reported performance
improvement of odr on cc1, of 42.30%:
...
$ ./dwz.master cc1 -o 1 -lnone
$ ./diff.sh cc1 1
.debug_info red: 44.84% 111527248 61527733
.debug_abbrev red: 40.28% 1722726 1028968
.debug_str red: 0% 6609355 6609355
total red: 42.30% 119859329 69166056
...
to 54.55%:
...
$ ./dwz.master cc1 -o 2 -lnone --odr
$ ./diff.sh cc1 2
.debug_info red: 57.46% 111527248 47449258
.debug_abbrev red: 75.08% 1722726 429434
.debug_str red: 0% 6609355 6609355
total red: 54.55% 119859329 54488047
...
pretty much disappears:
...
$ ./dwz.fix cc1 -o 3 -lnone --odr
$ ./diff.sh cc1 3
.debug_info red: 45.44% 111527248 60858640
.debug_abbrev red: 42.88% 1722726 984084
.debug_str red: 0% 6609355 6609355
total red: 42.89% 119859329 68452079
...
Any comments?
Thanks,
- Tom
Fix CK_BAD propagation for --odr
2021-02-16 Tom de Vries <tdevries@suse.de>
PR dwz/26252
* dwz.c (checksum_ref_die): Fix CK_BAD propagation for --odr.
(partition_found_dups): Add assert.
---
dwz.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/dwz.c b/dwz.c
index 17921f0..faf00d1 100644
--- a/dwz.c
+++ b/dwz.c
@@ -4187,24 +4187,31 @@ checksum_ref_die (dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die,
}
}
- if (!only_hash_name_p
- && (top_die == NULL || top_die->die_ck_state != CK_BAD))
- for (child = die->die_child; child; child = child->die_sib)
- {
- unsigned int r
- = checksum_ref_die (cu,
- top_die ? top_die
- : child->die_named_namespace
- ? NULL : child, child,
- second_idx, second_hash);
- if (top_die == NULL)
- assert (r == 0 && obstack_object_size (&ob) == 0);
-
- if (ret == 0 || (r && r < ret))
- ret = r;
- if (top_die && top_die->die_ck_state == CK_BAD)
- break;
- }
+ if (top_die == NULL || top_die->die_ck_state != CK_BAD)
+ {
+ hashval_t save_checksum = 0;
+ if (top_die)
+ save_checksum = top_die->u.p1.die_ref_hash;
+ for (child = die->die_child; child; child = child->die_sib)
+ {
+ unsigned int r
+ = checksum_ref_die (cu,
+ top_die ? top_die
+ : child->die_named_namespace
+ ? NULL : child, child,
+ second_idx, second_hash);
+ if (top_die == NULL)
+ assert (r == 0 && obstack_object_size (&ob) == 0);
+
+ if (ret == 0 || (r && r < ret))
+ ret = r;
+ if (top_die && top_die->die_ck_state == CK_BAD)
+ break;
+ }
+ if (top_die
+ && top_die->die_ck_state != CK_BAD && only_hash_name_p)
+ top_die->u.p1.die_ref_hash = save_checksum;
+ }
if (top_die == die)
{
@@ -7369,6 +7376,7 @@ partition_cmp (const void *p, const void *q)
static void
partition_found_dups (dw_die_ref die, struct obstack *vec)
{
+ assert (die->die_ck_state == CK_KNOWN);
obstack_ptr_grow (vec, die);
if (unlikely (verify_dups_p))
verify_dups (die, true);
next reply other threads:[~2021-02-16 9:11 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-16 9:11 Tom de Vries [this message]
2021-02-18 13:04 ` [committed] Add assert in partition_found_dups Tom de Vries
2021-02-18 14:20 ` [PATCH] Fix CK_BAD propagation for --odr Tom de Vries
2021-02-22 7:51 ` Tom de Vries
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210216091146.GA32276@delia \
--to=tdevries@suse.de \
--cc=dwz@sourceware.org \
--cc=jakub@redhat.com \
--cc=mark@klomp.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).