[Patches] [ python-Patches-1020188 ] Use Py_CLEAR where necessary to avoid crashes

[SourceForge.net]

Patches item #1020188, was opened at 2004-09-01 00:35
Message generated for change (Comment added) made by rhettinger
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1020188&group_id=5470

Category: Core (C code)
Group: None
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: Dima Dorfman (ddorfman)
Assigned to: Michael Hudson (mwh)
Summary: Use Py_CLEAR where necessary to avoid crashes

Initial Comment:
The Py_CLEAR macro was introduced to make it less tempting
to write incorrect code in the form

  Py_DECREF(self->field);
  self->field = NULL;

because that can expose a half-destroyed object if
deallocation can cause self->field to be read. This patch
fixes mistakes like this that still exist in the core.

Only cases that are potentially dangerous are fixed in this
patch. If self->field can only reference a special kind of
[builtin] object, then it's just a style bug because we know
that the builtin object isn't evil. Likewise if the code is
operating on an automatic variable. It might be nice to fix
those style bugs anyway, to discourage the broken idiom.


Just for kicks, here's a way to use this bug in reversed to
crash the interpreter:

  import array, gc, weakref
  a = array.array('c')
  wr = weakref.ref(a, lambda _: gc.get_referents(rev))
  rev = reversed(a)
  del a
  list(rev)

For me, this crashes immediately with a debug build and
after a couple tries otherwise.


Also attached is clearcand.py to help find these cases. It's
not comprehensive, but it's a start. Usage:

  $ find src -name '*.c' | python clearcand.py | fgrep -- '->'


The patch requires SF #1020185 to be applied for genobject.c
to compile without warnings.

----------------------------------------------------------------------

>Comment By: Raymond Hettinger (rhettinger)
Date: 2004-09-08 12:33

Message:
Logged In: YES 
user_id=80475

FWIW, rather than add specific testcases (closing the 
barndoor after the horse has escaped), it might be a good 
idea to fixup the OP's search code and add it to the toolbox.

Future errors of this type are better caught through code 
scans than by testing stuff that already works fine.

Of course, if highly motivated, you can do both :-)

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2004-09-01 07:32

Message:
Logged In: YES 
user_id=6656

Test cases would be nice.  If it's easy for someone whose thought 
about it, that would be great, if not assign to me and I'll get to it...

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2004-09-01 02:06

Message:
Logged In: YES 
user_id=80475

Accepted and applied.
See:
    Include/object.h 2.129
    Modules/itertoolsmodule.c 1.35
    Objects/enumobject.c 1.18
    Objects/genobject.c 1.4
    Objects/iterobject.c 1.19

Thanks for the patch.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1020188&group_id=5470