Fix undefined behaviour in directorout typemaps for void * and const char *&

if the same wrapped function is called more than once. Note that using returning
pointers in directors is still full of traps and not recommended. As such,
warning SWIGWARN_TYPEMAP_DIRECTOROUT_PTR (473) continues to be issued.

The debug Python interpreter failed 3 director testcases which have been
modified in this patch to work around unrecommended director usage
returning from director methods. These are C strings and std::string_view.

A Pyton reference count leak is chosen over undefined behaviour for
returning std::string_view.
This commit is contained in:
William S Fulton 2025-05-14 22:10:39 +01:00
parent 6353914211
commit 53f4751ffa
7 changed files with 40 additions and 13 deletions

View File

@ -7,6 +7,12 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
Version 4.4.0 (in progress)
===========================
2025-05-13: wsfulton
Fix undefined behaviour in directorout typemaps for void * and const char *& if
the same wrapped function is called more than once. Note that using returning
pointers in directors is still full of traps and not recommended. As such,
warning SWIGWARN_TYPEMAP_DIRECTOROUT_PTR (473) continues to be issued.
2025-05-11: wsfulton
[Python] Move the SwigPyObject, SwigPyPacked, SwigVarLink (renamed from
swigvarlink) support Python types to the SWIG runtime module (currently called

View File

@ -328,7 +328,7 @@ macro(size_t, pfx, sizet)
a << pfx##_##name;
b << def_##name;
if (a.str() != b.str()) {
std::cout << "failing in pfx""_""name : "
std::cout << "var_check failing in pfx""_""name : "
<< a.str() << " : " << b.str() << std::endl;
}
}
@ -339,7 +339,7 @@ macro(size_t, pfx, sizet)
a << pfx##_##name;
b << def_##name;
if (a.str() != b.str()) {
std::cout << "failing in pfx""_""name : "
std::cout << "var_array_check failing in pfx""_""name : "
<< a.str() << " : " << b.str() << std::endl;
}
%enddef
@ -352,7 +352,7 @@ macro(size_t, pfx, sizet)
a << pfx##_##name(pfx##_##tmp##name);
b << def_##name;
if (a.str() != b.str()) {
std::cout << "failing in pfx""_""name : "
std::cout << "call_check failing in pfx""_""name : "
<< a.str() << " : " << b.str() << std::endl;
}
}

View File

@ -49,7 +49,13 @@ class C(FooBar_int):
return 2
def get_name(self):
return FooBar_int.get_name(self) + " hello"
# We suppressed the warning:
# Warning 473: Returning a reference, pointer or pointer wrapper in a director method is not recommended.
# An extra reference to the returned string is now required to keep it from being deleted as the C code
# uses a pointer to the internal Python string which would otherwise be deleted and then the pointer would
# point to garbage.
self.hack_extra_reference = FooBar_int.get_name(self) + " hello"
return self.hack_extra_reference
pass

View File

@ -83,6 +83,13 @@ class PyTest (TestDirector):
def ident(self, x):
return x
# The warning for const char * and const char *& warning is suppressed:
# Warning 473: Returning a reference, pointer or pointer wrapper in a director method is not recommended.
# ident_extra_ref is a hack to add an extra reference in order to avoid returning a pointer to deleted memory.
def ident_extra_ref(self, x):
self.extra_ref = x
return x
def vval_bool(self, x): return self.ident(x)
def vval_schar(self, x): return self.ident(x)
@ -111,9 +118,9 @@ class PyTest (TestDirector):
def vval_char(self, x): return self.ident(x)
def vval_pchar(self, x): return self.ident(x)
def vval_pchar(self, x): return self.ident_extra_ref(x)
def vval_pcharc(self, x): return self.ident(x)
def vval_pcharc(self, x): return self.ident_extra_ref(x)
def vval_pint(self, x): return self.ident(x)
@ -151,9 +158,9 @@ class PyTest (TestDirector):
def vref_char(self, x): return self.ident(x)
def vref_pchar(self, x): return self.ident(x)
def vref_pchar(self, x): return self.ident_extra_ref(x)
def vref_pcharc(self, x): return self.ident(x)
def vref_pcharc(self, x): return self.ident_extra_ref(x)
def vref_pint(self, x): return self.ident(x)

View File

@ -87,10 +87,16 @@ namespace std {
PyObject *bytes = NULL;
if (PyUnicode_Check($input)) {
p = SWIG_PyUnicode_AsUTF8AndSize($input, &len, &bytes);
// Avoid undefined behaviour (p will be pointing to a temporary
// if bytes is not NULL which happens when Py_LIMITED_API is defined
// and < 0x030A0000) and just leak by not calling Py_XDECREF.
// Avoid undefined behaviour by leaking, macros match those in SWIG_PyUnicode_AsUTF8AndSize
%#if PY_VERSION_HEX >= 0x03030000
%# if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000
SWIG_Py_XINCREF($input);
%# else
// Py_XDECREF(bytes);
%# endif
%#else
// Py_XDECREF(bytes);
%#endif
} else {
p = PyBytes_AsString($input);
if (p) len = PyBytes_Size($input);

View File

@ -192,7 +192,8 @@
if (!SWIG_IsOK(res)) {
%dirout_fail(res, "$type");
}
static $*1_ltype tmp = buf;
static $*1_ltype tmp;
tmp = buf;
$result = &tmp;
if (alloc == SWIG_NEWOBJ) {
swig_acquire_ownership_array(buf);

View File

@ -74,7 +74,8 @@
if (!SWIG_IsOK(res)) {
%dirout_fail(res,"$type");
}
static $*ltype temp = %reinterpret_cast(argp, $*ltype);
static $*ltype temp;
temp = %reinterpret_cast(argp, $*ltype);
$result = &temp;
}