Backport rev. 51972:

Bug #1557232: fix seg fault with def f((((x)))) and def f(((x),)).

These tests should be improved.  Hopefully this fixes variations when
flipping back and forth between fpdef and fplist.

Lib/test/test_complex_args.py

@@ -0,0 +1,91 @@
+
+import unittest
+from test import test_support
+
+class ComplexArgsTestCase(unittest.TestCase):
+
+    def check(self, func, expected, *args):
+        self.assertEqual(func(*args), expected)
+
+    # These functions are tested below as lambdas too.  If you add a function test,
+    # also add a similar lambda test.
+
+    def test_func_parens_no_unpacking(self):
+        def f(((((x))))): return x
+        self.check(f, 1, 1)
+        # Inner parens are elided, same as: f(x,)
+        def f(((x)),): return x
+        self.check(f, 2, 2)
+
+    def test_func_1(self):
+        def f(((((x),)))): return x
+        self.check(f, 3, (3,))
+        def f(((((x)),))): return x
+        self.check(f, 4, (4,))
+        def f(((((x))),)): return x
+        self.check(f, 5, (5,))
+        def f(((x),)): return x
+        self.check(f, 6, (6,))
+
+    def test_func_2(self):
+        def f(((((x)),),)): return x
+        self.check(f, 2, ((2,),))
+
+    def test_func_3(self):
+        def f((((((x)),),),)): return x
+        self.check(f, 3, (((3,),),))
+
+    def test_func_complex(self):
+        def f((((((x)),),),), a, b, c): return x, a, b, c
+        self.check(f, (3, 9, 8, 7), (((3,),),), 9, 8, 7)
+
+        def f(((((((x)),)),),), a, b, c): return x, a, b, c
+        self.check(f, (3, 9, 8, 7), (((3,),),), 9, 8, 7)
+
+        def f(a, b, c, ((((((x)),)),),)): return a, b, c, x
+        self.check(f, (9, 8, 7, 3), 9, 8, 7, (((3,),),))
+
+    # Duplicate the tests above, but for lambda.  If you add a lambda test,
+    # also add a similar function test above.
+
+    def test_lambda_parens_no_unpacking(self):
+        f = lambda (((((x))))): x
+        self.check(f, 1, 1)
+        # Inner parens are elided, same as: f(x,)
+        f = lambda ((x)),: x
+        self.check(f, 2, 2)
+
+    def test_lambda_1(self):
+        f = lambda (((((x),)))): x
+        self.check(f, 3, (3,))
+        f = lambda (((((x)),))): x
+        self.check(f, 4, (4,))
+        f = lambda (((((x))),)): x
+        self.check(f, 5, (5,))
+        f = lambda (((x),)): x
+        self.check(f, 6, (6,))
+
+    def test_lambda_2(self):
+        f = lambda (((((x)),),)): x
+        self.check(f, 2, ((2,),))
+
+    def test_lambda_3(self):
+        f = lambda ((((((x)),),),)): x
+        self.check(f, 3, (((3,),),))
+
+    def test_lambda_complex(self):
+        f = lambda (((((x)),),),), a, b, c: (x, a, b, c)
+        self.check(f, (3, 9, 8, 7), (((3,),),), 9, 8, 7)
+
+        f = lambda ((((((x)),)),),), a, b, c: (x, a, b, c)
+        self.check(f, (3, 9, 8, 7), (((3,),),), 9, 8, 7)
+
+        f = lambda a, b, c, ((((((x)),)),),): (a, b, c, x)
+        self.check(f, (9, 8, 7, 3), 9, 8, 7, (((3,),),))
+
+
+def test_main():
+    test_support.run_unittest(ComplexArgsTestCase)
+
+if __name__ == "__main__":
+    test_main()

Misc/NEWS

@@ -16,6 +16,8 @@ Core and builtins
   not being recognized as a keyword after, e.g., this statement:
   from __future__ import division, with_statement
 
+- Bug #1557232: fix seg fault with def f((((x)))) and def f(((x),)).
+
 - Fix %zd string formatting on Mac OS X so it prints negative numbers.
 
 - Allow exception instances to be directly sliced again.

Python/ast.c

@@ -566,10 +566,17 @@ compiler_complex_args(struct compiling *c, const node *n)
     if (!args)
         return NULL;
 
+    /* fpdef: NAME | '(' fplist ')'
+       fplist: fpdef (',' fpdef)* [',']
+    */
     REQ(n, fplist);
     for (i = 0; i < len; i++) {
-        const node *child = CHILD(CHILD(n, 2*i), 0);
+        const node *fpdef_node = CHILD(n, 2*i);
+        const node *child;
         expr_ty arg;
+set_name:
+        /* fpdef_node is either a NAME or an fplist */
+        child = CHILD(fpdef_node, 0);
         if (TYPE(child) == NAME) {
     		if (!strcmp(STR(child), "None")) {
 	    		ast_error(child, "assignment to None");
@@ -579,7 +586,17 @@ compiler_complex_args(struct compiling *c, const node *n)
                        child->n_col_offset, c->c_arena);
 	    }
         else {
-            arg = compiler_complex_args(c, CHILD(CHILD(n, 2*i), 1));
+            assert(TYPE(fpdef_node) == fpdef);
+            /* fpdef_node[0] is not a name, so it must be a '(', get CHILD[1] */
+            child = CHILD(fpdef_node, 1);
+            assert(TYPE(child) == fplist);
+            /* NCH == 1 means we have (x), we need to elide the extra parens */
+            if (NCH(child) == 1) {
+                fpdef_node = CHILD(child, 0);
+                assert(TYPE(fpdef_node) == fpdef);
+                goto set_name;
+            }
+            arg = compiler_complex_args(c, child);
         }
         asdl_seq_SET(args, i, arg);
     }
@@ -637,6 +654,7 @@ ast_for_arguments(struct compiling *c, const node *n)
 	ch = CHILD(n, i);
 	switch (TYPE(ch)) {
             case fpdef:
+            handle_fpdef:
                 /* XXX Need to worry about checking if TYPE(CHILD(n, i+1)) is
                    anything other than EQUAL or a comma? */
                 /* XXX Should NCH(n) check be made a separate check? */
@@ -662,7 +680,11 @@ ast_for_arguments(struct compiling *c, const node *n)
 			asdl_seq_SET(args, k++, compiler_complex_args(c, ch));
 		    } else {
 			/* def foo((x)): setup for checking NAME below. */
+			/* Loop because there can be many parens and tuple
+			   upacking mixed in. */
 			ch = CHILD(ch, 0);
+			assert(TYPE(ch) == fpdef);
+			goto handle_fpdef;
 		    }
                 }
                 if (TYPE(CHILD(ch, 0)) == NAME) {