[C++-sig] Segfault with keyword arguments and overloads.

[Alex Mohr]

Oh!  I did miss your patch, sorry.  Thanks!

It looks to me like your patch calls PyTuple_GET_ITEM() with None as the 
python object (when kv is None).  The python docs for PyTuple_GET_ITEM 
say, "Like PyTuple_GetItem(), but does no checking of its arguments." 
To me that means that the object you pass really needs to be a tuple.

Here's the patch I'm using -- it simply checks to see if kv is None, 
which is true if the overload doesn't accept a keyword arg in that 
position, and if so it simply rejects the overload.  This is working for 
me in the example and across our codebase and testsuite.

I will file a bug as you suggest, and thanks again.

Alex

--- ./boost_1_53_0/libs/python/src/object/function.cpp.orig	2013-07-22 
17:38:54.000000000 -0700
+++ ./boost_1_53_0/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


On 8/8/2013 2:28 AM, Alex Leach wrote:
> Hi Alex,
>
> On Wed, 07 Aug 2013 18:06:24 +0100, Alex Mohr <amohr at pixar.com> wrote:
>
>> Thanks for your response.  I've responded to your comments in-line below.
>>
>> After further investigation, I believe this can be fixed by simply
>> checking for None when we get 'kv' from f->m_arg_names while resolving
>> keywords.  If we encounter None, that means the overload doesn't
>> accept a keyword argument in that position (so not enough positional
>> args were passed), and we can reject the overload.
>>
>> If it works, I think that's a simple, targeted fix that isn't
>> particularly risky.  I'll try to work up a patch.
>>
>
>
> Sounds like you missed it, but I did add a fairly simple patch to my
> previous email. I used your code as a test case and it worked fine; no
> seg faults. I've copied it again below...
>
> I don't have commit privileges to Boost Python, but I think from your
> explanations, that this is a valid bug in BP. So it would probably be
> worthwhile filing a bug report, at
> http://www.boost.org/development/bugs.html
>
> Unless someone with commit privileges wants to consider merging it? My
> only thought on the patch regards return value optimisation. Would
> moving the `PyObject *` declarations out of the loop impair the
> compiler's ability to perform RVO?
>
>
> Hope that helps anyway.
>
> Kind regards,
> Alex
>
>
> --- ./boost_1_53_0/libs/python/src/object/function.cpp.orig
> 2013-08-07 11:02:28.569236835 +0100
> +++ ./boost_1_53_0/libs/python/src/object/function.cpp  2013-08-07
> 11:56:10.926045853 +0100
> @@ -177,16 +177,18 @@
>                           // Grab remaining arguments by name from the
> keyword dictionary
>                           std::size_t n_actual_processed =
> n_unnamed_actual;
>
> +                        PyObject *kv, *k, *value;
>                           for (std::size_t arg_pos = n_unnamed_actual;
> arg_pos < max_arity ; ++arg_pos)
>                           {
>                               // Get the keyword[, value pair]
> corresponding
> -                            PyObject* kv =
> PyTuple_GET_ITEM(f->m_arg_names.ptr(), arg_pos);
> +                            kv = PyTuple_GET_ITEM(f->m_arg_names.ptr(),
> arg_pos);
>
>                               // If there were any keyword arguments,
>                               // look up the one we need for this
>                               // argument position
> -                            PyObject* value = n_keyword_actual
> -                                ? PyDict_GetItem(keywords,
> PyTuple_GET_ITEM(kv, 0))
> +                            k = PyTuple_GET_ITEM(kv, 0);
> +                            value = (k != NULL && n_keyword_actual)
> +                                ? PyDict_GetItem(keywords, k)
>                                   : 0;
>
>                               if (!value)