[PATCH] gdb: allow default_addressable_memory_unit_size to handle more cases

[PATCH] gdb: allow default_addressable_memory_unit_size to handle more cases

[Andrew Burgess]

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

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

[Simon Marchi]

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