Skip to content

Commit 21893fb

Browse files
authored
bpo-29587: allow chaining NULL exceptions in _gen_throw() (GH-19877)
This is a follow-up to GH-19823 that removes the check that the exception value isn't NULL, prior to calling _PyErr_ChainExceptions(). This enables implicit exception chaining for gen.throw() in more circumstances. The commit also adds a test that a particular code snippet involving gen.throw() doesn't crash. The test shows why the new `gi_exc_state.exc_type != Py_None` check that was added is necessary. Without the new check, the code snippet (as well as a number of other tests) crashes on certain platforms (e.g. Fedora but not Mac).
1 parent 0400a7f commit 21893fb

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

Lib/test/test_generators.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,26 @@ def f():
332332
context = cm.exception.__context__
333333
self.assertEqual((type(context), context.args), (KeyError, ('a',)))
334334

335+
def test_throw_after_none_exc_type(self):
336+
def g():
337+
try:
338+
raise KeyError
339+
except KeyError:
340+
pass
341+
342+
try:
343+
yield
344+
except Exception:
345+
# Without the `gi_exc_state.exc_type != Py_None` in
346+
# _gen_throw(), this line was causing a crash ("Segmentation
347+
# fault (core dumped)") on e.g. Fedora 32.
348+
raise RuntimeError
349+
350+
gen = g()
351+
gen.send(None)
352+
with self.assertRaises(RuntimeError) as cm:
353+
gen.throw(ValueError)
354+
335355

336356
class YieldFromTests(unittest.TestCase):
337357
def test_generator_gi_yieldfrom(self):

Objects/genobject.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -512,11 +512,12 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
512512
}
513513

514514
PyErr_Restore(typ, val, tb);
515-
/* XXX Should we also handle the case where exc_type is true and
516-
exc_value is false? */
517-
if (gen->gi_exc_state.exc_type && gen->gi_exc_state.exc_value) {
515+
/* XXX It seems like we shouldn't have to check not equal to Py_None
516+
here because exc_type should only ever be a class. But not including
517+
this check was causing crashes on certain tests e.g. on Fedora. */
518+
if (gen->gi_exc_state.exc_type && gen->gi_exc_state.exc_type != Py_None) {
518519
Py_INCREF(gen->gi_exc_state.exc_type);
519-
Py_INCREF(gen->gi_exc_state.exc_value);
520+
Py_XINCREF(gen->gi_exc_state.exc_value);
520521
Py_XINCREF(gen->gi_exc_state.exc_traceback);
521522
_PyErr_ChainExceptions(gen->gi_exc_state.exc_type,
522523
gen->gi_exc_state.exc_value, gen->gi_exc_state.exc_traceback);

0 commit comments

Comments
 (0)