Setting state of an exception object with a dic crashes Python 3.8.14

[xiaxinmeng]

Crash report

The following programs defined a class C. In C, we perform dic clear() operation. When we set state of exception object with the dic, it causes segmentation fault on main branch (68c46ae) and latest stable Python 3.8.14. But it does not trigger any crashing on Python-3.11.0rc2 and Python 3.9.0.

class C(str):
    def __hash__(self):
        d.clear()
        return 0

d = {}
d[C()] = C()

e = Exception()
e.__setstate__(d)

Error messages

Segmentation Fault (Core dumped)

Your environment

• CPython versions tested on: Python 3.8.14 and main branch (68c46ae)
• Operating system and architecture: [GCC 7.5.0] on linux

[gvanrossum]

I ran this in main under the debugger and it looks like the crash actually occurs during final cleanup, in ClearWeakRefs called from free_keys_object. Not sure if I have the guts to continue debugging.

[ofey404]

Reproduced this in python 3.10.2, and latest main (83a3de4)

I would try to dig a little deeper but can't guarantee there will be any output (my knowledge about the python runtime is limited).

[ofey404]

I debugged with following code and found:

• The cause is the d.clear() in Key.__hash__().
• Any value type will trigger the segfault, except some types (str and dict won't cause crush, but list would.)

import gc

class Key(str):
    def __hash__(self):
        d.clear()
        return 0

class Value(str):
    pass

d = {}
d[Key()] = Value()

e = Exception()
e.__setstate__(d)

gc.collect() # Segfault happens here

According the stacktrace, it dies in deduce_unreachable in GC module.

cpython/Modules/gcmodule.c

Lines 1095 to 1103 in ff54dd9

	deduce_unreachable(PyGC_Head *base, PyGC_Head *unreachable) {
	validate_list(base, collecting_clear_unreachable_clear);
	/* Using ob_refcnt and gc_refs, calculate which objects in the
	* container set are reachable from outside the set (i.e., have a
	* refcount greater than 0 when all the references within the
	* set are taken into account).
	*/
	update_refs(base); // gc_prev is used for gc_refs
	subtract_refs(base);

And finally if (Py_TYPE(v)->tp_repr == NULL) triggered the segfault.

cpython/Objects/object.c

Lines 409 to 411 in ff54dd9

	if (Py_TYPE(v)->tp_repr == NULL)
	return PyUnicode_FromFormat("<%s object at %p>",
	Py_TYPE(v)->tp_name, v);

[ofey404]

It seems that crash happening during garbage collector subtracting internal references:

cpython/Modules/gcmodule.c

Lines 465 to 482 in ff54dd9

	/* Subtract internal references from gc_refs. After this, gc_refs is >= 0
	* for all objects in containers, and is GC_REACHABLE for all tracked gc
	* objects not in containers. The ones with gc_refs > 0 are directly
	* reachable from outside containers, and so can't be collected.
	*/
	static void
	subtract_refs(PyGC_Head *containers)
	{
	traverseproc traverse;
	PyGC_Head *gc = GC_NEXT(containers);
	for (; gc != containers; gc = GC_NEXT(gc)) {
	PyObject *op = FROM_GC(gc);
	traverse = Py_TYPE(op)->tp_traverse;
	(void) traverse(op,
	(visitproc)visit_decref,
	op);
	}
	}

While traversing through the objects, it meets an object, and the _PyObject_ASSERT failed:

• The parent is not freed, but op is freed, so it triggers the assertion.

cpython/Modules/gcmodule.c

Lines 448 to 450 in ff54dd9

	visit_decref(PyObject *op, void *parent)
	{
	_PyObject_ASSERT(_PyObject_CAST(parent), !_PyObject_IsFreed(op));

Then it wants to dump the parent (the dict in this scenario) with message, but segfault happens during the process.

(During printing the representation of the value. Strange.)

cpython/Objects/dictobject.c

Line 2416 in 9f2f1dd

s = PyObject_Repr(value);

[ofey404]

What confused me is that, if I comment out the e.__setstate__ line, nothing happens during garbage collection.

Does the __setstate__ call put the dict into an inconsistent state? Or the problem lays in the d.clear() in key's hash function.

[ofey404]

The problem lies on the value's refcount: it becomes -2459565876494606881.

[ofey404]

Find the cause of wrongly set refcount:

cpython/Objects/typeobject.c

Lines 4017 to 4032 in 9f2f1dd

	/* Internal API to look for a name through the MRO, bypassing the method cache.
	This returns a borrowed reference, and might set an exception.
	'error' is set to: -1: error with exception; 1: error without exception; 0: ok */
	static PyObject *
	find_name_in_mro(PyTypeObject *type, PyObject *name, int *error)
	{
	Py_hash_t hash;
	if (!PyUnicode_CheckExact(name) ||
	(hash = _PyASCIIObject_CAST(name)->hash) == -1)
	{
	hash = PyObject_Hash(name);
	if (hash == -1) {
	*error = -1;
	return NULL;
	}
	}

The hash = PyObject_Hash(name); is exactly the line which touch the refcount.

---

So the problem is in _PyType_Lookup at _PyObject_GenericSetAttrWithDict. Finally it will call __hash__ method again (call d.clear()), and the refcount of value would be wrongly toggled.

cpython/Objects/object.c

Lines 1370 to 1390 in ff54dd9

	_PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
	PyObject *value, PyObject *dict)
	{
	PyTypeObject *tp = Py_TYPE(obj);
	PyObject *descr;
	descrsetfunc f;
	int res = -1;
	if (!PyUnicode_Check(name)){
	PyErr_Format(PyExc_TypeError,
	"attribute name must be string, not '%.200s'",
	Py_TYPE(name)->tp_name);
	return -1;
	}
	if (tp->tp_dict == NULL && PyType_Ready(tp) < 0)
	return -1;
	Py_INCREF(name);
	Py_INCREF(tp);
	descr = _PyType_Lookup(tp, name);

Full stack trace when the refcount is changed.

libc.so.6!__memset_avx2_unaligned_erms (Unknown Source:0)
_PyMem_DebugRawFree(void * ctx, void * p) (\home\ofey\Code\cpython-source-reading\cpython\Objects\obmalloc.c:2575)
_PyMem_DebugFree(void * ctx, void * ptr) (\home\ofey\Code\cpython-source-reading\cpython\Objects\obmalloc.c:2709)
PyObject_Free(void * ptr) (\home\ofey\Code\cpython-source-reading\cpython\Objects\obmalloc.c:742)
PyObject_GC_Del(void * op) (\home\ofey\Code\cpython-source-reading\cpython\Modules\gcmodule.c:2366)
unicode_dealloc(PyObject * unicode) (\home\ofey\Code\cpython-source-reading\cpython\Objects\unicodeobject.c:1557)
subtype_dealloc(PyObject * self) (\home\ofey\Code\cpython-source-reading\cpython\Objects\typeobject.c:1573)
_Py_Dealloc(PyObject * op) (\home\ofey\Code\cpython-source-reading\cpython\Objects\object.c:2383)
Py_DECREF(PyObject * op, int lineno, const char * filename) (\home\ofey\Code\cpython-source-reading\cpython\Include\object.h:549)
Py_XDECREF(PyObject * op) (\home\ofey\Code\cpython-source-reading\cpython\Include\object.h:625)
free_keys_object(PyDictKeysObject * keys) (\home\ofey\Code\cpython-source-reading\cpython\Objects\dictobject.c:672)
dictkeys_decref(PyDictKeysObject * dk) (\home\ofey\Code\cpython-source-reading\cpython\Objects\dictobject.c:324)
PyDict_Clear(PyObject * op) (\home\ofey\Code\cpython-source-reading\cpython\Objects\dictobject.c:2070)
dict_clear(PyDictObject * mp, PyObject * _unused_ignored) (\home\ofey\Code\cpython-source-reading\cpython\Objects\dictobject.c:3379)
method_vectorcall_NOARGS(PyObject * func, PyObject * const * args, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Objects\descrobject.c:454)
_PyObject_VectorcallTstate(PyThreadState * tstate, PyObject * callable, PyObject * const * args, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Include\internal\pycore_call.h:92)
PyObject_Vectorcall(PyObject * callable, PyObject * const * args, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Objects\call.c:300)
_PyEval_EvalFrameDefault(PyThreadState * tstate, _PyInterpreterFrame * frame, int throwflag) (\home\ofey\Code\cpython-source-reading\cpython\Python\ceval.c:4173)
_PyEval_EvalFrame(int throwflag, struct _PyInterpreterFrame * frame, PyThreadState * tstate) (\home\ofey\Code\cpython-source-reading\cpython\Include\internal\pycore_ceval.h:95)
_PyEval_Vector(PyThreadState * tstate, PyFunctionObject * func, PyObject * locals, PyObject * const * args, size_t argcount, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Python\ceval.c:5791)
_PyFunction_Vectorcall(PyObject * func, PyObject * const * stack, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Objects\call.c:396)
_PyObject_VectorcallTstate(PyObject * kwnames, size_t nargsf, PyObject * const * args, PyObject * callable, PyThreadState * tstate) (\home\ofey\Code\cpython-source-reading\cpython\Include\internal\pycore_call.h:92)
PyObject_CallOneArg(PyObject * func, PyObject * arg) (\home\ofey\Code\cpython-source-reading\cpython\Objects\call.c:378)
call_unbound_noarg(int unbound, PyObject * func, PyObject * self) (\home\ofey\Code\cpython-source-reading\cpython\Objects\typeobject.c:1759)
slot_tp_hash(PyObject * self) (\home\ofey\Code\cpython-source-reading\cpython\Objects\typeobject.c:8044)
PyObject_Hash(PyObject * v) (\home\ofey\Code\cpython-source-reading\cpython\Objects\object.c:769)
find_name_in_mro(PyTypeObject * type, PyObject * name, int * error) (\home\ofey\Code\cpython-source-reading\cpython\Objects\typeobject.c:4027)
_PyType_Lookup(PyTypeObject * type, PyObject * name) (\home\ofey\Code\cpython-source-reading\cpython\Objects\typeobject.c:4097)
_PyObject_GenericSetAttrWithDict(PyObject * obj, PyObject * name, PyObject * value, PyObject * dict) (\home\ofey\Code\cpython-source-reading\cpython\Objects\object.c:1390)
PyObject_GenericSetAttr(PyObject * obj, PyObject * name, PyObject * value) (\home\ofey\Code\cpython-source-reading\cpython\Objects\object.c:1463)
PyObject_SetAttr(PyObject * v, PyObject * name, PyObject * value) (\home\ofey\Code\cpython-source-reading\cpython\Objects\object.c:1022)
BaseException_setstate(PyObject * self, PyObject * state) (\home\ofey\Code\cpython-source-reading\cpython\Objects\exceptions.c:170)
method_vectorcall_O(PyObject * func, PyObject * const * args, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Objects\descrobject.c:482)
_PyObject_VectorcallTstate(PyThreadState * tstate, PyObject * callable, PyObject * const * args, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Include\internal\pycore_call.h:92)
PyObject_Vectorcall(PyObject * callable, PyObject * const * args, size_t nargsf, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Objects\call.c:300)
_PyEval_EvalFrameDefault(PyThreadState * tstate, _PyInterpreterFrame * frame, int throwflag) (\home\ofey\Code\cpython-source-reading\cpython\Python\ceval.c:4173)
_PyEval_EvalFrame(int throwflag, struct _PyInterpreterFrame * frame, PyThreadState * tstate) (\home\ofey\Code\cpython-source-reading\cpython\Include\internal\pycore_ceval.h:95)
_PyEval_Vector(PyThreadState * tstate, PyFunctionObject * func, PyObject * locals, PyObject * const * args, size_t argcount, PyObject * kwnames) (\home\ofey\Code\cpython-source-reading\cpython\Python\ceval.c:5791)
PyEval_EvalCode(PyObject * co, PyObject * globals, PyObject * locals) (\home\ofey\Code\cpython-source-reading\cpython\Python\ceval.c:591)
run_eval_code_obj(PyThreadState * tstate, PyCodeObject * co, PyObject * globals, PyObject * locals) (\home\ofey\Code\cpython-source-reading\cpython\Python\pythonrun.c:1713)
run_mod(mod_ty mod, PyObject * filename, PyObject * globals, PyObject * locals, PyCompilerFlags * flags, PyArena * arena) (\home\ofey\Code\cpython-source-reading\cpython\Python\pythonrun.c:1734)
pyrun_file(FILE * fp, PyObject * filename, int start, PyObject * globals, PyObject * locals, int closeit, PyCompilerFlags * flags) (\home\ofey\Code\cpython-source-reading\cpython\Python\pythonrun.c:1629)
_PyRun_SimpleFileObject(FILE * fp, PyObject * filename, int closeit, PyCompilerFlags * flags) (\home\ofey\Code\cpython-source-reading\cpython\Python\pythonrun.c:439)
_PyRun_AnyFileObject(FILE * fp, PyObject * filename, int closeit, PyCompilerFlags * flags) (\home\ofey\Code\cpython-source-reading\cpython\Python\pythonrun.c:78)
pymain_run_file_obj(PyObject * program_name, PyObject * filename, int skip_source_first_line) (\home\ofey\Code\cpython-source-reading\cpython\Modules\main.c:360)
pymain_run_file(const PyConfig * config) (\home\ofey\Code\cpython-source-reading\cpython\Modules\main.c:379)
pymain_run_python(int * exitcode) (\home\ofey\Code\cpython-source-reading\cpython\Modules\main.c:610)
Py_RunMain() (\home\ofey\Code\cpython-source-reading\cpython\Modules\main.c:689)
pymain_main(_PyArgv * args) (\home\ofey\Code\cpython-source-reading\cpython\Modules\main.c:719)
Py_BytesMain(int argc, char ** argv) (\home\ofey\Code\cpython-source-reading\cpython\Modules\main.c:743)
main(int argc, char ** argv) (\home\ofey\Code\cpython-source-reading\cpython\Programs\python.c:15)

[ofey404]

So... What should we do with it?

I think I need some guidance/reference now, about what _PyType_Lookup does, and what's the MRO.

[gvanrossum]

Thanks for the deep research!

I think the root cause is something that happens in BaseException_setstate(), in Python/exceptions.c. In particular these four lines:

        while (PyDict_Next(state, &i, &d_key, &d_value)) {
            if (PyObject_SetAttr(self, d_key, d_value) < 0)
                return NULL;
        }

are likely to end up missing an INCREF when the dict is cleared by the hash call. I suspect it's happening in the PyObject_SetAttr() call, since PyDict_Next() has no need for the hash.

Looking at the code for PyObject_SetAttr(), it does an INCREF(name) / DECREF(name) around the important bits, but it doesn't do the same for the value. Possibly it should -- or in this case the caller should?

[ofey404]

I tried to play with it, adding INCREF(value) / DECREF(value) pair around some suspicious place, but the Makefile crushed before I can run and debug what's happending inside.

I wonder how could I build and inspect INCREF and DECREF behavior correctly, and I would post this question in the Core-Mentorship list.

make -s -j
# make: *** [Makefile:1306: Python/deepfreeze/deepfreeze.c] Segmentation fault (core dumped)

[kumaraditya303]

It is indeed missing an incref before calling tp_hash slot in PyObject_SetAttr. See similar issue #92930

The following patch fixes it:

diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index 3703fdcda4..3680c6068b 100644
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -167,8 +167,14 @@ BaseException_setstate(PyObject *self, PyObject *state)
             return NULL;
         }
         while (PyDict_Next(state, &i, &d_key, &d_value)) {
-            if (PyObject_SetAttr(self, d_key, d_value) < 0)
+            Py_INCREF(d_key);
+            Py_INCREF(d_value);
+            int res = PyObject_SetAttr(self, d_key, d_value);
+            Py_DECREF(d_key);
+            Py_DECREF(d_value);
+            if (res < 0) {
                 return NULL;
+            }
         }
     }
     Py_RETURN_NONE;

[ofey404]

Wow, I'll check the issue. With this patch, the bug disappeared. Should I make a PR for the patch? @kumaraditya303

---

I guess it's fixed before, but the patch is reversed for some reason. I would look into the line history.

In the latest main, the inc/dec ref pair is still missing:

cpython/Objects/exceptions.c

Lines 158 to 175 in 273a819

	static PyObject *
	BaseException_setstate(PyObject *self, PyObject *state)
	{
	PyObject *d_key, *d_value;
	Py_ssize_t i = 0;
	if (state != Py_None) {
	if (!PyDict_Check(state)) {
	PyErr_SetString(PyExc_TypeError, "state is not a dictionary");
	return NULL;
	}
	while (PyDict_Next(state, &i, &d_key, &d_value)) {
	if (PyObject_SetAttr(self, d_key, d_value) < 0)
	return NULL;
	}
	}
	Py_RETURN_NONE;
	}

[kumaraditya303]

Wow, I'll check the issue. With this patch, the bug disappeared. Should I make a PR for the patch?

Feel free to create a PR with tests.

[ofey404]

I check the line history but didn't find much useful information. These lines seem to be unchanged since the cpython repo migrated from SVN in 2006? This is the commit.

git log -L '/PyObject_SetAttr(self, d_key, d_value)/,+5:Objects/exceptions.c'

  commit 4d70c3d9dded0f0fa7a73c67217a71111d05df4d
  Author: Thomas Wouters <[email protected]>
  Date:   Thu Jun 8 14:42:34 2006 +0000

....

  diff --git a/Objects/exceptions.c b/Objects/exceptions.c
  --- a/Objects/exceptions.c
  +++ b/Objects/exceptions.c
  @@ -167,0 +170,5 @@
  +            if (PyObject_SetAttr(self, d_key, d_value) < 0)
  +                return NULL;
  +        }
  +    }
  +    Py_RETURN_NONE;

[ofey404]

Feel free to create a PR with tests.

I would create a test triggering this behavior in Lib/test/test_exception.py ;)

[ofey404]

PR is created.

Lib/test/test_baseexception.py is a better place for this test to go, since:

cpython/Lib/test/test_baseexception.py

Lines 7 to 10 in 273a819

	class ExceptionClassTests(unittest.TestCase):
	"""Tests for anything relating to exception objects themselves (e.g.,
	inheritance hierarchy)"""

[gvanrossum]

Thanks for your investigation and patch!