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

PyObject_CallOneArg() usage triggers assert #4093

Closed
ariebovenberg opened this issue Apr 18, 2024 · 1 comment · Fixed by #4104
Closed

PyObject_CallOneArg() usage triggers assert #4093

ariebovenberg opened this issue Apr 18, 2024 · 1 comment · Fixed by #4104
Labels

Comments

@ariebovenberg
Copy link

ariebovenberg commented Apr 18, 2024

Bug Description

I'm getting an error using PyObject_CallOneArg() and I think this may be a bug. Feel free to close this issue if it isn't 👀

Steps to Reproduce

a minimal-ish example:

let zoneinfo_module = PyImport_ImportModule("zoneinfo\0".as_ptr().cast::<c_char>());
let zoneinfo_class =
    PyObject_GetAttrString(zoneinfo_module, "ZoneInfo\0".as_ptr().cast::<c_char>());
let zoneinfo_instance = PyObject_CallOneArg(
    zoneinfo_class,
    PyUnicode_FromString("Europe/Amsterdam\0".as_ptr().cast::<c_char>()),
);

this triggers the assert below. Indeed the n is set to 9223372036854775809 (max i64 + 1). The function seems to usually get smaller values like 0, or 3 (corresponding to the number of arguments).

// abstract_.rs
pub unsafe fn PyVectorcall_NARGS(n: size_t) -> Py_ssize_t {
    assert!(n <= (PY_SSIZE_T_MAX as size_t));
    (n as Py_ssize_t) & !PY_VECTORCALL_ARGUMENTS_OFFSET
}

The large value can be traced back directly to PyObject_CallOneArg

// in abstract_.rs
pub unsafe fn PyObject_CallOneArg(func: *mut PyObject, arg: *mut PyObject) -> *mut PyObject {
    assert!(!arg.is_null());
    let args_array = [std::ptr::null_mut(), arg];
    let args = args_array.as_ptr().offset(1); // For PY_VECTORCALL_ARGUMENTS_OFFSET
    let tstate = PyThreadState_GET();
    let nargsf = 1 | PY_VECTORCALL_ARGUMENTS_OFFSET;
    _PyObject_VectorcallTstate(tstate, func, args, nargsf as size_t, std::ptr::null_mut())
}

where 1 | PY_VECTORCALL_ARGUMENTS_OFFSET results in this massive number. Patching let nargsf = 1; gets things working—but it should obviously be derived from PY_VECTORCALL_ARGUMENTS_OFFSET in some way.

I'm still pretty new to the C API, so it isn't obvious to me what the answer is 😬. Of course I'd be happy to submit a PR if that'd help 🙂

Backtrace

No response

Your operating system and version

MacOS 14.4.1

Your Python version (python --version)

Python 3.12.0

Your Rust version (rustc --version)

1.77.2

Your PyO3 version

master branch

How did you install python? Did you use a virtualenv?

pyenv virtualenv 3.12.0 foo
pyenv local foo

I then added my code to the string_example example in the pyo3 repo

Additional Info

No response

@ariebovenberg ariebovenberg changed the title PyObject_CallOneArg() triggers assert PyObject_CallOneArg() usage triggers assert Apr 18, 2024
birkenfeld added a commit that referenced this issue Apr 20, 2024
Fixes #4093

- Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
- Change the assert in PyVectorcall_NARGS to check the recovered
  number, not the original value (which is > PY_SSIZE_T_MAX on
  purpose)
@birkenfeld
Copy link
Member

Thanks for the report! #4104 should fix this.

birkenfeld added a commit that referenced this issue Apr 20, 2024
Fixes #4093

- Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
- Change the assert in PyVectorcall_NARGS to check the recovered
  number, not the original value (which is > PY_SSIZE_T_MAX on
  purpose)
birkenfeld added a commit that referenced this issue Apr 21, 2024
Fixes #4093

- Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
- Change the assert in PyVectorcall_NARGS to check the recovered
  number, not the original value (which is > PY_SSIZE_T_MAX on
  purpose)
birkenfeld added a commit that referenced this issue Apr 22, 2024
Fixes #4093

- Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
- Remove the assert in PyVectorcall_NARGS and use checked cast
github-merge-queue bot pushed a commit that referenced this issue Apr 22, 2024
Fixes #4093

- Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
- Remove the assert in PyVectorcall_NARGS and use checked cast
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants