bpo-34080: Fix a memory leak in the compiler.

[serhiy-storchaka]

https://bugs.python.org/issue34080

[zhangyangyu]

In parsetok.c, err_ret->text is assigned a not malloced string, with Barry as BDFL ..., does it matter?

[serhiy-storchaka]

Interesting. This looks as a bug, but it is not related to this PR, because it sets err_ret->error = E_SYNTAX, while this PR fixes a case for err_ret->error = E_ERROR. Please open a separate issue for that bug.

[vstinner]

IHMO err_input() is fine, it shouldn't clear err->msg if err->error == E_ERROR. I would suggest to fix err_clear() instead:

diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index bcd1ca931d..1377be3cd5 100644
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -1324,6 +1324,10 @@ static void
 err_free(perrdetail *err)
 {
     Py_CLEAR(err->filename);
+    if (err->text != NULL) {
+        PyObject_FREE(err->text);
+        err->text = NULL;
+    }
 }
 
 /* Set the error appropriate to the given input error code (see errcode.h) */

[bedevere-bot]

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

[vstinner]

I would suggest to fix err_clear() instead: (...)

FYI "./python -m test -j0 -R 3:3 test_compile test_grammar" pass with this change, but then the following issue should be addressed as well?

"In parsetok.c, err_ret->text is assigned a not malloced string, with Barry as BDFL ..., does it matter?"

[zhangyangyu]

err_input is only called when the parsing operation fails. err_free is always called no matter the operation succeeds or fails. err->text has value only when error occurs while err->filename is always initialized. That may be why the cleaning logic is put in err_input. Anyway, no matter where it is put, I think we can remove the NULL test since PyObject_FREE can handle it itself.

@vstinner It's issue34084. Add you to the nosy list.

[vstinner]

I wrote PR #8242 to implement my proposed fix and fix also the issue spotted by @zhangyangyu.

[miss-islington]

Thanks @serhiy-storchaka for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6, 3.7.
🐍🍒⛏🤖

[bedevere-bot]

GH-8256 is a backport of this pull request to the 3.7 branch.

[bedevere-bot]

GH-8257 is a backport of this pull request to the 3.6 branch.

[bedevere-bot]

GH-8258 is a backport of this pull request to the 2.7 branch.