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

Message ID 0c082e1edb5933f2ac5a7aac06197f8957099628.1677582745.git.tankut.baris.aktemur@intel.com
State New
Headers
Series gdbserver: refactor regcache and allow gradually populating |

Commit Message

Aktemur, Tankut Baris Feb. 28, 2023, 11:28 a.m. UTC
  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(-)
  

Comments

Simon Marchi Dec. 22, 2023, 3:35 a.m. UTC | #1
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
  

Patch

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 ());
       memset ((void *) this->register_status, REG_UNAVAILABLE,
 	      tdesc->reg_defs.size ());
 #else
@@ -490,7 +490,7 @@  regcache::get_register_status (int regnum) const
 #ifndef IN_PROCESS_AGENT
   gdb_assert (regnum >= 0 && regnum < tdesc->reg_defs.size ());
   if (register_status != nullptr)
-    return (enum register_status) (register_status[regnum]);
+    return register_status[regnum];
   else
     return REG_VALID;
 #else
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index 3fcd4643a42..ad71a9acaec 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -46,8 +46,8 @@  struct regcache : public reg_buffer_common
   bool registers_owned = false;
   unsigned char *registers = nullptr;
 #ifndef IN_PROCESS_AGENT
-  /* One of REG_UNAVAILABLE or REG_VALID.  */
-  unsigned char *register_status = nullptr;
+  /* See gdbsupport/common-regcache.h.  */
+  enum register_status *register_status = nullptr;
 
   /* Constructors.  */
   regcache () = default;