From 5a4e346f534e342a13a939d9f0131ba11fc7c44a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sun, 14 May 2023 19:52:36 +0100 Subject: [PATCH 1/8] gh-104482: Fix error handling bugs in ast.c --- Python/ast.c | 7 ++++++- Python/compile.c | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Python/ast.c b/Python/ast.c index 50fc8e01fb3f69..6883d494d0bb60 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -580,7 +580,9 @@ validate_pattern(struct validator *state, pattern_ty p, int star_ok) break; } } - + if (ret == 0) { + break; + } ret = validate_patterns(state, p->v.MatchMapping.patterns, /*star_ok=*/0); break; case MatchClass_kind: @@ -620,6 +622,9 @@ validate_pattern(struct validator *state, pattern_ty p, int star_ok) } } + if (ret == 0) { + break; + } if (!validate_patterns(state, p->v.MatchClass.patterns, /*star_ok=*/0)) { ret = 0; break; diff --git a/Python/compile.c b/Python/compile.c index f8d0197e9f0682..bf5e4a52482a4a 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -567,6 +567,7 @@ PyCodeObject * _PyAST_Compile(mod_ty mod, PyObject *filename, PyCompilerFlags *pflags, int optimize, PyArena *arena) { + assert(!PyErr_Occurred()); struct compiler *c = new_compiler(mod, filename, pflags, optimize, arena); if (c == NULL) { return NULL; From 0d9d4b0a1c8057f70c87c79fdbd65cbcfa6d6318 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Sun, 14 May 2023 19:55:08 +0100 Subject: [PATCH 2/8] whitespace --- Python/ast.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/ast.c b/Python/ast.c index 6883d494d0bb60..685daf79482da3 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -621,7 +621,6 @@ validate_pattern(struct validator *state, pattern_ty p, int star_ok) break; } } - if (ret == 0) { break; } From 35dae7656bfa1c81abbb1088afe5a410fb184b89 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 14 May 2023 18:56:58 +0000 Subject: [PATCH 3/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst new file mode 100644 index 00000000000000..38121fa91a6e24 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst @@ -0,0 +1 @@ +Fix two error handling bugs in ast.c's parsing of pattern matching statements. From 72577b1253e122fc0153d77c8647c2e0cf49c36f Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 15 May 2023 18:30:47 +0100 Subject: [PATCH 4/8] add one more missing break --- Python/ast.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Python/ast.c b/Python/ast.c index 685daf79482da3..19d85a5e039267 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -613,6 +613,9 @@ validate_pattern(struct validator *state, pattern_ty p, int star_ok) break; } } + if (ret == 0) { + break; + } for (Py_ssize_t i = 0; i < asdl_seq_LEN(p->v.MatchClass.kwd_attrs); i++) { PyObject *identifier = asdl_seq_GET(p->v.MatchClass.kwd_attrs, i); @@ -624,6 +627,7 @@ validate_pattern(struct validator *state, pattern_ty p, int star_ok) if (ret == 0) { break; } + if (!validate_patterns(state, p->v.MatchClass.patterns, /*star_ok=*/0)) { ret = 0; break; From d925fff430dde6224958d9097d60c1b32f865bba Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 15 May 2023 19:24:55 +0100 Subject: [PATCH 5/8] add test to cover bad name case. Add assertions in validation functions to catch this as soon as it occurs --- Lib/test/test_ast.py | 6 ++++++ Python/ast.c | 37 +++++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py index fdd21aca06ffdd..34808ed8562e11 100644 --- a/Lib/test/test_ast.py +++ b/Lib/test/test_ast.py @@ -2035,6 +2035,12 @@ def test_stdlib_validates(self): kwd_attrs=[], kwd_patterns=[ast.MatchStar()] ), + ast.MatchClass( + constant_true, # invalid name + patterns=[], + kwd_attrs=['True'], + kwd_patterns=[pattern_1] + ), ast.MatchSequence( [ ast.MatchStar("True") diff --git a/Python/ast.c b/Python/ast.c index 19d85a5e039267..17f4663024db8a 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -46,6 +46,7 @@ static int validate_pattern(struct validator *, pattern_ty, int); static int validate_name(PyObject *name) { + assert(!PyErr_Occurred()); assert(PyUnicode_Check(name)); static const char * const forbidden[] = { "None", @@ -65,6 +66,7 @@ validate_name(PyObject *name) static int validate_comprehension(struct validator *state, asdl_comprehension_seq *gens) { + assert(!PyErr_Occurred()); Py_ssize_t i; if (!asdl_seq_LEN(gens)) { PyErr_SetString(PyExc_ValueError, "comprehension with no generators"); @@ -83,6 +85,7 @@ validate_comprehension(struct validator *state, asdl_comprehension_seq *gens) static int validate_keywords(struct validator *state, asdl_keyword_seq *keywords) { + assert(!PyErr_Occurred()); Py_ssize_t i; for (i = 0; i < asdl_seq_LEN(keywords); i++) if (!validate_expr(state, (asdl_seq_GET(keywords, i))->value, Load)) @@ -93,6 +96,7 @@ validate_keywords(struct validator *state, asdl_keyword_seq *keywords) static int validate_args(struct validator *state, asdl_arg_seq *args) { + assert(!PyErr_Occurred()); Py_ssize_t i; for (i = 0; i < asdl_seq_LEN(args); i++) { arg_ty arg = asdl_seq_GET(args, i); @@ -121,6 +125,7 @@ expr_context_name(expr_context_ty ctx) static int validate_arguments(struct validator *state, arguments_ty args) { + assert(!PyErr_Occurred()); if (!validate_args(state, args->posonlyargs) || !validate_args(state, args->args)) { return 0; } @@ -149,6 +154,7 @@ validate_arguments(struct validator *state, arguments_ty args) static int validate_constant(struct validator *state, PyObject *value) { + assert(!PyErr_Occurred()); if (value == Py_None || value == Py_Ellipsis) return 1; @@ -205,6 +211,7 @@ validate_constant(struct validator *state, PyObject *value) static int validate_expr(struct validator *state, expr_ty exp, expr_context_ty ctx) { + assert(!PyErr_Occurred()); VALIDATE_POSITIONS(exp); int ret = -1; if (++state->recursion_depth > state->recursion_limit) { @@ -465,6 +472,7 @@ ensure_literal_complex(expr_ty exp) static int validate_pattern_match_value(struct validator *state, expr_ty exp) { + assert(!PyErr_Occurred()); if (!validate_expr(state, exp, Load)) { return 0; } @@ -518,6 +526,7 @@ validate_pattern_match_value(struct validator *state, expr_ty exp) static int validate_capture(PyObject *name) { + assert(!PyErr_Occurred()); if (_PyUnicode_EqualToASCIIString(name, "_")) { PyErr_Format(PyExc_ValueError, "can't capture name '_' in patterns"); return 0; @@ -528,6 +537,7 @@ validate_capture(PyObject *name) static int validate_pattern(struct validator *state, pattern_ty p, int star_ok) { + assert(!PyErr_Occurred()); VALIDATE_POSITIONS(p); int ret = -1; if (++state->recursion_depth > state->recursion_limit) { @@ -693,6 +703,7 @@ _validate_nonempty_seq(asdl_seq *seq, const char *what, const char *owner) static int validate_assignlist(struct validator *state, asdl_expr_seq *targets, expr_context_ty ctx) { + assert(!PyErr_Occurred()); return validate_nonempty_seq(targets, "targets", ctx == Del ? "Delete" : "Assign") && validate_exprs(state, targets, ctx, 0); } @@ -700,15 +711,16 @@ validate_assignlist(struct validator *state, asdl_expr_seq *targets, expr_contex static int validate_body(struct validator *state, asdl_stmt_seq *body, const char *owner) { + assert(!PyErr_Occurred()); return validate_nonempty_seq(body, "body", owner) && validate_stmts(state, body); } static int validate_stmt(struct validator *state, stmt_ty stmt) { + assert(!PyErr_Occurred()); VALIDATE_POSITIONS(stmt); int ret = -1; - Py_ssize_t i; if (++state->recursion_depth > state->recursion_limit) { PyErr_SetString(PyExc_RecursionError, "maximum recursion depth exceeded during compilation"); @@ -779,7 +791,7 @@ validate_stmt(struct validator *state, stmt_ty stmt) case With_kind: if (!validate_nonempty_seq(stmt->v.With.items, "items", "With")) return 0; - for (i = 0; i < asdl_seq_LEN(stmt->v.With.items); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(stmt->v.With.items); i++) { withitem_ty item = asdl_seq_GET(stmt->v.With.items, i); if (!validate_expr(state, item->context_expr, Load) || (item->optional_vars && !validate_expr(state, item->optional_vars, Store))) @@ -790,7 +802,7 @@ validate_stmt(struct validator *state, stmt_ty stmt) case AsyncWith_kind: if (!validate_nonempty_seq(stmt->v.AsyncWith.items, "items", "AsyncWith")) return 0; - for (i = 0; i < asdl_seq_LEN(stmt->v.AsyncWith.items); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(stmt->v.AsyncWith.items); i++) { withitem_ty item = asdl_seq_GET(stmt->v.AsyncWith.items, i); if (!validate_expr(state, item->context_expr, Load) || (item->optional_vars && !validate_expr(state, item->optional_vars, Store))) @@ -803,7 +815,7 @@ validate_stmt(struct validator *state, stmt_ty stmt) || !validate_nonempty_seq(stmt->v.Match.cases, "cases", "Match")) { return 0; } - for (i = 0; i < asdl_seq_LEN(stmt->v.Match.cases); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(stmt->v.Match.cases); i++) { match_case_ty m = asdl_seq_GET(stmt->v.Match.cases, i); if (!validate_pattern(state, m->pattern, /*star_ok=*/0) || (m->guard && !validate_expr(state, m->guard, Load)) @@ -838,7 +850,7 @@ validate_stmt(struct validator *state, stmt_ty stmt) PyErr_SetString(PyExc_ValueError, "Try has orelse but no except handlers"); return 0; } - for (i = 0; i < asdl_seq_LEN(stmt->v.Try.handlers); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(stmt->v.Try.handlers); i++) { excepthandler_ty handler = asdl_seq_GET(stmt->v.Try.handlers, i); VALIDATE_POSITIONS(handler); if ((handler->v.ExceptHandler.type && @@ -864,7 +876,7 @@ validate_stmt(struct validator *state, stmt_ty stmt) PyErr_SetString(PyExc_ValueError, "TryStar has orelse but no except handlers"); return 0; } - for (i = 0; i < asdl_seq_LEN(stmt->v.TryStar.handlers); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(stmt->v.TryStar.handlers); i++) { excepthandler_ty handler = asdl_seq_GET(stmt->v.TryStar.handlers, i); if ((handler->v.ExceptHandler.type && !validate_expr(state, handler->v.ExceptHandler.type, Load)) || @@ -924,8 +936,8 @@ validate_stmt(struct validator *state, stmt_ty stmt) static int validate_stmts(struct validator *state, asdl_stmt_seq *seq) { - Py_ssize_t i; - for (i = 0; i < asdl_seq_LEN(seq); i++) { + assert(!PyErr_Occurred()); + for (Py_ssize_t i = 0; i < asdl_seq_LEN(seq); i++) { stmt_ty stmt = asdl_seq_GET(seq, i); if (stmt) { if (!validate_stmt(state, stmt)) @@ -943,8 +955,8 @@ validate_stmts(struct validator *state, asdl_stmt_seq *seq) static int validate_exprs(struct validator *state, asdl_expr_seq *exprs, expr_context_ty ctx, int null_ok) { - Py_ssize_t i; - for (i = 0; i < asdl_seq_LEN(exprs); i++) { + assert(!PyErr_Occurred()); + for (Py_ssize_t i = 0; i < asdl_seq_LEN(exprs); i++) { expr_ty expr = asdl_seq_GET(exprs, i); if (expr) { if (!validate_expr(state, expr, ctx)) @@ -963,8 +975,8 @@ validate_exprs(struct validator *state, asdl_expr_seq *exprs, expr_context_ty ct static int validate_patterns(struct validator *state, asdl_pattern_seq *patterns, int star_ok) { - Py_ssize_t i; - for (i = 0; i < asdl_seq_LEN(patterns); i++) { + assert(!PyErr_Occurred()); + for (Py_ssize_t i = 0; i < asdl_seq_LEN(patterns); i++) { pattern_ty pattern = asdl_seq_GET(patterns, i); if (!validate_pattern(state, pattern, star_ok)) { return 0; @@ -980,6 +992,7 @@ validate_patterns(struct validator *state, asdl_pattern_seq *patterns, int star_ int _PyAST_Validate(mod_ty mod) { + assert(!PyErr_Occurred()); int res = -1; struct validator state; PyThreadState *tstate; From 9dd4fee833938c02348abd9080b2c1d27fb30642 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Mon, 15 May 2023 19:39:18 +0100 Subject: [PATCH 6/8] Update news --- .../2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst index 38121fa91a6e24..71c66123682269 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst @@ -1 +1 @@ -Fix two error handling bugs in ast.c's parsing of pattern matching statements. +Fix three error handling bugs in ast.c's parsing of pattern matching statements. From 43a9d47e3db9563a804a225873bd1f8bb3ce0732 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 15 May 2023 20:45:43 +0100 Subject: [PATCH 7/8] missed a few loops --- Python/ast.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Python/ast.c b/Python/ast.c index 17f4663024db8a..f079e64bbdfc00 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -67,12 +67,11 @@ static int validate_comprehension(struct validator *state, asdl_comprehension_seq *gens) { assert(!PyErr_Occurred()); - Py_ssize_t i; if (!asdl_seq_LEN(gens)) { PyErr_SetString(PyExc_ValueError, "comprehension with no generators"); return 0; } - for (i = 0; i < asdl_seq_LEN(gens); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(gens); i++) { comprehension_ty comp = asdl_seq_GET(gens, i); if (!validate_expr(state, comp->target, Store) || !validate_expr(state, comp->iter, Load) || @@ -86,8 +85,7 @@ static int validate_keywords(struct validator *state, asdl_keyword_seq *keywords) { assert(!PyErr_Occurred()); - Py_ssize_t i; - for (i = 0; i < asdl_seq_LEN(keywords); i++) + for (Py_ssize_t i = 0; i < asdl_seq_LEN(keywords); i++) if (!validate_expr(state, (asdl_seq_GET(keywords, i))->value, Load)) return 0; return 1; @@ -97,8 +95,7 @@ static int validate_args(struct validator *state, asdl_arg_seq *args) { assert(!PyErr_Occurred()); - Py_ssize_t i; - for (i = 0; i < asdl_seq_LEN(args); i++) { + for (Py_ssize_t i = 0; i < asdl_seq_LEN(args); i++) { arg_ty arg = asdl_seq_GET(args, i); VALIDATE_POSITIONS(arg); if (arg->annotation && !validate_expr(state, arg->annotation, Load)) From 8370aefb19f8cf82f824db51991717c5a7f6f1a2 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Mon, 15 May 2023 21:11:51 +0100 Subject: [PATCH 8/8] fix news wording --- .../2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst index 71c66123682269..07c09a3a84a62c 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-05-14-18-56-54.gh-issue-104482.yaQsv8.rst @@ -1 +1 @@ -Fix three error handling bugs in ast.c's parsing of pattern matching statements. +Fix three error handling bugs in ast.c's validation of pattern matching statements.