public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/python: Add new gdb.Value.bytes attribute
@ 2023-10-18 14:57 Andrew Burgess
  2023-10-18 15:40 ` Eli Zaretskii
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andrew Burgess @ 2023-10-18 14:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Add a gdb.Value.bytes attribute.  This attribute contains the bytes of
the value (assuming the complete bytes of the value are available).

If the bytes of the gdb.Value are not available then accessing this
attribute raises an exception.

The bytes object returned from gdb.Value.bytes is cached within GDB so
that the same bytes object is returned each time.  The bytes object is
created on-demand though to reduce unnecessary work.

For some values we can of course obtain the same information by
reading inferior memory based on gdb.Value.address and
gdb.Value.type.sizeof, however, not every value is in memory, so we
don't always have an address.

The gdb.Value.bytes attribute will convert any value to a bytes
object, so long as the contents are available.  The value can be one
created purely in Python code, the value could be in a register,
or (of course) the value could be in memory.
---
 gdb/NEWS                              |  3 ++
 gdb/doc/python.texi                   |  7 ++++
 gdb/python/py-value.c                 | 35 +++++++++++++++++
 gdb/testsuite/gdb.python/py-value.exp | 56 ++++++++++++++++++++++++++-
 4 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 08d779010f0..d89df5dbb45 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -11,6 +11,9 @@
   ** New function gdb.notify_mi(NAME, DATA), that emits custom
      GDB/MI async notification.
 
+  ** New attribute gdb.Value.bytes that contains a bytes object
+     holding the contents of this value.
+
 *** Changes in GDB 14
 
 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 546b4d4b962..a2db6c1b863 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -916,6 +916,13 @@
 method is invoked.  
 @end defvar
 
+@defvar Value.bytes
+The value of this read-only attribute is a @code{bytes} object
+containing the bytes that make up this Value's value.  If the complete
+contents of this value are not available then accessing this attribute
+will raise an exception.
+@end defvar
+
 The following methods are provided:
 
 @defun Value.__init__ (val)
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 0bf1d6e0dae..87287cd9c7f 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -63,6 +63,7 @@ struct value_object {
   PyObject *address;
   PyObject *type;
   PyObject *dynamic_type;
+  PyObject *content_bytes;
 };
 
 /* List of all values which are currently exposed to Python. It is
@@ -86,6 +87,7 @@ valpy_clear_value (value_object *self)
   Py_CLEAR (self->address);
   Py_CLEAR (self->type);
   Py_CLEAR (self->dynamic_type);
+  Py_CLEAR (self->content_bytes);
 }
 
 /* Called by the Python interpreter when deallocating a value object.  */
@@ -1304,6 +1306,36 @@ valpy_get_is_lazy (PyObject *self, void *closure)
   Py_RETURN_FALSE;
 }
 
+/* Implements gdb.Value.bytes attribute.  */
+static PyObject *
+valpy_get_bytes (PyObject *self, void *closure)
+{
+  value_object *value_obj = (value_object *) self;
+  struct value *value = value_obj->value;
+
+  if (value_obj->content_bytes != nullptr)
+    {
+      Py_INCREF (value_obj->content_bytes);
+      return value_obj->content_bytes;
+    }
+
+  gdb::array_view<const gdb_byte> contents;
+  try
+    {
+      contents = value->contents ();
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+
+  value_obj->content_bytes
+    =  PyBytes_FromStringAndSize ((const char *) contents.data (),
+				  contents.size ());
+  Py_XINCREF (value_obj->content_bytes);
+  return value_obj->content_bytes;
+}
+
 /* Implements gdb.Value.fetch_lazy ().  */
 static PyObject *
 valpy_fetch_lazy (PyObject *self, PyObject *args)
@@ -1865,6 +1897,7 @@ value_to_value_object (struct value *val)
       val_obj->address = NULL;
       val_obj->type = NULL;
       val_obj->dynamic_type = NULL;
+      val_obj->content_bytes = nullptr;
       note_value (val_obj);
     }
 
@@ -2152,6 +2185,8 @@ static gdb_PyGetSetDef value_object_getset[] = {
     "Boolean telling whether the value is lazy (not fetched yet\n\
 from the inferior).  A lazy value is fetched when needed, or when\n\
 the \"fetch_lazy()\" method is called.", NULL },
+  { "bytes", valpy_get_bytes, nullptr,
+    "Return a bytearray containing the bytes of this value.", nullptr },
   {NULL}  /* Sentinel */
 };
 
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index cdfcd414cd4..89bfbfd8153 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -66,7 +66,14 @@ proc test_value_creation {} {
   gdb_test "python print ('result = %s' % i.address)" "= None" "test address attribute in non-addressable value"
 
   # Test creating / printing an optimized out value
-  gdb_test "python print(gdb.Value(gdb.Value(5).type.optimized_out()))"
+  gdb_test "python print(gdb.Value(gdb.Value(5).type.optimized_out()))" \
+      "<optimized out>"
+
+  # Test accessing the bytes of an optimised out value.
+  gdb_test "python print(gdb.Value(gdb.Value(5).type.optimized_out()).bytes)" \
+      [multi_line \
+	   "gdb\\.error: value has been optimized out" \
+	   "Error while executing Python code\\."]
 }
 
 # Check that we can call gdb.Value.__init__ to change a value.
@@ -485,6 +492,34 @@ proc test_subscript_regression {exefile lang} {
  gdb_py_test_silent_cmd "print {\"fu \",\"foo\",\"bar\"}" "Build array" 1
  gdb_py_test_silent_cmd "python marray = gdb.history(0)" "fetch marray" 0
  gdb_test "python print (marray\[1\]\[2\])" "o." "test multiple subscript"
+
+ # A Python helper function.  Fetch VAR_NAME from the inferior as a
+ # gdb.Value.  Read the bytes of the value based on its address, and
+ # the size of its type.  The compare these bytes to the value
+ # obtained from gdb.Value.bytes.  Assert that the two bytes objects
+ # match.
+ gdb_test_multiline "Create a function to check Value.bytes" \
+     "python" "" \
+     "def check_value_bytes(var_name):" "" \
+     "   val = gdb.parse_and_eval(var_name)" "" \
+     "   addr = val.address" "" \
+     "   len = val.type.sizeof" "" \
+     "   mem = gdb.selected_inferior().read_memory(addr, len)" "" \
+     "   mem_bytes = mem.tobytes()" "" \
+     "   val_bytes = val.bytes" "" \
+     "   assert mem_bytes == val_bytes" "" \
+     "end" ""
+
+ gdb_test_no_output { python check_value_bytes("a") }
+ gdb_test_no_output { python check_value_bytes("p") }
+ gdb_test_no_output { python check_value_bytes("i") }
+ gdb_test_no_output { python check_value_bytes("ptr_i") }
+ gdb_test_no_output { python check_value_bytes("embed") }
+ gdb_test_no_output { python check_value_bytes("fp1") }
+ gdb_test_no_output { python check_value_bytes("nullst") }
+ gdb_test_no_output { python check_value_bytes("st") }
+ gdb_test_no_output { python check_value_bytes("s") }
+ gdb_test_no_output { python check_value_bytes("u") }
 }
 
 # A few tests of gdb.parse_and_eval.
@@ -542,13 +577,30 @@ proc prepare_type_and_buffer {} {
 proc test_value_from_buffer {} {
   global gdb_prompt
 
+  # A Python helper function.  Create a bytes object from inferior
+  # memory LEN bytes starting at ADDR, and compare this to the bytes
+  # obtained from VAL.bytes.  Assert that the two bytes object match.
+  gdb_test_multiline "Create another function to check Value.bytes" \
+     "python" "" \
+     "def compare_value_bytes_to_mem(val, addr, len):" "" \
+     "   mem = gdb.selected_inferior().read_memory(addr, len)" "" \
+     "   mem_bytes = mem.tobytes()" "" \
+     "   val_bytes = val.bytes" "" \
+     "   assert mem_bytes == val_bytes" "" \
+     "end" ""
+
   prepare_type_and_buffer
   gdb_test "python v=gdb.Value(b,tp); print(v)" "1" \
             "construct value from buffer"
+  gdb_test_no_output { python compare_value_bytes_to_mem(v, addr, size_a0) }
   gdb_test "python v=gdb.Value(b\[size_a0:\],tp); print(v)" "2" \
             "convert 2nd elem of buffer to value"
+  gdb_test_no_output \
+      { python compare_value_bytes_to_mem(v, (int(addr) + size_a0), size_a0) }
   gdb_test "python v=gdb.Value(b\[2*size_a0:\],tp); print(v)" "3" \
            "convert 3rd elem of buffer to value"
+  gdb_test_no_output \
+      { python compare_value_bytes_to_mem(v, (int(addr) + (2 * size_a0)), size_a0) }
   gdb_test "python v=gdb.Value(b\[2*size_a0+1:\],tp); print(v)" \
            "ValueError: Size of type is larger than that of buffer object\..*" \
 	   "attempt to convert smaller buffer than size of type"
@@ -556,6 +608,8 @@ proc test_value_from_buffer {} {
                          "make array type" 0
   gdb_py_test_silent_cmd "python va=gdb.Value(b,atp)" \
                          "construct array value from buffer" 0
+  gdb_test_no_output \
+      { python compare_value_bytes_to_mem(va, addr, size_a0 * 3) }
   gdb_test "python print(va)" "\\{1, 2, 3\\}" "print array value"
   gdb_test "python print(va\[0\])" "1" "print first array element"
   gdb_test "python print(va\[1\])" "2" "print second array element"

base-commit: 29736fc507c7a9c6e797b7f83e8df4be73d37767
-- 
2.25.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb/python: Add new gdb.Value.bytes attribute
  2023-10-18 14:57 [PATCH] gdb/python: Add new gdb.Value.bytes attribute Andrew Burgess
@ 2023-10-18 15:40 ` Eli Zaretskii
  2023-10-20 17:21 ` Tom Tromey
  2023-10-23 17:55 ` [PATCHv2] " Andrew Burgess
  2 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2023-10-18 15:40 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Wed, 18 Oct 2023 15:57:15 +0100
> 
>  gdb/NEWS                              |  3 ++
>  gdb/doc/python.texi                   |  7 ++++
>  gdb/python/py-value.c                 | 35 +++++++++++++++++
>  gdb/testsuite/gdb.python/py-value.exp | 56 ++++++++++++++++++++++++++-
>  4 files changed, 100 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 08d779010f0..d89df5dbb45 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -11,6 +11,9 @@
>    ** New function gdb.notify_mi(NAME, DATA), that emits custom
>       GDB/MI async notification.
>  
> +  ** New attribute gdb.Value.bytes that contains a bytes object
> +     holding the contents of this value.
> +
>  *** Changes in GDB 14
>  
>  * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 546b4d4b962..a2db6c1b863 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -916,6 +916,13 @@
>  method is invoked.  
>  @end defvar
>  
> +@defvar Value.bytes
> +The value of this read-only attribute is a @code{bytes} object
> +containing the bytes that make up this Value's value.  If the complete
                                          ^^^^^
I think "Value" should be in @code.

> +contents of this value are not available then accessing this attribute
> +will raise an exception.
> +@end defvar
> +

The documentation parts are okay, but I wonder: is it enough to say
"the bytes that make up this Value"?  Shouldn't we say something about
endianness, for example?  Or what are "the bytes" of a string with
non-ASCII characters in it?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb/python: Add new gdb.Value.bytes attribute
  2023-10-18 14:57 [PATCH] gdb/python: Add new gdb.Value.bytes attribute Andrew Burgess
  2023-10-18 15:40 ` Eli Zaretskii
@ 2023-10-20 17:21 ` Tom Tromey
  2023-10-23 17:55 ` [PATCHv2] " Andrew Burgess
  2 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2023-10-20 17:21 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Add a gdb.Value.bytes attribute.  This attribute contains the bytes of
Andrew> the value (assuming the complete bytes of the value are available).

Thanks for doing this.  I think we've wanted this for a long time.
Also, it's half of https://sourceware.org/bugzilla/show_bug.cgi?id=13267

Andrew> +  value_obj->content_bytes
Andrew> +    =  PyBytes_FromStringAndSize ((const char *) contents.data (),
Andrew> +				  contents.size ());
Andrew> +  Py_XINCREF (value_obj->content_bytes);
Andrew> +  return value_obj->content_bytes;

I think Value.assign should clear the cache.

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2] gdb/python: Add new gdb.Value.bytes attribute
  2023-10-18 14:57 [PATCH] gdb/python: Add new gdb.Value.bytes attribute Andrew Burgess
  2023-10-18 15:40 ` Eli Zaretskii
  2023-10-20 17:21 ` Tom Tromey
@ 2023-10-23 17:55 ` Andrew Burgess
  2023-10-23 18:46   ` Eli Zaretskii
  2023-10-25 15:39   ` Tom Tromey
  2 siblings, 2 replies; 6+ messages in thread
From: Andrew Burgess @ 2023-10-23 17:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess, Eli Zaretskii, Tom Tromey

Thanks for the feedback on V1.

In V2:

  - Docs have been updated to be more explicit about the byte
    ordering, but have also been extended to cover the new
    functionality I've added in this update,

  - The cached value is now cleared correctly during Value.assign
    calls as Tom pointed out,

  - The Value.bytes attribute can now be assigned too.  This is
    similar to how Value.assign works.

---

Add a gdb.Value.bytes attribute.  This attribute contains the bytes of
the value (assuming the complete bytes of the value are available).

If the bytes of the gdb.Value are not available then accessing this
attribute raises an exception.

The bytes object returned from gdb.Value.bytes is cached within GDB so
that the same bytes object is returned each time.  The bytes object is
created on-demand though to reduce unnecessary work.

For some values we can of course obtain the same information by
reading inferior memory based on gdb.Value.address and
gdb.Value.type.sizeof, however, not every value is in memory, so we
don't always have an address.

The gdb.Value.bytes attribute will convert any value to a bytes
object, so long as the contents are available.  The value can be one
created purely in Python code, the value could be in a register,
or (of course) the value could be in memory.

The Value.bytes attribute can also be assigned too.  Assigning to this
attribute is similar to calling Value.assign, the value of the
underlying value is updated within the inferior.  The value assigned
to Value.bytes must be a buffer which contains exactly the correct
number of bytes (i.e. unlike value creation, we don't allow oversized
buffers).

To support this assignment like behaviour I've factored out the core
of valpy_assign.  I've also updated convert_buffer_and_type_to_value
so that it can (for my use case) check the exact buffer length.

The restrictions for when the Value.bytes can or cannot be written too
are exactly the same as for Value.assign.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=13267
---
 gdb/NEWS                              |   3 +
 gdb/doc/python.texi                   |  16 ++++
 gdb/python/py-value.c                 | 122 ++++++++++++++++++++++----
 gdb/testsuite/gdb.python/py-value.exp | 109 ++++++++++++++++++++++-
 4 files changed, 230 insertions(+), 20 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 08d779010f0..e54554dc044 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -11,6 +11,9 @@
   ** New function gdb.notify_mi(NAME, DATA), that emits custom
      GDB/MI async notification.
 
+  ** New read/write attribute gdb.Value.bytes that contains a bytes
+     object holding the contents of this value.
+
 *** Changes in GDB 14
 
 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 546b4d4b962..8cc3f92cbfe 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -916,6 +916,21 @@
 method is invoked.  
 @end defvar
 
+@defvar Value.bytes
+The value of this attribute is a @code{bytes} object containing the
+bytes that make up this @code{Value}'s complete value in little endian
+order.  If the complete contents of this value are not available then
+accessing this attribute will raise an exception.
+
+This attribute can also be assigned to.  The new value should be a
+buffer object (e.g.@: a @code{bytes} object), the length of the new
+buffer must exactly match the length of this @code{Value}'s type.  The
+bytes values in the new buffer should be in little endian order.
+
+As with @code{Value.assign} (@pxref{Value.assign}), if this value
+cannot be assigned to, then an exception will be thrown.
+@end defvar
+
 The following methods are provided:
 
 @defun Value.__init__ (val)
@@ -966,6 +981,7 @@
 behaves as though @var{type} was not passed at all.
 @end defun
 
+@anchor{Value.assign}
 @defun Value.assign (rhs)
 Assign @var{rhs} to this value, and return @code{None}.  If this value
 cannot be assigned to, or if the assignment is invalid for some reason
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 0bf1d6e0dae..f360f849ce6 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -63,6 +63,7 @@ struct value_object {
   PyObject *address;
   PyObject *type;
   PyObject *dynamic_type;
+  PyObject *content_bytes;
 };
 
 /* List of all values which are currently exposed to Python. It is
@@ -86,6 +87,7 @@ valpy_clear_value (value_object *self)
   Py_CLEAR (self->address);
   Py_CLEAR (self->type);
   Py_CLEAR (self->dynamic_type);
+  Py_CLEAR (self->content_bytes);
 }
 
 /* Called by the Python interpreter when deallocating a value object.  */
@@ -135,10 +137,14 @@ note_value (value_object *value_obj)
 /* Convert a python object OBJ with type TYPE to a gdb value.  The
    python object in question must conform to the python buffer
    protocol.  On success, return the converted value, otherwise
-   nullptr.  */
+   nullptr.  When REQUIRE_EXACT_SIZE_P is true the buffer OBJ must be the
+   exact length of TYPE.  When REQUIRE_EXACT_SIZE_P is false then the
+   buffer OBJ can be longer than TYPE, in which case only the least
+   significant bytes from the buffer are used.  */
 
 static struct value *
-convert_buffer_and_type_to_value (PyObject *obj, struct type *type)
+convert_buffer_and_type_to_value (PyObject *obj, struct type *type,
+				  bool require_exact_size_p)
 {
   Py_buffer_up buffer_up;
   Py_buffer py_buf;
@@ -157,7 +163,13 @@ convert_buffer_and_type_to_value (PyObject *obj, struct type *type)
       return nullptr;
     }
 
-  if (type->length () > py_buf.len)
+  if (require_exact_size_p && type->length () != py_buf.len)
+    {
+      PyErr_SetString (PyExc_ValueError,
+		       _("Size of type is not equal to that of buffer object."));
+      return nullptr;
+    }
+  else if (!require_exact_size_p && type->length () > py_buf.len)
     {
       PyErr_SetString (PyExc_ValueError,
 		       _("Size of type is larger than that of buffer object."));
@@ -196,7 +208,7 @@ valpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   if (type == nullptr)
     value = convert_value_from_python (val_obj);
   else
-    value = convert_buffer_and_type_to_value (val_obj, type);
+    value = convert_buffer_and_type_to_value (val_obj, type, false);
   if (value == nullptr)
     {
       gdb_assert (PyErr_Occurred ());
@@ -888,36 +900,53 @@ valpy_reinterpret_cast (PyObject *self, PyObject *args)
   return valpy_do_cast (self, args, UNOP_REINTERPRET_CAST);
 }
 
-/* Implementation of the "assign" method.  */
+/* Assign NEW_VALUE into SELF, handles 'struct value' reference counting,
+   and also clearing the bytes data cached within SELF.  Return true if
+   the assignment was successful, otherwise return false, in which case a
+   Python exception will be set.  */
 
-static PyObject *
-valpy_assign (PyObject *self_obj, PyObject *args)
+static bool
+valpy_assign_core (value_object *self, struct value *new_value)
 {
-  PyObject *val_obj;
-
-  if (! PyArg_ParseTuple (args, "O", &val_obj))
-    return nullptr;
-
-  struct value *val = convert_value_from_python (val_obj);
-  if (val == nullptr)
-    return nullptr;
-
   try
     {
-      value_object *self = (value_object *) self_obj;
-      value *new_value = value_assign (self->value, val);
+      new_value = value_assign (self->value, new_value);
+
       /* value_as_address returns a new value with the same location
 	 as the old one.  Ensure that this gdb.Value is updated to
 	 reflect the new value.  */
       new_value->incref ();
       self->value->decref ();
+      Py_CLEAR (self->content_bytes);
       self->value = new_value;
     }
   catch (const gdb_exception &except)
     {
-      GDB_PY_HANDLE_EXCEPTION (except);
+      gdbpy_convert_exception (except);
+      return false;
     }
 
+  return true;
+}
+
+/* Implementation of the "assign" method.  */
+
+static PyObject *
+valpy_assign (PyObject *self_obj, PyObject *args)
+{
+  PyObject *val_obj;
+
+  if (! PyArg_ParseTuple (args, "O", &val_obj))
+    return nullptr;
+
+  struct value *val = convert_value_from_python (val_obj);
+  if (val == nullptr)
+    return nullptr;
+
+  value_object *self = (value_object *) self_obj;
+  if (!valpy_assign_core (self, val))
+    return nullptr;
+
   Py_RETURN_NONE;
 }
 
@@ -1304,6 +1333,58 @@ valpy_get_is_lazy (PyObject *self, void *closure)
   Py_RETURN_FALSE;
 }
 
+/* Get gdb.Value.bytes attribute.  */
+
+static PyObject *
+valpy_get_bytes (PyObject *self, void *closure)
+{
+  value_object *value_obj = (value_object *) self;
+  struct value *value = value_obj->value;
+
+  if (value_obj->content_bytes != nullptr)
+    {
+      Py_INCREF (value_obj->content_bytes);
+      return value_obj->content_bytes;
+    }
+
+  gdb::array_view<const gdb_byte> contents;
+  try
+    {
+      contents = value->contents ();
+    }
+  catch (const gdb_exception &except)
+    {
+      GDB_PY_HANDLE_EXCEPTION (except);
+    }
+
+  value_obj->content_bytes
+    =  PyBytes_FromStringAndSize ((const char *) contents.data (),
+				  contents.size ());
+  Py_XINCREF (value_obj->content_bytes);
+  return value_obj->content_bytes;
+}
+
+/* Set gdb.Value.bytes attribute.  */
+
+static int
+valpy_set_bytes (PyObject *self_obj, PyObject *new_value_obj, void *closure)
+{
+  value_object *self = (value_object *) self_obj;
+
+  /* Create a new value from the buffer NEW_VALUE_OBJ.  We pass true here
+     to indicate that NEW_VALUE_OBJ must match exactly the type length.  */
+  struct value *new_value
+    = convert_buffer_and_type_to_value (new_value_obj, self->value->type (),
+					true);
+  if (new_value == nullptr)
+    return -1;
+
+  if (!valpy_assign_core (self, new_value))
+    return -1;
+
+  return 0;
+}
+
 /* Implements gdb.Value.fetch_lazy ().  */
 static PyObject *
 valpy_fetch_lazy (PyObject *self, PyObject *args)
@@ -1865,6 +1946,7 @@ value_to_value_object (struct value *val)
       val_obj->address = NULL;
       val_obj->type = NULL;
       val_obj->dynamic_type = NULL;
+      val_obj->content_bytes = nullptr;
       note_value (val_obj);
     }
 
@@ -2152,6 +2234,8 @@ static gdb_PyGetSetDef value_object_getset[] = {
     "Boolean telling whether the value is lazy (not fetched yet\n\
 from the inferior).  A lazy value is fetched when needed, or when\n\
 the \"fetch_lazy()\" method is called.", NULL },
+  { "bytes", valpy_get_bytes, valpy_set_bytes,
+    "Return a bytearray containing the bytes of this value.", nullptr },
   {NULL}  /* Sentinel */
 };
 
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index cdfcd414cd4..4b826f2d6ca 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -66,7 +66,8 @@ proc test_value_creation {} {
   gdb_test "python print ('result = %s' % i.address)" "= None" "test address attribute in non-addressable value"
 
   # Test creating / printing an optimized out value
-  gdb_test "python print(gdb.Value(gdb.Value(5).type.optimized_out()))"
+  gdb_test "python print(gdb.Value(gdb.Value(5).type.optimized_out()))" \
+      "<optimized out>"
 }
 
 # Check that we can call gdb.Value.__init__ to change a value.
@@ -542,13 +543,30 @@ proc prepare_type_and_buffer {} {
 proc test_value_from_buffer {} {
   global gdb_prompt
 
+  # A Python helper function.  Create a bytes object from inferior
+  # memory LEN bytes starting at ADDR, and compare this to the bytes
+  # obtained from VAL.bytes.  Assert that the two bytes object match.
+  gdb_test_multiline "Create another function to check Value.bytes" \
+     "python" "" \
+     "def compare_value_bytes_to_mem(val, addr, len):" "" \
+     "   mem = gdb.selected_inferior().read_memory(addr, len)" "" \
+     "   mem_bytes = mem.tobytes()" "" \
+     "   val_bytes = val.bytes" "" \
+     "   assert mem_bytes == val_bytes" "" \
+     "end" ""
+
   prepare_type_and_buffer
   gdb_test "python v=gdb.Value(b,tp); print(v)" "1" \
             "construct value from buffer"
+  gdb_test_no_output { python compare_value_bytes_to_mem(v, addr, size_a0) }
   gdb_test "python v=gdb.Value(b\[size_a0:\],tp); print(v)" "2" \
             "convert 2nd elem of buffer to value"
+  gdb_test_no_output \
+      { python compare_value_bytes_to_mem(v, (int(addr) + size_a0), size_a0) }
   gdb_test "python v=gdb.Value(b\[2*size_a0:\],tp); print(v)" "3" \
            "convert 3rd elem of buffer to value"
+  gdb_test_no_output \
+      { python compare_value_bytes_to_mem(v, (int(addr) + (2 * size_a0)), size_a0) }
   gdb_test "python v=gdb.Value(b\[2*size_a0+1:\],tp); print(v)" \
            "ValueError: Size of type is larger than that of buffer object\..*" \
 	   "attempt to convert smaller buffer than size of type"
@@ -556,6 +574,8 @@ proc test_value_from_buffer {} {
                          "make array type" 0
   gdb_py_test_silent_cmd "python va=gdb.Value(b,atp)" \
                          "construct array value from buffer" 0
+  gdb_test_no_output \
+      { python compare_value_bytes_to_mem(va, addr, size_a0 * 3) }
   gdb_test "python print(va)" "\\{1, 2, 3\\}" "print array value"
   gdb_test "python print(va\[0\])" "1" "print first array element"
   gdb_test "python print(va\[1\])" "2" "print second array element"
@@ -633,6 +653,92 @@ proc test_history_count {} {
     }
 }
 
+# Test the gdb.Value.bytes API.
+proc_with_prefix test_value_bytes { } {
+  # Test accessing the bytes of an optimised out value.
+  gdb_test "python print(gdb.Value(gdb.Value(5).type.optimized_out()).bytes)" \
+      [multi_line \
+	   "gdb\\.error: value has been optimized out" \
+	   "Error while executing Python code\\."]
+
+  # A Python helper function.  Fetch VAR_NAME from the inferior as a
+  # gdb.Value.  Read the bytes of the value based on its address, and
+  # the size of its type.  The compare these bytes to the value
+  # obtained from gdb.Value.bytes.  Assert that the two bytes objects
+  # match.
+  gdb_test_multiline "Create a function to check Value.bytes" \
+      "python" "" \
+      "def check_value_bytes(var_name):" "" \
+      "   val = gdb.parse_and_eval(var_name)" "" \
+      "   addr = val.address" "" \
+      "   len = val.type.sizeof" "" \
+      "   mem = gdb.selected_inferior().read_memory(addr, len)" "" \
+      "   mem_bytes = mem.tobytes()" "" \
+      "   val_bytes = val.bytes" "" \
+      "   assert mem_bytes == val_bytes" "" \
+      "end" ""
+
+  gdb_test_no_output { python check_value_bytes("a") }
+  gdb_test_no_output { python check_value_bytes("p") }
+  gdb_test_no_output { python check_value_bytes("i") }
+  gdb_test_no_output { python check_value_bytes("ptr_i") }
+  gdb_test_no_output { python check_value_bytes("embed") }
+  gdb_test_no_output { python check_value_bytes("fp1") }
+  gdb_test_no_output { python check_value_bytes("nullst") }
+  gdb_test_no_output { python check_value_bytes("st") }
+  gdb_test_no_output { python check_value_bytes("s") }
+  gdb_test_no_output { python check_value_bytes("u") }
+
+  # Check that gdb.Value.bytes changes after calling
+  # gdb.Value.assign().  The bytes value is cached within the Value
+  # object, so calling assign should clear the cache.
+  with_test_prefix "assign clears bytes cache" {
+      gdb_test_no_output "python v = gdb.parse_and_eval(\"i\")"
+      gdb_test_no_output "python bytes_before = v.bytes"
+      gdb_test_no_output "python v.assign(9)"
+      gdb_test_no_output "python bytes_after = v.bytes"
+      gdb_test_no_output "python assert(bytes_after != bytes_before)"
+  }
+
+  # Check that if we re-init a gdb.Value object the cached bytes for
+  # the Value are cleared.
+  with_test_prefix "re-init clears bytes cache" {
+      gdb_test_no_output "python w = gdb.Value(1)"
+      gdb_test_no_output "python bytes_before = w.bytes"
+      gdb_test_no_output "python w.__init__(3)"
+      gdb_test_no_output "python bytes_after = w.bytes"
+      gdb_test_no_output "python assert(bytes_after != bytes_before)"
+  }
+
+  # Check that we can assign to the Value.bytes field.
+  gdb_test_no_output "python i_value = gdb.parse_and_eval('i')" \
+      "evaluate i"
+  gdb_test_no_output "python i_bytes = i_value.bytes"
+  gdb_test_no_output "python i_bytes = bytes(\[b if b != 9 else 5 for b in i_bytes\])"
+  gdb_test_no_output "python i_value.bytes = i_bytes"
+  gdb_test "print i" " = 5"
+
+  # Check we get an exception if attempting to assign a buffer that is
+  # too big, or too small.
+  gdb_test_no_output "python bytes_as_int = \[x for x in i_bytes\]"
+  gdb_test_no_output "python bytes_as_int.append(0)"
+  gdb_test_no_output "python too_many_bytes = bytes(bytes_as_int)"
+  gdb_test "python i_value.bytes = too_many_bytes" \
+      "ValueError: Size of type is not equal to that of buffer object\\..*"
+  gdb_test_no_output "python bytes_as_int = bytes_as_int\[0:-2\]"
+  gdb_test_no_output "python too_few_bytes = bytes(bytes_as_int)"
+  gdb_test "python i_value.bytes = too_few_bytes" \
+      "ValueError: Size of type is not equal to that of buffer object\\..*"
+
+  # Check we get an exception writing to a not_lval.
+  gdb_test_no_output "python i_value = gdb.Value(9)" \
+      "reset i_value"
+  gdb_test_no_output "python i_bytes = i_value.bytes" \
+      "grab new value bytes"
+  gdb_test "python i_value.bytes = i_bytes" "not an lvalue.*" \
+      "cannot assign to not_lval value"
+}
+
 # Test Value.assign.
 proc test_assign {} {
     gdb_test_no_output "python i_value = gdb.parse_and_eval('i')" \
@@ -677,6 +783,7 @@ test_value_from_buffer
 test_value_sub_classes
 test_inferior_function_call
 test_assign
+test_value_bytes
 test_value_after_death
 
 # Test either C or C++ values. 

base-commit: 29736fc507c7a9c6e797b7f83e8df4be73d37767
-- 
2.25.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] gdb/python: Add new gdb.Value.bytes attribute
  2023-10-23 17:55 ` [PATCHv2] " Andrew Burgess
@ 2023-10-23 18:46   ` Eli Zaretskii
  2023-10-25 15:39   ` Tom Tromey
  1 sibling, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2023-10-23 18:46 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, tom

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>,
> 	Eli Zaretskii <eliz@gnu.org>,
> 	Tom Tromey <tom@tromey.com>
> Date: Mon, 23 Oct 2023 18:55:44 +0100
> 
> Thanks for the feedback on V1.
> 
> In V2:
> 
>   - Docs have been updated to be more explicit about the byte
>     ordering, but have also been extended to cover the new
>     functionality I've added in this update,
> 
>   - The cached value is now cleared correctly during Value.assign
>     calls as Tom pointed out,
> 
>   - The Value.bytes attribute can now be assigned too.  This is
>     similar to how Value.assign works.
> 
> ---
> 
> Add a gdb.Value.bytes attribute.  This attribute contains the bytes of
> the value (assuming the complete bytes of the value are available).
> 
> If the bytes of the gdb.Value are not available then accessing this
> attribute raises an exception.
> 
> The bytes object returned from gdb.Value.bytes is cached within GDB so
> that the same bytes object is returned each time.  The bytes object is
> created on-demand though to reduce unnecessary work.
> 
> For some values we can of course obtain the same information by
> reading inferior memory based on gdb.Value.address and
> gdb.Value.type.sizeof, however, not every value is in memory, so we
> don't always have an address.
> 
> The gdb.Value.bytes attribute will convert any value to a bytes
> object, so long as the contents are available.  The value can be one
> created purely in Python code, the value could be in a register,
> or (of course) the value could be in memory.
> 
> The Value.bytes attribute can also be assigned too.  Assigning to this
> attribute is similar to calling Value.assign, the value of the
> underlying value is updated within the inferior.  The value assigned
> to Value.bytes must be a buffer which contains exactly the correct
> number of bytes (i.e. unlike value creation, we don't allow oversized
> buffers).
> 
> To support this assignment like behaviour I've factored out the core
> of valpy_assign.  I've also updated convert_buffer_and_type_to_value
> so that it can (for my use case) check the exact buffer length.
> 
> The restrictions for when the Value.bytes can or cannot be written too
> are exactly the same as for Value.assign.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=13267
> ---
>  gdb/NEWS                              |   3 +
>  gdb/doc/python.texi                   |  16 ++++
>  gdb/python/py-value.c                 | 122 ++++++++++++++++++++++----
>  gdb/testsuite/gdb.python/py-value.exp | 109 ++++++++++++++++++++++-
>  4 files changed, 230 insertions(+), 20 deletions(-)

OK for the documentation parts, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2] gdb/python: Add new gdb.Value.bytes attribute
  2023-10-23 17:55 ` [PATCHv2] " Andrew Burgess
  2023-10-23 18:46   ` Eli Zaretskii
@ 2023-10-25 15:39   ` Tom Tromey
  1 sibling, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2023-10-25 15:39 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Eli Zaretskii, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Thanks for the feedback on V1.
Andrew> In V2:

Andrew>   - Docs have been updated to be more explicit about the byte
Andrew>     ordering, but have also been extended to cover the new
Andrew>     functionality I've added in this update,

Andrew>   - The cached value is now cleared correctly during Value.assign
Andrew>     calls as Tom pointed out,

Andrew>   - The Value.bytes attribute can now be assigned too.  This is
Andrew>     similar to how Value.assign works.

Looks good to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-25 15:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 14:57 [PATCH] gdb/python: Add new gdb.Value.bytes attribute Andrew Burgess
2023-10-18 15:40 ` Eli Zaretskii
2023-10-20 17:21 ` Tom Tromey
2023-10-23 17:55 ` [PATCHv2] " Andrew Burgess
2023-10-23 18:46   ` Eli Zaretskii
2023-10-25 15:39   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).