swig / swig

SWIG is a software development tool that connects programs written in C and C++ with a variety of high-level programming languages.
http://www.swig.org
Other
5.96k stars 1.26k forks source link

Generated code using pybuffer.i, fails to compile with Py_LIMITED_API and Python-3.13 #3059

Open julian-smith-artifex-com opened 5 months ago

julian-smith-artifex-com commented 5 months ago

If Py_LIMITED_API is defined and pybuffer.i is used, SWIG generated code uses PyObject_AsReadBuffer(), which is no longer available with Python-3.13.

This is with SWIG 4.2.1.

To reproduce:

Create file foo.i containing:

%include pybuffer.i

%pybuffer_binary(
        const unsigned char* PYTHON_BUFFER_DATA,
        size_t PYTHON_BUFFER_SIZE
        );

%{
const unsigned char* python_buffer_data(
        const unsigned char* PYTHON_BUFFER_DATA,
        size_t PYTHON_BUFFER_SIZE
        )
{
    return PYTHON_BUFFER_DATA;
}

%}

const unsigned char* python_buffer_data(
        const unsigned char* PYTHON_BUFFER_DATA,
        size_t PYTHON_BUFFER_SIZE
        );

Then run swig:

swig -Wall -c++ -python -module foo -o foo.i.cpp foo.i

The generated foo.i.cpp file has this sequence:

SWIGINTERN PyObject *_wrap_python_buffer_data(PyObject *self, PyObject *args) {
  PyObject *resultobj = 0;
  unsigned char *arg1 = (unsigned char *) 0 ;
  size_t arg2 ;
  PyObject *swig_obj[1] ;
  unsigned char *result = 0 ;

  (void)self;
  if (!args) SWIG_fail;
  swig_obj[0] = args;
  {
    int res; Py_ssize_t size = 0; const void *buf = 0;
#ifndef Py_LIMITED_API
    Py_buffer view;
    res = PyObject_GetBuffer(swig_obj[0], &view, PyBUF_CONTIG_RO);
#else
#if defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6))
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated"
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
#elif defined(_MSC_VER)
#pragma warning(push)
#pragma warning(disable: 4996)
#endif
    res = PyObject_AsReadBuffer(swig_obj[0], &buf, &size);
#if defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6))
#pragma GCC diagnostic pop
#elif defined(_MSC_VER)
#pragma warning(pop)
#endif
#endif
    if (res < 0) {
      PyErr_Clear();
      SWIG_exception_fail(SWIG_ArgError(res), "in method '" "python_buffer_data" "', argument " "1"" of type '" "(const unsigned char* PYTHON_BUFFER_DATA, size_t PYTHON_BUFFER_SIZE)""'");
    }
#ifndef Py_LIMITED_API
    size = view.len;
    buf = view.buf;
    PyBuffer_Release(&view);
#endif
    arg1 = (unsigned char *) buf;
    arg2 = (size_t) (size / sizeof(unsigned char const));
  }
  result = (unsigned char *)python_buffer_data((unsigned char const *)arg1,SWIG_STD_MOVE(arg2));
  resultobj = SWIG_NewPointerObj(SWIG_as_voidptr(result), SWIGTYPE_p_unsigned_char, 0 |  0 );
  return resultobj;
fail:
  return NULL;
}

And compilation with -DPy_LIMITED_API and Python-3.13 headers fails with:

error: ‘PyObject_AsReadBuffer’ was not declared in this scope; did you mean ‘PyObject_GetBuffer’?
    res = PyObject_AsReadBuffer(swig_obj[0], &buf, &size);
            ^~~~~~~~~~~~~~~~~~~~~
            PyObject_GetBuffer
julian-smith-artifex-com commented 5 months ago

It looks like this is from Lib/python/pybuffer.i in git.

Apparently the replacement for PyObject_AsReadBuffer() is PyObject_GetBuffer() which is only available in the stable ABI from 3.11.

So could pybuffer.i be modified to generate code like this?:

#ifdef Py_LIMITED_API
#if PY_VERSION_HEX < 0x0300b000
-- Python < 3.11, use old API.
-- use PyObject_AsReadBuffer()
#else
-- use PyObject_GetBuffer()
#endif 
#endif
jschueller commented 5 months ago

It looks fine with swig master + python 3.12 + -DPy_LIMITED_API=0x03040000, are you sure you're using the right includes ?

julian-smith-artifex-com commented 5 months ago

It's fine with Python-3.12, but breaks with Python-3.13's headers, because PyObject_AsReadBuffer() is not longer available.

jschueller commented 5 months ago

oh you're right

jschueller commented 5 months ago

it looks like the old apis are no more declared, but we can still use them if we redeclare it ourselves could you try #3063 ?

julian-smith-artifex-com commented 4 months ago

I've cloned the main swig repository https://github.com/swig/swig.git and cherry-picked https://github.com/jschueller/swig/tree/abi3buffer 418aece45dca8c Prefer PyObject_GetBuffer.

This appears to fix the problem - in my test case the generated C++ files compiles fine now with Python-3.13.

wsfulton commented 2 months ago

The new buffer protocol is available from 2.7. For info on the removal of the old buffer protocol, see https://github.com/python/cpython/issues/105186. Reading this and the linked issues, in summary, I understand the timeline is:

I think the old protocol should only be available if a user chooses to use it (probably only needed by limited API users). This would mean that SWIG would provide support that will always use the new buffer protocol by default, and as per the Python developers intentions will only be available in the limited API/stable ABI from 3.11 onwards as that is when it was added.

As the Python developers have removed it from the C API in 3.13, I feel the SWIG project has no right to hack it back in and it would be irresponsible to go against Python's plans to get rid of it altogether. It has gone, get used to it! Of course, there is nothing stopping individual SWIG users adding in the function prototypes if they wish to suffer the subsequent consequences and maintenance overhead.

In summary I propose we add a macro, such as SWIG_PYTHON_OLD_BUFFER_PROTOCOL, which will allow a user to change the default from the new to old protocol.

julian-smith-artifex-com commented 1 month ago

I agree that declaring the old protocol when it has been removed from the Python headers, is not a good solution. But we need to use the old protocol if Py_LIMITED_API is defined and the Python version is less than 3.11, otherwise SWIG code will not compile. And in these circumstances it is available in the Python headers.

If we do this (e.g. by implementing the suggestion in https://github.com/swig/swig/issues/3059#issuecomment-2437412822), there will be no need for manual control with SWIG_PYTHON_OLD_BUFFER_PROTOCOL.

jschueller commented 6 days ago

It turns out even we define the missing symbols they are not available to the interpreter: _python_pybuffer.abi3.so: undefined symbol: _Z21PyObject_AsReadBufferP7_objectPPKvPl We can try to emit a meaningful compilation error instead of just ending up to the undeclared symbol though I updated #3063