Re: [PATCH 19/26] gdbserver: fix the declared type of register_status in regcache

Simon Marchi
2023-12-22T03:35:15Z

On 2023-02-28 06:28, Tankut Baris Aktemur via Gdb-patches wrote:
> The register_status field of regcache is declared as `unsigned char *`.
> This is incorrect, because `enum register_status` from
> gdbsupport/common-regcache.h is based on signed char and
> REG_UNAVAILABLE is defined as -1. Fix the declared type.
>
> The get/set methods already use the correct type, but we update cast
> operations in two places.
> ---
> gdbserver/regcache.cc | 4 ++--
> gdbserver/regcache.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
> index 0e21c1aa7d1..09ea58bdbd6 100644
> --- a/gdbserver/regcache.cc
> +++ b/gdbserver/regcache.cc
> @@ -147,7 +147,7 @@ regcache::initialize (const target_desc *tdesc,
> = (unsigned char *) xcalloc (1, tdesc->registers_size);
> this->registers_owned = true;
> this->register_status
> - = (unsigned char *) xmalloc (tdesc->reg_defs.size ());
> + = (enum register_status *) xmalloc (tdesc->reg_defs.size ());

The malloc'ed size assumes that a register_status value is 1 byte long.
register_status is indeed 1 byte long, but since it's hidden behind
another name, it's not obvious. You could perhaps switch to:

this->register_status = XNEWVEC (enum register_status, tdesc->reg_defs.size ());

Or C++ify it:

this->register_status = new enum register_status[tdesc->reg_defs.size ()];

But then you have to change the free to a delete[], or store it in an
std::unique_ptr<enum register_status[]>.

Simon