[22/26] gdbserver: zero-out register values in regcache-discard

Message ID 877c74ccb7fb99d36242d9246d2824f181752a5a.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
  Zero-out register values when a regcache is discarded so that we avoid
garbage values left in the buffer.
---
 gdbserver/regcache.cc | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
  

Comments

Simon Marchi Dec. 22, 2023, 4:36 a.m. UTC | #1
On 2023-02-28 06:28, Tankut Baris Aktemur via Gdb-patches wrote:
> Zero-out register values when a regcache is discarded so that we avoid
> garbage values left in the buffer.
> ---
>  gdbserver/regcache.cc | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
> index 2befb30e337..644f436c681 100644
> --- a/gdbserver/regcache.cc
> +++ b/gdbserver/regcache.cc
> @@ -136,6 +136,7 @@ regcache_invalidate (void)
>  void
>  regcache::discard ()
>  {
> +  memset (registers, 0, tdesc->registers_size);
>  #ifndef IN_PROCESS_AGENT
>    memset ((void *) register_status, REG_UNKNOWN, tdesc->reg_defs.size ());
>  #endif
> @@ -149,16 +150,17 @@ regcache::initialize (const target_desc *tdesc,
>    if (regbuf == NULL)
>      {
>  #ifndef IN_PROCESS_AGENT
> -      /* Make sure to zero-initialize the register cache when it is
> -	 created, in case there are registers the target never
> -	 fetches.  This way they'll read as zero instead of
> -	 garbage.  */
>        this->tdesc = tdesc;
>        this->registers
> -	= (unsigned char *) xcalloc (1, tdesc->registers_size);
> +	= (unsigned char *) xmalloc (tdesc->registers_size);
>        this->registers_owned = true;
>        this->register_status
>  	= (enum register_status *) xmalloc (tdesc->reg_defs.size ());
> +
> +      /* Make sure to zero-initialize the register cache when it is
> +	 created, in case there are registers the target never
> +	 fetches.  This way they'll read as zero instead of
> +	 garbage.  */
>        discard ();
>  #else
>        gdb_assert_not_reached ("can't allocate memory from the heap");

Just curious, if we read and use the contents of a register that isn't
REG_VALID, it's a bug, right?  If so, shouldn't we instead make sure
this never happens?  After all, for a register that is not REG_VALID, a
value of 0 is just as much a "garbage value" as any other value.

Simon
  

Patch

diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
index 2befb30e337..644f436c681 100644
--- a/gdbserver/regcache.cc
+++ b/gdbserver/regcache.cc
@@ -136,6 +136,7 @@  regcache_invalidate (void)
 void
 regcache::discard ()
 {
+  memset (registers, 0, tdesc->registers_size);
 #ifndef IN_PROCESS_AGENT
   memset ((void *) register_status, REG_UNKNOWN, tdesc->reg_defs.size ());
 #endif
@@ -149,16 +150,17 @@  regcache::initialize (const target_desc *tdesc,
   if (regbuf == NULL)
     {
 #ifndef IN_PROCESS_AGENT
-      /* Make sure to zero-initialize the register cache when it is
-	 created, in case there are registers the target never
-	 fetches.  This way they'll read as zero instead of
-	 garbage.  */
       this->tdesc = tdesc;
       this->registers
-	= (unsigned char *) xcalloc (1, tdesc->registers_size);
+	= (unsigned char *) xmalloc (tdesc->registers_size);
       this->registers_owned = true;
       this->register_status
 	= (enum register_status *) xmalloc (tdesc->reg_defs.size ());
+
+      /* Make sure to zero-initialize the register cache when it is
+	 created, in case there are registers the target never
+	 fetches.  This way they'll read as zero instead of
+	 garbage.  */
       discard ();
 #else
       gdb_assert_not_reached ("can't allocate memory from the heap");