[1/3] gdb: Added builtin types for 24 bit integers.
Commit Message
* gdb/gdbtypes.h (struct builtin_type): New members builtin_int24
and builtin_uint24;
* gdb/gdbtypes.c: Initialize them.
* gdb/doc/gdb.texinfo: Add them.
---
gdb/doc/gdb.texinfo | 2 ++
gdb/gdbtypes.c | 4 ++++
gdb/gdbtypes.h | 2 ++
3 files changed, 8 insertions(+)
Comments
> From: John Darrington <john@darrington.wattle.id.au>
> Cc: John Darrington <john@darrington.wattle.id.au>
> Date: Thu, 23 Aug 2018 19:35:24 +0200
>
> * gdb/doc/gdb.texinfo: Add them.
This should mention the name of the node in which the change is being
made. Also, since gdb/doc/ has its own ChangeLog file, please make
the description specific, because it will be physically separate from
the other log entries.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 5068c0ac81..e4ecd57a9e 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
This part is OK.
Thanks.
On 2018-08-23 13:35, John Darrington wrote:
> * gdb/gdbtypes.h (struct builtin_type): New members builtin_int24
> and builtin_uint24;
> * gdb/gdbtypes.c: Initialize them.
> * gdb/doc/gdb.texinfo: Add them.
> ---
> gdb/doc/gdb.texinfo | 2 ++
> gdb/gdbtypes.c | 4 ++++
> gdb/gdbtypes.h | 2 ++
> 3 files changed, 8 insertions(+)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 5068c0ac81..e4ecd57a9e 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -42379,6 +42379,7 @@ Boolean type, occupying a single bit.
>
> @item int8
> @itemx int16
> +@itemx int24
> @itemx int32
> @itemx int64
> @itemx int128
> @@ -42386,6 +42387,7 @@ Signed integer types holding the specified
> number of bits.
>
> @item uint8
> @itemx uint16
> +@itemx uint24
> @itemx uint32
> @itemx uint64
> @itemx uint128
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 65b1211ec4..05bf7b1134 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5402,6 +5402,10 @@ gdbtypes_post_init (struct gdbarch *gdbarch)
> = arch_integer_type (gdbarch, 16, 0, "int16_t");
> builtin_type->builtin_uint16
> = arch_integer_type (gdbarch, 16, 1, "uint16_t");
> + builtin_type->builtin_int24
> + = arch_integer_type (gdbarch, 24, 0, "int24_t");
> + builtin_type->builtin_uint24
> + = arch_integer_type (gdbarch, 24, 1, "uint24_t");
> builtin_type->builtin_int32
> = arch_integer_type (gdbarch, 32, 0, "int32_t");
> builtin_type->builtin_uint32
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 14059ab3dc..eb7c365b71 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1611,6 +1611,8 @@ struct builtin_type
> struct type *builtin_uint8;
> struct type *builtin_int16;
> struct type *builtin_uint16;
> + struct type *builtin_int24;
> + struct type *builtin_uint24;
> struct type *builtin_int32;
> struct type *builtin_uint32;
> struct type *builtin_int64;
Hi John,
It's always useful to have a little commit message along with the patch
to explain why this is needed. What's your problem and how does it help
you solve it. Could you expand a bit on your patches?
Thanks,
Simon
On Thu, Aug 23, 2018 at 03:41:11PM -0400, Simon Marchi wrote:
Hi John,
It's always useful to have a little commit message along with the patch
to explain why this is needed. What's your problem and how does it help
you solve it. Could you expand a bit on your patches?
Thanks,
Hi Simon,
Sorry, I had presumed it was self evident from the final patch, which
adds support for the S12Z target - a 24 bit architecture.
It seems that up till now there has been no 24 bit targets, so the other
two patches as some necessary things to make that possible.
J'
On 2018-08-23 16:03, John Darrington wrote:
> On Thu, Aug 23, 2018 at 03:41:11PM -0400, Simon Marchi wrote:
>
> Hi John,
>
> It's always useful to have a little commit message along with the
> patch
> to explain why this is needed. What's your problem and how does
> it help
> you solve it. Could you expand a bit on your patches?
>
> Thanks,
>
> Hi Simon,
>
> Sorry, I had presumed it was self evident from the final patch, which
> adds support for the S12Z target - a 24 bit architecture.
Well this is new information to me :).
It is clear now, but somebody doing a git blame to know why 24-bit
integer types were added would only find the patch that adds them by
itself and wonder who uses that. A little message like
This patch adds 24-bit integer types, used when debugging on the S12Z
architecture (added by a later patch in this series).
clears that up. That might looks a bit silly, but I think it helps in
the long run.
> It seems that up till now there has been no 24 bit targets, so the
> other
> two patches as some necessary things to make that possible.
Thanks. Coming back to the code of the patch, I was wondering if these
24-bit types are useful or even relevant for any other architecture.
Would it work if you only defined the types for s12z architectures,
storing the reference in the gdbarch_tdep object?
Simon
On Thu, Aug 23, 2018 at 04:35:25PM -0400, Simon Marchi wrote:
It is clear now, but somebody doing a git blame to know why 24-bit
integer types were added would only find the patch that adds them by
itself and wonder who uses that. A little message like
This patch adds 24-bit integer types, used when debugging on the S12Z
architecture (added by a later patch in this series).
clears that up. That might looks a bit silly, but I think it helps in
the long run.
I fully agree with you. I've worked on other projects however had a
different opinion - they insisted that the checkin comment NOT contain
any rationale for the change, instead it should just summarize what
changed. I find that rather pointless but anyway ....
> It seems that up till now there has been no 24 bit targets, so the
> other
> two patches as some necessary things to make that possible.
Thanks. Coming back to the code of the patch, I was wondering if these
24-bit types are useful or even relevant for any other architecture.
There most certainly are plenty of 24 bit architectures especially in the
embedded world - just apparently none that gdb currently supports :(
Would it work if you only defined the types for s12z architectures,
storing the reference in the gdbarch_tdep object?
My first reaction is that it probably *could* be made to work, but not
in an elegant fashion. Somehow I'd have to avoid that gdb ever calls the
read_encoded_value function.
I do concede that adding DW_EH_PE_udata3 might be problematic since
it's not part of the dwarf standard. An alternative might be to rework
the read_encoded_value function to not rely on the dwarf enums (all it
really cares about is the size of the target's address space.
Regards
John
@@ -42379,6 +42379,7 @@ Boolean type, occupying a single bit.
@item int8
@itemx int16
+@itemx int24
@itemx int32
@itemx int64
@itemx int128
@@ -42386,6 +42387,7 @@ Signed integer types holding the specified number of bits.
@item uint8
@itemx uint16
+@itemx uint24
@itemx uint32
@itemx uint64
@itemx uint128
@@ -5402,6 +5402,10 @@ gdbtypes_post_init (struct gdbarch *gdbarch)
= arch_integer_type (gdbarch, 16, 0, "int16_t");
builtin_type->builtin_uint16
= arch_integer_type (gdbarch, 16, 1, "uint16_t");
+ builtin_type->builtin_int24
+ = arch_integer_type (gdbarch, 24, 0, "int24_t");
+ builtin_type->builtin_uint24
+ = arch_integer_type (gdbarch, 24, 1, "uint24_t");
builtin_type->builtin_int32
= arch_integer_type (gdbarch, 32, 0, "int32_t");
builtin_type->builtin_uint32
@@ -1611,6 +1611,8 @@ struct builtin_type
struct type *builtin_uint8;
struct type *builtin_int16;
struct type *builtin_uint16;
+ struct type *builtin_int24;
+ struct type *builtin_uint24;
struct type *builtin_int32;
struct type *builtin_uint32;
struct type *builtin_int64;