fix: measure sizeof(struct timespec) instead of reading __TIMESIZE - #20
Conversation
__TIMESIZE reports the port's default time_t width, so it still reads 32 on targets that gained a 64-bit time_t through Debian's time64 transition (_TIME_BITS=64 in the toolchain, e.g. armhf on trixie). On those targets the probe from diwic#19 does not emit alsa_sys_time64, libc::timespec stays 8 bytes, and libasound writes 16-byte timespecs past it (diwic/alsa-rs#158, RustAudio/cpal#1285). Measure the size directly instead: try to compile a _Static_assert that sizeof(struct timespec) == 16. The snippet is compiled but never run, so cross compiling against the target's sysroot keeps working. A control compile first checks that <time.h> yields a usable timespec, so a broken toolchain produces a warning and the safe 32-bit fallback instead of being misread as a pre-transition target. Verified on Raspberry Pi OS armhf: with this change the cfg is emitted on trixie (16-byte timespec) and still not on bookworm (8-byte timespec). 64-bit glibc and musl targets return early as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@roderickvd Can you review this patch? |
|
Nice! This should definitely be more robust than Reverting on what I had introduced myself, I think we should fall back to 16-byte |
|
@b0bbywan Thanks for the patch! How about this for even more robustness:
|
…rors Review feedback from diwic#20: - Measure sizeof(snd_htimestamp_t) from <alsa/asoundlib.h> instead of struct timespec from <time.h>: it is the exact type libasound writes through. The probe now runs after pkg-config so it can reuse the reported include paths. The alsa/ prefix is required since alsa-lib 1.2.5, when alsa.pc stopped adding -I${includedir}/alsa. - If the control snippet does not compile, fail the build with an explicit error instead of assuming a 32-bit time_t: guessing 8 can segfault and guessing 16 yields garbage timestamps, so there is no safe fallback to pick silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks both! I pushed a commit implementing @diwic's two suggestions. Probe One detail: I had to use Fail the build if the control snippet doesn't compile. No more warning + fallback: the build now stops with an error that explains the stakes and includes the failing compiler invocation. I think this also answers @roderickvd's question, instead of choosing between a possible segfault (assume 8 bytes) and silently nonsensical timestamps (assume 16), the failure is loud and diagnosable, and we can revisit if real-world fallout shows up. Since the snippet now needs the ALSA headers, a failing control compile means the crate build was doomed anyway (pkg-config or bindgen would trip over the same problem), so the hard failure should be near-zero risk for configurations that worked before. Re-verified in the same Raspberry Pi OS containers as the original patch:
A full |
|
@b0bbywan @roderickvd @d3d9 So I merged it (thanks!) and reworded the panic message slightly. |
|
A new release would be great. Yanking the previous one is up to you, not entirely necessary for me (nor was it entirely broken). Thanks all! |
|
Thx guys! I think 0.6.1 release is enough, no need to yank 6.0.0 in my opinion, since it's an edge case and previous version also have the same bug. |
Context
While cross-compiling an audio application for 32-bit Raspberry Pi OS (armhf), I hit the segfault described in diwic/alsa-rs#158 and RustAudio/cpal#1285: libasound writes a 16-byte
struct timespecwhilealsa-sysre-exports the 8-bytelibc::timespec.To be upfront: I'm a developer, but not a Rust developer, and I used Claude to track this down and to write the patch. The measurements and verification below were run for real in the two Raspberry Pi OS images, and I'm happy to rework anything.
Problem
The probe added in #19 reads
__TIMESIZE, but that macro reports the port's defaulttime_twidth. Debian's time64 transition works differently: it switches existing 32-bit targets to a 64-bittime_tby passing_TIME_BITS=64in the toolchain, which leaves__TIMESIZEat 32. So on transitioned targets such as armhf and i386 the cfg is not emitted, which is exactly where it is needed — forcing it by hand is what made @d3d9's test pass in #158.Measured in the two Raspberry Pi OS armhf images, by poisoning a 16-byte buffer and passing it to
snd_pcm_status_get_htstampon thenullPCM:libasound21.2.8-1+rpt1libasound2t641.2.14-1+rpt1sizeof(struct timespec)__TIMESIZE__USE_TIME_BITS64Change
Measure
sizeof(struct timespec)directly by compiling a_Static_assert, instead of inferring the width from a macro. The snippet is compiled but never run, so cross-compiling against the target's sysroot keeps working. A control compile first checks that<time.h>yields a usable timespec, so a broken toolchain produces a warning and the safe 32-bit fallback instead of being misread as a pre-transition target (the direction that corrupts memory).Checking
__USE_TIME_BITS64would also work on both images and is a smaller change if you prefer it; I went with the size because it holds regardless of which mechanism set the width.Verification
Same container, same libasound, trixie armhf:
On bookworm armhf the cfg is still (correctly) not emitted, since both
libc::timespecand the library's timespec are 8 bytes there. 64-bit glibc and musl targets return early at the existing guard, as before.One gap: I could not test riscv32, a port born with a 64-bit
time_twhere__TIMESIZEis already 64. The assertion should compile there and keep the cfg on, but that path is reasoned, not measured.