Re: [PATCH] gdb: allow default_addressable_memory_unit_size to handle more cases

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-04-21 12:59 p.m., Andrew Burgess wrote:
> Currently default_addressable_memory_unit_size always returns 1,
> indicating 1 byte is 1 octet.  If a target has something other than
> this (common) setup then the target should override the
> default_addressable_memory_unit_size.
> 
> However, the bfd library already knows about each targets octets per
> byte, so it seems redundant making targets override this method to
> tell GDB something it already knows (through bfd).
> 
> In this commit I propose to make default_addressable_memory_unit_size
> return a value based on bfd's bits per byte.  I checked, and for every
> target that GDB currently supports the bits per byte in bfd is 8, so
> the current behaviour will not change.
> 
> In fact, the only targets in bfd that have bits per byte set to
> something other than 8 can be found in cpu-tic4x.c and cpu-tic54x.c, I
> don't believe these are supported by GDB right now.
> 
> I don't propose to remove the ability to override
> default_addressable_memory_unit_size, this allows targets additional
> flexibility for how to handle weird combinations of byte sizes.
> 
> This change was motivated by an out of tree target I was working on,
> but it seemed like it was a good change that others might benefit
> from.
> 
> There should be no user visible changes after this commit.
> 
> gdb/ChangeLog:
> 
> 	* arch-utils.c (default_addressable_memory_unit_size): Return a
> 	value based on bfd's bits per byte.
> ---
>  gdb/ChangeLog    | 5 +++++
>  gdb/arch-utils.c | 8 +++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index e1d5afd6039..36681e96b61 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -1005,13 +1005,15 @@ default_gnu_triplet_regexp (struct gdbarch *gdbarch)
>    return gdbarch_bfd_arch_info (gdbarch)->arch_name;
>  }
>  
> -/* Default method for gdbarch_addressable_memory_unit_size.  By default, a memory byte has
> -   a size of 1 octet.  */
> +/* Default method for gdbarch_addressable_memory_unit_size.  The default is
> +   based on the bits_per_byte defined in the bfd library for the current
> +   architecture, this is usually 8-bits, and so this function will usually
> +   return 1 indicating 1 bytes is 1 octet.  */
>  
>  int
>  default_addressable_memory_unit_size (struct gdbarch *gdbarch)
>  {
> -  return 1;
> +  return gdbarch_bfd_arch_info (gdbarch)->bits_per_byte / 8;
>  }
>  
>  void
> 

I think that makes sense, and therefore LGTM.

Simon