Skip to content

gh-145122: Add _GUARD_NOS_ANY_DICT to prevent STORE_SUBSCR_DICT on frozendict - #145039

Merged
corona10 merged 5 commits into
python:mainfrom
corona10:gh-141510-fix
Feb 24, 2026
Merged

gh-145122: Add _GUARD_NOS_ANY_DICT to prevent STORE_SUBSCR_DICT on frozendict#145039
corona10 merged 5 commits into
python:mainfrom
corona10:gh-141510-fix

Conversation

@corona10

@corona10corona10 commented Feb 20, 2026

Copy link
Copy Markdown
Member

@corona10corona10 changed the title gh-141510: Add _GUARD_NOS_ANY_DICT to prevent STORE_SUBSCR_DICT on fr…gh-141510: Add _GUARD_NOS_ANY_DICT to prevent STORE_SUBSCR_DICT on frozendictFeb 20, 2026
@corona10

Copy link
Copy Markdown
MemberAuthor

Comment threadPython/optimizer_bytecodes.c Outdated
}
else if (sym_matches_type(tos, &PyFrozenDict_Type)) {
ADD_OP(_NOP, 0, 0);
sym_set_type(tos, &PyFrozenDict_Type);

@corona10corona10Feb 20, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I am not sure this is correct way to handle this.
Should I just follow the same as _GUARD_TOS_ANY_SET which does not call sym_set_type?

@corona10

corona10 commented Feb 20, 2026

Copy link
Copy Markdown
MemberAuthor

Without this PR, tthe est code will fail like this:

Raised RLIMIT_NOFILE: 256 -> 1024
Using random seed: 2311893955
0:00:00 load avg: 7.54 Run 1 test sequentially in a single process
0:00:00 load avg: 7.54 [1/1] test_opcache
test test_opcache failed -- Traceback (most recent call last):
File "/Users/dongheena/oss/cpython/Lib/test/test_opcache.py", line 1910, in test_store_subscr
with self.assertRaises(TypeError):
~~~~~~~~~~~~~~~~~^^^^^^^^^^^
AssertionError: TypeError not raised
0:00:00 load avg: 7.54 [1/1/1] test_opcache failed (1 failure)

Comment threadPython/bytecodes.c Outdated
EXIT_IF(!PyDict_CheckExact(o));
}

op(_GUARD_TOS_DICT, (tos -- tos)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This guard is no longer needed, it can be removed, no?

@corona10corona10Feb 21, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

You’re right, I think we can remove the guard. However, let’s wait for Ken and Mark’s opinions on whether deleting it would be safe (I think yes), or if they prefer to leave it as is for potential future use.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If a uop is no longer needed it's best to remove it so that we don't leave things to bitrot in the code.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Thanks! I remove the _GUARD_TOS_DICT

Comment threadLib/test/test_opcache.py
@corona10corona10 changed the title gh-141510: Add _GUARD_NOS_ANY_DICT to prevent STORE_SUBSCR_DICT on frozendictgh-145122: Add _GUARD_NOS_ANY_DICT to prevent STORE_SUBSCR_DICT on frozendictFeb 23, 2026
Comment threadPython/optimizer_bytecodes.c
@corona10

Copy link
Copy Markdown
MemberAuthor

@Fidget-Spinner Could you take a look? The fix in bytecode.c looks good to me, but I’d appreciate your review of the changes in optimized_bytecode.c.

@corona10
corona10 merged commit 0f759f1 into python:mainFeb 24, 2026
91 of 92 checks passed
@corona10
corona10 deleted the gh-141510-fix branch February 24, 2026 15:48
brijkapadia pushed a commit to brijkapadia/cpython that referenced this pull request Feb 28, 2026
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants

@corona10@vstinner@Fidget-Spinner@cocolato@savannahostrowski