public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 15/20] Add attribute::get_unsigned method
Date: Mon, 30 Mar 2020 11:57:13 -0400	[thread overview]
Message-ID: <7a56ce4e-28f6-fe7f-2005-0516152e36d1@simark.ca> (raw)
In-Reply-To: <20200328192208.11324-16-tom@tromey.com>

On 2020-03-28 3:22 p.m., Tom Tromey wrote:
> This introduces a new attribute::get_unsigned method and changes a few
> spots to use it.
> 
> gdb/ChangeLog
> 2020-03-28  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.c (dw2_get_file_names_reader)
> 	(dwarf2_build_include_psymtabs, handle_DW_AT_stmt_list)
> 	(dwarf2_cu::setup_type_unit_groups, fill_in_loclist_baton)
> 	(dwarf2_symbol_mark_computed): Use get_unsigned.
> 	* dwarf2/attribute.h (struct attribute) <get_unsigned>: New
> 	method.
> 	<form_is_section_offset>: Update comment.
> ---
>  gdb/ChangeLog          | 10 ++++++++++
>  gdb/dwarf2/attribute.h | 11 ++++++++++-
>  gdb/dwarf2/read.c      | 22 +++++++++++-----------
>  3 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
> index 9b387e5df05..546156283b3 100644
> --- a/gdb/dwarf2/attribute.h
> +++ b/gdb/dwarf2/attribute.h
> @@ -82,9 +82,18 @@ struct attribute
>      return u.unsnd;
>    }
>  
> +  /* Return the unsigned value.  Requires that the form be an unsigned
> +     form, and that reprocessing not be needed.  */
> +  ULONGEST get_unsigned () const
> +  {
> +    gdb_assert (form_is_unsigned ());
> +    gdb_assert (!requires_reprocessing);

I see what you're trying to do here, but I don't think it's really useful to assert !requires_reprocessing.

For string and address forms that require reprocessing, it's true that we set an unsigned value, but the
form is still some string of address form (we don't change it to some unsigned form).  So it's not really
possible for form_is_unsigned()  to return true, and requires_reprocessing to be true.

Instead, gdb_assert (!requires_reprocessing) should be added to ::address and ::string.

Note that I don't mind if we leave the assert in this function, it's true in any case that requires_reprocessing
should be false at this point.

Also, I noted that this method is named "get_unsigned", since you obviously can't name it "unsigned".  If you
used my idea to name the others "as_string" and "as_address", this one could be "as_unsigned", and the names
would all be coherent.

Simon


  reply	other threads:[~2020-03-30 15:57 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-28 19:21 [PATCH 00/20] Make DWARF attribute references safe Tom Tromey
2020-03-28 19:21 ` [PATCH 01/20] Add attribute::value_as_string method Tom Tromey
2020-03-28 19:21 ` [PATCH 02/20] Rename struct attribute accessors Tom Tromey
2020-03-30  8:58   ` Aktemur, Tankut Baris
2020-03-30 23:39     ` Tom Tromey
2020-03-30 14:45   ` Simon Marchi
2020-03-30 23:39     ` Tom Tromey
2020-03-28 19:21 ` [PATCH 03/20] Avoid using DW_* macros in dwarf2/attribute.c Tom Tromey
2020-03-28 19:21 ` [PATCH 04/20] Change some uses of DW_STRING to string method Tom Tromey
2020-03-30 14:56   ` Simon Marchi
2020-03-30 23:53     ` Tom Tromey
2020-03-28 19:21 ` [PATCH 05/20] Remove some uses of DW_STRING_IS_CANONICAL Tom Tromey
2020-03-30 15:02   ` Simon Marchi
2020-03-31  0:01     ` Tom Tromey
2020-03-28 19:21 ` [PATCH 06/20] Remove DW_STRING and DW_STRING_IS_CANONICAL Tom Tromey
2020-03-30 15:10   ` Simon Marchi
2020-03-31  0:23     ` Tom Tromey
2020-03-28 19:21 ` [PATCH 07/20] Remove DW_BLOCK Tom Tromey
2020-03-30 15:13   ` Simon Marchi
2020-03-28 19:21 ` [PATCH 08/20] Remove DW_SIGNATURE Tom Tromey
2020-03-28 19:21 ` [PATCH 09/20] Remove DW_SND Tom Tromey
2020-03-28 19:21 ` [PATCH 10/20] Use setter for attribute's unsigned value Tom Tromey
2020-03-28 19:21 ` [PATCH 11/20] Add reprocessing flag to struct attribute Tom Tromey
2020-03-30 15:32   ` Simon Marchi
2020-04-04 14:02     ` Tom Tromey
2020-03-28 19:22 ` [PATCH 12/20] Remove DW_ADDR Tom Tromey
2020-03-30 15:40   ` Simon Marchi
2020-04-04 14:05     ` Tom Tromey
2020-03-28 19:22 ` [PATCH 13/20] Change how reprocessing is done Tom Tromey
2020-03-30 15:46   ` Simon Marchi
2020-04-04 14:14     ` Tom Tromey
2020-03-28 19:22 ` [PATCH 14/20] Change how accessibility is handled in dwarf2/read.c Tom Tromey
2020-03-30 15:50   ` Simon Marchi
2020-03-28 19:22 ` [PATCH 15/20] Add attribute::get_unsigned method Tom Tromey
2020-03-30 15:57   ` Simon Marchi [this message]
2020-04-04 14:17     ` Tom Tromey
2020-03-28 19:22 ` [PATCH 16/20] Change is_valid_DW_AT_defaulted to a method on attribute Tom Tromey
2020-03-30 16:00   ` Simon Marchi
2020-04-04 14:23     ` Tom Tromey
2020-03-28 19:22 ` [PATCH 17/20] Change die_info methods to check the attribute's form Tom Tromey
2020-03-30 16:02   ` Simon Marchi
2020-03-30 19:04     ` Tom Tromey
2020-03-30 20:18       ` Simon Marchi
2020-03-30 20:26         ` Tom Tromey
2020-03-28 19:22 ` [PATCH 18/20] Add attribute::virtuality method Tom Tromey
2020-03-28 19:22 ` [PATCH 19/20] Add attribute::boolean method Tom Tromey
2020-03-28 19:22 ` [PATCH 20/20] Remove DW_UNSND Tom Tromey

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=7a56ce4e-28f6-fe7f-2005-0516152e36d1@simark.ca \
    --to=simark@simark.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

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