bpo-29988: Test signal safety of with statements #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ncoghlan
force-pushed
the
bpo-29988-signal-safe-with-statements
branch
from
fcf622d to
2fc137e
Compare
njsmith
reviewed
Sep 5, 2017
Python/ceval.c
Outdated
| @@ -1014,6 +1017,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) | |||
| assert(PyBytes_GET_SIZE(co->co_code) % sizeof(_Py_CODEUNIT) == 0); | |||
| assert(_Py_IS_ALIGNED(PyBytes_AS_STRING(co->co_code), sizeof(_Py_CODEUNIT))); | |||
| first_instr = (_Py_CODEUNIT *) PyBytes_AS_STRING(co->co_code); | |||
| handle_pending_after = first_instr; | |||
There was a problem hiding this comment.
Should this be first_instr - 1 or maybe the check down below should be >=?
njsmith
reviewed
Sep 5, 2017
Python/ceval.c
Outdated
| if (Py_MakePendingCalls() < 0) | ||
| goto error; | ||
| /* Allow for subsequent jumps backwards in the bytecode */ | ||
| handle_pending_after = first_instr; |
There was a problem hiding this comment.
Is it possible to get a jump backwards without the &pendingcalls_to_do becoming true in the mean time, so that this never gets reset?
There was a problem hiding this comment.
Yeah, it is, so the check should be restructured a bit to handle that case.
There was a problem hiding this comment.
Maybe the simplest fix would be for the frame to track a start and stop (with the start being implicitly set to the offset of the DEFER_PENDING_UNTIL opcode), and make the check if (_Py_atomic_load_relaxed(&pendingcalls_to_do) && !(defer_pending_start <= next_instr && next_instr < defer_pending_end). Using a standard half-open range here would also make it easy to say "there is currently no defer pending region": just set defer_pending_start = defer_pending_end.
There was a problem hiding this comment.
Yeah, that would work, and means I can put the check back inside the pending_calls_to_do guard again.
ncoghlan
force-pushed
the
bpo-29988-signal-safe-with-statements
branch
from
1bea258 to
ed32551
Compare
Given the ability to get trace hooks to run after every opcode (instead of only between lines), its possible to provoke failures in with statements, where `__enter__` or `__aenter__` can run successfully, but a poorly timed external exception (e.g. KeyboardInterrupt) can lead to `__exit__` or `__aexit__` still being skipped.
- adds a new trace event type ("opcode") for
more reliable injection of the error
- moves the error injection into a C helper
function so it actually happens in the frame
of interest immediately after the offset of
interest
- without this, the exception gets raised as soon as the CM's __exit__ method starts running - for the async case, it turns out there's nothing we can do to solve this at the byte code level. Instead, we need to somehow push *all* pending call processing back to the event loop and inject a synthetic await into the current frame
ncoghlan
force-pushed
the
bpo-29988-signal-safe-with-statements
branch
from
99b63d6 to
4bb4a38
Compare
ncoghlan
changed the title
ncoghlan
pushed a commit
that referenced
this pull request
Jun 28, 2020
```
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x7f008bf19667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
#1 0x7f007a0bee4a in subprocess_fork_exec /home/heimes/dev/python/cpython/Modules/_posixsubprocess.c:774
#2 0xe0305b in cfunction_call Objects/methodobject.c:546
```
Signed-off-by: Christian Heimes <christian@python.org>
|
This PR is stale because it has been open for 30 days with no activity. |
ncoghlan
pushed a commit
that referenced
this pull request
Aug 19, 2022
…python#91466) Fix an uninitialized bool in exception print context. `struct exception_print_context.need_close` was uninitialized. Found by oss-fuzz in a test case running under the undefined behavior sanitizer. https://oss-fuzz.com/testcase-detail/6217746058182656 ``` Python/pythonrun.c:1241:28: runtime error: load of value 253, which is not a valid value for type 'bool' #0 0xbf2203 in print_chained cpython3/Python/pythonrun.c:1241:28 #1 0xbea4bb in print_exception_cause_and_context cpython3/Python/pythonrun.c:1320:19 #2 0xbea4bb in print_exception_recursive cpython3/Python/pythonrun.c:1470:13 #3 0xbe9e39 in _PyErr_Display cpython3/Python/pythonrun.c:1517:9 ``` Pretty obvious what the ommission was upon code inspection.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Given the ability to get trace hooks to run after
every opcode (instead of only between lines), its
possible to provoke failures in with statements,
where
__enter__or__aenter__can run successfully,but a poorly timed external exception (e.g.
KeyboardInterrupt) can lead to
__exit__or__aexit__still being skipped.