[Numpy-discussion] object array alignment issues
Michael Droettboom
mdroe@stsci....
Mon Oct 19 09:55:43 CDT 2009
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 <mdroe@stsci.edu
> <mailto:mdroe@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
More information about the NumPy-Discussion
mailing list