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 3630D3858002 for ; Sun, 14 Feb 2021 21:23:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3630D3858002 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 4EE4AAB98; Sun, 14 Feb 2021 21:23:29 +0000 (UTC) Subject: Re: [PATCH] Handle DW_FORM_implicit_const for DW_AT_decl_line To: Mark Wielaard , dwz@sourceware.org, jakub@redhat.com References: <20210214085202.GA16780@delia> <655872bb0320602ddab5663078db3c65c07464c0.camel@klomp.org> From: Tom de Vries Message-ID: <241b7a3b-a712-d425-1b83-2639e2524082@suse.de> Date: Sun, 14 Feb 2021 22:23:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <655872bb0320602ddab5663078db3c65c07464c0.camel@klomp.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, NICE_REPLY_A, 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: Sun, 14 Feb 2021 21:23:31 -0000 On 2/14/21 8:10 PM, Mark Wielaard wrote: > Hi Tom, > > On Sun, 2021-02-14 at 09:52 +0100, Tom de Vries wrote: >> due to the DW_AT_decl_line attribute having different forms, >> DW_FORM_data1 and >> DW_FORM_implicit_const. >> >> Fix this in checksum_die and die_eq_1 by adding dedicated handling for >> DW_AT_decl_line, similar to how that's done for DW_AT_decl_file. >> >> Any comments? > > Yes, this makes sense. Like you said, we do something like this already > for decl/call_file. I think technically the die_eq_1 part could be put > under the same switch case. But maybe you find it clearer to do it > separately to mirror the checksum_die part? > Well, the first parts are similar, but after the "(value1 == 0) ^ (value2 == 0))" things diverge quite a bit. I agree there's room for sharing, but I wonder whether perhaps that's better done using an inline function. > Together with your DW_LANG_C_plus_plus_{03,11,14} patch it fixes the -- > odr testsuite failures I was seeing. > Thanks for confirming. Committed. > As a follow up this might actually make sense for more attributes. Ack. I went down that road initially but felt things got too complicated for a simple bug fix, so reverted to this more simple approach, but agreed, a more generic solution would be better. Thanks, - Tom > Where we know the attribute value is interpreted as an signed or > unsigned value (so we know what data[124] represents). This is actually > slightly tricky, because it isn't always immediately clear which > attribute values DWARF intends to be signed/unsigned, and it might even > differ per language (or whether or not an attribute is associated with > a [referenced] signed or unsigned type). > > But the following seem to be always be unsigned values that might > qualify because they have unsigned constants assigned: > > DW_AT_encoding > DW_AT_decimal_sign > DW_AT_endianity > DW_AT_accessibility > DW_AT_visibility > DW_AT_virtuality > DW_AT_language > DW_AT_address_class > DW_AT_identifier_case > DW_AT_calling_convention > DW_AT_inline > DW_AT_ordering > DW_AT_discr_list > DW_AT_defaulted > > And the following qualify because they describe sizes, or counts, when > constant class: > DW_AT_byte_size > DW_AT_bit_size > DW_AT_count > DW_AT_digit_count > DW_AT_rank > DW_AT_string_length_bit_size > DW_AT_string_length_byte_size > DW_AT_alignment > > As are these when constant class, indicating positive offsets > DW_AT_high_pc > DW_AT_start_scope > DW_AT_data_member_location > DW_AT_data_bit_offset > > Cheers, > > Mark >