Patchwork [v2,04/10] Add regcache raw_compare method

login
register
mail settings
Submitter Alan Hayward
Date June 6, 2018, 3:16 p.m.
Message ID <20180606151629.36602-5-alan.hayward@arm.com>
Download mbox | patch
Permalink /patch/27662/
State New
Headers show

Comments

Alan Hayward - June 6, 2018, 3:16 p.m.
raw_compare returns the same as a memcmp (0 for success,
the diff otherwise). Kept with tristate return as this
feels potentally more useful than a simple true/false return.
(Happy to change if not).

2018-06-06  Alan Hayward  <alan.hayward@arm.com>

gdb/
	* common/common-regcache.h (raw_compare): New function.
	* regcache.c (regcache::raw_compare): Likewise.
	* regcache.h (regcache::raw_compare): New declaration.

gdbserver/
	* regcache.c (regcache::raw_compare): New function.
	* regcache.h (regcache::raw_compare): New declaration.
---
 gdb/common/common-regcache.h |  1 +
 gdb/gdbserver/regcache.c     | 10 ++++++++++
 gdb/gdbserver/regcache.h     |  5 +++++
 gdb/regcache.c               | 15 +++++++++++++++
 gdb/regcache.h               | 11 +++++++++++
 5 files changed, 42 insertions(+)
Simon Marchi - June 7, 2018, 8:55 p.m.
On 2018-06-06 11:16 AM, Alan Hayward wrote:
> raw_compare returns the same as a memcmp (0 for success,
> the diff otherwise). Kept with tristate return as this
> feels potentally more useful than a simple true/false return.
> (Happy to change if not).

I would err on the bool side, but I don't really mind, the important is
that it's documented properly.  We still use many ints as bool in GDB, so
someone seeing an int return value could easily think it's 0 -> not equal,
1 -> equal.

Speaking of doc, I would suggest (as in the previous patch) to centralize
the doc in the class/struct at the root, so we don't duplicate it.

> 2018-06-06  Alan Hayward  <alan.hayward@arm.com>
> 
> gdb/
> 	* common/common-regcache.h (raw_compare): New function.
> 	* regcache.c (regcache::raw_compare): Likewise.
> 	* regcache.h (regcache::raw_compare): New declaration.
> 
> gdbserver/
> 	* regcache.c (regcache::raw_compare): New function.
> 	* regcache.h (regcache::raw_compare): New declaration.
> ---
>  gdb/common/common-regcache.h |  1 +
>  gdb/gdbserver/regcache.c     | 10 ++++++++++
>  gdb/gdbserver/regcache.h     |  5 +++++
>  gdb/regcache.c               | 15 +++++++++++++++
>  gdb/regcache.h               | 11 +++++++++++
>  5 files changed, 42 insertions(+)
> 
> diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h
> index 487da0a7fb..e91439bec5 100644
> --- a/gdb/common/common-regcache.h
> +++ b/gdb/common/common-regcache.h
> @@ -67,6 +67,7 @@ struct reg_buffer_common
>    virtual ~reg_buffer_common () = default;
>    virtual void raw_supply (int regnum, const void *buf) = 0;
>    virtual void raw_collect (int regnum, void *buf) const = 0;
> +  virtual int raw_compare (int regnum, const void *buf, int offset) const = 0;
>    virtual register_status get_register_status (int regnum) const = 0;
>  };
>  
> diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
> index 857721ee3d..9648428349 100644
> --- a/gdb/gdbserver/regcache.c
> +++ b/gdb/gdbserver/regcache.c
> @@ -501,3 +501,13 @@ regcache::get_register_status (int regnum) const
>    return REG_VALID;
>  #endif
>  }
> +
> +/* See gdbserver/regcache.h.  */
> +
> +int
> +regcache::raw_compare (int regnum, const void *buf, int offset) const
> +{
> +  gdb_assert (register_size (tdesc, regnum) > offset);

Should we check ">= offset"?  I think it could be useful for some edge cases
where we could avoid an "if (offset < size)" in the caller, if we know offset
could be equal to size.  memcmp would return 0 (equal), which is fine.

Maybe assign "register_size (tdesc, regnum)" to a variable (e.g. reg_size)
and use it in both places?

> +  return memcmp (buf, register_data (this, regnum, 1) + offset,
> +		 register_size (tdesc, regnum) - offset);
> +}
> diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
> index 1842f1d9cf..b26f39a8ad 100644
> --- a/gdb/gdbserver/regcache.h
> +++ b/gdb/gdbserver/regcache.h
> @@ -50,6 +50,11 @@ struct regcache : public reg_buffer_common
>  
>    void raw_collect (int regnum, void *buf) const override;
>  
> +  /* Compare the contents of the register stored in the regcache (ignoring the
> +     first OFFSET bytes) to the contents of BUF (without any offset).  Returns
> +     the same result as memcmp.  */
> +  int raw_compare (int regnum, const void *buf, int offset) const override;
> +
>    enum register_status get_register_status (int regnum) const override;
>  };
>  
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index a914b548ca..383e355e9f 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1082,6 +1082,21 @@ regcache::collect_regset (const struct regset *regset,
>    transfer_regset (regset, NULL, regnum, NULL, buf, size);
>  }
>  
> +/* See regcache.h.  */
> +
> +int
> +regcache::raw_compare (int regnum, const void *buf, int offset) const
> +{
> +  const char *regbuf;
> +  size_t size;
> +
> +  gdb_assert (buf != NULL);
> +  assert_regnum (regnum);

Should we assert that offset is < or <= size here too?

Simon

Patch

diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h
index 487da0a7fb..e91439bec5 100644
--- a/gdb/common/common-regcache.h
+++ b/gdb/common/common-regcache.h
@@ -67,6 +67,7 @@  struct reg_buffer_common
   virtual ~reg_buffer_common () = default;
   virtual void raw_supply (int regnum, const void *buf) = 0;
   virtual void raw_collect (int regnum, void *buf) const = 0;
+  virtual int raw_compare (int regnum, const void *buf, int offset) const = 0;
   virtual register_status get_register_status (int regnum) const = 0;
 };
 
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 857721ee3d..9648428349 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -501,3 +501,13 @@  regcache::get_register_status (int regnum) const
   return REG_VALID;
 #endif
 }
+
+/* See gdbserver/regcache.h.  */
+
+int
+regcache::raw_compare (int regnum, const void *buf, int offset) const
+{
+  gdb_assert (register_size (tdesc, regnum) > offset);
+  return memcmp (buf, register_data (this, regnum, 1) + offset,
+		 register_size (tdesc, regnum) - offset);
+}
diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
index 1842f1d9cf..b26f39a8ad 100644
--- a/gdb/gdbserver/regcache.h
+++ b/gdb/gdbserver/regcache.h
@@ -50,6 +50,11 @@  struct regcache : public reg_buffer_common
 
   void raw_collect (int regnum, void *buf) const override;
 
+  /* Compare the contents of the register stored in the regcache (ignoring the
+     first OFFSET bytes) to the contents of BUF (without any offset).  Returns
+     the same result as memcmp.  */
+  int raw_compare (int regnum, const void *buf, int offset) const override;
+
   enum register_status get_register_status (int regnum) const override;
 };
 
diff --git a/gdb/regcache.c b/gdb/regcache.c
index a914b548ca..383e355e9f 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1082,6 +1082,21 @@  regcache::collect_regset (const struct regset *regset,
   transfer_regset (regset, NULL, regnum, NULL, buf, size);
 }
 
+/* See regcache.h.  */
+
+int
+regcache::raw_compare (int regnum, const void *buf, int offset) const
+{
+  const char *regbuf;
+  size_t size;
+
+  gdb_assert (buf != NULL);
+  assert_regnum (regnum);
+
+  regbuf = (const char *) register_buffer (regnum);
+  size = m_descr->sizeof_register[regnum];
+  return memcmp (buf, regbuf + offset, size - offset);
+}
 
 /* Special handling for register PC.  */
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index b559a10752..03fb4f2452 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -169,6 +169,12 @@  public:
     gdb_assert (false);
   }
 
+  virtual int raw_compare (int regnum, const void *buf,
+			   int offset) const override
+  {
+    gdb_assert (false);
+  }
+
 protected:
   /* Assert on the range of REGNUM.  */
   void assert_regnum (int regnum) const;
@@ -325,6 +331,11 @@  public:
   void collect_regset (const struct regset *regset, int regnum,
 		       void *buf, size_t size) const;
 
+  /* Compare the contents of the register stored in the regcache (ignoring the
+     first OFFSET bytes) to the contents of BUF (without any offset).  Returns
+     the same result as memcmp.  */
+  int raw_compare (int regnum, const void *buf, int offset) const override;
+
   /* Return REGCACHE's ptid.  */
 
   ptid_t ptid () const