microsoft / mimalloc

mimalloc is a compact general purpose allocator with excellent performance.
MIT License
9.74k stars 791 forks source link

Tests fail on armhf and armv7 on Alpine Linux (musl libc) since mimalloc 2.1.4 & 1.8.4 #895

Closed jirutka closed 1 month ago

jirutka commented 1 month ago

Tests have been failing on armhf and armv7 on Alpine Linux Edge (musl libc) since 2.1.4 and 1.8.4, including 2.1.6 and 1.8.6.

There’s no issue with 2.1.2 and 1.8.2.

>>> mimalloc2: Testing debug build
Test project /builds/alpine/aports/community/mimalloc2/src/mimalloc-2.1.4/out/debug
    Start 1: test-api
    Start 2: test-api-fill
    Start 3: test-stress
1/3 Test #1: test-api .........................Bus error***Exception:   0.00 sec
test: malloc-zero...  mimalloc: warning: unable to allocate aligned OS memory directly, fall back to over-allocation (size: 0x8000000 bytes, address: 0xEFDD2000, alignment: 0x400000, commit: 1)
2/3 Test #2: test-api-fill ....................Bus error***Exception:   0.00 sec
test: zeroinit-zalloc-small...  mimalloc: warning: unable to allocate aligned OS memory directly, fall back to over-allocation (size: 0x8000000 bytes, address: 0xEF894000, alignment: 0x400000, commit: 1)
3/3 Test #3: test-stress ......................Bus error***Exception:   0.00 sec
Using 32 threads with a 25% load-per-thread and 50 iterations (allow large objects)
mimalloc: warning: unable to allocate aligned OS memory directly, fall back to over-allocation (size: 0x8000000 bytes, address: 0xEFC64000, alignment: 0x400000, commit: 1)
0% tests passed, 3 tests failed out of 3
Total Test time (real) =   0.01 sec
The following tests FAILED:
      1 - test-api (Bus error)
      2 - test-api-fill (Bus error)
      3 - test-stress (Bus error)
Errors while running CTest

Full log: https://gitlab.alpinelinux.org/alpine/aports/-/jobs/1384901

daanx commented 1 month ago

Ah, that is not good. This is probably because the thread id is incorrect. Did you try to build with cmake ../.. -DMI_LIBC_MUSL=ON ? (this is a new switch in 1.8.6/2.1.6.). Let me know if that fixes things. Otherwise, we need to probably not use __builtin_thread_pointer and make the test for that fail (I am guessing this is the case as the test fails on ARM).

Let me know if you can. Thanks!

daanx commented 1 month ago

Well, I managed to repro (inside a docker image on qemu) and somehow the error is caused by the meta data allocation inside a static array; I tried this on aarch64 and x86 (32-bit) and it works fine ... I do not quite understand why this fails with a BUS error (signal 7) on Alpine + MUSL + 32-bit ? For now, I "fixed" it by disabling fast meta data allocation and falling back to allocating OS memory instead but it would be best if I can understand the root cause. TBC :-)

note: you need to build with cmake ../.. -DMI_LIBC_MUSL=ON; I will make that automatically once I figure out how to detect musl libc / alpine at cmake time.

daanx commented 1 month ago

Well, it turns out that the atomic write to the purge_expire field (here) causes the BUS error if using a static array (while the OS allocated memory works somehow). If I make purge_expire use a 32-bit value (instead of the 64-bit mi_msecs_t) the BUS error disappears. The alignment seems right so I am not sure what is the cause -- maybe a bug in the implementation of 64-bit atomics?

edit: Even though mi_arena_static_zalloc allocates to the right alignment, it turns out aligning the whole static array fixes the issue! I don't quite understand how this can be the case... but I will go ahead and push a fix.

jirutka commented 1 month ago

I can confirm that 8fd1184272fdf5c93f4776a36086911b55fb315e fixed the problem – tests pass on all architectures now.