From ecef7689ca1644dfb436f21bba0c38ef1ea54c80 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 21 Aug 2023 15:08:28 +0900 Subject: [PATCH 1/7] gh-107265: Fix code_hash for ENTER_EXECUTOR case --- Lib/test/test_capi/test_misc.py | 6 ++++-- Objects/codeobject.c | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index ea0504333bab00..1cd4c56b49bba5 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2341,7 +2341,7 @@ def long_loop(): long_loop() self.assertEqual(opt.get_count(), 10) - def test_code_richcompare(self): + def test_code_restore_for_ENTER_EXECUTOR(self): def testfunc(x): i = 0 while i < x: @@ -2350,7 +2350,9 @@ def testfunc(x): opt = _testinternalcapi.get_counter_optimizer() with temporary_optimizer(opt): testfunc(1000) - self.assertEqual(testfunc.__code__, testfunc.__code__.replace()) + code, replace_code = testfunc.__code__, testfunc.__code__.replace() + self.assertEqual(code, replace_code) + self.assertEqual(hash(code), hash(replace_code)) def get_first_executor(func): diff --git a/Objects/codeobject.c b/Objects/codeobject.c index c34905c3196063..4a3fcd0a506534 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1884,9 +1884,20 @@ code_hash(PyCodeObject *co) SCRAMBLE_IN(Py_SIZE(co)); for (int i = 0; i < Py_SIZE(co); i++) { int deop = _Py_GetBaseOpcode(co, i); - SCRAMBLE_IN(deop); - SCRAMBLE_IN(_PyCode_CODE(co)[i].op.arg); - i += _PyOpcode_Caches[deop]; + if (deop == ENTER_EXECUTOR) { + // Assume that deopt of ENTER_EXECUTOR will be ENTER_EXECUTOR. + const int exec_index = _PyCode_CODE(co)[i].op.arg; + _PyExecutorObject *exec = co->co_executors->executors[exec_index]; + assert(exec != NULL); + SCRAMBLE_IN(exec->vm_data.opcode); + SCRAMBLE_IN(exec->vm_data.oparg); + i += _PyOpcode_Caches[exec->vm_data.opcode]; + } + else { + SCRAMBLE_IN(deop); + SCRAMBLE_IN(_PyCode_CODE(co)[i].op.arg); + i += _PyOpcode_Caches[deop]; + } } if ((Py_hash_t)uhash == -1) { return -2; From be27a2bb9ddc80fdfdca86ce1b424376b64f5693 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 21 Aug 2023 15:35:11 +0900 Subject: [PATCH 2/7] nit --- Objects/codeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 4a3fcd0a506534..8f65c916f164db 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1885,7 +1885,7 @@ code_hash(PyCodeObject *co) for (int i = 0; i < Py_SIZE(co); i++) { int deop = _Py_GetBaseOpcode(co, i); if (deop == ENTER_EXECUTOR) { - // Assume that deopt of ENTER_EXECUTOR will be ENTER_EXECUTOR. + // Assume that deop of ENTER_EXECUTOR will be ENTER_EXECUTOR. const int exec_index = _PyCode_CODE(co)[i].op.arg; _PyExecutorObject *exec = co->co_executors->executors[exec_index]; assert(exec != NULL); From 969719663cd42440ef009f06e631296874875269 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 21 Aug 2023 16:39:10 +0900 Subject: [PATCH 3/7] Add assert --- Objects/codeobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 8f65c916f164db..d0bbc118964eef 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1889,6 +1889,7 @@ code_hash(PyCodeObject *co) const int exec_index = _PyCode_CODE(co)[i].op.arg; _PyExecutorObject *exec = co->co_executors->executors[exec_index]; assert(exec != NULL); + assert(exec->vm_data.opcode != ENTER_EXECUTOR); SCRAMBLE_IN(exec->vm_data.opcode); SCRAMBLE_IN(exec->vm_data.oparg); i += _PyOpcode_Caches[exec->vm_data.opcode]; From 2197675563492aff3f3f7160f310d11f6c5bc559 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 22 Aug 2023 00:01:01 +0900 Subject: [PATCH 4/7] Fix code_richcompare not to change code object. --- Objects/codeobject.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index d0bbc118964eef..f5f73e92da57a2 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1781,30 +1781,33 @@ code_richcompare(PyObject *self, PyObject *other, int op) for (int i = 0; i < Py_SIZE(co); i++) { _Py_CODEUNIT co_instr = _PyCode_CODE(co)[i]; _Py_CODEUNIT cp_instr = _PyCode_CODE(cp)[i]; + uint8_t co_code = co_instr.op.code; + uint8_t co_arg = co_instr.op.arg; + uint8_t cp_code = cp_instr.op.code; + uint8_t cp_arg = cp_instr.op.arg; - if (co_instr.op.code == ENTER_EXECUTOR) { - const int exec_index = co_instr.op.arg; + if (co_code == ENTER_EXECUTOR) { + const int exec_index = co_arg; _PyExecutorObject *exec = co->co_executors->executors[exec_index]; - co_instr.op.code = exec->vm_data.opcode; - co_instr.op.arg = exec->vm_data.oparg; + co_code = exec->vm_data.opcode; + co_arg = exec->vm_data.oparg; } - assert(co_instr.op.code != ENTER_EXECUTOR); - co_instr.op.code = _PyOpcode_Deopt[co_instr.op.code]; + assert(co_code != ENTER_EXECUTOR); + co_code = _PyOpcode_Deopt[co_code]; - if (cp_instr.op.code == ENTER_EXECUTOR) { - const int exec_index = cp_instr.op.arg; + if (cp_code == ENTER_EXECUTOR) { + const int exec_index = cp_arg; _PyExecutorObject *exec = cp->co_executors->executors[exec_index]; - cp_instr.op.code = exec->vm_data.opcode; - cp_instr.op.arg = exec->vm_data.oparg; + cp_code = exec->vm_data.opcode; + cp_arg = exec->vm_data.oparg; } - assert(cp_instr.op.code != ENTER_EXECUTOR); - cp_instr.op.code = _PyOpcode_Deopt[cp_instr.op.code]; + assert(cp_code != ENTER_EXECUTOR); + cp_code = _PyOpcode_Deopt[cp_code]; - eq = co_instr.cache == cp_instr.cache; - if (!eq) { + if (co_code != cp_code || co_arg != cp_arg) { goto unequal; } - i += _PyOpcode_Caches[co_instr.op.code]; + i += _PyOpcode_Caches[co_code]; } /* compare constants */ From 2bd37a995973590ba2790869d09c8dfd2f09e43b Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 22 Aug 2023 00:57:46 +0900 Subject: [PATCH 5/7] Address code review --- Objects/codeobject.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index f5f73e92da57a2..9d54ceb9a0666c 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1886,22 +1886,22 @@ code_hash(PyCodeObject *co) SCRAMBLE_IN(co->co_firstlineno); SCRAMBLE_IN(Py_SIZE(co)); for (int i = 0; i < Py_SIZE(co); i++) { - int deop = _Py_GetBaseOpcode(co, i); - if (deop == ENTER_EXECUTOR) { - // Assume that deop of ENTER_EXECUTOR will be ENTER_EXECUTOR. - const int exec_index = _PyCode_CODE(co)[i].op.arg; - _PyExecutorObject *exec = co->co_executors->executors[exec_index]; + _Py_CODEUNIT co_instr = _PyCode_CODE(co)[i]; + uint8_t co_code = co_instr.op.code; + uint8_t co_arg = co_instr.op.arg; + if (co_code == ENTER_EXECUTOR) { + _PyExecutorObject *exec = co->co_executors->executors[co_arg]; assert(exec != NULL); assert(exec->vm_data.opcode != ENTER_EXECUTOR); - SCRAMBLE_IN(exec->vm_data.opcode); - SCRAMBLE_IN(exec->vm_data.oparg); - i += _PyOpcode_Caches[exec->vm_data.opcode]; + co_code = exec->vm_data.opcode; + co_arg = exec->vm_data.oparg; } else { - SCRAMBLE_IN(deop); - SCRAMBLE_IN(_PyCode_CODE(co)[i].op.arg); - i += _PyOpcode_Caches[deop]; + co_code = _PyOpcode_Deopt[co_code]; } + SCRAMBLE_IN(co_code); + SCRAMBLE_IN(co_arg); + i += _PyOpcode_Caches[co_code]; } if ((Py_hash_t)uhash == -1) { return -2; From 8446dfd4fbe614361245708934abf2bc4c783d51 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 22 Aug 2023 01:16:36 +0900 Subject: [PATCH 6/7] fix --- Objects/codeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 9d54ceb9a0666c..37084ca2bf4da4 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1897,7 +1897,7 @@ code_hash(PyCodeObject *co) co_arg = exec->vm_data.oparg; } else { - co_code = _PyOpcode_Deopt[co_code]; + co_code = _Py_GetBaseOpcode(co, i); } SCRAMBLE_IN(co_code); SCRAMBLE_IN(co_arg); From be782e940b4c1a30fde0f3cdc30229f8f77bc58b Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 22 Aug 2023 01:47:16 +0900 Subject: [PATCH 7/7] nit --- Objects/codeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 37084ca2bf4da4..dca5804a91d2cd 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1893,7 +1893,7 @@ code_hash(PyCodeObject *co) _PyExecutorObject *exec = co->co_executors->executors[co_arg]; assert(exec != NULL); assert(exec->vm_data.opcode != ENTER_EXECUTOR); - co_code = exec->vm_data.opcode; + co_code = _PyOpcode_Deopt[exec->vm_data.opcode]; co_arg = exec->vm_data.oparg; } else {