From 21893fbb74e8fde2931fbed9b511e2a41362b1ab Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Sun, 3 May 2020 00:07:57 -0700 Subject: 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). --- Lib/test/test_generators.py | 20 ++++++++++++++++++++ Objects/genobject.c | 9 +++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 4d96f44..5824ecd 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -332,6 +332,26 @@ class GeneratorThrowTest(unittest.TestCase): 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 41a63ae..b27fa92 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -512,11 +512,12 @@ throw_here: } 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); -- cgit v0.12