From: Guido van Rossum <guido@python.org>
Date: Sat, 26 Aug 2023 23:22:40 +0000 (-0700)
Subject: [3.12] gh-108487: Change assert that should've been DEOPT_IF (#108509)
X-Git-Tag: v3.12.0rc2~59
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bbdd8895a5aced4cd4e66a5c6e3471636f28df6b;p=thirdparty%2FPython%2Fcpython.git

[3.12] gh-108487: Change assert that should've been DEOPT_IF (#108509)

* Remove assert that should've been DEOPT_IF

The assert(method != NULL) in CALL_NO_KW_LIST_APPEND is wrong --
this condition should lead to a deoptimization, and indeed there
is a DEOPT_IF two lines later that will trigger if method == NULL.

This would crash in a devious repro scenario (first seen live
in boto3 tests) when compiled with assertions enabled.
In a production version there is no crash, so impact is limited.

(The crash also appears in main; I will prepare a separate PR.)

* Add back a different assert(self != NULL)

* 📜🤖 Added by blurb_it.

---------

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
---

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-26-04-33-18.gh-issue-108487.aUFxqf.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-26-04-33-18.gh-issue-108487.aUFxqf.rst
new file mode 100644
index 000000000000..1117bcd7e985
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-26-04-33-18.gh-issue-108487.aUFxqf.rst	
@@ -0,0 +1 @@
+Change an assert that would cause a spurious crash in a devious case that should only trigger deoptimization.
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index dc2ae221f0bd..5e80e06205ae 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2992,9 +2992,9 @@ dummy_func(
         inst(CALL_NO_KW_LIST_APPEND, (unused/1, unused/2, method, self, args[oparg] -- unused)) {
             assert(kwnames == NULL);
             assert(oparg == 1);
-            assert(method != NULL);
             PyInterpreterState *interp = _PyInterpreterState_GET();
             DEOPT_IF(method != interp->callable_cache.list_append, CALL);
+            assert(self != NULL);
             DEOPT_IF(!PyList_Check(self), CALL);
             STAT_INC(CALL, hit);
             if (_PyList_AppendTakeRef((PyListObject *)self, args[0]) < 0) {
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index b0a363ce9aa1..a3c049569268 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -4248,9 +4248,9 @@
             #line 2993 "Python/bytecodes.c"
             assert(kwnames == NULL);
             assert(oparg == 1);
-            assert(method != NULL);
             PyInterpreterState *interp = _PyInterpreterState_GET();
             DEOPT_IF(method != interp->callable_cache.list_append, CALL);
+            assert(self != NULL);
             DEOPT_IF(!PyList_Check(self), CALL);
             STAT_INC(CALL, hit);
             if (_PyList_AppendTakeRef((PyListObject *)self, args[0]) < 0) {