From 70d05eb8977a52944647a7b2791126aa9d216c2f Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 05:11:48 +0100 Subject: [PATCH 1/8] Fix `exec(, closure=)` segfault --- Lib/test/test_builtin.py | 7 +++++++ .../2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst | 2 ++ Python/bltinmodule.c | 1 + 3 files changed, 10 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 913d007a126d72..a3d1efe2d80064 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -984,6 +984,13 @@ def four_freevars(): closure=my_closure) self.assertEqual(result, 2520) + # should fail: closure isn't allowed + # when source is a string + self.assertRaises(TypeError, + exec, + "pass", + closure=my_closure) + # should fail: closure isn't allowed # for functions without free vars self.assertRaises(TypeError, diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst new file mode 100644 index 00000000000000..ca8b9ecde6722e --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst @@ -0,0 +1,2 @@ +``exec()`` called with a string ``source`` argument and non-``None`` ``closure`` argument no longer fails with a segmentation +fault. Patch by Bartosz Sławecki. diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 46a6fd9a8ef017..0c7b45c0b9b48d 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -1175,6 +1175,7 @@ builtin_exec_impl(PyObject *module, PyObject *source, PyObject *globals, if (closure != NULL) { PyErr_SetString(PyExc_TypeError, "closure can only be used when source is a code object"); + goto error; } PyObject *source_copy; const char *str; From 559a7ecec384d5e23743a5fb0812eb8a0b1a5ec9 Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 05:46:09 +0100 Subject: [PATCH 2/8] Tune test cases --- Lib/test/test_builtin.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index a3d1efe2d80064..ae24e52f0095c6 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -984,13 +984,6 @@ def four_freevars(): closure=my_closure) self.assertEqual(result, 2520) - # should fail: closure isn't allowed - # when source is a string - self.assertRaises(TypeError, - exec, - "pass", - closure=my_closure) - # should fail: closure isn't allowed # for functions without free vars self.assertRaises(TypeError, @@ -1021,6 +1014,20 @@ def four_freevars(): three_freevars.__globals__, closure=my_closure) + # should fail: incorrect closure isn't allowed + # when source is a string + self.assertRaises(TypeError, + exec, + "pass", + closure=object()) + + # should fail: correct closure isn't allowed + # when source is a string + self.assertRaises(TypeError, + exec, + "pass", + closure=my_closure) + # should fail: closure tuple with one non-cell-var my_closure[0] = int my_closure = tuple(my_closure) From 66daed5fb5f33b86625c5bef138b4806b2267f54 Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 05:50:05 +0100 Subject: [PATCH 3/8] Fixup test comments --- Lib/test/test_builtin.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index ae24e52f0095c6..e9622f8184bcea 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1014,15 +1014,15 @@ def four_freevars(): three_freevars.__globals__, closure=my_closure) - # should fail: incorrect closure isn't allowed - # when source is a string + # should fail: anything passed to closure= isn't allowed + # when the source is a string self.assertRaises(TypeError, exec, "pass", closure=object()) - # should fail: correct closure isn't allowed - # when source is a string + # should fail: correct closure= argument isn't allowed + # when the source is a string self.assertRaises(TypeError, exec, "pass", From 52398f7eb2bc7c9d154ca6be4c136e32d07058ac Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 05:52:50 +0100 Subject: [PATCH 4/8] Fixup `my_closure` after tests --- Lib/test/test_builtin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index e9622f8184bcea..f67cbb647a974c 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1013,6 +1013,7 @@ def four_freevars(): three_freevars.__code__, three_freevars.__globals__, closure=my_closure) + my_closure = tuple(my_closure) # should fail: anything passed to closure= isn't allowed # when the source is a string @@ -1029,6 +1030,7 @@ def four_freevars(): closure=my_closure) # should fail: closure tuple with one non-cell-var + my_closure = list(my_closure) my_closure[0] = int my_closure = tuple(my_closure) self.assertRaises(TypeError, From eee517cc73536f8dd5fe7b0a75c15df47dfe7fcb Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 05:54:00 +0100 Subject: [PATCH 5/8] Use `int` as an arbitrary object --- Lib/test/test_builtin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index f67cbb647a974c..87914ade5b518f 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -1020,7 +1020,7 @@ def four_freevars(): self.assertRaises(TypeError, exec, "pass", - closure=object()) + closure=int) # should fail: correct closure= argument isn't allowed # when the source is a string From a968687111a17a4eedda90e2fc66ef0788a5430d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20S=C5=82awecki?= Date: Thu, 13 Feb 2025 10:41:28 +0100 Subject: [PATCH 6/8] Reformat NEWS entry Co-authored-by: sobolevn --- .../2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst index ca8b9ecde6722e..10f71aa71e0d0a 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst @@ -1,2 +1,2 @@ -``exec()`` called with a string ``source`` argument and non-``None`` ``closure`` argument no longer fails with a segmentation +:func:`builtins.exec` called with a string ``source`` argument and non-``None`` ``closure`` argument no longer fails with a segmentation fault. Patch by Bartosz Sławecki. From d23e263ab3c2454835c92e4ecf56193b431b7537 Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 10:57:46 +0100 Subject: [PATCH 7/8] Fix `exec` crossref --- .../2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst index 10f71aa71e0d0a..af0b5134fefb0d 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst @@ -1,2 +1,2 @@ -:func:`builtins.exec` called with a string ``source`` argument and non-``None`` ``closure`` argument no longer fails with a segmentation +:func:`exec` called with a string ``source`` argument and non-``None`` ``closure`` argument no longer fails with a segmentation fault. Patch by Bartosz Sławecki. From 7b731baad0b9b5f1313b150b1bbb1066f65c645f Mon Sep 17 00:00:00 2001 From: bswck Date: Thu, 13 Feb 2025 11:41:43 +0100 Subject: [PATCH 8/8] Correct the NEWS entry (this wasn't a segfault!) --- .../2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst index af0b5134fefb0d..f9e135f1902b8a 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-13-05-09-31.gh-issue-130070.C8c9gK.rst @@ -1,2 +1 @@ -:func:`exec` called with a string ``source`` argument and non-``None`` ``closure`` argument no longer fails with a segmentation -fault. Patch by Bartosz Sławecki. +Fixed an assertion error for :func:`exec` passed a string ``source`` and a non-``None`` ``closure``. Patch by Bartosz Sławecki.