Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bus error on SPARC for test_recfunctions.TestRecFunctions:test_find_duplicates #4314

Closed
matthew-brett opened this issue Feb 17, 2014 · 11 comments · Fixed by #4544
Closed

Bus error on SPARC for test_recfunctions.TestRecFunctions:test_find_duplicates #4314

matthew-brett opened this issue Feb 17, 2014 · 11 comments · Fixed by #4544

Comments

@matthew-brett
Copy link
Contributor

I am getting a bus error on Debian SPARC during testing of the recarray functions.

On standard Debian squeeze system (uname -a):

Linux vagus 2.6.32-5-sparc64-smp #1 SMP Tue Sep 24 00:00:54 UTC 2013 sparc64 GNU/Linux

Running:

./runtests.py -t numpy/lib/tests/test_recfunctions.py:TestRecFunctions --verbose

gives:

Building, see build.log...
Build OK
Running unit tests for numpy
NumPy version 1.9.0.dev-297f54b
NumPy is installed in /home/matthew/dev_trees/numpy/build/testenv/lib/python2.6/site-packages/numpy
Python version 2.6.6 (r266:84292, Dec 26 2010, 23:29:26) [GCC 4.4.5 20100913 (prerelease)]
nose version 1.3.0
test_drop_fields (test_recfunctions.TestRecFunctions) ... ok
test_find_duplicates (test_recfunctions.TestRecFunctions) ... Bus error
@juliantaylor
Copy link
Contributor

hm the test creates a void dtype with element size 2, but void dtypes have alignment 1 set which triggers the crash in @seberg new mapiter_trivial_get function

@juliantaylor
Copy link
Contributor

can be reproduced on x86 with:

--- a/numpy/core/src/multiarray/lowlevel_strided_loops.c.src
+++ b/numpy/core/src/multiarray/lowlevel_strided_loops.c.src
@@ -1438,6 +1438,8 @@ mapiter_trivial_@name@(PyArrayObject *self, PyArrayObject *ind,

 #if @isget@
 #if @elsize@
+            assert(npy_is_aligned(result_ptr, sizeof(@copytype@)));
+            assert(npy_is_aligned(self_ptr, sizeof(@copytype@)));
             *(@copytype@ *)result_ptr = *(@copytype@ *)self_ptr;
 #else
             copyswap(result_ptr, self_ptr, 0, self);

@seberg
Copy link
Member

seberg commented Feb 17, 2014

Oh, I was not aware that this could be problematic, so we need extra checks in both of those inner loop functions.

@juliantaylor
Copy link
Contributor

it should be enough to check if the data is aligned to itemsize which you are using here instead of checking the dtype alignment

@seberg
Copy link
Member

seberg commented Feb 19, 2014

@juliantaylor, maybe we should just update the nditer aligned flag to do this as well, or is there any downside to that? Or in fact, maybe it already does... The trivial loop doesn't use nditer/npyiter, but checking alignment for the other by hand is somewhat annoying.

@seberg seberg self-assigned this Mar 24, 2014
@seberg seberg added this to the 1.9 blockers milestone Mar 24, 2014
@charris
Copy link
Member

charris commented Mar 24, 2014

@seberg Has this been fixed? I think we need it for 1.9.

@juliantaylor
Copy link
Contributor

we could add back a special case in _IsAligned for void and string arrays, they require the highest alignment required by the platform, which would 16 on spark (or 32 for clongdouble?) and probably 4 or 8 on all others

@juliantaylor
Copy link
Contributor

or we just bump the alignment required for strings and void types in the descriptions, or is there any reason it is set to 1?

@juliantaylor
Copy link
Contributor

hm seems to required for memoryviews so putting it in _IsAligned is probably better

juliantaylor added a commit to juliantaylor/numpy that referenced this issue Mar 25, 2014
Requires 16 bytes alignment from string and flexible dtypes, as
processing functions might access them on the itemsize which can be
larger than 1 byte (e.g. 8 bytes strings)
16 byte the largest alignment required for all numpy copy loops.
Closes numpygh-4314
@seberg
Copy link
Member

seberg commented Mar 26, 2014

I see you had a look at it @juliantaylor, thanks. I don't quite understand all the alignment stuff, but if the current alignment makes sense, how about just changing our aligned functions a little, and have an _isAlignedTo(arr, itemsize) or something like it, so that it is easy to check if it is aligned for this purpose and not just for the dtype itself?

@juliantaylor
Copy link
Contributor

such a function would be useful, though we don't need it anywhere
the proposed fix does something equivalent, so far I know only string and void types have dtype alignments that do not match the itemsize, one could generalize it to (dtype.alignment != dtype.itemsize)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants