public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Define gdb.Value(val, type) constructor
@ 2019-02-18 14:58 Kevin Buettner
  2019-02-18 15:03 ` [PATCH 1/4] Define unique_ptr specialization for Py_buffer Kevin Buettner
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Kevin Buettner @ 2019-02-18 14:58 UTC (permalink / raw)
  To: gdb-patches

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().

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

* [PATCH 1/4] Define unique_ptr specialization for Py_buffer
  2019-02-18 14:58 [PATCH 0/4] Define gdb.Value(val, type) constructor Kevin Buettner
@ 2019-02-18 15:03 ` Kevin Buettner
  2019-02-18 15:06 ` [PATCH 2/4] Define gdb.Value(bufobj, type) constructor Kevin Buettner
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Kevin Buettner @ 2019-02-18 15:03 UTC (permalink / raw)
  To: gdb-patches

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 */

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

* [PATCH 2/4] Define gdb.Value(bufobj, type) constructor
  2019-02-18 14:58 [PATCH 0/4] Define gdb.Value(val, type) constructor Kevin Buettner
  2019-02-18 15:03 ` [PATCH 1/4] Define unique_ptr specialization for Py_buffer Kevin Buettner
@ 2019-02-18 15:06 ` Kevin Buettner
  2019-02-18 22:46   ` Tom Tromey
  2019-02-19  2:41   ` Simon Marchi
  2019-02-18 15:07 ` [PATCH 3/4] Add tests for " Kevin Buettner
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Kevin Buettner @ 2019-02-18 15:06 UTC (permalink / raw)
  To: gdb-patches

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..154b175a21 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 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 (PyTuple_Size (args) != 1)
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "O|O", keywords,
+					&val_obj, &type_obj))
+    return nullptr;
+
+  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_RuntimeError,
+			   _("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;

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

* [PATCH 3/4] Add tests for gdb.Value(bufobj, type) constructor
  2019-02-18 14:58 [PATCH 0/4] Define gdb.Value(val, type) constructor Kevin Buettner
  2019-02-18 15:03 ` [PATCH 1/4] Define unique_ptr specialization for Py_buffer Kevin Buettner
  2019-02-18 15:06 ` [PATCH 2/4] Define gdb.Value(bufobj, type) constructor Kevin Buettner
@ 2019-02-18 15:07 ` Kevin Buettner
  2019-02-19  2:46   ` Simon Marchi
  2019-02-18 15:09 ` [PATCH 4/4] Document two argument form of gdb.Value constructor Kevin Buettner
  2019-02-19  2:48 ` [PATCH 0/4] Define gdb.Value(val, type) constructor Simon Marchi
  4 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2019-02-18 15:07 UTC (permalink / raw)
  To: gdb-patches

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 | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 4f04aa9417..ba7de2166e 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -510,6 +510,47 @@ 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"
+}
+
 # Build C version of executable.  C++ is built later.
 if { [build_inferior "${binfile}" "c"] < 0 } {
     return -1
@@ -538,6 +579,7 @@ if ![runto_main] then {
 }
 
 test_value_in_inferior
+test_value_from_buffer
 test_inferior_function_call
 test_value_after_death
 

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

* [PATCH 4/4] Document two argument form of gdb.Value constructor
  2019-02-18 14:58 [PATCH 0/4] Define gdb.Value(val, type) constructor Kevin Buettner
                   ` (2 preceding siblings ...)
  2019-02-18 15:07 ` [PATCH 3/4] Add tests for " Kevin Buettner
@ 2019-02-18 15:09 ` Kevin Buettner
  2019-02-18 16:13   ` Eli Zaretskii
  2019-02-19  2:48 ` [PATCH 0/4] Define gdb.Value(val, type) constructor Simon Marchi
  4 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2019-02-18 15:09 UTC (permalink / raw)
  To: gdb-patches

gdb/doc/ChangeLog:
    
            * python.texi (Values From Inferior): Document second form
            of Value.__init__.
---
 gdb/doc/python.texi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 2860361c33..f6df32e76d 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -735,6 +735,13 @@ 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 gdb
+value of type @{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}.
+
 @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

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

* Re: [PATCH 4/4] Document two argument form of gdb.Value constructor
  2019-02-18 15:09 ` [PATCH 4/4] Document two argument form of gdb.Value constructor Kevin Buettner
@ 2019-02-18 16:13   ` Eli Zaretskii
  2019-02-18 17:25     ` Kevin Buettner
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2019-02-18 16:13 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> Date: Mon, 18 Feb 2019 08:09:07 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> 
> +@defun Value.__init__ (@var{val}, @r{[}, type @r{]})
> +This second form of the @code{gdb.Value} constructor returns a gdb
> +value of type @{type} where the value contents are taken from the

"gdb value" should be "@code{gdb.value}", right?

> +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}.

I think elsewhere we capitalize Python.

OK with those two fixed.

Thanks.

P.S. Does this warrant a NEWS entry?

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

* Re: [PATCH 4/4] Document two argument form of gdb.Value constructor
  2019-02-18 16:13   ` Eli Zaretskii
@ 2019-02-18 17:25     ` Kevin Buettner
  2019-02-18 17:45       ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2019-02-18 17:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: Eli Zaretskii

On Mon, 18 Feb 2019 18:13:31 +0200
Eli Zaretskii <eliz@gnu.org> wrote:
 
> P.S. Does this warrant a NEWS entry?

I didn't think so at first, but after reviewing NEWS, I see that similar
sorts of things are mentioned there.

Below is an updated patch incorporating both the NEWS entry and the
other changes that you asked for.

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 | 7 +++++++
 2 files changed, 10 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..8758b626c9 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -735,6 +735,13 @@ 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 @{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}.
+
 @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

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

* Re: [PATCH 4/4] Document two argument form of gdb.Value constructor
  2019-02-18 17:25     ` Kevin Buettner
@ 2019-02-18 17:45       ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2019-02-18 17:45 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> Date: Mon, 18 Feb 2019 10:25:07 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> On Mon, 18 Feb 2019 18:13:31 +0200
> Eli Zaretskii <eliz@gnu.org> wrote:
>  
> > P.S. Does this warrant a NEWS entry?
> 
> I didn't think so at first, but after reviewing NEWS, I see that similar
> sorts of things are mentioned there.
> 
> Below is an updated patch incorporating both the NEWS entry and the
> other changes that you asked for.
> 
> 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__.

Thanks, this is fine.

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

* Re: [PATCH 2/4] Define gdb.Value(bufobj, type) constructor
  2019-02-18 15:06 ` [PATCH 2/4] Define gdb.Value(bufobj, type) constructor Kevin Buettner
@ 2019-02-18 22:46   ` Tom Tromey
  2019-02-19  0:57     ` Kevin Buettner
  2019-02-19  2:41   ` Simon Marchi
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2019-02-18 22:46 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

FWIW I think this whole series looks quite reasonable and would be a
good addition.

Kevin> +  Py_buffer py_buf;

Elsewhere in the Python code, Py_buffer is only used conditionally on
"#ifdef IS_PY3K".  I don't know exactly why, though.  Py_buffer seems to
be in Python 2.7, maybe it wasn't in some earlier version that gdb still
supports?

I don't know whether anyone still relies on older versions of Python
(2.7 was released in 2010 so it seems plenty old at this point; after
all we require compilers released after this).

However, this seems like a decision to make consciously.
I'm not completely sure how to proceed, though one idea might be to find
the most recent Python 2.[456] compatibility patch and then asking the
author whether this support is still relevant.

thanks,
Tom

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

* Re: [PATCH 2/4] Define gdb.Value(bufobj, type) constructor
  2019-02-18 22:46   ` Tom Tromey
@ 2019-02-19  0:57     ` Kevin Buettner
  2019-02-19 15:19       ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Buettner @ 2019-02-19  0:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

On Mon, 18 Feb 2019 15:45:57 -0700
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> FWIW I think this whole series looks quite reasonable and would be a
> good addition.

Thanks for looking it over.

> Kevin> +  Py_buffer py_buf;  
> 
> Elsewhere in the Python code, Py_buffer is only used conditionally on
> "#ifdef IS_PY3K".  I don't know exactly why, though.  Py_buffer seems to
> be in Python 2.7, maybe it wasn't in some earlier version that gdb still
> supports?

FWIW, I've tested my patch using both Python 2.7 and Python 3.7, which
are what I have on my F29 machine.

> I don't know whether anyone still relies on older versions of Python
> (2.7 was released in 2010 so it seems plenty old at this point; after
> all we require compilers released after this).
> 
> However, this seems like a decision to make consciously.
> I'm not completely sure how to proceed, though one idea might be to find
> the most recent Python 2.[456] compatibility patch and then asking the
> author whether this support is still relevant.

I didn't know of that compatibility patch, though I'll try to track it
down.  (A pointer would be appreciated if you have one handy.)

I decided that I had better educate myself about the old buffer
protocol.  I came across this page, which describes Python's old
buffer protocol:

https://docs.python.org/2/c-api/objbuffer.html

The old buffer protocol includes the functions PyObject_CheckReadBuffer()
and PyObject_AsReadBuffer() which are used in the non-IS_PY3K code in
GDB.

According to that page, the old buffer protocol was introduced in
Python 1.6, but is deprecated in the 2.X series.  It also indicates
that support for the new buffer protocol, which is what I used in my
patch, has been backported to Python 2.6.  2.6 was released in Oct
of 2008.  (I'm thinking that indicating that the old buffer protocol is
deprecated in 2.X is a typo.  I think they meant 3.X.)

I'll try removing use of the old buffer protocol from GDB and then
see if anything breaks with Python 2.7 (and maybe even 2.6 if I can
figure out how to make that happen).  If we get rid of the IS_PY3K
ifdefs from GDB, we'll definitely be making this decision consciously.

Kevin

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

* Re: [PATCH 2/4] Define gdb.Value(bufobj, type) constructor
  2019-02-18 15:06 ` [PATCH 2/4] Define gdb.Value(bufobj, type) constructor Kevin Buettner
  2019-02-18 22:46   ` Tom Tromey
@ 2019-02-19  2:41   ` Simon Marchi
  1 sibling, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2019-02-19  2:41 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On 2019-02-18 10:06, Kevin Buettner wrote:
> +/* 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 released
> +         when it goes out of scope.  */

Seems to be missing a "be" in "Cause it to be released".

Simon

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

* Re: [PATCH 3/4] Add tests for gdb.Value(bufobj, type) constructor
  2019-02-18 15:07 ` [PATCH 3/4] Add tests for " Kevin Buettner
@ 2019-02-19  2:46   ` Simon Marchi
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2019-02-19  2:46 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On 2019-02-18 10:07, Kevin Buettner wrote:
> 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 | 42 
> +++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.python/py-value.exp
> b/gdb/testsuite/gdb.python/py-value.exp
> index 4f04aa9417..ba7de2166e 100644
> --- a/gdb/testsuite/gdb.python/py-value.exp
> +++ b/gdb/testsuite/gdb.python/py-value.exp
> @@ -510,6 +510,47 @@ 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"
> +}

Just a nit: I would suggest testing the code path when the second 
argument does not have the right type (is not a gdb.Type).

Simon

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

* Re: [PATCH 0/4] Define gdb.Value(val, type) constructor
  2019-02-18 14:58 [PATCH 0/4] Define gdb.Value(val, type) constructor Kevin Buettner
                   ` (3 preceding siblings ...)
  2019-02-18 15:09 ` [PATCH 4/4] Document two argument form of gdb.Value constructor Kevin Buettner
@ 2019-02-19  2:48 ` Simon Marchi
  4 siblings, 0 replies; 15+ messages in thread
From: Simon Marchi @ 2019-02-19  2:48 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On 2019-02-18 09:58, Kevin Buettner 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().

Apart from the two small nits I sent, this LGTM.

Simon

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

* Re: [PATCH 2/4] Define gdb.Value(bufobj, type) constructor
  2019-02-19  0:57     ` Kevin Buettner
@ 2019-02-19 15:19       ` Tom Tromey
  2019-02-19 15:28         ` Ulrich Weigand
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2019-02-19 15:19 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches, Tom Tromey, Ulrich Weigand

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

>> However, this seems like a decision to make consciously.
>> I'm not completely sure how to proceed, though one idea might be to find
>> the most recent Python 2.[456] compatibility patch and then asking the
>> author whether this support is still relevant.

Kevin> I didn't know of that compatibility patch, though I'll try to track it
Kevin> down.  (A pointer would be appreciated if you have one handy.)

I just recall it happening from time to time.  Using:

    git log --grep 'Python 2\.[4-6]'

... shows a couple maybe uninteresting patches, followed by:

commit 49840f2a6669ae2366c522da41edf615785b3626
Author: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Date:   Sun Mar 16 15:01:24 2014 +0100

    Fix Python 2.4 build break
    
    This fixes a build failure against Python 2.4 by casting away "const"
    on the second argument to PyObject_GetAttrString.  Similar casts to
    support Python 2.4 were already present in a number of other places.
    
    gdb/
    2014-03-16  Ulrich Weigand  <uweigand@de.ibm.com>
    
            * python/py-value.c (get_field_flag): Cast flag_name argument to
            PyObject_GetAttrString to support Python 2.4.

So, I've CC'd Ulrich.

Tom

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

* Re: [PATCH 2/4] Define gdb.Value(bufobj, type) constructor
  2019-02-19 15:19       ` Tom Tromey
@ 2019-02-19 15:28         ` Ulrich Weigand
  0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Weigand @ 2019-02-19 15:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Kevin Buettner, gdb-patches, Tom Tromey

Tom Tromey wrote:
> >>>>> "Kevin" =3D=3D Kevin Buettner <kevinb@redhat.com> writes:
> 
> >> However, this seems like a decision to make consciously.
> >> I'm not completely sure how to proceed, though one idea might be to find
> >> the most recent Python 2.[456] compatibility patch and then asking the
> >> author whether this support is still relevant.
> 
> Kevin> I didn't know of that compatibility patch, though I'll try to track =
> it
> Kevin> down.  (A pointer would be appreciated if you have one handy.)
> 
> I just recall it happening from time to time.  Using:
> 
>     git log --grep 'Python 2\.[4-6]'

So I've been running an SPU build bot on RHEL 5, which still has Python 2.4
as system version.  Those patches were from when I noticed build breaks
because of that.

However, a while back I switched my build bot to use a private Python 2.7
instead of the system Python 2.4, so I guess at this point I'm not even
noticing any 2.4 related issues.  As far as I'm concerned, I'd be fine
with removing compatibility with older Python versions ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

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

end of thread, other threads:[~2019-02-19 15:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 14:58 [PATCH 0/4] Define gdb.Value(val, type) constructor Kevin Buettner
2019-02-18 15:03 ` [PATCH 1/4] Define unique_ptr specialization for Py_buffer Kevin Buettner
2019-02-18 15:06 ` [PATCH 2/4] Define gdb.Value(bufobj, type) constructor Kevin Buettner
2019-02-18 22:46   ` Tom Tromey
2019-02-19  0:57     ` Kevin Buettner
2019-02-19 15:19       ` Tom Tromey
2019-02-19 15:28         ` Ulrich Weigand
2019-02-19  2:41   ` Simon Marchi
2019-02-18 15:07 ` [PATCH 3/4] Add tests for " Kevin Buettner
2019-02-19  2:46   ` Simon Marchi
2019-02-18 15:09 ` [PATCH 4/4] Document two argument form of gdb.Value constructor Kevin Buettner
2019-02-18 16:13   ` Eli Zaretskii
2019-02-18 17:25     ` Kevin Buettner
2019-02-18 17:45       ` Eli Zaretskii
2019-02-19  2:48 ` [PATCH 0/4] Define gdb.Value(val, type) constructor Simon Marchi

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).