https://github.com/python/cpython/commit/21893fbb74e8fde2931fbed9b511e2a4136... commit: 21893fbb74e8fde2931fbed9b511e2a41362b1ab branch: master author: Chris Jerdonek <chris.jerdonek@gmail.com> committer: GitHub <noreply@github.com> date: 2020-05-03T00:07:57-07:00 summary: 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). files: M Lib/test/test_generators.py M Objects/genobject.c diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 4d96f44b15062..5824ecd7c37e8 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -332,6 +332,26 @@ def f(): context = cm.exception.__context__ self.assertEqual((type(context), context.args), (KeyError, ('a',))) + def test_throw_after_none_exc_type(self): + def g(): + try: + raise KeyError + except KeyError: + pass + + try: + yield + except Exception: + # Without the `gi_exc_state.exc_type != Py_None` in + # _gen_throw(), this line was causing a crash ("Segmentation + # fault (core dumped)") on e.g. Fedora 32. + raise RuntimeError + + gen = g() + gen.send(None) + with self.assertRaises(RuntimeError) as cm: + gen.throw(ValueError) + class YieldFromTests(unittest.TestCase): def test_generator_gi_yieldfrom(self): diff --git a/Objects/genobject.c b/Objects/genobject.c index 41a63ae2e666a..b27fa929a2625 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -512,11 +512,12 @@ _gen_throw(PyGenObject *gen, int close_on_genexit, } PyErr_Restore(typ, val, tb); - /* XXX Should we also handle the case where exc_type is true and - exc_value is false? */ - if (gen->gi_exc_state.exc_type && gen->gi_exc_state.exc_value) { + /* XXX It seems like we shouldn't have to check not equal to Py_None + here because exc_type should only ever be a class. But not including + this check was causing crashes on certain tests e.g. on Fedora. */ + if (gen->gi_exc_state.exc_type && gen->gi_exc_state.exc_type != Py_None) { Py_INCREF(gen->gi_exc_state.exc_type); - Py_INCREF(gen->gi_exc_state.exc_value); + Py_XINCREF(gen->gi_exc_state.exc_value); Py_XINCREF(gen->gi_exc_state.exc_traceback); _PyErr_ChainExceptions(gen->gi_exc_state.exc_type, gen->gi_exc_state.exc_value, gen->gi_exc_state.exc_traceback);