[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