This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: _PyInstance_Lookup() defeats its purpose
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, arigo, eric.araujo, ezio.melotti, georg.brandl, pitrou, serhiy.storchaka
Priority: low Keywords:

Created on 2010-09-03 09:30 by arigo, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (9)
msg115426 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2010-09-03 09:30
_PyInstance_Lookup() is documented as follows:

    The point of this routine is that it never calls arbitrary Python
    code, so is always "safe":  all it does is dict lookups.

But of course dict lookups can call arbitrary Python code.  This function has no purpose, because it's not possible to be "safe".  I checked in a crasher:

http://svn.python.org/projects/python/branches/release27-maint/Lib/test/crashers/gc_has_finalizer.py
msg115444 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-09-03 12:08
I'm not sure anyone will be interested in this: it doesn't affect 3.x.
msg116139 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-09-12 00:22
It’s not clear whether you want a doc fix or a behavior change.
msg116154 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2010-09-12 08:21
I propose that we first attempt to fix the crasher; depending on the solution we might then either fix the doc or the code for _PyInstance_Lookup().

If no-one is willing to fix this bug I am fine to let it go.  But somehow I am sure that there is code *somewhere* that sticks non-string keys in old-style classes or instances, and whose __eq__ has side-effects, like caching.  This would create hard-to-reproduce and hard-to-diagnose segfaults.  That's mainly the reason why I think that it should be fixed.
msg160267 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-05-09 09:56
I tracked this down a bit and this is what I found:
has_finalizer in Modules/gcmodule.c calls
    return _PyInstance_Lookup(op, delstr) != NULL;
_PyInstance_Lookup in Modules/classobject.c calls
    v = class_lookup(inst->in_class, name, &klass);
    where inst is (PyInstanceObject *)op;
class_lookup in Modules/classobject.c eventually calls
    PyObject *value = PyDict_GetItem(cp->cl_dict, name);
    where cp is (PyClassObject *)inst->in_class
and since cp is not a valid pointer, cp->cl_dict results in the segfault after a few recursive calls of class_lookup.

Confirmed that this only affects 2.7.
msg160269 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-09 10:19
Well, I don't think there's any point in trying to fixing this now.
msg160310 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2012-05-09 19:31
Unlike other crashers I'm a bit concerned about this one.  It can occur on any code that stores custom instances as keys in the __dict__ of an old-style instance.  Such code might be unusual-looking, but certainly not unheard-of.  And the segfault that we get then is very rare and impossible to reproduce in practice because it depends on when exactly the GC is running and on the low bits of hashes colliding.
msg366806 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2020-04-19 21:51
Python 2 is EOL.
msg370482 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-05-31 15:19
Python 2.7 is no longer supported.
History
Date User Action Args
2022-04-11 14:57:06adminsetgithub: 53960
2020-05-31 15:19:57serhiy.storchakasetstatus: open -> closed

nosy: + serhiy.storchaka
messages: + msg370482

resolution: out of date
stage: needs patch -> resolved
2020-04-19 21:51:36ZackerySpytzsetnosy: + ZackerySpytz
messages: + msg366806
2012-05-09 19:31:09arigosetmessages: + msg160310
2012-05-09 10:19:20pitrousetnosy: + pitrou
messages: + msg160269
2012-05-09 09:56:33ezio.melottisetnosy: + ezio.melotti
messages: + msg160267
2010-09-12 08:21:44arigosetmessages: + msg116154
2010-09-12 00:22:27eric.araujosetnosy: + eric.araujo
messages: + msg116139
2010-09-03 12:08:15georg.brandlsetnosy: + georg.brandl
messages: + msg115444
2010-09-03 09:30:10arigocreate