diff mbox series

[v2,11/11] gdbserver: fix the declared type of register_status in regcache

Message ID 20241230-upstream-gdbserver-regcache-v2-11-020a9514fcf0@intel.com
State New
Headers
Series gdbserver: refactor regcache |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Tankut Baris Aktemur Dec. 30, 2024, 10:49 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.

Now that we are modifying the declaration, also use a unique_ptr
and make the field private.

The get/set methods already use the correct type, but we update cast
operations in two places.
---
 gdbserver/regcache.cc | 22 +++++++++++-----------
 gdbserver/regcache.h  | 11 ++++++++---
 2 files changed, 19 insertions(+), 14 deletions(-)

Comments

Simon Marchi Jan. 7, 2025, 5:40 a.m. UTC | #1 
On 2024-12-30 05:49, Tankut Baris Aktemur 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.
> 
> Now that we are modifying the declaration, also use a unique_ptr
> and make the field private.
> 
> The get/set methods already use the correct type, but we update cast
> operations in two places.

Oh well, you can disregard my previous comments about changing the type
of this field and making it private :).  I should really find a way to
review an entire series before sending the feedback...

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon
diff mbox series

Patch

diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index 562524306dfb36c8be88be5056b2fdddb6ca0b3c..54826e886143349923877deaf8b5c7f556eddfc0 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -120,7 +120,7 @@  regcache::regcache (const target_desc *tdesc,
   this->registers = regbuf;
   this->registers_owned = false;
 #ifndef IN_PROCESS_AGENT
-  this->register_status = nullptr;
+  m_register_status = nullptr;
 #endif
   this->registers_fetched = false;
 }
@@ -139,8 +139,8 @@  regcache::regcache (const target_desc *tdesc)
   this->registers
     = (unsigned char *) xmalloc (tdesc->registers_size);
   this->registers_owned = true;
-  this->register_status
-    = (unsigned char *) xmalloc (tdesc->reg_defs.size ());
+  size_t num_regs = tdesc->reg_defs.size ();
+  m_register_status.reset (new enum register_status[num_regs]);
   reset (REG_UNKNOWN);
 }
 
@@ -148,7 +148,6 @@  regcache::~regcache ()
 {
   if (registers_owned)
     free (registers);
-  free (register_status);
 }
 
 #endif
@@ -161,7 +160,8 @@  regcache::reset (enum register_status status)
      of garbage.  */
   memset (this->registers, 0, this->tdesc->registers_size);
 #ifndef IN_PROCESS_AGENT
-  memset (this->register_status, status, this->tdesc->reg_defs.size ());
+  for (int i = 0; i < this->tdesc->reg_defs.size (); i++)
+    set_register_status (i, status);
 #endif
 }
 
@@ -174,8 +174,8 @@  regcache::copy_from (regcache *src)
 
   memcpy (this->registers, src->registers, src->tdesc->registers_size);
 #ifndef IN_PROCESS_AGENT
-  if (this->register_status != nullptr && src->register_status != nullptr)
-    memcpy (this->register_status, src->register_status,
+  if (m_register_status != nullptr && src->m_register_status != nullptr)
+    memcpy (m_register_status.get (), src->m_register_status.get (),
 	    src->tdesc->reg_defs.size ());
 #endif
   this->registers_fetched = src->registers_fetched;
@@ -486,8 +486,8 @@  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]);
+  if (m_register_status != nullptr)
+    return m_register_status[regnum];
   else
     return REG_VALID;
 #else
@@ -500,8 +500,8 @@  regcache::set_register_status (int regnum, enum register_status status)
 {
 #ifndef IN_PROCESS_AGENT
   gdb_assert (regnum >= 0 && regnum < tdesc->reg_defs.size ());
-  if (register_status != nullptr)
-    register_status[regnum] = status;
+  if (m_register_status != nullptr)
+    m_register_status[regnum] = status;
 #endif
 }
 
diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
index b5a0b85982515dd545c6f79688ba7a4fd177b3d6..6828611550e71ffea8af5fec4f4a073b8644da67 100644
--- a/gdbserver/regcache.h
+++ b/gdbserver/regcache.h
@@ -20,6 +20,7 @@ 
 #define GDBSERVER_REGCACHE_H
 
 #include "gdbsupport/common-regcache.h"
+#include <memory>
 
 struct thread_info;
 struct target_desc;
@@ -43,9 +44,6 @@  struct regcache : public reg_buffer_common
   bool registers_owned = false;
   unsigned char *registers = nullptr;
 #ifndef IN_PROCESS_AGENT
-  /* See gdbsupport/common-regcache.h.  */
-  unsigned char *register_status = nullptr;
-
   /* Constructors.  */
   regcache (const target_desc *tdesc);
 
@@ -80,6 +78,13 @@  struct regcache : public reg_buffer_common
 
   /* Copy the contents of SRC into this regcache.  */
   void copy_from (regcache *src);
+
+private:
+
+#ifndef IN_PROCESS_AGENT
+  /* See gdbsupport/common-regcache.h.  */
+  std::unique_ptr<enum register_status[]> m_register_status;
+#endif
 };
 
 regcache *get_thread_regcache (thread_info *thread, bool fetch = true);