From 97dd42693598c29850856f842933fbcbb77c60e4 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 27 Aug 2017 10:14:54 +0300 Subject: [PATCH 1/3] bpo-31286: Fixed stack usage in absolute imports with binding a submodule to a name. --- Lib/test/test_import/__init__.py | 8 ++++++ .../2017-08-27-10-14-32.bpo-31286.ecSW2d.rst | 1 + Python/compile.c | 26 ++++++++++++------- 3 files changed, 25 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 1b04de24dd16e44..ad8865c3ba65811 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -237,6 +237,14 @@ def test_import_name_binding(self): import test.support as y self.assertIs(y, test.support, y.__name__) + def test_issue31286(self): + # import in finally resulted in SystemError + try: + x = ... + finally: + import test.support.script_helper as x + self.assertIs(x, test.support.script_helper) + def test_failing_reload(self): # A failing reload should leave the module object in sys.modules. source = TESTFN + os.extsep + "py" diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst new file mode 100644 index 000000000000000..b34bfc3fa336e59 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst @@ -0,0 +1 @@ +Fixed stack usage in absolute imports with binding a submodule to a name. diff --git a/Python/compile.c b/Python/compile.c index 78e797a2c331f36..e547c2fd591c498 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2673,28 +2673,34 @@ compiler_import_as(struct compiler *c, identifier name, identifier asname) If there is a dot in name, we need to split it and emit a IMPORT_FROM for each name. */ - Py_ssize_t dot = PyUnicode_FindChar(name, '.', 0, - PyUnicode_GET_LENGTH(name), 1); + Py_ssize_t len = PyUnicode_GET_LENGTH(name); + Py_ssize_t dot = PyUnicode_FindChar(name, '.', 0, len, 1); if (dot == -2) return 0; if (dot != -1) { /* Consume the base module name to get the first attribute */ - Py_ssize_t pos = dot + 1; - while (dot != -1) { + while (1) { + Py_ssize_t pos = dot + 1; PyObject *attr; - dot = PyUnicode_FindChar(name, '.', pos, - PyUnicode_GET_LENGTH(name), 1); + dot = PyUnicode_FindChar(name, '.', pos, len, 1); if (dot == -2) return 0; - attr = PyUnicode_Substring(name, pos, - (dot != -1) ? dot : - PyUnicode_GET_LENGTH(name)); + attr = PyUnicode_Substring(name, pos, (dot != -1) ? dot : len); if (!attr) return 0; ADDOP_O(c, IMPORT_FROM, attr, names); Py_DECREF(attr); - pos = dot + 1; + if (dot == -1) { + break; + } + ADDOP(c, ROT_TWO); + ADDOP(c, POP_TOP); } + if (!compiler_nameop(c, asname, Store)) { + return 0; + } + ADDOP(c, POP_TOP); + return 1; } return compiler_nameop(c, asname, Store); } From 225ffd16b7fb8e73241a6ffbd80bb2137a42511a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 27 Aug 2017 21:29:24 +0300 Subject: [PATCH 2/3] Add more tests. --- Lib/test/test_import/__init__.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index ad8865c3ba65811..ddef27d9cd4527b 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -238,12 +238,21 @@ def test_import_name_binding(self): self.assertIs(y, test.support, y.__name__) def test_issue31286(self): - # import in finally resulted in SystemError + # import in a 'finally' block resulted in SystemError try: x = ... finally: import test.support.script_helper as x - self.assertIs(x, test.support.script_helper) + + # import in a 'while' loop resulted in stack overflow + i = 0 + while i < 10: + import test.support.script_helper as x + i += 1 + + # import in a 'for' loop resulted in segmentation fault + for i in range(2): + import test.support.script_helper as x def test_failing_reload(self): # A failing reload should leave the module object in sys.modules. From 22169d852de0b72f88415442cb1765403d321e0f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 29 Aug 2017 13:27:20 +0300 Subject: [PATCH 3/3] Remove the NEWS entry. --- .../Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst deleted file mode 100644 index b34bfc3fa336e59..000000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2017-08-27-10-14-32.bpo-31286.ecSW2d.rst +++ /dev/null @@ -1 +0,0 @@ -Fixed stack usage in absolute imports with binding a submodule to a name.