Skip to content

Error handling of RERAISE is strange #122759

Closed
Closed
Error handling of RERAISE is strange#122759
Assignees
sobolevn
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@sobolevn
sobolevn
opened on Aug 6, 2024

Bug report

There's a strange pattern used in RERAISE opcode:

cpython/Python/bytecodes.c

Lines 1174 to 1189 in 4767a6e

tier1 inst(RERAISE, (values[oparg], exc_st -- values[oparg])) {
PyObject *exc = PyStackRef_AsPyObjectBorrow(exc_st);
assert(oparg >= 0 && oparg <= 2);
if (oparg) {
PyObject *lasti = PyStackRef_AsPyObjectBorrow(values[0]);
if (PyLong_Check(lasti)) {
frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame)) + PyLong_AsLong(lasti);
assert(!_PyErr_Occurred(tstate));
}
else {
assert(PyLong_Check(lasti));
_PyErr_SetString(tstate, PyExc_SystemError, "lasti is not an int");
ERROR_NO_POP();
}
}

Especially these lines:

cpython/Python/bytecodes.c

Lines 1180 to 1187 in 4767a6e

if (PyLong_Check(lasti)) {
frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame)) + PyLong_AsLong(lasti);
assert(!_PyErr_Occurred(tstate));
}
else {
assert(PyLong_Check(lasti));
_PyErr_SetString(tstate, PyExc_SystemError, "lasti is not an int");
ERROR_NO_POP();

It looks like the assert call is not needed here:

  • In debug builds it will always crash, since PyLong_Check(lasti) will always be false
  • In non-debug builds it will set SystemError as it should be (at least, I think so). That's the only line that matches r'assert\(.*\);\n\s+_PyErr'

I have a PR ready.

Linked PRs

Activity

  1. sobolevn
    added
    type-bugAn unexpected behavior, bug, or error
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    on Aug 6, 2024
  2. sobolevn
    self-assigned this
    on Aug 6, 2024
  3. sobolevn
    added 2 commits that reference this issue on Aug 6, 2024

    pythongh-122759: Remove `assert` from `RERAISE` error handling

    d4b2214

    gh-122759: Remove `assert` from `RERAISE` error handling (#122760)

    61a8bf2

  4. hugovk commented on Aug 8, 2024

    @hugovk
    hugovk
    on Aug 8, 2024
    Member

    Triage: closing because the linked PR is merged, please re-open if still needed.

  5. hugovk
    closed this as completedon Aug 8, 2024
  6. blhsing
    added a commit that references this issue on Aug 22, 2024

    pythongh-122759: Remove `assert` from `RERAISE` error handling (pytho…

    0c5e26f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

interpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions