Skip to content

Fix crash on startup when compiled with AVX#331

Open
BlueManCZ wants to merge 1 commit into
davy7125:masterfrom
BlueManCZ:fix-fastmaths-avx-alignment
Open

Fix crash on startup when compiled with AVX#331
BlueManCZ wants to merge 1 commit into
davy7125:masterfrom
BlueManCZ:fix-fastmaths-avx-alignment

Conversation

@BlueManCZ

Copy link
Copy Markdown

Problem

Polyphone segfaults on startup (general-protection fault on the audio thread) when built with AVX enabled — e.g. -march=native, or any -march that turns on AVX. It's deterministic on source-based / optimized distributions (Gentoo, Arch with native flags, CachyOS…). Baseline builds (-march=x86-64, SSE only) are unaffected, which is why it hasn't shown up widely.

Cause

The FastMaths helpers in sources/core/fastmaths.cpp promise the compiler 32-byte alignment via __builtin_assume_aligned(ptr, 32), but the buffers passed in aren't guaranteed to be:

  • engine buffers (_dataL, _voiceData, …) come from plain new float[] → only 16-byte aligned on x86-64;
  • SoundEngine::addData runs addVectors on the sound-card output buffer, which the engine doesn't allocate;
  • multiply4/multiply8 get sinc-table coefficients at arbitrary offsets (&sincWindow[frac * order + j]), so coeffs can be 4-byte aligned.

With AVX, GCC trusts the promise and auto-vectorizes the generic loops (e.g. addVectors's a[i] += b[i]) into aligned 256-bit moves (vmovaps %ymm). When a buffer isn't actually 32-byte aligned at runtime, that move raises a #GP. Confirmed from a core dump: faulting instruction vmovaps (%rdi,%rax,1),%ymm0 with rdi % 32 == 16.

Fix

Remove the unsafe alignment assumptions. Auto-vectorization still applies (unaligned moves cost the same as aligned ones on current x86), and the aarch64 NEON paths already use unaligned vld1q_f32, so they're unaffected.

Verified with -march=alderlake: Polyphone crashes immediately on launch before the change, and starts normally after.

If you'd rather keep aligned 256-bit AVX for the buffer-wide functions, the alternative is allocating the engine buffers 32-byte aligned (operator new(…, std::align_val_t(32))) — but that can't cover the sound-card output buffer or the sinc-table offsets, so dropping the assumption is the robust fix.

BlueManCZ added a commit to BlueManCZ/edgets that referenced this pull request Jun 19, 2026
Switch the buffer-alignment fix to remove the unsafe
__builtin_assume_aligned() calls outright, matching the upstream patch
(davy7125/polyphone#331), rather than lowering the assumed alignment.
The FastMaths SIMD helpers told the compiler their buffers were 32-byte
aligned via __builtin_assume_aligned(ptr, 32). However the buffers passed
to them are only 16-byte aligned at best (the engine buffers from
new float[], and the sound card output buffer) or are accessed at
arbitrary offsets (the sinc-table coefficients in multiply4/multiply8).

When Polyphone is built with AVX enabled (e.g. -march=native), the generic
loops auto-vectorize into aligned 256-bit moves (vmovaps %ymm). Whenever a
buffer is not actually 32-byte aligned at runtime, that move raises a
general-protection fault and the audio thread crashes on startup. Default
baseline builds (SSE only, 16-byte movaps) never hit it, which is why it
shows up on source-based/optimized distributions.

Remove the unsafe alignment assumptions. Auto-vectorization still applies
with unaligned moves, which have no measurable cost on current x86, and the
aarch64 NEON paths already use unaligned vld1q_f32.
@BlueManCZ
BlueManCZ force-pushed the fix-fastmaths-avx-alignment branch from 690e043 to c83874e Compare June 19, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant