From f733efd3c0e1863c0a315af5c741f879c8ee1db4 Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Fri, 18 Mar 2022 00:08:01 -0400 Subject: [PATCH 1/3] Use different capsule names with and without -builtin Types generated with and without -builtin are not compatible. Mixing them in a common type list leads to crashes. Avoid this by using different capsule names: "type_pointer_capsule" without -builtin and "type_pointer_capsule_builtin" with. See #1684 --- Examples/test-suite/common.mk | 3 ++- Examples/test-suite/python/Makefile.in | 9 ++++++++- .../python/python_runtime_data_runme.py | 11 +++++++++++ Examples/test-suite/python_runtime_data.list | 2 ++ Examples/test-suite/python_runtime_data_builtin.i | 15 +++++++++++++++ .../test-suite/python_runtime_data_nobuiltin.i | 3 +++ Lib/python/pyhead.swg | 7 ++++++- Lib/python/pyrun.swg | 2 +- Source/Modules/python.cxx | 3 +++ 9 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 Examples/test-suite/python/python_runtime_data_runme.py create mode 100644 Examples/test-suite/python_runtime_data.list create mode 100644 Examples/test-suite/python_runtime_data_builtin.i create mode 100644 Examples/test-suite/python_runtime_data_nobuiltin.i diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index 48e0546c1..85b27979d 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -847,9 +847,10 @@ swig_and_compile_c = \ swig_and_compile_multi_cpp = \ for f in `cat $(top_srcdir)/$(EXAMPLES)/$(TEST_SUITE)/$*.list` ; do \ + swigopt=$$(name=SWIGOPT_$$f; eval echo \$$$$name); \ $(MAKE) -f $(top_builddir)/$(EXAMPLES)/Makefile SRCDIR='$(SRCDIR)' CXXSRCS='$(CXXSRCS)' \ SWIG_LIB_DIR='$(SWIG_LIB_DIR)' SWIGEXE='$(SWIGEXE)' \ - LIBS='$(LIBS)' INCLUDES='$(INCLUDES)' SWIGOPT='$(SWIGOPT)' NOLINK=true \ + LIBS='$(LIBS)' INCLUDES='$(INCLUDES)' SWIGOPT="$${swigopt:-$(SWIGOPT)}" NOLINK=true \ TARGET="$(TARGETPREFIX)$${f}$(TARGETSUFFIX)" INTERFACEDIR='$(INTERFACEDIR)' INTERFACE="$$f.i" \ $(LANGUAGE)$(VARIANT)_cpp; \ done diff --git a/Examples/test-suite/python/Makefile.in b/Examples/test-suite/python/Makefile.in index 80116be13..57106edd1 100644 --- a/Examples/test-suite/python/Makefile.in +++ b/Examples/test-suite/python/Makefile.in @@ -93,6 +93,9 @@ C_TEST_CASES += \ python_nondynamic \ python_varargs_typemap \ +MULTI_CPP_TEST_CASES += \ + python_runtime_data \ + include $(srcdir)/../common.mk # Overridden variables here @@ -102,6 +105,10 @@ VALGRIND_OPT += --suppressions=pythonswig.supp # Custom tests - tests with additional commandline options #python_flatstaticmethod.cpptest: SWIGOPT += -flatstaticmethod +python_runtime_data.multicpptest: override SWIG_FEATURES := $(filter-out -builtin,$(SWIG_FEATURES)) +python_runtime_data.multicpptest: override SWIGOPT := $(filter-out -builtin,$(SWIGOPT)) +python_runtime_data.multicpptest: export SWIGOPT_python_runtime_data_builtin = $(SWIGOPT) -builtin + # Rules for the different types of tests %.cpptest: $(setup) @@ -149,7 +156,7 @@ clean: rm -f hugemod.h hugemod_a.i hugemod_b.i hugemod_a.py hugemod_b.py hugemod_runme.py rm -f imports_a.py imports_b.py mod_a.py mod_b.py multi_import_a.py rm -f multi_import_b.py multi_import_d.py packageoption_a.py packageoption_b.py packageoption_c.py - rm -f template_typedef_cplx2.py + rm -f template_typedef_cplx2.py python_runtime_data_builtin.py python_runtime_data_nobuiltin.py hugemod_runme = hugemod$(SCRIPTPREFIX) diff --git a/Examples/test-suite/python/python_runtime_data_runme.py b/Examples/test-suite/python/python_runtime_data_runme.py new file mode 100644 index 000000000..341315ee3 --- /dev/null +++ b/Examples/test-suite/python/python_runtime_data_runme.py @@ -0,0 +1,11 @@ +import python_runtime_data_builtin as builtin +import python_runtime_data_nobuiltin as nobuiltin + +assert builtin.is_python_builtin() +assert not nobuiltin.is_python_builtin() + +for i in range(1, 5): + v1 = builtin.vectord([1.] * i) + assert len(v1) == i + v2 = nobuiltin.vectord([1.] * i) + assert len(v2) == i diff --git a/Examples/test-suite/python_runtime_data.list b/Examples/test-suite/python_runtime_data.list new file mode 100644 index 000000000..e88ef0c3b --- /dev/null +++ b/Examples/test-suite/python_runtime_data.list @@ -0,0 +1,2 @@ +python_runtime_data_builtin +python_runtime_data_nobuiltin diff --git a/Examples/test-suite/python_runtime_data_builtin.i b/Examples/test-suite/python_runtime_data_builtin.i new file mode 100644 index 000000000..1001b837c --- /dev/null +++ b/Examples/test-suite/python_runtime_data_builtin.i @@ -0,0 +1,15 @@ +// Test swig_runtime_data with and without -builtin + +%module python_runtime_data_builtin + +%inline %{ +#ifdef SWIGPYTHON_BUILTIN +bool is_python_builtin() { return true; } +#else +bool is_python_builtin() { return false; } +#endif +%} + +%include std_vector.i + +%template(vectord) std::vector; diff --git a/Examples/test-suite/python_runtime_data_nobuiltin.i b/Examples/test-suite/python_runtime_data_nobuiltin.i new file mode 100644 index 000000000..18c041468 --- /dev/null +++ b/Examples/test-suite/python_runtime_data_nobuiltin.i @@ -0,0 +1,3 @@ +%module python_runtime_data_nobuiltin + +%include "python_runtime_data_builtin.i" diff --git a/Lib/python/pyhead.swg b/Lib/python/pyhead.swg index 669990719..d3730a8fa 100644 --- a/Lib/python/pyhead.swg +++ b/Lib/python/pyhead.swg @@ -80,7 +80,12 @@ SWIG_Python_str_FromChar(const char *c) /* SWIGPY_USE_CAPSULE is no longer used within SWIG itself, but some user interface files check for it. */ # define SWIGPY_USE_CAPSULE -# define SWIGPY_CAPSULE_NAME ("swig_runtime_data" SWIG_RUNTIME_VERSION ".type_pointer_capsule" SWIG_TYPE_TABLE_NAME) +#ifdef SWIGPYTHON_BUILTIN +# define SWIGPY_CAPSULE_ATTR_NAME "type_pointer_capsule_builtin" SWIG_TYPE_TABLE_NAME +#else +# define SWIGPY_CAPSULE_ATTR_NAME "type_pointer_capsule" SWIG_TYPE_TABLE_NAME +#endif +# define SWIGPY_CAPSULE_NAME ("swig_runtime_data" SWIG_RUNTIME_VERSION "." SWIGPY_CAPSULE_ATTR_NAME) #if PY_VERSION_HEX < 0x03020000 #define PyDescr_TYPE(x) (((PyDescrObject *)(x))->d_type) diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg index 3d3443946..081bb2cde 100644 --- a/Lib/python/pyrun.swg +++ b/Lib/python/pyrun.swg @@ -1686,7 +1686,7 @@ SWIG_Python_SetModule(swig_module_info *swig_module) { #endif PyObject *pointer = PyCapsule_New((void *) swig_module, SWIGPY_CAPSULE_NAME, SWIG_Python_DestroyModule); if (pointer && module) { - if (PyModule_AddObject(module, "type_pointer_capsule" SWIG_TYPE_TABLE_NAME, pointer) == 0) { + if (PyModule_AddObject(module, SWIGPY_CAPSULE_ATTR_NAME, pointer) == 0) { Swig_Capsule_global = pointer; } else { Py_DECREF(pointer); diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx index 69c13db48..e3b6faeed 100644 --- a/Source/Modules/python.cxx +++ b/Source/Modules/python.cxx @@ -4234,6 +4234,9 @@ public: printSlot(f, getSlot(n, "feature:python:am_await"), "am_await", "unaryfunc"); printSlot(f, getSlot(n, "feature:python:am_aiter"), "am_aiter", "unaryfunc"); printSlot(f, getSlot(n, "feature:python:am_anext"), "am_anext", "unaryfunc"); + Printv(f, "# if PY_VERSION_HEX >= 0x030a0000\n", NIL); + printSlot(f, getSlot(n, "feature:python:am_send"), "am_send", "sendfunc"); + Printv(f, "# endif\n", NIL); Printf(f, " },\n"); Printv(f, "#endif\n", NIL); From 3ab288dfa48e34788da25cf0ec5b3bdd0e8d63f6 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Sat, 26 Mar 2022 15:06:13 +0000 Subject: [PATCH 2/3] Different capsule names for builtin changes entry --- CHANGES.current | 8 ++++++++ .../test-suite/python/python_runtime_data_runme.py | 12 ++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGES.current b/CHANGES.current index 64e4c4402..56cb2c560 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,14 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.1.0 (in progress) =========================== +2022-03-26: eltoder + [Python] #1684 Use different capsule names with and without -builtin + + Types generated with and without -builtin are not compatible. Mixing + them in a common type list leads to crashes. Avoid this by using + different capsule names: "type_pointer_capsule" without -builtin and + "type_pointer_capsule_builtin" with. + 2022-03-15: ianlancetaylor [Go] Don't convert arrays to pointers if there is a "gotype" typemap entry. diff --git a/Examples/test-suite/python/python_runtime_data_runme.py b/Examples/test-suite/python/python_runtime_data_runme.py index 341315ee3..063bf82d1 100644 --- a/Examples/test-suite/python/python_runtime_data_runme.py +++ b/Examples/test-suite/python/python_runtime_data_runme.py @@ -1,11 +1,15 @@ import python_runtime_data_builtin as builtin import python_runtime_data_nobuiltin as nobuiltin -assert builtin.is_python_builtin() -assert not nobuiltin.is_python_builtin() +def swig_assert(a): + if not a: + raise RuntimeError("Failed") + +swig_assert(builtin.is_python_builtin()) +swig_assert(not nobuiltin.is_python_builtin()) for i in range(1, 5): v1 = builtin.vectord([1.] * i) - assert len(v1) == i + swig_assert(len(v1) == i) v2 = nobuiltin.vectord([1.] * i) - assert len(v2) == i + swig_assert(len(v2) == i) From f2dd436a5b6c71d1c552a9238a33980f2f0bb529 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Sat, 26 Mar 2022 15:16:22 +0000 Subject: [PATCH 3/3] Rework swig_and_compile_multi_cpp makefile helper Seems less cryptic and more maintainable to me --- Examples/test-suite/common.mk | 14 ++++++++------ Examples/test-suite/python/Makefile.in | 5 ++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index 85b27979d..ddbb75d22 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -831,6 +831,13 @@ endif partialcheck: $(MAKE) check CC=true CXX=true LDSHARED=true CXXSHARED=true RUNTOOL=true COMPILETOOL=true +swig_and_compile_cpp_helper = \ + $(MAKE) -f $(top_builddir)/$(EXAMPLES)/Makefile SRCDIR='$(SRCDIR)' CXXSRCS='$(CXXSRCS)' \ + SWIG_LIB_DIR='$(SWIG_LIB_DIR)' SWIGEXE='$(SWIGEXE)' \ + LIBS='$(LIBS)' INCLUDES='$(INCLUDES)' SWIGOPT=$(2) NOLINK=true \ + TARGET="$(TARGETPREFIX)$(1)$(TARGETSUFFIX)" INTERFACEDIR='$(INTERFACEDIR)' INTERFACE="$(1).i" \ + $(LANGUAGE)$(VARIANT)_cpp + swig_and_compile_cpp = \ $(MAKE) -f $(top_builddir)/$(EXAMPLES)/Makefile SRCDIR='$(SRCDIR)' CXXSRCS='$(CXXSRCS)' \ SWIG_LIB_DIR='$(SWIG_LIB_DIR)' SWIGEXE='$(SWIGEXE)' \ @@ -847,12 +854,7 @@ swig_and_compile_c = \ swig_and_compile_multi_cpp = \ for f in `cat $(top_srcdir)/$(EXAMPLES)/$(TEST_SUITE)/$*.list` ; do \ - swigopt=$$(name=SWIGOPT_$$f; eval echo \$$$$name); \ - $(MAKE) -f $(top_builddir)/$(EXAMPLES)/Makefile SRCDIR='$(SRCDIR)' CXXSRCS='$(CXXSRCS)' \ - SWIG_LIB_DIR='$(SWIG_LIB_DIR)' SWIGEXE='$(SWIGEXE)' \ - LIBS='$(LIBS)' INCLUDES='$(INCLUDES)' SWIGOPT="$${swigopt:-$(SWIGOPT)}" NOLINK=true \ - TARGET="$(TARGETPREFIX)$${f}$(TARGETSUFFIX)" INTERFACEDIR='$(INTERFACEDIR)' INTERFACE="$$f.i" \ - $(LANGUAGE)$(VARIANT)_cpp; \ + $(call swig_and_compile_cpp_helper,$${f},'$(SWIGOPT)'); \ done swig_and_compile_external = \ diff --git a/Examples/test-suite/python/Makefile.in b/Examples/test-suite/python/Makefile.in index 57106edd1..a85b2984a 100644 --- a/Examples/test-suite/python/Makefile.in +++ b/Examples/test-suite/python/Makefile.in @@ -105,9 +105,12 @@ VALGRIND_OPT += --suppressions=pythonswig.supp # Custom tests - tests with additional commandline options #python_flatstaticmethod.cpptest: SWIGOPT += -flatstaticmethod +# Make sure just python_runtime_data_builtin.i uses the -builtin option. Note: does not use python_runtime_data.list for all steps. python_runtime_data.multicpptest: override SWIG_FEATURES := $(filter-out -builtin,$(SWIG_FEATURES)) python_runtime_data.multicpptest: override SWIGOPT := $(filter-out -builtin,$(SWIGOPT)) -python_runtime_data.multicpptest: export SWIGOPT_python_runtime_data_builtin = $(SWIGOPT) -builtin +python_runtime_data.multicpptest: swig_and_compile_multi_cpp = \ + $(call swig_and_compile_cpp_helper,python_runtime_data_builtin,'$(SWIGOPT) -builtin') && \ + $(call swig_and_compile_cpp_helper,python_runtime_data_nobuiltin,'$(SWIGOPT)') # Rules for the different types of tests %.cpptest: