From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id A83943846034 for ; Tue, 16 Feb 2021 09:11:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A83943846034 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id BE58BAF50; Tue, 16 Feb 2021 09:11:49 +0000 (UTC) Date: Tue, 16 Feb 2021 10:11:47 +0100 From: Tom de Vries To: dwz@sourceware.org, jakub@redhat.com, mark@klomp.org Subject: [PATCH] Fix CK_BAD propagation for --odr Message-ID: <20210216091146.GA32276@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: dwz@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Dwz mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Feb 2021 09:11:52 -0000 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 > <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 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);