Re: [PATCH] Handle DW_FORM_implicit_const for DW_AT_decl_line

[Tom de Vries <tdevries@suse.de>]

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
>