8000 BUG: crash from inside highway running the numpy tests with asan on MacOS with Clang15 · Issue #27023 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: crash from inside highway running the numpy tests with asan on MacOS with Clang15 #27023

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.

Sign up for GitHub

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

Jump to bottom
Closed
ngoldbaum opened this issue Jul 24, 2024 · 7 comments
Closed

BUG: crash from inside highway running the numpy tests with asan on MacOS with Clang15 #27023

ngoldbaum opened this issue Jul 24, 2024 · 7 comments
Assignees
@Mousius
Labels
00 - Bug component: SIMD Issues in SIMD (fast instruction sets) code or machinery

Comments

@ngoldbaum
Copy link
Member

If I build NumPy with Clang 15 from homebrew on MacOS:

CC=$(brew --prefix llvm@15)/bin/clang \
CXX=$(brew --prefix llvm@15)/bin/clang++ \
LDFLAGS="-L/$(brew --prefix llvm@15)/lib" \
CPPFLAGS="-I/$(brew --prefix llvm@15)/include" \
spin build --clean -- -Db_sanitize=address,undefined --buildtype=debug

And then run the tests like so:

PYTHONPATH=$PWD/build-install/usr/lib/python3.12/site-packages ASAN_OPTIONS=detect_leaks=0:symbolize=1:strict_init_order=true:allocator_may_return_null=1:halt_on_error=1 UBSAN_OPTIONS=halt_on_error=0 DYLD_INSERT_LIBRARIES=/opt/homebrew/Cellar/llvm@15/15.0.7/lib/clang/15.0.7/lib/darwin/libclang_rt.asan_osx_dynamic.dylib $(pyenv which python) $(pyenv which pytest) -svx "build-install/usr/lib/python3.12/site-packages/numpy/_core/tests/test_multiarray.py"

I get a segfault coming from the internals of highway:

build-install/usr/lib/python3.12/site-packages/numpy/_core/tests/test_multiarray.py::TestMethods::test_sort Abort at vqsort-inl.h:2168: Assert 0
    #0 0x1010f9480 in __sanitizer_print_stack_trace+0x28 (libclang_rt.asan_osx_dynamic.dylib:arm64+0x49480) (BuildId: 3b5251098aeb340ca4b47d5ff02ddb3832000000200000000100000000000b00)
    #1 0x10f7b5364 in hwy::Abort(char const*, int, char const*, ...) abort.cc:58
    #2 0x10f7b4a58 in void hwy::N_NEON::VQSortStatic<double>(double*, unsigned long, hwy::SortAscending) vqsort-inl.h:2168
    #3 0x10f7b4a08 in void np::highway::qsort_simd::QSort_ASIMD<double>(double*, long) highway_qsort.dispatch.cpp:17
    #4 0x10e6e5a7c in bool quicksort_dispatch<double>(double*, long) quicksort.cpp:105
    #5 0x10e6e59e0 in quicksort_double quicksort.cpp:814
    #6 0x10e370f40 in _new_sortlike item_selection.c:1265
    #7 0x10e36f7e0 in PyArray_Sort item_selection.c:1566
    #8 0x10e48b140 in array_sort methods.c:1317
    #9 0x10224a898 in method_vectorcall_FASTCALL_KEYWORDS+0xa8 (libpython3.12.dylib:arm64+0x5e898) (BuildId: 5f2b9f823b583ece98800e23be4e540432000000200000000100000000050e00)
    #10 0x10223eda0 in PyObject_Vectorcall+0x54 (libpython3.12.dylib:arm64+0x52da0) (BuildId: 5f2b9f823b583ece98800e23be4e540432000000200000000100000000050e00)
    #11 0x10232f6c0 in _PyEval_EvalFrameDefault+0x92a8 (libpython3.12.dylib:arm64+0x1436c0) (BuildId: 5f2b9f823b583ece98800e23be4e540432000000200000000100000000050e00)
    #12 0x10223eda0 in PyObject_Vectorcall+0x54 (libpython3.12.dylib:arm64+0x52da0) (BuildId: 5f2b9f823b583ece98800e23be4e540432000000200000000100000000050e00)
@ngoldbaum ngoldbaum added 00 - Bug component: SIMD Issues in SIMD (fast instruction sets) code or machinery labels Jul 24, 2024
@ngoldbaum
Copy link
Member Author

I can reliably reproduce this although admittedly this is a weird setup so if anyone wants me to do things to help debug I'm happy to do so.

@Mousius
Copy link
Member
Mousius commented Jul 24, 2024

Doesn't look like a SegFault, Highway has decided that VQSort isn't available:
https://github.com/google/highway/blob/1dbb1180e05c55b648f2508d3f97bf26c6f926a8/hwy/contrib/sort/vqsort-inl.h#L2158-L2169

Looks like there's logic added due to crashes in certain compilers, sanitizers and target combinations:
https://github.com/google/highway/blob/44ad2983f7bd7d197c0b46ef881c096fc842a3bb/hwy/contrib/sort/shared-inl.h#L130-L138

Can you try wrapping these with #if VQSORT_ENABLED?

@Mousius Mousius self-assigned this Jul 24, 2024
@ngoldbaum ngoldbaum changed the title BUG: segfault from inside highway running the numpy tests with asan on MacOS with Clang15 BUG: build failure from inside highway running the numpy tests with asan on MacOS with Clang15 Jul 24, 2024
@ngoldbaum ngoldbaum changed the title BUG: build failure from inside highway running the numpy tests with asan on MacOS with Clang15 BUG: crash from inside highway running the numpy tests with asan on MacOS with Clang15 Jul 24, 2024
@ngoldbaum
Copy link
Member Author

Not sure if I added the macros correctly, but if I do it like so:

diff --git a/numpy/_core/src/npysort/highway_qsort.dispatch.cpp b/numpy/_core/src/npysort/highway_qsort.dispatch.cpp
index 38adfc6de8..6c1ebfc577 100644
--- a/numpy/_core/src/npysort/highway_qsort.dispatch.cpp
+++ b/numpy/_core/src/npysort/highway_qsort.dispatch.cpp
@@ -2,11 +2,16 @@
 #define VQSORT_ONLY_STATIC 1
 #include "hwy/contrib/sort/vqsort-inl.h"
 
+#if VQSORT_ENABLED
 #define DISPATCH_VQSORT(TYPE) \
 template<> void NPY_CPU_DISPATCH_CURFX(QSort)(TYPE *arr, intptr_t size) \
 { \
     hwy::HWY_NAMESPACE::VQSortStatic(arr, size, hwy::SortAscending()); \
-} \
+}
+#else
+#define DISPATCH_VQSORT(TYPE) \
+template<> void NPY_CPU_DISPATCH_CURFX(QSort)(TYPE *arr, intptr_t size) {}
+#endif
 
 namespace np { namespace highway { namespace qsort_simd {
 
@@ -16,5 +21,5 @@ namespace np { namespace highway { namespace qsort_simd {
     DISPATCH_VQSORT(uint64_t)
     DISPATCH_VQSORT(double)
     DISPATCH_VQSORT(float)
-
 } } } // np::highway::qsort_simd
+
diff --git a/numpy/_core/src/npysort/highway_qsort_16bit.dispatch.cpp b/numpy/_core/src/npysort/highway_qsort_16bit.dispatch.cpp
index 35b6cc58c7..07ff576299 100644
--- a/numpy/_core/src/npysort/highway_qsort_16bit.dispatch.cpp
+++ b/numpy/_core/src/npysort/highway_qsort_16bit.dispatch.cpp
@@ -5,7 +5,6 @@
 #include "quicksort.hpp"
 
 namespace np { namespace highway { namespace qsort_simd {
-
 template<> void NPY_CPU_DISPATCH_CURFX(QSort)(Half *arr, intptr_t size)
 {
 #if HWY_HAVE_FLOAT16
@@ -16,11 +15,15 @@ template<> void NPY_CPU_DISPATCH_CURFX(QSort)(Half *arr, intptr_t size)
 }
 template<> void NPY_CPU_DISPATCH_CURFX(QSort)(uint16_t *arr, intptr_t size)
 {
+#if VQSORT_ENABLED
     hwy::HWY_NAMESPACE::VQSortStatic(arr, size, hwy::SortAscending());
+#endif
 }
 template<> void NPY_CPU_DISPATCH_CURFX(QSort)(int16_t *arr, intptr_t size)
 {
+#if VQSORT_ENABLED
     hwy::HWY_NAMESPACE::VQSortStatic(arr, size, hwy::SortAscending());
+#endif
 }
-
 } } } // np::highway::qsort_simd
+

Then I avoid the crash in the tests, but I also see a test failure in test_multiarray.py::TestMethods::test_sort so there must be some other plumbing elsewhere that needs VQSORT_ENABLED macros or I need to put in defaults that don't use highway in the templates I stubbed out.

@Mousius
Copy link
Member
Mousius commented Jul 25, 2024 
 template<> void NPY_CPU_DISPATCH_CURFX(QSort)(uint16_t *arr, intptr_t size)
 {
+#if VQSORT_ENABLED
     hwy::HWY_NAMESPACE::VQSortStatic(arr, size, hwy::SortAscending());
+#endif
 }

This means that if VQSORT_ENABLED is true, then this function is a blank function which does nothing and then returns. We need to remove the function entirely:

+#if VQSORT_ENABLED
 template<> void NPY_CPU_DISPATCH_CURFX(QSort)(uint16_t *arr, intptr_t size)
 {
     hwy::HWY_NAMESPACE::VQSortStatic(arr, size, hwy::SortAscending());
 }
+#endif

Then in the dispatch logic, it'll get a nullptr and dispatch the default path.

@neutrinoceros neutrinoceros mentioned this i 8000 ssue Jul 25, 2024
Closed
@ngoldbaum
Copy link
Member Author
ngoldbaum commented Jul 26, 2024 •
edited
Loading

If I do that dlopen complains about missing symbols:

ImportError: dlopen(/Users/goldbaum/Documents/numpy/build-install/usr/lib/python3.12/site-packages/numpy/_core/_multiarray_umath.cpython-312-darwin.so, 0x0002): symbol not found in flat namespace '__ZN2np7highway10qsort_simd13QSort_ASIMDHPIsEEvPT_l'

@Mousius
Copy link
Member
Mousius commented Jul 26, 2024

Argh, the symbol is still defined outside of dynamic dispatch, and I don't know a nice way of not doing that so I've done a workaround in #27050. Does that work for you @ngoldbaum ?

@ngoldbaum
Copy link
Member Author

Fixed by #27050

@ngoldbaum ngoldbaum mentioned this issue Aug 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Mousius Mousius

Labels
00 - Bug component: SIMD Issues in SIMD (fast instruction sets) code or machinery
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ngoldbaum @Mousius
0