[lldb] Drop support for the Buffer Protocol

[JDevlieghere]

This is an alternative solution to the issue described in #167990, which can be summarized as that we cannot target Python 3.8 with the stable API and support building for Python 3.13 and later due to the buffer protocol.

The approach taken in this PR, and proposed by Ismail, is to sidesteps the issue by dropping support for the buffer protocol. The only two users are SBFile::Read and SBFile::Write. Instead, we support PyBytes and PyByteArray which are the builtin types that conform to the buffer protocol. Technically, this means a small regression, where those methods could previously take custom types that conform to Python's buffer protocol. Like Ismail, I think this is acceptable given the alternatives.

Co-authored-by: Med Ismail Bennani ismail@bennani.ma

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

This is an alternative solution to the issue described in #167990, which can be summarized as that we cannot target Python 3.8 with the stable API and support building for Python 3.13 and later due to the buffer protocol.

The approach taken in this PR, and proposed by Ismail, is to sidesteps the issue by dropping support for the buffer protocol. The only two users are SBFile::Read and SBFile::Write. Instead, we support PyBytes and PyByteArray which are the builtin types that conform to the buffer protocol. Technically, this means a small regression, where those methods could previously take custom types that conform to Python's buffer protocol. Like Ismail, I think this is acceptable given the alternatives.

---

Full diff: https://github.com/llvm/llvm-project/pull/168144.diff

3 Files Affected:

• (modified) lldb/bindings/python/python-typemaps.swig (+28-55)
• (modified) lldb/bindings/python/python-wrapper.swig (+5-2)
• (modified) lldb/bindings/python/python.swig (+5)

diff --git a/lldb/bindings/python/python-typemaps.swig b/lldb/bindings/python/python-typemaps.swig index 3baeaa7770e6c..59bba480e7395 100644 --- a/lldb/bindings/python/python-typemaps.swig +++ b/lldb/bindings/python/python-typemaps.swig @@ -220,9 +220,9 @@ AND call SWIG_fail at the same time, because it will result in a double free. } // Disable default type checking for this method to avoid SWIG dispatch issues. -//  +// // Problem: SBThread::GetStopDescription has two overloads: -// 1. GetStopDescription(char* dst_or_null, size_t dst_len)  +// 1. GetStopDescription(char* dst_or_null, size_t dst_len) // 2. GetStopDescription(lldb::SBStream& stream) // // SWIG generates a dispatch function to select the correct overload based on argument types. @@ -230,9 +230,9 @@ AND call SWIG_fail at the same time, because it will result in a double free. // However, this dispatcher doesn't consider typemaps that transform function signatures. // // In Python, our typemap converts GetStopDescription(char*, size_t) to GetStopDescription(int). -// The dispatcher still checks against the original (char*, size_t) signature instead of  +// The dispatcher still checks against the original (char*, size_t) signature instead of // the transformed (int) signature, causing type matching to fail. -// This only affects SBThread::GetStopDescription since the type check also matches  +// This only affects SBThread::GetStopDescription since the type check also matches // the argument name, which is unique to this function. %typemap(typecheck, precedence=SWIG_TYPECHECK_POINTER) (char *dst_or_null, size_t dst_len) "" @@ -628,61 +628,34 @@ template <> bool SetNumberFromPyObject<double>(double &number, PyObject *obj) { } } -// These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i, -// and fixed so they will not crash if PyObject_GetBuffer fails. -// https://github.com/swig/swig/issues/1640 -// -// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper, -// doing it right away is not legal according to the python buffer protocol. -%inline %{ -struct Py_buffer_RAII { - Py_buffer buffer = {}; - Py_buffer_RAII(){}; - Py_buffer &operator=(const Py_buffer_RAII &) = delete; - Py_buffer_RAII(const Py_buffer_RAII &) = delete; - ~Py_buffer_RAII() { - if (buffer.obj) - PyBuffer_Release(&buffer); - } -}; -%} -%define %pybuffer_mutable_binary(TYPEMAP, SIZE) -%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) { - int res; - Py_ssize_t size = 0; - void *buf = 0; - res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE); - if (res < 0) { - PyErr_Clear(); - %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum); - } - size = view.buffer.len; - buf = view.buffer.buf; - $1 = ($1_ltype)buf; - $2 = ($2_ltype)(size / sizeof($*1_type)); -} -%enddef - -%define %pybuffer_binary(TYPEMAP, SIZE) -%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) { - int res; - Py_ssize_t size = 0; - const void *buf = 0; - res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO); - if (res < 0) { - PyErr_Clear(); - %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum); +// Typemap for SBFile::Write. +%typemap(in) (const uint8_t *buf, size_t num_bytes) { + if (PythonByteArray::Check($input)) { + PythonByteArray bytearray(PyRefType::Borrowed, $input); + $1 = (uint8_t *)bytearray.GetBytes().data(); + $2 = bytearray.GetSize(); + } else if (PythonBytes::Check($input)) { + PythonBytes bytes(PyRefType::Borrowed, $input); + $1 = (uint8_t *)bytes.GetBytes().data(); + $2 = bytes.GetSize(); + } else { + PyErr_SetString(PyExc_ValueError, "Expecting a bytes or bytearray object"); + SWIG_fail; } - size = view.buffer.len; - buf = view.buffer.buf; - $1 = ($1_ltype)buf; - $2 = ($2_ltype)(size / sizeof($*1_type)); } -%enddef -%pybuffer_binary(const uint8_t *buf, size_t num_bytes); -%pybuffer_mutable_binary(uint8_t *buf, size_t num_bytes); +// Typemap for SBFile::Read. +%typemap(in) (uint8_t *buf, size_t num_bytes) { + if (PythonByteArray::Check($input)) { + PythonByteArray bytearray(PyRefType::Borrowed, $input); + $1 = (uint8_t *)bytearray.GetBytes().data(); + $2 = bytearray.GetSize(); + } else { + PyErr_SetString(PyExc_ValueError, "Expecting a bytearray"); + SWIG_fail; + } +} %typemap(in) (const char **symbol_name, uint32_t num_names) { using namespace lldb_private; diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig index 3a0995e84f643..9b9245eac14ce 100644 --- a/lldb/bindings/python/python-wrapper.swig +++ b/lldb/bindings/python/python-wrapper.swig @@ -128,8 +128,11 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallTypeScript( PyObject *pfunc_impl = nullptr; - if (pyfunct_wrapper && *pyfunct_wrapper && - PyFunction_Check(*pyfunct_wrapper)) { + if (pyfunct_wrapper && *pyfunct_wrapper  +#ifndef Py_LIMITED_API + && PyFunction_Check(*pyfunct_wrapper) +#endif + ) { pfunc_impl = (PyObject *)(*pyfunct_wrapper); if (pfunc_impl->ob_refcnt == 1) { Py_XDECREF(pfunc_impl); diff --git a/lldb/bindings/python/python.swig b/lldb/bindings/python/python.swig index 4a5a39dc4b06d..b2823f98acac8 100644 --- a/lldb/bindings/python/python.swig +++ b/lldb/bindings/python/python.swig @@ -59,6 +59,11 @@ except ImportError: // Parameter types will be used in the autodoc string. %feature("autodoc", "1"); +// Include lldb-python first as it sets Py_LIMITED_API. +%begin %{ +#include "../source/Plugins/ScriptInterpreter/Python/lldb-python.h" +%} + %pythoncode%{ import uuid import re 

[bulbazord]

Technically, this means a small regression, where those methods could previously take custom types that conform to Python's buffer protocol.

Is this something a user of LLDB could notice? Do we have any tests or examples that take advantage of this functionality? It might be a good idea to add something to the release notes if it's user visible, even if it's tiny.

[bulbazord]

Actually, given how small and niche this is, I don't expect anyone to care or notice.

[JDevlieghere]

Actually, given how small and niche this is, I don't expect anyone to care or notice.

I drewthe same conclusion. Someone that is relying on that must know what they're doing (and how to make it work again).