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

Message ID 20190219144158.7df6921b@f29-4.lan
State New
Headers show
Series
  • Define gdb.Value(val, type) constructor
Related show

Commit Message

Kevin Buettner Feb. 19, 2019, 9:41 p.m.
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(-)

Comments

Simon Marchi Feb. 20, 2019, 3:43 a.m. | #1
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
Tom Tromey Feb. 20, 2019, 6:03 p.m. | #2
>>>>> "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
Kevin Buettner Feb. 26, 2019, 5:34 p.m. | #3
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

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;