Use Python 2.[67] / 3.X / PEP 3118 buffer protocol
Commit Message
This patch removes the non-IS_PY3K code in infpy_write_memory()
and infpy_search_memory(). In both cases, the remaining code
from these ifdefs is related to use of the PEP 3118 buffer protocol.
(Deleted code is either due to simplification or related to use of the
old buffer protocol.) PEP 3118 is sometimes referred to as the "new"
buffer protocol, though it's not that new anymore.
The link below describes new features in Python 2.6. In particular,
it says that the buffer protocol described by PEP 3118 is in Python
2.6. It also says (at the top of the page) that Python 2.6 was
released on Oct 1, 2008.
https://docs.python.org/3/whatsnew/2.6.html#pep-3118-revised-buffer-protocol
The last security release for the Python 2.6 series was 2.6.9. It was
released on Oct 29, 2013. According to this document...
https://www.python.org/download/releases/2.6.9/
...support for the 2.6 series has ended:
With the 2.6.9 release, and five years after its first release,
the Python 2.6 series is now officially retired. All official
maintenance for Python 2.6, including security patches, has ended.
For ongoing maintenance releases, please see the Python 2.7
series.
As noted earlier, Python 2.6, Python 2.7, and Python 3.X all have
support for the PEP 3118 buffer protocol. Python releases prior
to 2.6 use an older buffer protocol. Since Python 2.6 has been
retired for a good while now, it seems reasonable to me to remove
code using the older buffer protocol from GDB.
I have also simplified some of the code via use of the Py_buffer
unique_ptr specialization which I introduced in the two argument
gdb.Value constructor patch series. Therefore, there is a dependency
on patch #1 from that series.
I have tested against both Python 2.7.15 and 3.7.2. I see no
regressions among the non-racy tests. I've also verified that
PyBuffer_Release is being called when the affected functions exit
while running the tests in gdb.python/py-inferior.exp by hand. I've
also tried running valgrind on GDB while running this test, but I'm
puzzled by the results that I'm seeing - I'm seeing no additional
leaks when I comment out the Py_buffer_up lines that I introduced.
That said, I'm not seeing any leaks that obviously originate from
either infpy_write_memory() or infpy_search_memory().
gdb/ChangeLog:
* python/py-inferior.c (infpy_write_memory): Remove non-IS_PY3K
code from these functions. Remove corresponding ifdefs. Use
Py_buffer_up instead of explicit calls to PyBuffer_Release.
Remove gotos and target of gotos.
(infpy_search_memory): Likewise.
---
gdb/python/py-inferior.c | 64 ++++++------------------------------------------
1 file changed, 8 insertions(+), 56 deletions(-)
Comments
>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
Kevin> This patch removes the non-IS_PY3K code in infpy_write_memory()
Kevin> and infpy_search_memory(). In both cases, the remaining code
Kevin> from these ifdefs is related to use of the PEP 3118 buffer protocol.
Kevin> (Deleted code is either due to simplification or related to use of the
Kevin> old buffer protocol.) PEP 3118 is sometimes referred to as the "new"
Kevin> buffer protocol, though it's not that new anymore.
Thanks for doing this.
Kevin> The link below describes new features in Python 2.6. In particular,
Kevin> it says that the buffer protocol described by PEP 3118 is in Python
Kevin> 2.6. It also says (at the top of the page) that Python 2.6 was
Kevin> released on Oct 1, 2008.
I think probably the NEWS file should be updated to mention the minimum
version bump. gdb.texinfo also currently says that Python 2.4 is the
minimum, so that should also be updated.
Kevin> I have tested against both Python 2.7.15 and 3.7.2. I see no
Kevin> regressions among the non-racy tests. I've also verified that
Kevin> PyBuffer_Release is being called when the affected functions exit
Kevin> while running the tests in gdb.python/py-inferior.exp by hand. I've
Kevin> also tried running valgrind on GDB while running this test, but I'm
Kevin> puzzled by the results that I'm seeing - I'm seeing no additional
Kevin> leaks when I comment out the Py_buffer_up lines that I introduced.
Maybe it's possible that PyBuffer_Release doesn't need to do anything in
particular in some scenarios? Like if the buffer it refers to is shared
with some other object? You'd have to dive into the Python
implementation to find out. Meanwhile if there is no leak introduced by
this code, then it seems fine to me.
The patch itself looks good to me.
Tom
On 02/19/2019 09:51 PM, Kevin Buettner wrote:
> As noted earlier, Python 2.6, Python 2.7, and Python 3.X all have
> support for the PEP 3118 buffer protocol. Python releases prior
> to 2.6 use an older buffer protocol. Since Python 2.6 has been
> retired for a good while now, it seems reasonable to me to remove
> code using the older buffer protocol from GDB.
>
> I have also simplified some of the code via use of the Py_buffer
> unique_ptr specialization which I introduced in the two argument
> gdb.Value constructor patch series. Therefore, there is a dependency
> on patch #1 from that series.
Not objecting, but I think you should send an email to gdb@ with
a clear subject indicating that you're proposing to drop support
for Python < 2.6.
Thanks,
Pedro Alves
On Wed, 20 Feb 2019 19:05:26 +0000
Pedro Alves <palves@redhat.com> wrote:
> On 02/19/2019 09:51 PM, Kevin Buettner wrote:
> > As noted earlier, Python 2.6, Python 2.7, and Python 3.X all have
> > support for the PEP 3118 buffer protocol. Python releases prior
> > to 2.6 use an older buffer protocol. Since Python 2.6 has been
> > retired for a good while now, it seems reasonable to me to remove
> > code using the older buffer protocol from GDB.
>
> Not objecting, but I think you should send an email to gdb@ with
> a clear subject indicating that you're proposing to drop support
> for Python < 2.6.
Okay, I've done this. See:
https://sourceware.org/ml/gdb/2019-02/msg00020.html
Kevin
On Wed, 20 Feb 2019 11:13:35 -0700
Tom Tromey <tom@tromey.com> wrote:
> Kevin> The link below describes new features in Python 2.6. In particular,
> Kevin> it says that the buffer protocol described by PEP 3118 is in Python
> Kevin> 2.6. It also says (at the top of the page) that Python 2.6 was
> Kevin> released on Oct 1, 2008.
>
> I think probably the NEWS file should be updated to mention the minimum
> version bump. gdb.texinfo also currently says that Python 2.4 is the
> minimum, so that should also be updated.
I've just posted a separate patch which does this.
Kevin
On Tue, 19 Feb 2019 14:51:10 -0700
Kevin Buettner <kevinb@redhat.com> wrote:
> gdb/ChangeLog:
>
> * python/py-inferior.c (infpy_write_memory): Remove non-IS_PY3K
> code from these functions. Remove corresponding ifdefs. Use
> Py_buffer_up instead of explicit calls to PyBuffer_Release.
> Remove gotos and target of gotos.
> (infpy_search_memory): Likewise.
Pushed.
Kevin
@@ -596,31 +596,23 @@ infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
CORE_ADDR addr, length;
PyObject *addr_obj, *length_obj = NULL;
static const char *keywords[] = { "address", "buffer", "length", NULL };
-#ifdef IS_PY3K
Py_buffer pybuf;
if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "Os*|O", keywords,
&addr_obj, &pybuf, &length_obj))
return NULL;
+ Py_buffer_up buffer_up (&pybuf);
buffer = (const gdb_byte *) pybuf.buf;
buf_len = pybuf.len;
-#else
- if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "Os#|O", keywords,
- &addr_obj, &buffer, &buf_len,
- &length_obj))
- return NULL;
-
- buffer = (const gdb_byte *) buffer;
-#endif
if (get_addr_from_python (addr_obj, &addr) < 0)
- goto fail;
+ return nullptr;
if (!length_obj)
length = buf_len;
else if (get_addr_from_python (length_obj, &length) < 0)
- goto fail;
+ return nullptr;
TRY
{
@@ -632,18 +624,9 @@ infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
}
END_CATCH
-#ifdef IS_PY3K
- PyBuffer_Release (&pybuf);
-#endif
GDB_PY_HANDLE_EXCEPTION (except);
Py_RETURN_NONE;
-
- fail:
-#ifdef IS_PY3K
- PyBuffer_Release (&pybuf);
-#endif
- return NULL;
}
/* Destructor of Membuf objects. */
@@ -752,7 +735,6 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
const gdb_byte *buffer;
CORE_ADDR found_addr;
int found = 0;
-#ifdef IS_PY3K
Py_buffer pybuf;
if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "OOs*", keywords,
@@ -760,42 +742,21 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
&pybuf))
return NULL;
+ Py_buffer_up buffer_up (&pybuf);
buffer = (const gdb_byte *) pybuf.buf;
pattern_size = pybuf.len;
-#else
- PyObject *pattern;
- const void *vbuffer;
-
- if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "OOO", keywords,
- &start_addr_obj, &length_obj,
- &pattern))
- return NULL;
-
- if (!PyObject_CheckReadBuffer (pattern))
- {
- PyErr_SetString (PyExc_RuntimeError,
- _("The pattern is not a Python buffer."));
-
- return NULL;
- }
-
- if (PyObject_AsReadBuffer (pattern, &vbuffer, &pattern_size) == -1)
- return NULL;
-
- buffer = (const gdb_byte *) vbuffer;
-#endif
if (get_addr_from_python (start_addr_obj, &start_addr) < 0)
- goto fail;
+ return nullptr;
if (get_addr_from_python (length_obj, &length) < 0)
- goto fail;
+ return nullptr;
if (!length)
{
PyErr_SetString (PyExc_ValueError,
_("Search range is empty."));
- goto fail;
+ return nullptr;
}
/* Watch for overflows. */
else if (length > CORE_ADDR_MAX
@@ -803,7 +764,7 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
{
PyErr_SetString (PyExc_ValueError,
_("The search range is too large."));
- goto fail;
+ return nullptr;
}
TRY
@@ -818,21 +779,12 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
}
END_CATCH
-#ifdef IS_PY3K
- PyBuffer_Release (&pybuf);
-#endif
GDB_PY_HANDLE_EXCEPTION (except);
if (found)
return PyLong_FromLong (found_addr);
else
Py_RETURN_NONE;
-
- fail:
-#ifdef IS_PY3K
- PyBuffer_Release (&pybuf);
-#endif
- return NULL;
}
/* Implementation of gdb.Inferior.is_valid (self) -> Boolean.