I've filed a bug and attached a patch: http://projects.scipy.org/numpy/ticket/1267
No guarantees that I've found all of the alignment issues. I did a grep for "PyObject **" to find possible locations where PyObject * in arrays were being dereferenced. If I could write a unit test to make it fall over on Solaris, then I fixed it, otherwise I left it alone. For example, there are places where misaligned dereferencing is theoretically possible (OBJECT_dot, OBJECT_compare), but a higher level function already did a "BEHAVED" array cast. In those cases I added a unit test so hopefully we'll be able to catch it in the future if the caller no longer ensures well-behavedness. The unit tests are passing with this patch on Sparc (SunOS 5.8), x86 (RHEL 4) and x86_64 (RHEL 4). Those of you who care about less common architectures may want to try the patch out. Since I don't know the alignment requirements of all of the supported platforms, I erred on the side of caution: only x86 and x86_64 will perform unaligned pointer dereferencing -- Everything else will use the slower-but-sure-to-work memcpy approach. That can easily be changed in npy_cpu.h if necessary. Mike Charles R Harris wrote: > > > On Sun, Oct 18, 2009 at 6:04 AM, Michael Droettboom <md...@stsci.edu > <mailto:md...@stsci.edu>> wrote: > > On 10/16/2009 11:35 PM, Travis Oliphant wrote: > > > > On Oct 15, 2009, at 11:40 AM, Michael Droettboom wrote: > > > >> I recently committed a regression test and bugfix for object > pointers in > >> record arrays of unaligned size (meaning where each record is not a > >> multiple of sizeof(PyObject **)). > >> > >> For example: > >> > >> a1 = np.zeros((10,), dtype=[('o', 'O'), ('c', 'c')]) > >> a2 = np.zeros((10,), 'S10') > >> # This copying would segfault > >> a1['o'] = a2 > >> > >> http://projects.scipy.org/numpy/ticket/1198 > >> > >> Unfortunately, this unit test has opened up a whole hornet's > nest of > >> alignment issues on Solaris. The various reference counting > functions > >> (PyArray_INCREF etc.) in refcnt.c all fail on unaligned object > pointers, > >> for instance. Interestingly, there are comments in there saying > >> "handles misaligned data" (eg. line 190), but in fact it > doesn't, and > >> doesn't look to me like it would. But I won't rule out a > mistake in > >> building it on my part. > > > > Thanks for this bug report. It would be very helpful if you > could > > provide the line number where the code is giving a bus error and > > explain why you think the code in question does not handle > misaligned > > data (it still seems like it should to me --- but perhaps I must be > > missing something --- I don't have a Solaris box to test on). > > Perhaps, the real problem is elsewhere (such as other places > where the > > mistake of forgetting about striding needing to be aligned also > before > > pursuing the fast alignment path that you pointed out in another > place > > of code). > > > > This was the thinking for why the code (that I think is in question) > > should handle mis-aligned data: > > > > 1) pointers that are not aligned to the correct size need to be > copied > > to an aligned memory area before being de-referenced. > > 2) static variables defined in a function will be aligned by the C > > compiler. > > > > So, what the code in refcnt.c does is to copy the value in the NumPy > > data-area (i.e. pointed to by it->dataptr) to another memory > location > > (the stack variable temp), dereference it and then increment it's > > reference count. > > > > 196: temp = (PyObject **)it->dataptr; > > 197: Py_XINCREF(*temp); > This is exactly an instance that fails. Let's say we have a > PyObject at > an aligned location 0x4000 (PyObjects themselves always seem to be > aligned -- I strongly suspect CPython is enforcing that). Then, > we can > create a recarray such that some of the PyObject*'s in it are at > unaligned locations. For example, if the dtype is 'O,c', you have a > record stride of 5 which creates unaligned PyObject*'s: > > OOOOcOOOOcOOOOc > 0123456789abcde > ^ ^ > > Now in the code above, let's assume that it->dataptr points to an > unaligned location, 0x8005. Assigning it to temp puts the same > unaligned value in temp, 0x8005. That is: > > &temp == 0x1000 /* The location of temp *is* on the stack and > aligned */ > temp == 0x8005 /* But its value as a pointer points to an unaligned > memory location */ > *temp == 0x4000 /* Dereferencing it should get us back to the > original > PyObject * pointer, but dereferencing an > unaligned memory location > fails with a bus error on Solaris */ > > So the bus error occurs on line 197. > > Note that something like: > > PyObject* temp; > temp = *(PyObject **)it->dataptr; > > would also fail. > > The solution (this is what works for me, though there may be a > better way): > > PyObject *temp; /* NB: temp is now a (PyObject *), not a (PyObject > **) */ > /* memcpy works byte-by-byte, so can handle an unaligned > assignment */ > memcpy(&temp, it->dataptr, sizeof(PyObject *)); > Py_XINCREF(temp); > > I'm proposing adding a macro which on Intel/AMD would be defined as: > > #define COPY_PYOBJECT_PTR(dst, src) (*(dst) = *(src)) > > and on alignment-required platforms as: > > #define COPY_PYOBJECT_PTR(dst, src) (memcpy((dst), (src), > sizeof(PyObject *)) > > and it would be used something like: > > COPY_PYOBJECT_PTR(&temp, it->dataptr); > > > This looks right to me, but I'll let Travis sign off on it. > > <snip> > > Chuck > > ------------------------------------------------------------------------ > > _______________________________________________ > NumPy-Discussion mailing list > NumPy-Discussion@scipy.org > http://mail.scipy.org/mailman/listinfo/numpy-discussion > -- Michael Droettboom Science Software Branch Operations and Engineering Division Space Telescope Science Institute Operated by AURA for NASA _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
