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 5888F385801D for ; Mon, 22 Feb 2021 15:11:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5888F385801D 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 63596AEDD; Mon, 22 Feb 2021 15:11:37 +0000 (UTC) Subject: [PATCH] Fix DW_AT_decl_file for odr From: Tom de Vries To: dwz@sourceware.org, jakub@redhat.com, mark@klomp.org References: <20210222082439.GA14387@delia> Message-ID: Date: Mon, 22 Feb 2021 16:11:36 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210222082439.GA14387@delia> Content-Type: multipart/mixed; boundary="------------5408959C5EBF217EF44E27CA" Content-Language: en-US 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: Mon, 22 Feb 2021 15:11:40 -0000 This is a multi-part message in MIME format. --------------5408959C5EBF217EF44E27CA Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit [ was: Re: [FTR] Fix DW_AT_decl_file for odr ] On 2/22/21 9:24 AM, Tom de Vries wrote: > Hi, > > [ For now, this is a for-the-record posting. I'm working on a different fix > for this. ] > And this is the proposed fix. Thanks, - Tom --------------5408959C5EBF217EF44E27CA Content-Type: text/x-patch; charset=UTF-8; name="0001-Fix-DW_AT_decl_file-for-odr.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="0001-Fix-DW_AT_decl_file-for-odr.patch" Fix DW_AT_decl_file for odr Consider odr-struct. It has two structs aaa (from different CUs), each with members of type bbb and ccc, but in one case bbb is a decl, in the other case ccc is a decl. When doing odr, we end up with one struct aaa, and no decls: ... $ dwz --odr odr-struct $ readelf -wi odr-struct \ | egrep -A2 "DW_TAG_structure" \ | egrep "DW_TAG|DW_AT_name|DW_AT_decl" <1><19>: Abbrev Number: 25 (DW_TAG_structure_type) <1a> DW_AT_name : ccc <1><2f>: Abbrev Number: 25 (DW_TAG_structure_type) <30> DW_AT_name : aaa <1><4b>: Abbrev Number: 25 (DW_TAG_structure_type) <4c> DW_AT_name : bbb ... Now consider using the same file for multifile optimization, combined with odr: ... $ cp odr-struct 1; cp 1 2; dwz -m 3 1 2 --odr ... The desired outcome is that the structs aaa are unified (same as above) in both 1 and 2, and then moved to multifile 3. The multifile looks good: ... $ readelf -wi 3 \ | egrep -A2 "DW_TAG_structure" \ | egrep "DW_TAG|DW_AT_name|DW_AT_decl" <1><14>: Abbrev Number: 15 (DW_TAG_structure_type) <15> DW_AT_name : ccc <1><2a>: Abbrev Number: 15 (DW_TAG_structure_type) <2b> DW_AT_name : aaa <1><46>: Abbrev Number: 15 (DW_TAG_structure_type) <47> DW_AT_name : bbb ... but some struct types are left in 1: ... $ readelf -wi 1 \ | egrep -A2 "DW_TAG_structure" \ | egrep "DW_TAG|DW_AT_name|DW_AT_decl" <1><1e>: Abbrev Number: 12 (DW_TAG_structure_type) <1f> DW_AT_name : aaa <1><3d>: Abbrev Number: 12 (DW_TAG_structure_type) <3e> DW_AT_name : bbb ... The problem is that the DW_AT_decl_file is different for struct bbb in 1 and 3, and that causes the struct types to linger in 1. The problem can already be shown without multifile mode using: .... $ dwz odr-struct --odr $ llvm-dwarfdump odr-struct \ | grep -A3 struct \ | egrep -v "^--|DW_AT_byte_size" \ | sed 's%/.*/%%' 0x00000019: DW_TAG_structure_type DW_AT_name ("ccc") DW_AT_decl_file ("odr.cc") 0x0000002f: DW_TAG_structure_type DW_AT_name ("aaa") DW_AT_decl_file ("odr.h") 0x0000004b: DW_TAG_structure_type DW_AT_name ("bbb") DW_AT_decl_file ("odr.cc") ... The DW_AT_decl_file for struct bbb should be odr-2.cc. The problem is caused by by odr: odr allows defs and decls to be part of the same duplicate chain, which breaks the invariant that DIEs in the duplicate chain are isomorph. During write_die, the first DIE in the chain is written out as the representative copy, which means having a decl as the first in the chain is counterproductive. We have reorder_dups to fix this problem, which detects if a duplicate chain starts with a decl and then moves the first def before it. However, this breaks another variant: that for each partition, all representative DIEs are from the same CU. Consequently, the file table of the partition may not match with the DW_AT_decl_file number. In other words, the reordered duplicate chain is in the wrong partition. Fix this by: - ignoring ODR_DECL DIEs at the start of a duplicate chain when partitioning - moving the reorder_dups call to the start of partial unit creating, such that we get the correct refcu. This also breaks the invariant checked in create_import_tree that partition_dups doesn't generate two seperate partial units with the same set of referrer CUs, so we allow this for odr. 2021-02-19 Tom de Vries PR dwz/27438 * dwz.c (partition_cmp): Ignore ODR_DECL dies at the start of a duplicate chain. (partition_dups_1): Same. Move call to reorder_dups earlier. (create_import_tree): Allow partial units with same set of referrers for odr. * testsuite/dwz.tests/odr-struct-multifile.sh: New test. --- dwz.c | 44 ++++++++++++++++++++---- testsuite/dwz.tests/odr-struct-multifile.sh | 53 +++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/dwz.c b/dwz.c index 076f39c..59ead79 100644 --- a/dwz.c +++ b/dwz.c @@ -7399,8 +7399,20 @@ partition_cmp (const void *p, const void *q) dw_die_ref die2 = *(dw_die_ref *) q; dw_die_ref ref1, ref2; dw_cu_ref last_cu1 = NULL, last_cu2 = NULL; - for (ref1 = die1, ref2 = die2;; - ref1 = ref1->die_nextdup, ref2 = ref2->die_nextdup) + ref1 = die1; + ref2 = die2; + if (odr_active_p && odr_mode != ODR_BASIC) + { + while (ref1 && die_odr_state (die_cu (ref1), ref1) == ODR_DECL) + ref1 = ref1->die_nextdup; + if (ref1 == NULL) + ref1 = die1; + while (ref2 && die_odr_state (die_cu (ref2), ref2) == ODR_DECL) + ref2 = ref2->die_nextdup; + if (ref2 == NULL) + ref2 = die2; + } + for (;; ref1 = ref1->die_nextdup, ref2 = ref2->die_nextdup) { dw_cu_ref ref1cu = NULL; dw_cu_ref ref2cu = NULL; @@ -7927,8 +7939,20 @@ partition_dups_1 (dw_die_ref *arr, size_t vec_size, dw_die_ref ref1, ref2; dw_cu_ref last_cu1 = NULL, last_cu2 = NULL; size_t this_cnt = 0; - for (ref1 = arr[i], ref2 = arr[j];; - ref1 = ref1->die_nextdup, ref2 = ref2->die_nextdup) + ref1 = arr[i]; + ref2 = arr[j]; + if (odr_active_p && odr_mode != ODR_BASIC) + { + while (ref1 && die_odr_state (die_cu (ref1), ref1) == ODR_DECL) + ref1 = ref1->die_nextdup; + if (ref1 == NULL) + ref1 = arr[i]; + while (ref2 && die_odr_state (die_cu (ref2), ref2) == ODR_DECL) + ref2 = ref2->die_nextdup; + if (ref2 == NULL) + ref2 = arr[j]; + } + for (;; ref1 = ref1->die_nextdup, ref2 = ref2->die_nextdup) { dw_cu_ref ref1cu = NULL; dw_cu_ref ref2cu = NULL; @@ -8090,6 +8114,13 @@ partition_dups_1 (dw_die_ref *arr, size_t vec_size, && (ignore_size || orig_size > new_size)); if (force) { + if (odr_active_p && odr_mode != ODR_BASIC) + for (k = i; k < j; k++) + { + if (second_phase && !arr[k]->die_ref_seen) + continue; + arr[k] = reorder_dups (arr[k]); + } dw_die_ref die, *diep; dw_cu_ref refcu = die_cu (arr[i]); dw_cu_ref partial_cu = pool_alloc (dw_cu, sizeof (struct dw_cu)); @@ -8133,8 +8164,6 @@ partition_dups_1 (dw_die_ref *arr, size_t vec_size, dw_die_ref child; if (second_phase && !arr[k]->die_ref_seen) continue; - if (odr_active_p && odr_mode != ODR_BASIC) - arr[k] = reorder_dups (arr[k]); if (dump_pus_p) dump_die (arr[k]); child = copy_die_tree (die, arr[k]); @@ -9399,6 +9428,9 @@ create_import_tree (void) IPU2 node into IPU, by removing all incoming edges of IPU2 and moving over all outgoing edges of IPU2 to IPU. */ + if (odr_active_p && odr_mode != ODR_BASIC + && ipu2->idx < npus + ncus) + continue; assert (ipu2->idx >= npus + ncus); size_inc = 0; if (edge_cost) diff --git a/testsuite/dwz.tests/odr-struct-multifile.sh b/testsuite/dwz.tests/odr-struct-multifile.sh new file mode 100644 index 0000000..cc462c9 --- /dev/null +++ b/testsuite/dwz.tests/odr-struct-multifile.sh @@ -0,0 +1,53 @@ +if ! $execs/dwz-for-test --odr -v 2>/dev/null; then + exit 77 +fi + +cp $execs/odr-struct 1 +cp 1 2 + +for name in aaa bbb ccc; do + cnt=$(readelf -wi 1 | grep -c "DW_AT_name.*:.*$name" || true) + [ $cnt -eq 2 ] +done + +for name in member_one member_two member_three member_four; do + cnt=$(readelf -wi 1 | grep -c "DW_AT_name.*:.*$name" || true) + case $name in + member_one|member_two) + [ $cnt -eq 2 ] + ;; + member_three|member_four) + [ $cnt -eq 1 ] + ;; + esac +done + +decl_cnt=$(readelf -wi 1 | grep -c "DW_AT_declaration" || true) + +$execs/dwz-for-test --odr 1 2 -m 3 + +verify-dwarf.sh 1 +verify-dwarf.sh 3 + +for name in aaa bbb ccc; do + cnt=$(readelf -wi 3 | grep -c "DW_AT_name.*:.*$name" || true) + [ $cnt -eq 1 ] +done + +for name in member_one member_two member_three member_four; do + cnt=$(readelf -wi 3 | grep -c "DW_AT_name.*:.*$name" || true) + [ $cnt -eq 1 ] +done + + +for name in aaa bbb ccc; do + cnt=$(readelf -wi 1 | grep -c "DW_AT_name.*:.*$name" || true) + [ $cnt -eq 0 ] +done + +for name in member_one member_two member_three member_four; do + cnt=$(readelf -wi 1 | grep -c "DW_AT_name.*:.*$name" || true) + [ $cnt -eq 0 ] +done + +rm -f 1 2 3 --------------5408959C5EBF217EF44E27CA--