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

gh-99845: Clean up _PyObject_VAR_SIZE() usage #99847

Merged
merged 1 commit into from
Nov 29, 2022
Merged

gh-99845: Clean up _PyObject_VAR_SIZE() usage #99847

merged 1 commit into from
Nov 29, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Nov 28, 2022

  • code_sizeof() now uses unsigned type to compute the result.
  • Fix _PyObject_ComputedDictPointer(): cast _PyObject_VAR_SIZE() to Py_ssize_t, rather than long.
  • Clarify that _PyObject_VAR_SIZE() uses an unsigned type (size_t).

* code_sizeof() now uses unsigned type to compute the result.
* Fix _PyObject_ComputedDictPointer(): cast _PyObject_VAR_SIZE() to
  Py_ssize_t, rather than long.
* Clarify that _PyObject_VAR_SIZE() uses an unsigned type (size_t).
Py_ssize_t tsize = Py_SIZE(obj);
if (tsize < 0) {
tsize = -tsize;
}
size_t size = _PyObject_VAR_SIZE(tp, tsize);
assert(size <= (size_t)PY_SSIZE_T_MAX);
dictoffset += (Py_ssize_t)size;
Copy link
Member

Choose a reason for hiding this comment

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

It should be backported.

@vstinner
Copy link
Member Author

vstinner commented Dec 1, 2022

The dictoffset code was different in 2001 when the cast was added: commit 6d483d3

Change:

-               dictoffset += size;
+               dictoffset += (long)size;

Code:

PyObject **
_PyObject_GetDictPtr(PyObject *obj)
{
        long dictoffset;
        ...
        if (dictoffset < 0) {
                size_t size;
                _PyObject_VAR_SIZE(size, tp, ((PyVarObject *)obj)->ob_size);
                dictoffset += (long)size;
                assert(dictoffset > 0);
                assert(dictoffset % SIZEOF_VOID_P == 0);
        }
        ...
}

The dictoffset type was changed in 2006 by commit 725507b without updating the cast, forgotten in the enhancement.

My change, replace (long) cast with (Py_ssize_t) only impact 64-bit Windows. On other platforms, long and Py_ssize_t should be the same type (either 32-bit or 64-bit signed integer, int32_t or int64_t).

@vstinner
Copy link
Member Author

vstinner commented Dec 1, 2022

I created PR #99922 to backport the cast fix to Python 3.11 (and 3.10).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants