public inbox for
 help / color / mirror / Atom feed
From: Tom de Vries <>
To: Mark Wielaard <>,,
Subject: Re: [PATCH] Handle DW_FORM_implicit_const for DW_AT_decl_line
Date: Sun, 14 Feb 2021 22:23:28 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

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

> 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.

- 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

      parent reply	other threads:[~2021-02-14 21:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-14  8:52 Tom de Vries
2021-02-14 19:10 ` Mark Wielaard
2021-02-14 19:19   ` Jakub Jelinek
2021-02-14 21:27     ` Tom de Vries
2021-02-15 12:54       ` Tom de Vries
2021-02-14 21:23   ` Tom de Vries [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).