Repository navigation
fix(alloc): request 16-byte alignment for small bemo_alloc sizes - #50
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
=========================================
Coverage 86.78% 86.79%
+ Complexity 616 615 -1
=========================================
Files 60 60
Lines 16912 16930 +18
Branches 490 490
=========================================
+ Hits 14677 14694 +17
- Misses 1911 1913 +2
+ Partials 324 323 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
|
clippy::undocumented_unsafe_blocks requires a SAFETY comment on the
unsafe { mi_malloc(size) } block in mi_malloc_aligned; the other unsafe
blocks in buffer.rs already have them. Fixes the macOS CI failures.
Detail bug report: View on Detail
Closes #45
Change
bemo_alloc/alloc::allocatedocument "aligned to at least 16", but called plainmi_malloc(size). On bundled-mimalloc builds (non-Apple: Linux/musl) sizes 1–8 are served from mimalloc's 8-byte block class, so roughly half of returned pointers were congruent to 8 mod 16. This is reachable from C embedders, the JVM FFM binding, GraalVM Native Image, and Netty'sNativeDirectByteBufAllocator(which forwardscapacityverbatim). Apple / miri /rust-allocatorwere unaffected — theremi_mallocresolves to a 16-aligned Rust-allocator wrapper.allocate()now callsmi_malloc_aligned(size, 16), still paired with the existingmi_free.mi_malloc_alignedacross all three allocator resolution paths inbuffer.rs: bundled mimalloc (libmimalloc-sysre-export), embedder-supplied mimalloc (extern "C"), and therust_allocshim (delegates tomi_mallocfor any alignment ≤ 16 — unchanged behavior, since itsMI_ALIGNheader already yields 16-aligned payloads).Bufferallocation path is untouched.No new ABI symbols, no version bump, no layout/ownership/lifetime change —
mi_malloc_alignedis apub(crate)alias, andbemo_alloc/bemo_free/bemo_alloc_liveare unchanged. The fix makes the implementation honor a contract already published ininclude/bemo.h.Validation
Platform: Linux x86-64 (glibc, static musl, and the Elide toolchain).
cargo check -p bemo --no-default-features,cargo clippy --workspace --all-targets --locked -- -D warnings, andcargo fmt --all --checkall clean.allocmodule): pass on default bundled-mimalloc, under--profile ci, under--features rust-allocator, and on the staticx86_64-unknown-linux-musltarget — the production config where the bug reproduced. The newevery_size_class_returns_16_aligned_storageholds 32 allocations of each size 1–64 at once and asserts 16-byte alignment + first/last-byte writability; it fails on the buggymi_malloc(size 1 returned a pointer not aligned to 16, left: 8) and passes with the fix.crates/bemo/tests/abi.rs(14/14) andbemo-ffi(1/1) pass. Fullbemosuite: 378/380 — the 2 failures (persistent_keepalive_reuses_receive_registration_and_fixed_send_state,arm_receive_failure_leaves_socket_dormant) are pre-existing driver/http socket flakes that reproduce identically on a clean checkout (git stash), unrelated to this change.tests/static.cto allocate sizes 1–16 through the publishedbemo_allocand assert 16-alignment + per-byte writability +bemo_alloc_liveaccounting +bemo_free;tests/abi.c(4-thread cross-thread alloc/free, the Netty release pattern) — both pass linked against the debug static lib, andtests/static.calso passes under the ASAN and TSAN native stages.cargo miri test -p bemo --no-default-features --features rust-allocator --lib --test abi --test buffers --test pool --test workload— clean, no UB on therust_allocheader path.alloctests with no violations; the instrumentedtests/static.cnative stage passes under both. (Stock ASAN/TSAN userust-allocator, so they cover the rust_alloc path; the bundled-mimalloc path is covered by the musl + unit tests.)make test-embedder): all 4 cases pass —gnu-mimalloc,gnu-rust-allocator,musl-libc-mimalloc(static),musl-rust-allocator(static) — each linkingtests/embedderagainst the embedder's allocator with the Elide clang and checking all 97 header exports are defined exactly once.make test-musl): 379/381 (same 2 pre-existing flakes); alloc tests pass on the production musl + bundled-mimalloc target.make test-jvm—FfmContractandCAbiContract(tests/abi.cvslibbemo_ffi.so) bothfailures=0, and NettyNativeDirectAllocatorTest(forwards capacities 1–8 tobemo_alloc)failures=0. One unrelated JVM contract,NativeSslInteropTest, failed only because the sandbox denies exec of/usr/bin/openssl.native-imagewas not on PATH — I located it, made it executable, and setJAVA_HOME; all Java modules and theCapiContracttest sources compiled, but GraalVM native-image image generation then failed because the JVM cannotposix_spawn/usr/bin/gccin this sandbox (posix_spawn failed, error: 13 (Permission denied)) — the same sandbox restriction that blocksNativeSslInteropTest.gccitself works from the shell, so this is an environment limitation, not a code issue. Since this change adds no symbols/layouts and does not alter ownership/lifetime, the Native Image C API contract is unaffected by the fix.Automatic Fixes PRs can be configured here.