[PATCH v2 0/4] Define gdb.Value(val, type) constructor

[PATCH v2 0/4] Define gdb.Value(val, type) constructor

[Kevin Buettner]

This four part patch series defines a two argument constructor for
gdb.Value.

gdb.Value currently has a one argument constructor.  It takes a python
value, figures out some potentially suitable gdb type and then
constructs a gdb value of that type.

The two argument version that I'm introducing is useful for
constructing a gdb value of a specified type from a buffer of bytes. 
It takes the form gdb.Value (val, type).  VAL is a python buffer object,
i.e. an object from which bytes may be read using python's buffer
protocol.  TYPE is a gdb type perhaps obtained by calling
gdb.lookup_type().

Changes between the original series and this v2 series are as follows:

Patch #1: No changes.

Patch #2:

  Fix wording of comment noted by Simon.
  
  Change PyExc_RuntimeError to PyExc_TypeError for "type argument must
  be a gdb.Type" error.  I found this while adding the additional test
  that Simon suggested.

Patch #3:

  Add test which invokes gdb.Value where second argument is not
  a gdb.Type.  (Suggested by Simon.)

Patch #4:

  Add NEWS entry.  (Requested by Eli.)

  Made other python.texi changes requested by Eli.  

  Added missing "@end defun".

[PATCH v2 1/4] Define unique_ptr specialization for Py_buffer

[Kevin Buettner]

This patch causes PyBuffer_Release() to be called when the associated
buffer goes out of scope.  I've been using it as follows:

 ...
 Py_buffer_up buffer_up;
 Py_buffer py_buf;

 if (PyObject_CheckBuffer (obj)
     && PyObject_GetBuffer (obj, &py_buf, PyBUF_SIMPLE) == 0)
   {
      /* Got a buffer, py_buf, out of obj.  Cause it to released
	 when it goes out of scope.  */
     buffer_up.reset (&py_buf);
   }
   ...

This snippet of code was taken directly from an upcoming patch to
python-value.c.

gdb/ChangeLog:
    
            * python/python-internal.h (Py_buffer_deleter): New struct.
            (Py_buffer_up): New typedef.
---
 gdb/python/python-internal.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 3cb9ebc1ee..d11af83c8e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -801,4 +801,17 @@ struct varobj;
 struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
 					    PyObject *printer);
 
+/* Deleter for Py_buffer unique_ptr specialization.  */
+
+struct Py_buffer_deleter
+{
+  void operator() (Py_buffer *b) const
+  {
+    PyBuffer_Release (b);
+  }
+};
+
+/* A unique_ptr specialization for Py_buffer.  */
+typedef std::unique_ptr<Py_buffer, Py_buffer_deleter> Py_buffer_up;
+
 #endif /* PYTHON_PYTHON_INTERNAL_H */

[PATCH v2 2/4] Define gdb.Value(bufobj, type) constructor

[Kevin Buettner]

Provided a buffer BUFOBJ and a type TYPE, construct a gdb.Value object
with type TYPE, where the value's contents are taken from BUFOBJ.

E.g...

(gdb) python import struct
(gdb) python unsigned_int_type=gdb.lookup_type('unsigned int')
(gdb) python b=struct.pack('=I',0xdeadbeef)
(gdb) python v=gdb.Value(b, unsigned_int_type) ; print("%#x" % v)
0xdeadbeef

This two argument form of the gdb.Value constructor may also be used
to obtain gdb values from selected portions of buffers read with
Inferior.read_memory().  The test case (which is in a separate patch)
demonstrates this use case.

gdb/ChangeLog:
    
	* python/py-value.c (convert_buffer_and_type_to_value): New
	function.
	(valpy_new): Parse arguments via gdb_PyArg_ParseTupleAndKeywords.
	Add support for handling an optional second argument.  Call
	convert_buffer_and_type_to_value as appropriate.
---
 gdb/python/py-value.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 62 insertions(+), 10 deletions(-)

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 20ef5822f8..af5a75a4a4 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -107,22 +107,68 @@ note_value (value_object *value_obj)
   values_in_python = 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.  */
+
+static struct value *
+convert_buffer_and_type_to_value (PyObject *obj, struct type *type)
+{
+  Py_buffer_up buffer_up;
+  Py_buffer py_buf;
+
+  if (PyObject_CheckBuffer (obj) 
+      && PyObject_GetBuffer (obj, &py_buf, PyBUF_SIMPLE) == 0)
+    {
+      /* Got a buffer, py_buf, out of obj.  Cause it to be released
+         when it goes out of scope.  */
+      buffer_up.reset (&py_buf);
+    }
+  else
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Object must support the python buffer protocol."));
+      return nullptr;
+    }
+
+  if (TYPE_LENGTH (type) > py_buf.len)
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Size of type is larger than that of buffer object."));
+      return nullptr;
+    }
+
+  return value_from_contents (type, (const gdb_byte *) py_buf.buf);
+}
+
 /* Called when a new gdb.Value object needs to be allocated.  Returns NULL on
    error, with a python exception set.  */
 static PyObject *
-valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
+valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
 {
-  struct value *value = NULL;   /* Initialize to appease gcc warning.  */
-  value_object *value_obj;
+  static const char *keywords[] = { "val", "type", NULL };
+  PyObject *val_obj = nullptr;
+  PyObject *type_obj = nullptr;
+
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "O|O", keywords,
+					&val_obj, &type_obj))
+    return nullptr;
 
-  if (PyTuple_Size (args) != 1)
+  struct type *type = nullptr;
+
+  if (type_obj != nullptr)
     {
-      PyErr_SetString (PyExc_TypeError, _("Value object creation takes only "
-					  "1 argument"));
-      return NULL;
+      type = type_object_to_type (type_obj);
+      if (type == nullptr)
+        {
+	  PyErr_SetString (PyExc_TypeError,
+			   _("type argument must be a gdb.Type."));
+	  return nullptr;
+	}
     }
 
-  value_obj = (value_object *) subtype->tp_alloc (subtype, 1);
+  value_object *value_obj = (value_object *) subtype->tp_alloc (subtype, 1);
   if (value_obj == NULL)
     {
       PyErr_SetString (PyExc_MemoryError, _("Could not allocate memory to "
@@ -130,8 +176,14 @@ valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
       return NULL;
     }
 
-  value = convert_value_from_python (PyTuple_GetItem (args, 0));
-  if (value == NULL)
+  struct value *value;
+
+  if (type == nullptr)
+    value = convert_value_from_python (val_obj);
+  else
+    value = convert_buffer_and_type_to_value (val_obj, type);
+
+  if (value == nullptr)
     {
       subtype->tp_free (value_obj);
       return NULL;

Re: [PATCH v2 2/4] Define gdb.Value(bufobj, type) constructor

[Simon Marchi]

On 2019-02-19 16:41, Kevin Buettner wrote:
> Provided a buffer BUFOBJ and a type TYPE, construct a gdb.Value object
> with type TYPE, where the value's contents are taken from BUFOBJ.
> 
> E.g...
> 
> (gdb) python import struct
> (gdb) python unsigned_int_type=gdb.lookup_type('unsigned int')
> (gdb) python b=struct.pack('=I',0xdeadbeef)
> (gdb) python v=gdb.Value(b, unsigned_int_type) ; print("%#x" % v)
> 0xdeadbeef
> 
> This two argument form of the gdb.Value constructor may also be used
> to obtain gdb values from selected portions of buffers read with
> Inferior.read_memory().  The test case (which is in a separate patch)
> demonstrates this use case.
> 
> gdb/ChangeLog:
> 
> 	* python/py-value.c (convert_buffer_and_type_to_value): New
> 	function.
> 	(valpy_new): Parse arguments via gdb_PyArg_ParseTupleAndKeywords.
> 	Add support for handling an optional second argument.  Call
> 	convert_buffer_and_type_to_value as appropriate.
> ---
>  gdb/python/py-value.c | 72 
> ++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 62 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index 20ef5822f8..af5a75a4a4 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -107,22 +107,68 @@ note_value (value_object *value_obj)
>    values_in_python = 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.  */
> +
> +static struct value *
> +convert_buffer_and_type_to_value (PyObject *obj, struct type *type)
> +{
> +  Py_buffer_up buffer_up;
> +  Py_buffer py_buf;
> +
> +  if (PyObject_CheckBuffer (obj)
> +      && PyObject_GetBuffer (obj, &py_buf, PyBUF_SIMPLE) == 0)
> +    {
> +      /* Got a buffer, py_buf, out of obj.  Cause it to be released
> +         when it goes out of scope.  */
> +      buffer_up.reset (&py_buf);
> +    }
> +  else
> +    {
> +      PyErr_SetString (PyExc_TypeError,
> +		       _("Object must support the python buffer protocol."));
> +      return nullptr;
> +    }
> +
> +  if (TYPE_LENGTH (type) > py_buf.len)
> +    {
> +      PyErr_SetString (PyExc_TypeError,
> +		       _("Size of type is larger than that of buffer object."));
> +      return nullptr;
> +    }

Another small thing I didn't spot when reading v1: I think it would be 
more appropriate to raise a ValueError in this last case.  TypeError 
would be if the arguments were of the wrong Python type, which is not 
the case here.  It just happens that the value we handle is a (GDB) 
type, but that's not the same kind of type.  If you do that change, 
don't forget to update the tests as well.

Otherwise, the whole series still LGTM.

Simon

Re: [PATCH v2 2/4] Define gdb.Value(bufobj, type) constructor

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Otherwise, the whole series still LGTM.

+1 from me too.  Thank you for doing this.

Tom

Re: [PATCH v2 2/4] Define gdb.Value(bufobj, type) constructor

[Kevin Buettner]

On Tue, 19 Feb 2019 22:43:09 -0500
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> > +  if (TYPE_LENGTH (type) > py_buf.len)
> > +    {
> > +      PyErr_SetString (PyExc_TypeError,
> > +		       _("Size of type is larger than that of buffer object."));
> > +      return nullptr;
> > +    }  
> 
> Another small thing I didn't spot when reading v1: I think it would be 
> more appropriate to raise a ValueError in this last case.  TypeError 
> would be if the arguments were of the wrong Python type, which is not 
> the case here.  It just happens that the value we handle is a (GDB) 
> type, but that's not the same kind of type.  If you do that change, 
> don't forget to update the tests as well.

I've made that change and have updated the test also.

Kevin

[PATCH v2 3/4] Add tests for gdb.Value(bufobj, type) constructor

[Kevin Buettner]

gdb/testsuite/ChangeLog:

	* gdb.python/py-value.exp (test_value_from_buffer): New proc with
	call from main program.
---
 gdb/testsuite/gdb.python/py-value.exp | 45 +++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 4f04aa9417..a41631a842 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -510,6 +510,50 @@ proc test_float_conversion {} {
     gdb_test "python print(float(gdb.Value(0)))" "0\\.0"
 }
 
+proc test_value_from_buffer {} {
+  global gdb_prompt
+  global gdb_py_is_py3k
+
+  gdb_py_test_silent_cmd "python tp=gdb.lookup_type('int')" "look up int type" 0
+  gdb_py_test_silent_cmd "python size_a=gdb.parse_and_eval('sizeof(a)')" \
+                         "find size of a" 0
+  gdb_py_test_silent_cmd "python size_a0=gdb.parse_and_eval('sizeof(a\[0\])')" \
+                         "find size of element of a" 0
+  gdb_py_test_silent_cmd "python addr=gdb.parse_and_eval('&a')" \
+                         "find address of a" 0
+  gdb_py_test_silent_cmd "python b=gdb.selected_inferior().read_memory(addr,size_a)" \
+                         "read buffer from memory" 1
+  gdb_test "python v=gdb.Value(b,tp); print(v)" "1" \
+            "construct value from buffer"
+  gdb_test "python v=gdb.Value(b\[size_a0:\],tp); print(v)" "2" \
+            "convert 2nd elem of buffer to value"
+  gdb_test "python v=gdb.Value(b\[2*size_a0:\],tp); print(v)" "3" \
+           "convert 3rd elem of buffer to value"
+  gdb_test "python v=gdb.Value(b\[2*size_a0+1:\],tp); print(v)" \
+           "TypeError: Size of type is larger than that of buffer object\..*" \
+	   "attempt to convert smaller buffer than size of type"
+  gdb_py_test_silent_cmd "python atp=tp.array(2) ; print(atp)" \
+                         "make array type" 0
+  gdb_py_test_silent_cmd "python va=gdb.Value(b,atp)" \
+                         "construct array value from buffer" 0
+  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"
+  gdb_test "python print(va\[2\])" "3" "print third array element"
+  gdb_test "python print(va\[3\])" "gdb\.error: no such vector element.*" \
+           "print out of bounds array element"
+  gdb_py_test_silent_cmd "python atpbig=tp.array(3)" "make bigger array type" 0
+  gdb_test "python vabig=gdb.Value(b,atpbig)" \
+           "TypeError: Size of type is larger than that of buffer object\..*" \
+	   "attempt to construct large value with small buffer" 
+  gdb_test "python v=gdb.Value(2048,tp)" \
+           "TypeError: Object must support the python buffer protocol\..*" \
+           "attempt to construct value from buffer with non-buffer object"
+  gdb_test "python v=gdb.Value(b,'int'); print(v)" \
+           "TypeError: type argument must be a gdb\.Type\..*" \
+	   "attempt to construct value with string as type"
+}
+
 # Build C version of executable.  C++ is built later.
 if { [build_inferior "${binfile}" "c"] < 0 } {
     return -1
@@ -538,6 +582,7 @@ if ![runto_main] then {
 }
 
 test_value_in_inferior
+test_value_from_buffer
 test_inferior_function_call
 test_value_after_death
 

[PATCH v2 4/4] Document two argument form of gdb.Value constructor

[Kevin Buettner]

gdb/ChangeLog:

	* NEWS: Mention two argument form of gdb.Value constructor.

gdb/doc/ChangeLog:

	* python.texi (Values From Inferior): Document second form
	of Value.__init__.
---
 gdb/NEWS            | 3 +++
 gdb/doc/python.texi | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index eaef6aa384..9ad7ff4885 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -202,6 +202,9 @@ FreeBSD/riscv			riscv*-*-freebsd*
      gdb.SYMBOL_TYPES_DOMAIN are now deprecated.  These were never
      correct and did not work properly.
 
+  ** The gdb.Value type has a new constructor, which is used to construct a
+     gdb.Value from a Python buffer object and a gdb.Type.
+
 * Configure changes
 
 --enable-ubsan
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 2860361c33..7f6f52c4df 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -735,6 +735,14 @@ its result is used.
 @end table
 @end defun
 
+@defun Value.__init__ (@var{val}, @r{[}, type @r{]})
+This second form of the @code{gdb.Value} constructor returns a
+@code{gdb.Value} of type @var{type} where the value contents are taken
+from the Python buffer object specified by @var{val}.  The number of
+bytes in the Python buffer object must be greater than or equal to the
+size of @var{type}.
+@end defun
+
 @defun Value.cast (type)
 Return a new instance of @code{gdb.Value} that is the result of
 casting this instance to the type described by @var{type}, which must

Re: [PATCH v2 4/4] Document two argument form of gdb.Value constructor

[Eli Zaretskii]

> Date: Tue, 19 Feb 2019 14:45:53 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Mention two argument form of gdb.Value constructor.
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Values From Inferior): Document second form
> 	of Value.__init__.

OK for this part.  Thanks.

Re: [PATCH v2 0/4] Define gdb.Value(val, type) constructor

[Kevin Buettner]

I've pushed this series.

Kevin

On Tue, 19 Feb 2019 14:33:56 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> This four part patch series defines a two argument constructor for
> gdb.Value.
> 
> gdb.Value currently has a one argument constructor.  It takes a python
> value, figures out some potentially suitable gdb type and then
> constructs a gdb value of that type.
> 
> The two argument version that I'm introducing is useful for
> constructing a gdb value of a specified type from a buffer of bytes. 
> It takes the form gdb.Value (val, type).  VAL is a python buffer object,
> i.e. an object from which bytes may be read using python's buffer
> protocol.  TYPE is a gdb type perhaps obtained by calling
> gdb.lookup_type().
> 
> Changes between the original series and this v2 series are as follows:
> 
> Patch #1: No changes.
> 
> Patch #2:
> 
>   Fix wording of comment noted by Simon.
>   
>   Change PyExc_RuntimeError to PyExc_TypeError for "type argument must
>   be a gdb.Type" error.  I found this while adding the additional test
>   that Simon suggested.
> 
> Patch #3:
> 
>   Add test which invokes gdb.Value where second argument is not
>   a gdb.Type.  (Suggested by Simon.)
> 
> Patch #4:
> 
>   Add NEWS entry.  (Requested by Eli.)
> 
>   Made other python.texi changes requested by Eli.  
> 
>   Added missing "@end defun".