From 010669fc2bd792fc60311a9e14d52bc9a4b86092 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Wed, 6 Jan 2021 14:24:58 +0200 Subject: [PATCH 01/10] bpo-42827: Fix crash on SyntaxError in multiline expressions When trying to extract the error line for the error message there are two distinct cases: 1. The input comes from a file, which means that we can extract the error line by using `PyErr_ProgramTextObject` and which we already do. 2. The input does not come from a file, at which point we need to get the source code from the tokenizer: * If the tokenizer's current line number is the same with the line of the error, we get the line from `tok->buf` and we're ready. * Else, we can extract the error line from the source code in the following two ways: * If the input comes from a string we have all the input in `tok->str` and we can extract the error line from it. * If the input comes from stdin, i.e. the interactive prompt, we do not have access to the previous line. That's why a new field `tok->stdin_content` is added which holds the whole input for the current (multiline) statement or expression. We can then extract the error line from `tok->stdin_content` like we do in the string case above. --- Lib/test/test_exceptions.py | 3 +++ Parser/pegen.c | 24 ++++++++++++++++++++++-- Parser/tokenizer.c | 20 ++++++++++++++++++++ Parser/tokenizer.h | 1 + 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 864422390ad3025..eb70d7b4e497241 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -209,6 +209,9 @@ def testSyntaxErrorOffset(self): check('x = "a', 1, 7) check('lambda x: x = 2', 1, 1) check('f{a + b + c}', 1, 2) + check('[file for str(file) in []\n])', 1, 11) + check('[\nfile\nfor str(file)\nin\n[]\n]', 3, 5) + check('[file for\n str(file) in []]', 2, 2) # Errors thrown by compile.c check('class foo:return 1', 1, 11) diff --git a/Parser/pegen.c b/Parser/pegen.c index 188fd282b760436..ad90b16b669222c 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -380,6 +380,21 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...) return NULL; } +PyObject * +get_error_line(Parser *p, int lineno) +{ + char *cur_line = p->tok->fp == NULL ? p->tok->str : p->tok->stdin_content; + for (int i = 0; i < lineno - 1; i++) { + cur_line = strchr(cur_line, '\n') + 1; + } + + char *next_newline; + if ((next_newline = strchr(cur_line, '\n')) == NULL) { // This is the last line + return PyUnicode_DecodeUTF8(cur_line, strlen(cur_line), "replace"); + } + return PyUnicode_DecodeUTF8(cur_line, next_newline - cur_line, "replace"); +} + void * _PyPegen_raise_error_known_location(Parser *p, PyObject *errtype, Py_ssize_t lineno, Py_ssize_t col_offset, @@ -416,8 +431,13 @@ _PyPegen_raise_error_known_location(Parser *p, PyObject *errtype, } if (!error_line) { - Py_ssize_t size = p->tok->inp - p->tok->buf; - error_line = PyUnicode_DecodeUTF8(p->tok->buf, size, "replace"); + if (p->tok->lineno == lineno) { + Py_ssize_t size = p->tok->inp - p->tok->buf; + error_line = PyUnicode_DecodeUTF8(p->tok->buf, size, "replace"); + } + else { + error_line = get_error_line(p, lineno); + } if (!error_line) { goto error; } diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c index 96539bd556529aa..eca8025366c4aa2 100644 --- a/Parser/tokenizer.c +++ b/Parser/tokenizer.c @@ -81,6 +81,7 @@ tok_new(void) tok->decoding_readline = NULL; tok->decoding_buffer = NULL; tok->type_comments = 0; + tok->stdin_content = NULL; tok->async_hacks = 0; tok->async_def = 0; @@ -856,6 +857,25 @@ tok_nextc(struct tok_state *tok) if (translated == NULL) return EOF; newtok = translated; + if (tok->stdin_content == NULL) { + tok->stdin_content = PyMem_Malloc(strlen(translated) + 1); + if (tok->stdin_content == NULL) { + tok->done = E_NOMEM; + return EOF; + } + strcpy(tok->stdin_content, translated); + tok->stdin_content[strlen(translated)] = 0; + } + else { + char *new_str = PyMem_Malloc(strlen(tok->stdin_content) + strlen(translated) + 1); + if (new_str == NULL) { + tok->done = E_NOMEM; + return EOF; + } + sprintf(new_str, "%s%s", tok->stdin_content, translated); + PyMem_Free(tok->stdin_content); + tok->stdin_content = new_str; + } } if (tok->encoding && newtok && *newtok) { /* Recode to UTF-8 */ diff --git a/Parser/tokenizer.h b/Parser/tokenizer.h index 5660ea38e9443d4..b659f34796e4241 100644 --- a/Parser/tokenizer.h +++ b/Parser/tokenizer.h @@ -37,6 +37,7 @@ struct tok_state { int atbol; /* Nonzero if at begin of new line */ int pendin; /* Pending indents (if > 0) or dedents (if < 0) */ const char *prompt, *nextprompt; /* For interactive prompting */ + char *stdin_content; int lineno; /* Current line number */ int first_lineno; /* First line of a single line or multi line string expression (cf. issue 16806) */ From c9285266db52df1a1bfcea0804b64abfefc91675 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Wed, 6 Jan 2021 16:12:37 +0200 Subject: [PATCH 02/10] Remove smelly symbol --- Parser/pegen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index ad90b16b669222c..9cfcc74cc3f2a5c 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -380,7 +380,7 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...) return NULL; } -PyObject * +static PyObject * get_error_line(Parser *p, int lineno) { char *cur_line = p->tok->fp == NULL ? p->tok->str : p->tok->stdin_content; From 5b9dfed39277d443e6ecb48de52d0e69720c0025 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Wed, 6 Jan 2021 17:06:43 +0200 Subject: [PATCH 03/10] Add news blurb --- .../Core and Builtins/2021-01-06-17-06-37.bpo-42827.jtRR0D.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-01-06-17-06-37.bpo-42827.jtRR0D.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-01-06-17-06-37.bpo-42827.jtRR0D.rst b/Misc/NEWS.d/next/Core and Builtins/2021-01-06-17-06-37.bpo-42827.jtRR0D.rst new file mode 100644 index 000000000000000..8e40ab6a65341a3 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-01-06-17-06-37.bpo-42827.jtRR0D.rst @@ -0,0 +1,2 @@ +Fix a crash when working out the error line of a :exc:`SyntaxError` in some +multi-line expressions. From bdfc2a4df2cff9e2be8be13fd71f78e35f1283b1 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Thu, 7 Jan 2021 20:10:09 +0200 Subject: [PATCH 04/10] Add PyMem_Free for the new field in PyTokenizer_Free --- Parser/tokenizer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c index eca8025366c4aa2..bf3e09e0d3278ad 100644 --- a/Parser/tokenizer.c +++ b/Parser/tokenizer.c @@ -817,6 +817,8 @@ PyTokenizer_Free(struct tok_state *tok) PyMem_Free(tok->buf); if (tok->input) PyMem_Free(tok->input); + if (tok->stdin_content) + PyMem_Free(tok->stdin_content); PyMem_Free(tok); } From 75e714ca64b45e73ac41ec52ab28e05315d47b52 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 8 Jan 2021 00:24:00 +0200 Subject: [PATCH 05/10] Update lineno type to Py_ssize_t Co-authored-by: Pablo Galindo --- Parser/pegen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index 9cfcc74cc3f2a5c..cd49d8624ae0793 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -381,7 +381,7 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...) } static PyObject * -get_error_line(Parser *p, int lineno) +get_error_line(Parser *p, Py_ssize_t lineno) { char *cur_line = p->tok->fp == NULL ? p->tok->str : p->tok->stdin_content; for (int i = 0; i < lineno - 1; i++) { From 78a0f9c8125060785562587faafb0035aa56884b Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 8 Jan 2021 00:55:11 +0200 Subject: [PATCH 06/10] Address feedback --- Parser/pegen.c | 8 +++++++- Parser/tokenizer.c | 3 +-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index cd49d8624ae0793..6131c1a8ca6a686 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -383,6 +383,10 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...) static PyObject * get_error_line(Parser *p, Py_ssize_t lineno) { + // If p->tok->fp == NULL, the we're parsing from a string, which means that + // the whole source is stored in p->tok->str. If not, then we're parsing + // from the REPL, so the source lines of the current (multi-line) statement + // are stored in p->tok->stdin_content char *cur_line = p->tok->fp == NULL ? p->tok->str : p->tok->stdin_content; for (int i = 0; i < lineno - 1; i++) { cur_line = strchr(cur_line, '\n') + 1; @@ -390,7 +394,7 @@ get_error_line(Parser *p, Py_ssize_t lineno) char *next_newline; if ((next_newline = strchr(cur_line, '\n')) == NULL) { // This is the last line - return PyUnicode_DecodeUTF8(cur_line, strlen(cur_line), "replace"); + next_newline = cur_line + strlen(cur_line); } return PyUnicode_DecodeUTF8(cur_line, next_newline - cur_line, "replace"); } @@ -431,6 +435,8 @@ _PyPegen_raise_error_known_location(Parser *p, PyObject *errtype, } if (!error_line) { + // PyErr_ProgramTextObject returned NULL, so we're not parsing from a file + assert(p->tok->fp == NULL || p->tok->fp == stdin); if (p->tok->lineno == lineno) { Py_ssize_t size = p->tok->inp - p->tok->buf; error_line = PyUnicode_DecodeUTF8(p->tok->buf, size, "replace"); diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c index bf3e09e0d3278ad..62cd2966231b8a3 100644 --- a/Parser/tokenizer.c +++ b/Parser/tokenizer.c @@ -865,8 +865,7 @@ tok_nextc(struct tok_state *tok) tok->done = E_NOMEM; return EOF; } - strcpy(tok->stdin_content, translated); - tok->stdin_content[strlen(translated)] = 0; + sprintf(tok->stdin_content, "%s", translated); } else { char *new_str = PyMem_Malloc(strlen(tok->stdin_content) + strlen(translated) + 1); From 1fbb092f0c5cb1bf03ff00e9b0b77595f993d931 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 8 Jan 2021 02:25:42 +0200 Subject: [PATCH 07/10] Remove assertion in raise error function --- Parser/pegen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index 6131c1a8ca6a686..50c7c3c519dba04 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -435,8 +435,8 @@ _PyPegen_raise_error_known_location(Parser *p, PyObject *errtype, } if (!error_line) { - // PyErr_ProgramTextObject returned NULL, so we're not parsing from a file - assert(p->tok->fp == NULL || p->tok->fp == stdin); + // PyErr_ProgramTextObject returned NULL, so it either failed or we're + // not parsing from a file if (p->tok->lineno == lineno) { Py_ssize_t size = p->tok->inp - p->tok->buf; error_line = PyUnicode_DecodeUTF8(p->tok->buf, size, "replace"); From c37af495cfefeb2c0a3bee9be4bef3a263a57425 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 8 Jan 2021 21:07:56 +0200 Subject: [PATCH 08/10] Fix comment Co-authored-by: Irit Katriel --- Parser/pegen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index 50c7c3c519dba04..a441ecadce6b74b 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -383,7 +383,7 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...) static PyObject * get_error_line(Parser *p, Py_ssize_t lineno) { - // If p->tok->fp == NULL, the we're parsing from a string, which means that + // If p->tok->fp == NULL, then we're parsing from a string, which means that // the whole source is stored in p->tok->str. If not, then we're parsing // from the REPL, so the source lines of the current (multi-line) statement // are stored in p->tok->stdin_content From 75cca9f5b8501630b9c2f65426884177ebf1c51a Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 8 Jan 2021 21:10:05 +0200 Subject: [PATCH 09/10] Fix second comment --- Parser/pegen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index a441ecadce6b74b..8c7308a02dc2de1 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -435,8 +435,8 @@ _PyPegen_raise_error_known_location(Parser *p, PyObject *errtype, } if (!error_line) { - // PyErr_ProgramTextObject returned NULL, so it either failed or we're - // not parsing from a file + // PyErr_ProgramTextObject was not called or returned NULL, in which case + // we are either parsing from a file or it unexpectedly failed if (p->tok->lineno == lineno) { Py_ssize_t size = p->tok->inp - p->tok->buf; error_line = PyUnicode_DecodeUTF8(p->tok->buf, size, "replace"); From 5f4fd515bd42b4bae5af9f31c0588d766b76aa84 Mon Sep 17 00:00:00 2001 From: Lysandros Nikolaou Date: Fri, 8 Jan 2021 22:01:26 +0200 Subject: [PATCH 10/10] Improve comments and assertions --- Parser/pegen.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/Parser/pegen.c b/Parser/pegen.c index 8c7308a02dc2de1..a6f97929255ac24 100644 --- a/Parser/pegen.c +++ b/Parser/pegen.c @@ -383,10 +383,12 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...) static PyObject * get_error_line(Parser *p, Py_ssize_t lineno) { - // If p->tok->fp == NULL, then we're parsing from a string, which means that - // the whole source is stored in p->tok->str. If not, then we're parsing - // from the REPL, so the source lines of the current (multi-line) statement - // are stored in p->tok->stdin_content + /* If p->tok->fp == NULL, then we're parsing from a string, which means that + the whole source is stored in p->tok->str. If not, then we're parsing + from the REPL, so the source lines of the current (multi-line) statement + are stored in p->tok->stdin_content */ + assert(p->tok->fp == NULL || p->tok->fp == stdin); + char *cur_line = p->tok->fp == NULL ? p->tok->str : p->tok->stdin_content; for (int i = 0; i < lineno - 1; i++) { cur_line = strchr(cur_line, '\n') + 1; @@ -435,8 +437,15 @@ _PyPegen_raise_error_known_location(Parser *p, PyObject *errtype, } if (!error_line) { - // PyErr_ProgramTextObject was not called or returned NULL, in which case - // we are either parsing from a file or it unexpectedly failed + /* PyErr_ProgramTextObject was not called or returned NULL. If it was not called, + then we need to find the error line from some other source, because + p->start_rule != Py_file_input. If it returned NULL, then it either unexpectedly + failed or we're parsing from a string or the REPL. There's a third edge case where + we're actually parsing from a file, which has an E_EOF SyntaxError and in that case + `PyErr_ProgramTextObject` fails because lineno points to last_file_line + 1, which + does not physically exist */ + assert(p->tok->fp == NULL || p->tok->fp == stdin || p->tok->done == E_EOF); + if (p->tok->lineno == lineno) { Py_ssize_t size = p->tok->inp - p->tok->buf; error_line = PyUnicode_DecodeUTF8(p->tok->buf, size, "replace");