[ python-Bugs-952807 ] segfault in subclassing datetime.date & pickling

[SourceForge.net]

Bugs item #952807, was opened at 2004-05-12 15:30
Message generated for change (Comment added) made by tim_one
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=952807&group_id=5470

Category: Extension Modules
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Thomas Wouters (twouters)
Assigned to: Nobody/Anonymous (nobody)
Summary: segfault in subclassing datetime.date & pickling

Initial Comment:
datetime.date does not take subclassing into account
properly. datetime.date's tp_new has special code for
unpickling (the single-string argument) which calls
PyObject_New() directly, which doesn't account for the
fact that subclasses may participate in cycle-gc (even
if datetime.date objects do not.)

The result is a segfault in code that unpickles
instances of subclasses of datetime.date:

import pickle, datetime
class mydate(datetime.date): pass
s = pickle.dumps(mydate.today())
broken = pickle.loads(s)
del broken

The 'del broken' is what causes the segfault: the
'mydate' class/type is supposed to participate in GC,
but because of datetime.date's shortcut, that part of
the object is never initialized (nor allocated, I
presume.) The 'broken' instance reaches 0 refcounts,
the GC gets triggered and it reads garbage memory. To
'prove' that the problem isn't caused by pickle itself:

class mydate(datetime.date): pass
broken = mydate('\x07\xd4\x05\x0c')
del broken

causes the same crash, in the GC code.


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

>Comment By: Tim Peters (tim_one)
Date: 2004-06-05 23:30

Message:
Logged In: YES 
user_id=31435

Thank you!  I'm attaching your patch to this report.

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

Comment By: Jiwon Seo (jiwon)
Date: 2004-06-05 20:07

Message:
Logged In: YES 
user_id=595483

It was as you expected.  =^)
I fixed it in the same way.

Here is the patch.
http://seojiwon.dnip.net:8000/~jiwon/tmp/datetime.diff (too
long to copy&paste here)

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

Comment By: Tim Peters (tim_one)
Date: 2004-06-05 18:13

Message:
Logged In: YES 
user_id=31435

I expect that datetime.datetime and datetime.time objects 
must have the same kind of vulnerability.  Jiwon, can you 
address those too while you're at it?

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

Comment By: Jiwon Seo (jiwon)
Date: 2004-06-05 10:57

Message:
Logged In: YES 
user_id=595483

Here is the patch of datetimemodule and test code for it. I
just read the summary, and made the datetimemodule patch as
is said, and added a testcode for it.

*** Modules/datetimemodule.c.orig       Sat Jun  5 23:49:26 2004
--- Modules/datetimemodule.c    Sat Jun  5 23:47:05 2004
***************
*** 2206,2212 ****
        {
                PyDateTime_Date *me;

!               me = PyObject_New(PyDateTime_Date, type);
                if (me != NULL) {
                        char *pdata = PyString_AS_STRING(state);
                        memcpy(me->data, pdata,
_PyDateTime_DATE_DATASIZE);
--- 2206,2212 ----
        {
                PyDateTime_Date *me;

!               me = (PyDateTime_Date *)
(type->tp_alloc(type, 0));
                if (me != NULL) {
                        char *pdata = PyString_AS_STRING(state);
                        memcpy(me->data, pdata,
_PyDateTime_DATE_DATASIZE);

test code patch
*** Lib/test/test_datetime.py.orig      Sat Jun  5 23:49:44 2004
--- Lib/test/test_datetime.py   Sat Jun  5 23:52:52 2004
***************
*** 510,515 ****
--- 510,517 ----
          dt2 = dt - delta
          self.assertEqual(dt2, dt - days)

+ class SubclassDate(date): pass
+
  class TestDate(HarmlessMixedComparison):
      # Tests here should pass for both dates and datetimes,
except for a
      # few tests that TestDateTime overrides.
***************
*** 1028,1033 ****
--- 1030,1044 ----
          self.assertEqual(dt2.extra, 7)
          self.assertEqual(dt1.toordinal(), dt2.toordinal())
          self.assertEqual(dt2.newmeth(-7), dt1.year +
dt1.month - 7)
+
+     def test_pickling_subclass_date(self):
+
+         args = 6, 7, 23
+         orig = SubclassDate(*args)
+         for pickler, unpickler, proto in pickle_choices:
+             green = pickler.dumps(orig, proto)
+             derived = unpickler.loads(green)
+             self.assertEqual(orig, derived)

      def test_backdoor_resistance(self):
          # For fast unpickling, the constructor accepts a
pickle string.

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

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