[lldb] Use Python Bytes instead of Buffer for Binary I/O (NFC)

[JDevlieghere]

Binary I/O (also called buffered I/O) expects bytes-like objects and produces bytes objects [1]. Switch from using a Python buffer to using Python bytes to read the data. This eliminates calls to functions that aren't part of the Python stable C API.

[1] https://docs.python.org/3/library/io.html#binary-i-o

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Binary I/O (also called buffered I/O) expects bytes-like objects and produces bytes objects [1]. Switch from using a Python buffer to using Python bytes to read the data. This eliminates calls to functions that aren't part of the Python stable C API.

[1] https://docs.python.org/3/library/io.html#binary-i-o

---

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

1 Files Affected:

• (modified) lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (+6-40)

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index e124954057520..3950a4fd7a20c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1085,40 +1085,6 @@ class SimplePythonFile : public OwnedPythonFile<NativeFile> {
 char SimplePythonFile::ID = 0;
 } // namespace
 
-namespace {
-class PythonBuffer {
-public:
-  PythonBuffer &operator=(const PythonBuffer &) = delete;
-  PythonBuffer(const PythonBuffer &) = delete;
-
-  static Expected<PythonBuffer> Create(PythonObject &obj,
-                                       int flags = PyBUF_SIMPLE) {
-    Py_buffer py_buffer = {};
-    PyObject_GetBuffer(obj.get(), &py_buffer, flags);
-    if (!py_buffer.obj)
-      return llvm::make_error<PythonException>();
-    return PythonBuffer(py_buffer);
-  }
-
-  PythonBuffer(PythonBuffer &&other) {
-    m_buffer = other.m_buffer;
-    other.m_buffer.obj = nullptr;
-  }
-
-  ~PythonBuffer() {
-    if (m_buffer.obj)
-      PyBuffer_Release(&m_buffer);
-  }
-
-  Py_buffer &get() { return m_buffer; }
-
-private:
-  // takes ownership of the buffer.
-  PythonBuffer(const Py_buffer &py_buffer) : m_buffer(py_buffer) {}
-  Py_buffer m_buffer;
-};
-} // namespace
-
 // Shared methods between TextPythonFile and BinaryPythonFile
 namespace {
 class PythonIOFile : public OwnedPythonFile<File> {
@@ -1212,12 +1178,12 @@ class BinaryPythonFile : public PythonIOFile {
       num_bytes = 0;
       return Status();
     }
-    auto pybuffer = PythonBuffer::Create(pybuffer_obj.get());
-    if (!pybuffer)
-      // Cloning since the wrapped exception may still reference the PyThread.
-      return Status::FromError(pybuffer.takeError()).Clone();
-    memcpy(buf, pybuffer.get().get().buf, pybuffer.get().get().len);
-    num_bytes = pybuffer.get().get().len;
+    PythonBytes pybytes(PyRefType::Owned, pybuffer_obj->get());
+    if (!pybytes)
+      return Status::FromErrorString("not a byte array");
+    llvm::ArrayRef<uint8_t> bytes = pybytes.GetBytes();
+    memcpy(buf, bytes.begin(), bytes.size());
+    num_bytes = bytes.size();
     return Status();
   }
 };

[JDevlieghere]

Part of #151617

[bulbazord]

This subtly changes behavior from raising a python exception to returning the string "not a byte array". Is that not a breaking change?

[JDevlieghere]

Good point, this should create a PythonError