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

get_hash_info might potentially swallow errors in sysmodule.c #115320

Closed
sobolevn opened this issue Feb 12, 2024 · 0 comments
Closed

get_hash_info might potentially swallow errors in sysmodule.c #115320

sobolevn opened this issue Feb 12, 2024 · 0 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Feb 12, 2024

Bug report

cpython/Python/sysmodule.c

Lines 1494 to 1526 in 54bde5d

static PyObject *
get_hash_info(PyThreadState *tstate)
{
PyObject *hash_info;
int field = 0;
PyHash_FuncDef *hashfunc;
hash_info = PyStructSequence_New(&Hash_InfoType);
if (hash_info == NULL)
return NULL;
hashfunc = PyHash_GetFuncDef();
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(8*sizeof(Py_hash_t)));
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromSsize_t(_PyHASH_MODULUS));
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(_PyHASH_INF));
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(0)); // This is no longer used
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(_PyHASH_IMAG));
PyStructSequence_SET_ITEM(hash_info, field++,
PyUnicode_FromString(hashfunc->name));
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(hashfunc->hash_bits));
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(hashfunc->seed_bits));
PyStructSequence_SET_ITEM(hash_info, field++,
PyLong_FromLong(Py_HASH_CUTOFF));
if (_PyErr_Occurred(tstate)) {
Py_CLEAR(hash_info);
return NULL;
}
return hash_info;

This code only checks for errors in the very last line:

cpython/Python/sysmodule.c

Lines 1522 to 1526 in 54bde5d

if (_PyErr_Occurred(tstate)) {
Py_CLEAR(hash_info);
return NULL;
}
return hash_info;

I think that this pattern should not be used. Because it can potentially swallow previous errors on only show the very last one.

I propose to refactor this code to show the first error instead.

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 12, 2024
@sobolevn sobolevn self-assigned this Feb 12, 2024
sobolevn added a commit to sobolevn/cpython that referenced this issue Feb 12, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 4, 2024
…llow errors (pythonGH-115321)

(cherry picked from commit 207030f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 4, 2024
…llow errors (pythonGH-115321)

(cherry picked from commit 207030f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this issue Mar 4, 2024
…allow errors (GH-115321) (#116324)

gh-115320: Refactor `get_hash_info` in `sysmodule.c` not to swallow errors (GH-115321)
(cherry picked from commit 207030f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this issue Mar 4, 2024
…allow errors (GH-115321) (#116323)

gh-115320: Refactor `get_hash_info` in `sysmodule.c` not to swallow errors (GH-115321)
(cherry picked from commit 207030f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@sobolevn sobolevn closed this as completed Mar 4, 2024
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant