From 3a23017bb2309f6ba44090fb322f9681bf49c4c3 Mon Sep 17 00:00:00 2001 From: Neal Norwitz Date: Fri, 22 Sep 2006 08:18:10 +0000 Subject: 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. Backport candidate. --- Lib/test/test_complex_args.py | 91 +++++++++++++++++++++++++++++++++++++++++++ Misc/NEWS | 2 + Python/ast.c | 26 ++++++++++++- 3 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 Lib/test/test_complex_args.py diff --git a/Lib/test/test_complex_args.py b/Lib/test/test_complex_args.py new file mode 100644 index 0000000..c6d50a9 --- /dev/null +++ b/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() diff --git a/Misc/NEWS b/Misc/NEWS index e93a80e..d7f1e75 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,8 @@ What's New in Python 2.6 alpha 1? Core and builtins ----------------- +- 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. diff --git a/Python/ast.c b/Python/ast.c index 9e0c184..f490268 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -560,10 +560,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"); @@ -573,7 +580,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); } @@ -631,6 +648,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? */ @@ -656,7 +674,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 + unpacking mixed in. */ ch = CHILD(ch, 0); + assert(TYPE(ch) == fpdef); + goto handle_fpdef; } } if (TYPE(CHILD(ch, 0)) == NAME) { -- cgit v0.12