Opened 9 years ago

Last modified 9 years ago

#8978 new Bugs

Segfault crash in boost.python with keyword arguments and overloads.

Reported by: amohr@… Owned by: Ralf W. Grosse-Kunstleve
Milestone: To Be Determined Component: python USE GITHUB
Version: Boost 1.51.0 Severity: Showstopper
Keywords: Cc:

Description

Please see the thread on the python c++ sig email list here (lacking protocol prefix to pass spam check): mail.python.org/pipermail/cplusplus-sig/2013-August/017012.html

Here is a minimal repro case:


C++ #include <boost/python.hpp>

static void f1(int a0, int a1) { } static void f2(int a0, int a1, int a2) { }

BOOST_PYTHON_MODULE(kwargCrash) {

boost::python::def("f", f1, (arg("a1")=2)); boost::python::def("f", f2, (arg("a2")=2));

}

# Python import kwargCrash kwargCrash.f(0, a1=2)


Version found info: boost 1.51.0, Python 2.7.5, gcc 4.8.1.

Please see the list thread linked above for a detailed explanation of why the crash occurs. In short, boost.python calls PyTuple_GET_ITEM() on None, and then unconditionally uses the result as a key to look up in a dict. This boost.python code hasn't changed in a long time. I suspect Python has changed, and is now returning NULL here instead of None, and calling PyDict_GetItem with NULL crashes.

Here is a patch to the boost.python code that fixes the bug. It simply checks to see if it got None from the m_arg_names tuple, and if so, that means the overload does not accept a keyword arg in that position, so it rejects the overload. This fixes the crash and works correctly across our codebase and testsuite.

If the patch looks good, it would be great to apply it and add the repro case to the test suite.

Thanks!

--- ./libs/python/src/object/function.cpp.orig 2013-07-22 17:38:54.000000000 -0700 +++ ./libs/python/src/object/function.cpp 2013-08-07 10:25:26.963988000 -0700 @@ -182,6 +182,16 @@

Get the keyword[, value pair] corresponding PyObject* kv = PyTuple_GET_ITEM(f->m_arg_names.ptr(), arg_pos);

+ If kv is None, this overload does not accept a + keyword argument in this position, meaning that + the caller did not supply enough positional + arguments. Reject the overload. + if (kv == Py_None) { + PyErr_Clear(); + inner_args = handle<>(); + break; + } +

If there were any keyword arguments, look up the one we need for this argument position

Change History (1)

comment:1 by amohr@…, 9 years ago

Oh man, I didn't realize it would format my input. Here's the repro and patch again, hopefully better formatted.

// C++
#include <boost/python.hpp>

static void f1(int a0, int a1) { }
static void f2(int a0, int a1, int a2) { }

BOOST_PYTHON_MODULE(kwargCrash) {
    boost::python::def("f", f1, (arg("a1")=2));
    boost::python::def("f", f2, (arg("a2")=2));
}

# Python
import kwargCrash
kwargCrash.f(0, a1=2) 


--- ./libs/python/src/object/function.cpp.orig	2013-07-22 17:38:54.000000000 -0700
+++ ./libs/python/src/object/function.cpp	2013-08-07 10:25:26.963988000 -0700
@@ -182,6 +182,16 @@
                             // Get the keyword[, value pair] corresponding
                             PyObject* kv = PyTuple_GET_ITEM(f->m_arg_names.ptr(), arg_pos);
 
+                            // If kv is None, this overload does not accept a
+                            // keyword argument in this position, meaning that
+                            // the caller did not supply enough positional
+                            // arguments.  Reject the overload.
+                            if (kv == Py_None) {
+                                PyErr_Clear();
+                                inner_args = handle<>();
+                                break;
+                            }
+
                             // If there were any keyword arguments,
                             // look up the one we need for this
                             // argument position


Note: See TracTickets for help on using tickets.