Fix CK_BAD propagation for --odr With the reproducer from PR26252 we get: ... $ dwz 2 -o 2.z --odr -lnone dwz: dwz.c:7396: 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. Specially: - undo all modifications related to odr in checksum_ref_die - After calling checksum_ref_die in read_debug_info: - override die_ref_hash for odr DIEs, and - recalculate die_ref_hash for all other DIEs. We still have the same amount of compression with cc1, that is: without odr we have 42.30% reduction: ... $ dwz cc1 -o cc1.z -lnone --no-odr $ diff.sh cc1 cc1.z .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 ... and with odr but without the bug fix we have 54.55%: ... $ dwz cc1 -o cc1.z -lnone --odr $ diff.sh cc1 cc1.z .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 ... and with odr and the bug fix still 54.55%: ... $ dwz cc1 -o cc1.z -lnone --odr $ ../measure/diff.sh cc1 cc1.z .debug_info red: 57.46% 111527248 47446501 .debug_abbrev red: 75.51% 1722726 422027 .debug_str red: 0% 6609355 6609355 total red: 54.55% 119859329 54477883 ... 2021-02-18 Tom de Vries PR dwz/26252 * dwz.c (checksum_ref_die): Undo modifications related to odr. (read_debug_info): After calling checksum_ref_die, override die_ref_hash for odr DIEs, and recalculate die_ref_hash for all other DIEs. --- dwz.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 69 insertions(+), 20 deletions(-) diff --git a/dwz.c b/dwz.c index 0b751fc..62ebe97 100644 --- a/dwz.c +++ b/dwz.c @@ -3994,7 +3994,6 @@ checksum_ref_die (dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die, unsigned int i, ret = 0; unsigned char *ptr; dw_die_ref child; - bool only_hash_name_p; if (top_die == die) { @@ -4025,7 +4024,6 @@ checksum_ref_die (dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die, else assert (top_die == NULL || die->die_ck_state == CK_KNOWN); t = die->die_abbrev; - only_hash_name_p = odr && die_odr_state (die_cu (die), die) != ODR_NONE; for (i = 0; i < t->nattr; ++i) if (t->attr[i].attr != DW_AT_sibling) switch (t->attr[i].form) @@ -4209,24 +4207,25 @@ 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) + { + 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 == die) { @@ -6742,6 +6741,24 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) for (cu = cuf; cu; cu = cu->cu_next) checksum_ref_die (cu, NULL, cu->cu_die, NULL, NULL); + if (odr) + { + for (cu = cuf; cu; cu = cu->cu_next) + { + dw_die_ref die; + FOREACH_LOW_TOPLEVEL_DIE_IN_CU (die, cu) + { + if (die->die_ck_state != CK_KNOWN) + continue; + if (die_odr_state (die_cu (die), die) != ODR_NONE) + die->u.p1.die_ref_hash = die->u.p1.die_hash; + else + die->die_ref_hash_computed = 0; + } + } + for (cu = cuf; cu; cu = cu->cu_next) + checksum_ref_die (cu, NULL, cu->cu_die, NULL, NULL); + } if (dump_dies_p) for (cu = cuf; cu; cu = cu->cu_next) @@ -7220,6 +7237,20 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) if (checksum_die (dso, cu, NULL, cu->cu_die)) goto fail; checksum_ref_die (cu, NULL, cu->cu_die, NULL, NULL); + if (odr) + { + dw_die_ref die; + FOREACH_LOW_TOPLEVEL_DIE_IN_CU (die, cu) + { + if (die->die_ck_state != CK_KNOWN) + continue; + if (die_odr_state (die_cu (die), die) != ODR_NONE) + die->u.p1.die_ref_hash = die->u.p1.die_hash; + else + die->die_ref_hash_computed = 0; + } + checksum_ref_die (cu, NULL, cu->cu_die, NULL, NULL); + } if (dump_dies_p) dump_dies (0, cu->cu_die); @@ -7292,6 +7323,24 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) for (cu = first_cu; cu; cu = cu->cu_next) checksum_ref_die (cu, NULL, cu->cu_die, NULL, NULL); + if (odr) + { + for (cu = first_cu; cu; cu = cu->cu_next) + { + dw_die_ref die; + FOREACH_LOW_TOPLEVEL_DIE_IN_CU (die, cu) + { + if (die->die_ck_state != CK_KNOWN) + continue; + if (die_odr_state (die_cu (die), die) != ODR_NONE) + die->u.p1.die_ref_hash = die->u.p1.die_hash; + else + die->die_ref_hash_computed = 0; + } + } + for (cu = first_cu; cu; cu = cu->cu_next) + checksum_ref_die (cu, NULL, cu->cu_die, NULL, NULL); + } if (dump_dies_p) for (cu = first_cu; cu; cu = cu->cu_next)