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.97k stars 1.26k forks source link

4.3.0 regression with Python bindings and PYTHONWARNINGS="error" #3061

Open rouault opened 5 months ago

rouault commented 5 months ago

Below is a simplified reproducer of https://github.com/conda-forge/gdal-feedstock/issues/995:

$ echo "int foo() { return 0; }" > foo.c
$ gcc foo.c -shared -fPIC -o libfoo.so
$ echo "%inline { int foo(); }" > foo.i
$ ~/install-swig-master/bin/swig -module foo -python foo.i
$ gcc -g  -I/usr/include/python3.8 foo_wrap.c -shared -fPIC -o _foo.cpython-38-x86_64-linux-gnu.so -L. -lfoo
$ PYTHONWARNINGS="error" LD_LIBRARY_PATH=$PWD PYTHONPATH=$PWD python -c "import foo"
Segmentation (fault)

$ PYTHONMALLOC=malloc PYTHONWARNINGS="error" LD_LIBRARY_PATH=$PWD PYTHONPATH=$PWD valgrind python -c "import foo"
==604088== Memcheck, a memory error detector
==604088== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==604088== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==604088== Command: python -c import\ foo
==604088== 
==604088== Invalid read of size 8
==604088==    at 0x5B57E2: _PyObject_New (in /usr/bin/python3.8)
==604088==    by 0x485524D: SWIG_Python_newvarlink (foo_wrap.c:1567)
==604088==    by 0x4855343: SWIG_globals (foo_wrap.c:1597)
==604088==    by 0x485714B: SWIG_Python_DestroyModule (foo_wrap.c:2965)
==604088==    by 0x60C8DC: ??? (in /usr/bin/python3.8)
==604088==    by 0x5BF352: ??? (in /usr/bin/python3.8)
==604088==    by 0x5C225B: ??? (in /usr/bin/python3.8)
==604088==    by 0x6A502B: ??? (in /usr/bin/python3.8)
==604088==    by 0x5C1D78: ??? (in /usr/bin/python3.8)
==604088==    by 0x6837A2: PyImport_Cleanup (in /usr/bin/python3.8)
==604088==    by 0x67E03E: Py_FinalizeEx (in /usr/bin/python3.8)
==604088==    by 0x6B584C: Py_RunMain (in /usr/bin/python3.8)
==604088==  Address 0x20 is not stack'd, malloc'd or (recently) free'd
==604088== 
==604088== 
==604088== Process terminating with default action of signal 11 (SIGSEGV)
==604088==  Access not within mapped region at address 0x20
==604088==    at 0x5B57E2: _PyObject_New (in /usr/bin/python3.8)
==604088==    by 0x485524D: SWIG_Python_newvarlink (foo_wrap.c:1567)
==604088==    by 0x4855343: SWIG_globals (foo_wrap.c:1597)
==604088==    by 0x485714B: SWIG_Python_DestroyModule (foo_wrap.c:2965)
==604088==    by 0x60C8DC: ??? (in /usr/bin/python3.8)
==604088==    by 0x5BF352: ??? (in /usr/bin/python3.8)
==604088==    by 0x5C225B: ??? (in /usr/bin/python3.8)
==604088==    by 0x6A502B: ??? (in /usr/bin/python3.8)
==604088==    by 0x5C1D78: ??? (in /usr/bin/python3.8)
==604088==    by 0x6837A2: PyImport_Cleanup (in /usr/bin/python3.8)
==604088==    by 0x67E03E: Py_FinalizeEx (in /usr/bin/python3.8)
==604088==    by 0x6B584C: Py_RunMain (in /usr/bin/python3.8)

git bisect points to: f121300b6b9ab4f30e90218fce9ba460708338b2 is the first bad commit

commit f121300b6b9ab4f30e90218fce9ba460708338b2
Author: Clinton Stimpson <cjstimp@sandia.gov>
Date:   Sun Oct 2 22:13:18 2022 -0600

    Use more portable PyType_FromSpec with Python 3.4+

    This makes progress towards building with Py_LIMITED_API.
    If using -builtin, PyType_FromSpec is enabled with Python 3.9+

 Lib/python/pyrun.swg      |  10 ++--
 Lib/python/pyruntime.swg  |   1 +
 Source/Modules/python.cxx | 120 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+), 3 deletions(-)

Manually editing foo_wrap.c to comment the line 1466 (on f121300b6b9ab4f30e90218fce9ba460708338b2, or line 207 with v4.3.0 tag) "#define SWIG_HEAPTYPES" also fixes the issue.

rouault commented 5 months ago

CC @clintonstimpson

rouault commented 5 months ago
$  PYTHONWARNINGS=always LD_LIBRARY_PATH=$PWD PYTHONPATH=$PWD python -c "import foo"
<frozen importlib._bootstrap>:219: DeprecationWarning: builtin type SwigPyPacked has no __module__ attribute
<frozen importlib._bootstrap>:219: DeprecationWarning: builtin type SwigPyObject has no __module__ attribute
sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute
ojwb commented 5 months ago
$  PYTHONWARNINGS=always LD_LIBRARY_PATH=$PWD PYTHONPATH=$PWD python -c "import foo"
<frozen importlib._bootstrap>:219: DeprecationWarning: builtin type SwigPyPacked has no __module__ attribute
<frozen importlib._bootstrap>:219: DeprecationWarning: builtin type SwigPyObject has no __module__ attribute
sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute

This part is #2881.

jesper-friis commented 2 months ago

I also see this issue. I think the reason for this segfault is that SWIG assumes that PyType_FromSpec() never returns NULL when called from swig_varlink_type() with SWIG_HEAPTYPES defined.

However, with Python 3.11 (and probably earlier version as well) PyType_FromSpec() issues a DeprecationWarning: builtin type swigvarlink has no __module__ attribute. With PYTHONWARNINGS=error this warning is turned into an error which causes PyType_FromSpec() to return NULL.

This NULL is then returned by swig_varlink_type() and causes a segfault in SWIG_Python_newvarlink():

/* Create a variable linking object for use later */
SWIGINTERN PyObject *
SWIG_Python_newvarlink(void) {
  swig_varlinkobject *result = PyObject_New(swig_varlinkobject, swig_varlink_type());
  if (result) {
    result->vars = 0;
  }
  return ((PyObject*) result);
}

when it calls PyObject_New() with a NULL pointer as second argument.

The correct fix seems for this issue seems that swig_varlink_type() should define a __module__ slot.

However, this will not prevent SWIG causing segfaults in the case of future DeprecationWarnings from PyType_FromSpec(). SWIG_Python_newvarlink() should probably also be updated to safely handle this case.

rouault commented 4 days ago

This issue also affects 4.3.1