Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] segmentation fault when numpy is not available #4377

Closed
maxbachmann opened this issue Sep 15, 2021 · 6 comments · Fixed by #4378
Closed

[BUG] segmentation fault when numpy is not available #4377

maxbachmann opened this issue Sep 15, 2021 · 6 comments · Fixed by #4378

Comments

@maxbachmann
Copy link
Contributor

maxbachmann commented Sep 15, 2021

Describe the bug
When using cimport numpy and numpy is not available at runtime, a segmentation fault is thrown.

To Reproduce
I used the following code

# distutils: language=c++
# cython: language_level=3, binding=True

cimport numpy as np

def test():
    pass

and uninstalled numpy before importing the module. For Cython 3.0a1 and Cython 0.29.24 this leads to an import error, while for any version newer than Cython 3.0a1 this causes a segmentation fault.

Expected behavior
No segmentation fault should be thrown.

Environment (please complete the following information):

  • OS: tested on Windows and Fedora
  • Python version: tested on 3.7 and 3.9
  • Cython version >3.0a1

Additional context
running the import in gdb leads to the following backtrace:

Program received signal SIGSEGV, Segmentation fault.
__Pyx_CLineForTraceback (c_line=3988, tstate=0x55555555afa0) at src/test.cpp:5923
5923	        __PYX_PY_DICT_LOOKUP_IF_MODIFIED(
(gdb) bt
#0  __Pyx_CLineForTraceback (c_line=3988, tstate=0x55555555afa0) at src/test.cpp:5923
#1  __Pyx_AddTraceback (filename=0x7ffff7fba89b "test.pyx", py_line=2, c_line=3988, 
    funcname=0x7ffff7fba886 "init test") at src/test.cpp:6105
#2  __pyx_pymod_exec_test (__pyx_pyinit_module=<optimized out>) at src/test.cpp:4057
#3  0x00007ffff7ddcdc3 in PyModule_ExecDef (module=<module at remote 0x7fffea250090>, def=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Objects/moduleobject.c:399
#4  0x00007ffff7ddcd34 in exec_builtin_or_dynamic (mod=<module at remote 0x7fffea250090>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/import.c:2248
#5  _imp_exec_builtin_impl (mod=<module at remote 0x7fffea250090>, module=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/import.c:2341
#6  _imp_exec_builtin (module=<optimized out>, mod=<module at remote 0x7fffea250090>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/clinic/import.c.h:388
#7  0x00007ffff7d6a36b in cfunction_vectorcall_O (
    func=<built-in method exec_dynamic of module object at remote 0x7fffea3746d0>, args=0x7fffea2de8f8, 
    nargsf=<optimized out>, kwnames=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Objects/methodobject.c:516
#8  0x00007ffff7d63fc7 in do_call_core (kwdict={}, callargs=(<module at remote 0x7fffea250090>,), 
    func=<built-in method exec_dynamic of module object at remote 0x7fffea3746d0>, tstate=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:5095
#9  _PyEval_EvalFrameDefault (tstate=<optimized out>, f=<optimized out>, throwflag=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:3580
#10 0x00007ffff7d5d00d in _PyEval_EvalFrame (throwflag=0, 
    f=Frame 0x7fffea2d7ac0, for file <frozen importlib._bootstrap>, line 228, in _call_with_frames_removed (f=<built-in method exec_dynamic of module object at remote 0x7fffea3746d0>, args=(<module at remote 0x7fffea250090>,), kwds={}), tstate=0x55555555afa0) at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/internal/pycore_ceval.h:40
#11 _PyEval_EvalCode (tstate=<optimized out>, _co=<optimized out>, globals=<optimized out>, locals=<optimized out>, 
    args=<optimized out>, argcount=<optimized out>, kwnames=0x0, kwargs=0x7fffea2d7e18, kwcount=0, kwstep=1, 
    defs=0x0, defcount=0, kwdefs=0x0, closure=0x0, name='_call_with_frames_removed', 
    qualname='_call_with_frames_removed') at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:4327
#12 0x00007ffff7d6acee in _PyFunction_Vectorcall (func=<optimized out>, stack=<optimized out>, 
    nargsf=<optimized out>, kwnames=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Objects/call.c:396
#13 0x00007ffff7d6305e in _PyObject_VectorcallTstate (kwnames=0x0, nargsf=<optimized out>, args=0x7fffea2d7e08, 
    callable=<function at remote 0x7fffea38b430>, tstate=0x55555555afa0)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/cpython/abstract.h:118
#14 PyObject_Vectorcall (kwnames=0x0, nargsf=<optimized out>, args=0x7fffea2d7e08, 
    callable=<function at remote 0x7fffea38b430>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/cpython/abstract.h:127
#15 call_function (kwnames=0x0, oparg=<optimized out>, pp_stack=<synthetic pointer>, tstate=0x55555555afa0)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:5075
#16 _PyEval_EvalFrameDefault (tstate=<optimized out>, f=<optimized out>, throwflag=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:3487
#17 0x00007ffff7d6afe3 in _PyEval_EvalFrame (throwflag=0, 
    f=Frame 0x7fffea2d7c80, for file <frozen importlib._bootstrap_external>, line 1181, in exec_module (self=<ExtensionFileLoader(name='rapidfuzz.test', path='/home/max/RapidFuzz/.venv/lib64/python3.9/site-packages/rapidfuzz/test.cpython-39-x86_64-linux-gnu.so') at remote 0x7fffea2498b0>, module=<module at remote 0x7fffea250090>), 
    tstate=0x55555555afa0) at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/internal/pycore_ceval.h:40
#18 function_code_fastcall (tstate=0x55555555afa0, co=<optimized out>, args=<optimized out>, nargs=2, 
    globals=<optimized out>) at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Objects/call.c:330
#19 0x00007ffff7d5e5eb in _PyObject_VectorcallTstate (kwnames=0x0, nargsf=<optimized out>, args=0x7fffea310f50, 
    callable=<function at remote 0x7fffea34f790>, tstate=0x55555555afa0)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/cpython/abstract.h:118
#20 PyObject_Vectorcall (kwnames=0x0, nargsf=<optimized out>, args=0x7fffea310f50, 
    callable=<function at remote 0x7fffea34f790>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/cpython/abstract.h:127
#21 call_function (kwnames=0x0, oparg=<optimized out>, pp_stack=<synthetic pointer>, tstate=0x55555555afa0)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:5075
--Type <RET> for more, q to quit, c to continue without paging--
#22 _PyEval_EvalFrameDefault (tstate=<optimized out>, f=<optimized out>, throwflag=<optimized out>)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Python/ceval.c:3504
#23 0x00007ffff7d6afe3 in _PyEval_EvalFrame (throwflag=0, 
    f=Frame 0x7fffea310dd0, for file <frozen importlib._bootstrap>, line 680, in _load_unlocked (spec=<ModuleSpec(name='rapidfuzz.test', loader=<ExtensionFileLoader(name='rapidfuzz.test', path='/home/max/RapidFuzz/.venv/lib64/python3.9/site-packages/rapidfuzz/test.cpython-39-x86_64-linux-gnu.so') at remote 0x7fffea2498b0>, origin='/home/max/RapidFuzz/.venv/lib64/python3.9/site-packages/rapidfuzz/test.cpython-39-x86_64-linux-gnu.so', loader_state=None, submodule_search_locations=None, _set_fileattr=True, _cached=None, _initializing=True) at remote 0x7fffea2492b0>, module=<module at remote 0x7fffea250090>), tstate=0x55555555afa0)
    at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Include/internal/pycore_ceval.h:40
#24 function_code_fastcall (tstate=0x55555555afa0, co=<optimized out>, args=<optimized out>, nargs=1, 
    globals=<optimized out>) at /usr/src/debug/python3.9-3.9.7-1.fc34.x86_64/Objects/call.c:330

For the full backtrace check the following file:
backtrace.txt

@da-woods
Copy link
Contributor

It looks to be f18e3c9 (i.e. my fault).

@da-woods
Copy link
Contributor

I think this is kind of intentional (partly).

If you're doing cimport numpy then you should be calling the Numpy C function import_array too. A lot of people were forgetting to do it (and getting odd crashes later because of it), so we started adding it in automatically.

To disable this autogeneration you can do:

# at global scope
<void>np.import_array

(i.e. just "use" the symbol from Cython's point of view).

It looks like np.import_array crashes when the Numpy module isn't present, so possibly we should guard against that. But I'm not sure how right now.

@da-woods
Copy link
Contributor

da-woods commented Sep 15, 2021

What I'm a little puzzled by is that the first thing import_array does is to try to import the Python module and raise an exception if that fails. So it should be pretty safe:

https://github.com/numpy/numpy/blob/1f95d7914ff23ffeb95767de79c55b4f702e6f2f/numpy/core/code_generators/generate_numpy_api.py#L44-L52


Edit: OK - the problem isn't in import_array - it actually happens if you raise any exception in __Pyx_InitGlobals

@maxbachmann
Copy link
Contributor Author

maxbachmann commented Sep 15, 2021

If you're doing cimport numpy then you should be calling the Numpy C function import_array too. A lot of people were forgetting to do it (and getting odd crashes later because of it), so we started adding it in automatically.

Yes I received a bug report yesterday, about my library crashing on arm since I started to use numpy: rapidfuzz/Levenshtein#12
My first thought was that I forgot to call import_array (which I did forget 😄) and was very surprised to find, that this is automatically done by Cython.

@da-woods
Copy link
Contributor

__Pyx_PyDict_GetItemStr(*cython_runtime_dict, __pyx_n_s_cline_in_traceback)

but we import Numpy before initializing the string-tab... Should be fairly easily fixed I think

@maxbachmann
Copy link
Contributor Author

I received a bug report yesterday, about my library crashing on arm since I started to use numpy: rapidfuzz/Levenshtein#12
My first thought was that I forgot to call import_array (which I did forget smile) and was very surprised to find, that this is automatically done by Cython.

I attempted to fix this by manually running np.import_array https://github.com/maxbachmann/RapidFuzz/blob/7cc1e5a053c6f2e7d84dd7348b766a2044a48c71/src/cpp_process_cdist.pyx#L44. This did indeed fix the segmentation fault. Now I am down to being binary incompatible, since pibuildwheels does not correctly use oldest-supported-numpy piwheels/packages#212 ...

da-woods added a commit to da-woods/cython that referenced this issue Sep 15, 2021
This can happen (rarely) with exceptions very early in the
module init process.

Fixes cython#4377

I also cleaned up a few inline functions to have return statements.

I'm struggling to see how to add a test for this (so I've
omitted it) but happy to take any suggestions
scoder pushed a commit that referenced this issue Sep 27, 2021
This can happen (rarely) with exceptions that occur very early in the module init process.

Fixes #4377

Uses a fake Numpy module for testing to make a version of "import_array" that always fails.
0dminnimda added a commit to 0dminnimda/cython that referenced this issue Oct 26, 2021
commit 1461e51
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Mon Oct 25 14:29:02 2021 +0200

    Clean up the NumPy integration test by moving the doctests into the functions that they test.

commit 68bb716
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Mon Oct 25 14:14:24 2021 +0200

    Remove dead test code.

commit 4a74678
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Sun Oct 24 21:27:44 2021 +0200

    Use newer test dependencies in Py3.6+. (Excluding 3.10 for now to give the projects a bit more time.)

commit 9d1ffd5
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Sun Oct 24 23:13:50 2021 +0100

    Initial support for Python 3.11 (cythonGH-4414)

    * Add a basic replacement for PyCode_New().

    An optimized versions would be nice, but this is intended to work sufficiently to start testing. Also, CPython 3.11 might actually add a new C-API function to simplify setting the current code position. That might be used instead.

    * Disable introspection of frame object with vectorcall

    This feature looked to only be used for early Python versions that don't have the full vectorcall protocol (and the contents of the frame object are changed in Python 3.11).

commit 346c81f
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Sun Oct 24 21:18:10 2021 +0200

    Make sure that version dependent special methods are correctly and completely excluded via preprocessor guards.
    Previously, implementing "__div__" could fail in Py3 (if the code for adapting the Python wrapper was generated) or would at least generate C compiler warnings about unused "__div__" C functions.

commit 3748c3c
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Sun Oct 24 13:22:34 2021 +0200

    Add Py3.10 as CI test target.

commit 0f84a57
Author: Max Bachmann <kontakt@maxbachmann.de>
Date:   Sat Oct 23 22:06:37 2021 +0200

    Update incorrect version support comment for pycapsule.pxd (cythonGH-4426)

commit c25c87d
Author: Dobatymo <Dobatymo@users.noreply.github.com>
Date:   Sat Oct 23 03:33:02 2021 +0800

    Fix libcpp map/set/multiset/unordered type issues (cythonGH-4410)

    Fix insert return types, constness and input iterator templates.
    Fix typing in iterators and add constructor to allow explicit conversion from iterator to const_iterator.

commit f776da0
Author: Dobatymo <Dobatymo@users.noreply.github.com>
Date:   Sat Oct 23 03:28:53 2021 +0800

    Add C++ multimap/unordered_multimap (cythonGH-4419)

commit c83fd44
Author: 0dminnimda <0dminnimda@gmail.com>
Date:   Fri Oct 22 21:44:11 2021 +0300

    Introduce new shell syntax for ci-run.sh to improve Windows support (cythonGH-4400)

commit c8c9a12
Merge: 174ca03 f53ac52
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Thu Oct 21 19:02:11 2021 +0200

    Merge branch '0.29.x'

commit f53ac52
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Tue May 25 11:20:54 2021 +0200

    docs: Use the Cython + IPython lexers that come with Pygments to avoid having to maintain our own ones.

commit 174ca03
Author: account-login <account-login@users.noreply.github.com>
Date:   Wed Oct 20 17:03:20 2021 +0800

    Add some missing functions to libcpp maps and string (cythonGH-4395)

    * add swap() to libcpp.string
    * add load_factor() to libcpp.unordered_map and libcpp.unordered_set

commit 42a4af2
Merge: 53b0eb2 fb5d29e
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Mon Oct 18 20:38:22 2021 +0200

    Merge branch '0.29.x'

commit fb5d29e
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Mon Oct 18 19:36:54 2021 +0100

    Fix tracing after adapting it to Py3.11 (cythonGH-4420)

commit 53b0eb2
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Mon Oct 18 19:36:54 2021 +0100

    Fix tracing after adapting it to Py3.11 (cythonGH-4420)

commit 5f820ed
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Mon Oct 18 11:10:05 2021 +0100

    Fix fused.__self__ tests on PyPy (cythonGH-4417)

    PyPy v7.3.6 looks to have added a helpful "did you mean..." to the AttributeError exception. It's currently tripping up these tests.

commit 4df1103
Merge: f6eeeda cbddad2
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Mon Oct 18 12:05:10 2021 +0200

    Merge branch '0.29.x'

commit cbddad2
Author: Victor Stinner <vstinner@python.org>
Date:   Mon Oct 18 12:03:17 2021 +0200

    Make Profile.c use PyThreadState_EnterTracing() (cythonGH-4411)

    Instead of __Pyx_SetTracing(), Profile.c now uses PyThreadState_EnterTracing() and PyThreadState_LeaveTracing(), which were added to Python 3.11.0a2:
    python/cpython#28542

    When these functions are used, Cython no longer sets directly PyThreadState.cframe.use_tracing.

commit f6eeeda
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Sun Oct 17 19:01:52 2021 +0100

    Fix fused cpdef default arguments (cythonGH-4413)

    A couple of things were going wrong:
    * they're creating CloneNodes (but not requiring the contents of the clone of the clone node to be temp)
    * assignment from a clone node generates cleanup code (which is against the general rules of a clone node), and also loses a reference via giveref
    * cpdef functions cause a small memory leak (cython#4412) by assigning to their constants twice. This is unfortunately difficult to test for. With this patch we no longer leak, but still duplicate a little bit of work.

commit a0571a6
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Sun Oct 17 18:51:08 2021 +0100

    Import TextTestResult in test runner instead of _TextTestResult (cythonGH-4415)

    All the versions we currently test are new enough that the alias is no longer necessary.

commit c129b15
Author: Dobatymo <Dobatymo@users.noreply.github.com>
Date:   Fri Oct 15 16:31:32 2021 +0800

    Fix wrong type in unordered_multiset::swap() (cythonGH-4408)

commit 72c18e7
Author: 0dminnimda <0dminnimda@gmail.com>
Date:   Thu Oct 7 10:56:43 2021 +0300

    Improve ci-run.sh (cythonGH-4398)

commit 454a498
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Wed Oct 6 07:16:08 2021 +0100

    Improve "import_array" guard (cythonGH-4397)

    Stop using NPY_NDARRAYOBJECT_H since:
    a) in principle it's private
    b) Numpy has renamed it
    and use a public symbol instead.

    I think the existing tests are adequate - we just aren't yet testing
    against a new enough version of Numpy to have caught it yet.

    Closes cython#4396
    Closes cython#4394

commit 97c05e7
Author: Stefan Behnel <stefan_ml@behnel.de>
Date:   Sat Oct 2 11:08:43 2021 +0200

    Make a compile test runnable.

commit 8c7b0f3
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Fri Oct 1 10:29:34 2021 +0100

    Handle function "outer_attrs" more consistently (cythonGH-4375)

    A few children of function nodes need to be consistently evaluated
    outside the function scope. This PR attempts to do so and thus
    fixes cython#4367.

commit 494f517
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Fri Oct 1 10:22:36 2021 +0100

    Change gcc version check in test runner to a numeric comparison (cythonGH-4359)

    The string comparison was reporting '11' < '4' (so OpenMP tests were being skipped on GCC 11)

commit cce3693
Author: Christian Clauss <cclauss@me.com>
Date:   Wed Sep 29 09:49:45 2021 +0200

    Fix typo discovered by codespell (cython#4387)

commit daa0a44
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Tue Sep 28 08:48:12 2021 +0100

     Fix the name of attributes in the common ABI module  (cythonGH-4376)

    Attribute names used to be fully qualified like "_cython_3_0_0a9.cython_function_or_method" instead of the plain name.

    Closes cython#4373

commit fa8db66
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Mon Sep 27 10:11:12 2021 +0100

    Avoid AddTraceback() if stringtab isn't set up (cythonGH-4378)

    This can happen (rarely) with exceptions that occur very early in the module init process.

    Fixes cython#4377

    Uses a fake Numpy module for testing to make a version of "import_array" that always fails.

commit f94f26a
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Mon Sep 27 09:58:20 2021 +0100

    Make __Pyx_CoroutineAwaitType non-pickleable (cythonGH-4381)

    This is explicitly tested for: https://github.com/cython/cython/blob/aea4e6b84b38223c540266f8c57093ee2039f284/tests/run/test_coroutines_pep492.pyx#L2400

    It turns out some earlier versions of Python assume that
    C-API classes without a dict or slot are pickleable by the class
    name. Currently it isn't pickleable because the class name lookup
    is failing but this change makes it more robust.

    See cython#4376

commit 7403055
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Fri Sep 24 11:05:52 2021 +0100

    Avoid unnecessary binding of fused functions on class lookup (cythonGH-4370)

    Among other things this makes it pickleable by ensuring that it's the same object each time.

commit e2a23fe
Author: da-woods <dw-git@d-woods.co.uk>
Date:   Mon Sep 20 08:34:34 2021 +0100

    Remove usused "FetchCommonPointer" utility code (cythonGH-4380)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants