Movatterモバイル変換

corona10 requested a review fromgvanrossum

August 20, 2023 11:09

corona10 requested a review frommarkshannon as acode owner

August 20, 2023 11:09

bedevere-bot mentioned this pull request

Decoding instructions should handle ENTER_EXECUTOR#107265

Open

10 tasks

bedevere-bot added the awaiting core review label

corona10 changed the title~~gh-107265: Fix code_richcompare to lookup actual opcode when ENTER_EX…~~gh-107265: Fix code_richcompare for ENTER_EXECUTOR case

Merge remote-tracking branch 'upstream/main' intopythongh-107265

25d3f5b

gvanrossum reviewed

Copy link

Member

gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LG, but could you add a test for this? You could add it totest_capi/test_misc.py inTestOptimizerAPI.

Add test and compare oparg too

fb0ab4e

Copy link

MemberAuthor

corona10 commentedAug 21, 2023•
edited
Loading

LG, but could you add a test for this? You could add it to test_capi/test_misc.py in TestOptimizerAPI.

Thanks, I added the test, and if I understand the intention correctly.
co_instr.op.arg andcp_instr.op.arg should also be updated.
If not, comparingco_instr.cache == cp_instr.cache will be failed anyway.

Please let me know if I understand wrongly.

Copy link

MemberAuthor

corona10 commentedAug 21, 2023•
edited
Loading

pydoc test failure is unrelated to this PR, I checked it locally and asked to Discord too

gvanrossum approved these changes

Copy link

Member

gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes, thearg must also be restored, since it is overwritten (with the index of the executor to be used in this code object's array of executors). So it looks like writing the test was useful!

bedevere-bot added awaiting merge and removed awaiting core review labels

Merge remote-tracking branch 'upstream/main' intopythongh-107265

70a29b9

corona10enabled auto-merge (squash)

August 21, 2023 05:03

corona10 merged commit4fdf3fd intopython:main

bedevere-bot removed the awaiting merge label

corona10 deleted the gh-107265 branch

August 21, 2023 05:50

markshannon reviewed

Objects/codeobject.c

		if (co_instr.op.code == ENTER_EXECUTOR) {
		const int exec_index = co_instr.op.arg;
		_PyExecutorObject *exec = co->co_executors->executors[exec_index];
		co_instr.op.code = exec->vm_data.opcode;

Copy link

Member

markshannonAug 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Comparing two code objects must not modify the code object.

The interaction between executor, the specializer and instrumentation is subtle and likely to break without care.
This will leak executors, as it flip-flops betweenJUMP_BACKWARDS andENTER_EXECUTOR, or worse if an optimizer assumes that a single instruction will only be seen once.

markshannon reviewed