Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

specialized PRECALL opcodes don't check types #92063

Closed
sweeneyde opened this issue Apr 29, 2022 · 5 comments
Closed

specialized PRECALL opcodes don't check types #92063

sweeneyde opened this issue Apr 29, 2022 · 5 comments
Labels
3.11 interpreter-core release-blocker type-crash

Comments

@sweeneyde
Copy link

@sweeneyde sweeneyde commented Apr 29, 2022

On the main branch, on both Windows and Linux, this crashes:

class Thing:
    pass

thing = Thing()

for i in range(10):
    print(i)
    try:
        str.upper(thing)
    except TypeError:
        pass

print("ok")

Other methods like str.split, bytes.split, and list.sort fail similarly. I caught this by running

./python -m test test_descr -m test_proxy_call -R3:20

Looking through stack traces, it appears the failure is on the res = cfunc(...) call in PRECALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS instruction. It looks to be a 3.11-only bug.
@sweeneyde sweeneyde added 3.11 type-crash interpreter-core labels Apr 29, 2022
@sweeneyde
Copy link
Author

@sweeneyde sweeneyde commented Apr 29, 2022 •
edited

If we're trying to emulate method_vectorcall_FASTCALL_KEYWORDS, then we're at least missing out on how method_check_args calls descr_check, which calls PyObject_TypeCheck(obj, descr->d_type). So we need one of these:

DEOPT_IF(!PyObject_TypeCheck(PEEK(nargs), callable->d_type));

or

DEOPT_IF(!Py_IS_TYPE(PEEK(nargs), callable->d_type));

I might have an off-by-one error.

cc @Fidget-Spinner @kumaraditya303

@sweeneyde
Copy link
Author

@sweeneyde sweeneyde commented Apr 30, 2022

Marking as release blocker: it's easy to trigger but hard to reason about if you don't know about specialization, so it could create Heisenbugs, which would be unfortunate in the beta release.

@corona10
Copy link

@corona10 corona10 commented Apr 30, 2022 •
edited

FYI, macOS meets the same issue as expected.

@sweeneyde sweeneyde mentioned this issue Apr 30, 2022
Merged
@sweeneyde
Copy link
Author

@sweeneyde sweeneyde commented Apr 30, 2022

It looks like PRECALL_NO_KW_METHOD_DESCRIPTOR_NOARGS is affected as well.

@sweeneyde
Copy link
Author

@sweeneyde sweeneyde commented Apr 30, 2022

PRECALL_NO_KW_METHOD_DESCRIPTOR_O as well

@sweeneyde sweeneyde changed the title PRECALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS doesn't check types specialized PRECALL opcodes don't check types Apr 30, 2022
markshannon pushed a commit that referenced this issue Apr 30, 2022
@sweeneyde
gh-92063: Enforce types in specialized PRECALL opcodes (GH-92068)
868b1af 
* Check the types of PRECALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_NOARGS as well

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_O

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_FAST
@sweeneyde sweeneyde closed this Apr 30, 2022
gvanrossum pushed a commit to gvanrossum/cpython that referenced this issue May 1, 2022
@sweeneyde @gvanrossum
pythongh-92063: Enforce types in specialized PRECALL opcodes (pythonG…
acf36d5 
…H-92068)

* Check the types of PRECALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_NOARGS as well

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_O

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_FAST
gvanrossum pushed a commit to gvanrossum/cpython that referenced this issue May 1, 2022
@sweeneyde @gvanrossum
pythongh-92063: Enforce types in specialized PRECALL opcodes (pythonG…
26b2f3f 
…H-92068)

* Check the types of PRECALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDS

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_NOARGS as well

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_O

* fix PRECALL_NO_KW_METHOD_DESCRIPTOR_FAST
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.11 interpreter-core release-blocker type-crash
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@corona10 @sweeneyde