cast between incompatible function types reported by gcc 8 for swig & python

[D4N]

Summary

swig with Python seems to produce C source code that triggers some of the new warnings provided by gcc 8.

Reproducer

I am running swig 3.0.12 & gcc 8.1.1 both from the Fedora 28 repositories. The same issue appears on Archlinux, too.

It can be reproduced with the following two files:
test.c:

void triple_it(int *input) { *input *= 3 * (*input); }

and test.i:

%module test
%pointer_functions(int, intp);
%{
void triple_it(int *input);
%}
void triple_it(int *input);

Then running swig & gcc:

$ swig -o test_wrap.c -python test.i
$ gcc -o test.os -c -Wall -Wextra -Werror -O2 -fPIC test.c
$ gcc -o test_wrap.os -c -Wall -Wextra -Werror -O2 -fPIC -I/usr/include/python3.6m test_wrap.c
test_wrap.c:1819:23: error: cast between incompatible function types from ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} to ‘PyObject * (*)(PyObject *,
PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"disown",  (PyCFunction)SwigPyObject_disown,  METH_NOARGS,  (char *)"releases ownership of the pointer"},
                       ^
test_wrap.c:1820:23: error: cast between incompatible function types from ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} to ‘PyObject * (*)(PyObject *,
PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"acquire", (PyCFunction)SwigPyObject_acquire, METH_NOARGS,  (char *)"acquires ownership of the pointer"},
                       ^
test_wrap.c:1823:23: error: cast between incompatible function types from ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} to ‘PyObject * (*)(PyObject *,
PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"next",    (PyCFunction)SwigPyObject_next,    METH_NOARGS,  (char *)"returns the next 'this' object"},
                       ^
test_wrap.c:1824:23: error: cast between incompatible function types from ‘PyObject * (*)(SwigPyObject *)’ {aka ‘struct _object * (*)(struct <anonymous> *)’} to ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=cast-function-type]
   {(char *)"__repr__",(PyCFunction)SwigPyObject_repr,    METH_NOARGS,  (char *)"returns object representation"},
                       ^
cc1: all warnings being treated as errors

GCC complains about this part of the code:

static PyMethodDef
swigobject_methods[] = {
  {(char *)"disown",  (PyCFunction)SwigPyObject_disown,  METH_NOARGS,  (char *)"releases ownership of the pointer"},
  {(char *)"acquire", (PyCFunction)SwigPyObject_acquire, METH_NOARGS,  (char *)"acquires ownership of the pointer"},
  {(char *)"own",     (PyCFunction)SwigPyObject_own,     METH_VARARGS, (char *)"returns/sets ownership of the pointer"},
  {(char *)"append",  (PyCFunction)SwigPyObject_append,  METH_O,       (char *)"appends another 'this' object"},
  {(char *)"next",    (PyCFunction)SwigPyObject_next,    METH_NOARGS,  (char *)"returns the next 'this' object"},
  {(char *)"__repr__",(PyCFunction)SwigPyObject_repr,    METH_NOARGS,  (char *)"returns object representation"},
  {0, 0, 0, 0}  
};

Workaround

Ignore the warnings or make them non fatal by adding -Wno-error=cast-function-type to the compilation flags.

[ojwb]

Looks like we're currently deliberately defining these functions with the wrong signature when METH_NOARGS is defined (which it should be for any Python >= 2.2). That was correct for Python 2.2:

METH_NOARGS
Methods without parameters don't need to check whether arguments are given if they are listed with the METH_NOARGS flag. They need to be of type PyNoArgsFunction: they expect a single single PyObject* as a parameter. When used with object methods, this parameter is typically named self and will hold a reference to the object instance.

But Python 2.3 changed that:

METH_NOARGS
Methods without parameters don't need to check whether arguments are given if they are listed with the METH_NOARGS flag. They need to be of type PyCFunction. When used with object methods, the first parameter is typically named self and will hold a reference to the object instance. In all cases the second parameter will be NULL.

With typical calling conventions we get away with this (which is how it has gone unnoticed for so long) but it really should be fixed, and we're officially dropping support for such old Python versions in 4.0.0, so it looks like we just need to drop the METH_NOARGS checks and use the two parameter form that's already there.

Does this trigger with anything already in SWIG's testsuite?

[ojwb]

I've now installed GCC8 and the answer is yes - e.g. this results in many errors:

make check-python-examples CXXFLAGS='-Wall -W -Werror'

So no new testcases needed for this.

[D4N]

I have also seen the following issue pop up on Archlinux (which uses gcc 8.1.0) but not on Fedora (which has gcc 8.1.1), so maybe that is a false positive:

serial_receiver/receiver_wrap.c: In function 'SWIG_Python_FixMethods':
serial_receiver/receiver_wrap.c:4359:15: error: 'strncpy' output truncated before terminating nul copying 10 bytes from a string of the same length [-Werror=stringop-truncation]
               strncpy(buff, "swig_ptr: ", 10);
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Unfortunately that is not triggered by my reproducer but that is taken from a test pipeline of my project. The swig file looks like this:

%module receiver
%include "stdint.i"
%include "carrays.i"
%include "cpointer.i"

%array_functions(uint8_t, byteArray);
%pointer_functions(double, doublep);
%{
#include <stdint.h>
int func(uint8_t *, size_t, uint8_t, size_t, const char *restrict, size_t, double *);
%}
int func(uint8_t *, size_t, uint8_t, size_t, const char *restrict, size_t, double *);

(I have changed the function name & removed the parameter names since it is internal and I am embarrassed to have written that code)

[ojwb]

The strncpy issue was actually fixed over a year ago by 9825fcb, but that was just after the last release though, so 3.0.12 will still exhibit that. It's a false positive essentially as the code isn't expecting a terminating nul to be added, so you can safely just ignore/suppress the warning.

[D4N]

Ah, I just checked the Fedora sources for swig and they already included that patch. That's why it's not showing up on Fedora but on Archlinux. Glad to hear that it's not an issue anymore.

[q-p]

I'm seeing another warning from gcc-8 when wrapping std::vector<std::shared_ptr<T>> to Python

py/Flucs.swigwrap_2.cxx: In instantiation of 'static Type swig::traits_as<Type, swig::pointer_category>::as(PyObject*, bool) [with Type = std::shared_ptr<flis::Para::Node>; PyObject = _object]':
py/Flucs.swigwrap_2.cxx:4204:64:   required from 'Type swig::as(PyObject*, bool) [with Type = std::shared_ptr<flis::Para::Node>; PyObject = _object]'
py/Flucs.swigwrap_2.cxx:4771:20:   required from 'swig::SwigPySequence_Ref<T>::operator T() const [with T = std::shared_ptr<flis::Para::Node>]'
py/Flucs.swigwrap_2.cxx:5007:30:   required from 'void swig::assign(const SwigPySeq&, Seq*) [with SwigPySeq = swig::SwigPySequence_Cont<std::shared_ptr<flis::Para::Node> >; Seq = std::vector<std::shared_ptr<flis::Para::Node> >]'
py/Flucs.swigwrap_2.cxx:5029:12:   required from 'static int swig::traits_asptr_stdseq<Seq, T>::asptr(PyObject*, swig::traits_asptr_stdseq<Seq, T>::sequence**) [with Seq = std::vector<std::shared_ptr<flis::Para::Node> >; T = std::shared_ptr<flis::Para::Node>; PyObject = _object; swig::traits_asptr_stdseq<Seq, T>::sequence = std::vector<std::shared_ptr<flis::Para::Node> >]'
py/Flucs.swigwrap_2.cxx:5090:52:   required from 'static int swig::traits_asptr<std::vector<_RealType> >::asptr(PyObject*, std::vector<_RealType>**) [with T = std::shared_ptr<flis::Para::Node>; PyObject = _object]'
py/Flucs.swigwrap_2.cxx:4096:37:   required from 'int swig::asptr(PyObject*, Type**) [with Type = std::vector<std::shared_ptr<flis::Para::Node> >; PyObject = _object]'
py/Flucs.swigwrap_2.cxx:6968:34:   required from here
py/Flucs.swigwrap_2.cxx:4179:8: warning: 'void* memset(void*, int, size_t)' clearing an object of type 'class std::shared_ptr<flis::Para::Node>' with no trivial copy-assignment; use assignment or value-initialization instead [-Wclass-memaccess]
  memset(v_def,0,sizeof(Type));
  ~~~~~~^~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/local/Cellar/gcc/8.1.0/include/c++/8.1.0/memory:81,
                 from ../infrastructure/include/flis/Common/ObjectInterface.hpp:42,
                 from py/Flucs.swigwrap_2.cxx:3207:
/usr/local/Cellar/gcc/8.1.0/include/c++/8.1.0/bits/shared_ptr.h:103:11: note: 'class std::shared_ptr<flis::Para::Node>' declared here
     class shared_ptr : public __shared_ptr<_Tp>
           ^~~~~~~~~~

But then, vector<shared_ptr> seems to have some inherent problems anyhow (see for example #731 or #773), this may be (part of) the same problem...?

[ojwb]

@q-p Please don't tag new problems onto an existing issue - the only connection with the incompatible cast being tracked here is that GCC 8 warns about the two cases. Any fixes required will be entirely separate.

However that warning should also already be fixed in git master (by #1027 and related earlier changes) - the only hit for memset(v_def in the current code is in a comment in a testcase which tries to suppress this warning for clang (I'll remove that as it's no longer needed).

[ojwb]

(

(I'll remove that as it's no longer needed).

Removed in 7d11c29.
)

As for the actual topic of this issue, I've now pushed the clean up of the supported Python versions to git master. The warnings here still remain, but fixing them should be less painful now.

[mromberg]

The function cast thing is also something I looked at briefly. From my understanding (which could
be wrong), this is an issue that swig can't really do much of anything about. The root of the problem is
how python deals with these function pointers. And it is not a problem that is specific to python. Other projects are using a similar technique which gcc8 is now warning about.

What happens is that python has a structure that has a function pointer and a flag. The function pointer is "generic" (as far as python is concerned). So, every function listed in this data structure must be of this type or the compiler will complain. But the pointer in the structure is designed to point at functions that take differing numbers of arguments. Which is why the cast is universally done in the first place. Later on, inside the guts of python, another cast is done using the flag that is also part of the data structure back to the (assuming the flag matches the real function type) correct function type. The whole thing is a little nauseating. But it is one way to work around limitations of C. Now gcc catches that the cast is bogus. It always was. But now the warning.

But there is no way for swig or anyone else to fix this because it is how python (and other C based projects) are dealing with overloaded functions. I do remember seeing a patch for gcc itself to not issue this new warning for cases like python and other things that do this. So, this warning may just go away if the gcc developers decide the warning needs adjustment. Or python itself may have to undergo major surgery on this data structure. Which will pretty much break many python C modules along with swig.

In my projects particular case, I just suppress this gcc warning and will wait for the dust to settle. My guess is that the new gcc warning will just go away with time as these sorts of casts seem rather common with C programs/libraries.

[ojwb]

In the cases I looked at, the type of the function actually changed during Python's 2.x history and we should be able to fix the (currently incorrect) function type we're defining. Those case are definitely worth fixing as they're technically undefined behaviour, and it'll be easier to do so now that we aren't trying to support all those old Python 2.x versions.

It could well be that there are cases which are acting as you describe though.

[ojwb]

It seems we can fix this except when keyword argument support is enabled i.e. METH_KEYWORD in the Python C API, which is an area where the Python C API relies on casting of function pointers to a pointer of a different function pointer type, and addressing that would have to be done by the Python developers.

[ojwb]

Actually, GCC provides a way to suppress -Wcast-function-type - cast the function pointer via void(*)(void). I think that's worth doing for the remaining cases. Reopening - will push a patch once I've had a chance to check it actually makes us warning clean here.