[PATCH] Fix CK_BAD propagation for --odr

Tom de Vries tdevries@suse.de
Tue Feb 16 09:11:47 GMT 2021


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);


More information about the Dwz mailing list