musl sanitizer fixes - #123
Open
thevar1able wants to merge 5 commits into
Open
Conversation
Two independent musl issues surfaced by building sanitizer runtimes for musl targets for the first time: - sanitizer_platform_limits_posix.cpp includes <asm/ptrace.h> on aarch64 (needed for struct user_pt_regs / user_fpsimd_state), which transitively includes the kernel UAPI <asm/sigcontext.h>. musl's own <signal.h> (via bits/signal.h) already defines the same structs (sigcontext, _aarch64_ctx, fpsimd_context, esr_context, extra_context, sve_context) with identical layout, so the kernel header's copy is a hard redefinition error. Same issue for struct sysinfo (linux/sysinfo.h vs musl's own sys/sysinfo.h, transitively pulled in via linux/sysctl.h -> linux/kernel.h). Block the redundant kernel definitions via their own include guards when SANITIZER_MUSL, instead of touching the kernel headers or musl. - msan_interceptors.cpp intercepts getrlimit64/prlimit64 unconditionally on non-BSD Linux, referencing struct_rlimit64_sz - which is only defined under SANITIZER_GLIBC (these are glibc's large-file-support variants; musl does not have distinct getrlimit64/prlimit64 symbols at all, they are #defined to getrlimit/prlimit). This left an undefined symbol at link time for every consumer of clang_rt_msan under musl. Gate the getrlimit64/prlimit64 interceptors to SANITIZER_GLIBC; __getrlimit and prlimit (already unconditional) cover musl. Verified by cross-compiling sanitizer_platform_limits_posix.cpp for aarch64-musl directly (reproduces the redefinitions without the fix, clean with it) and by building clang_rt_msan plus a full link+run of a musl+MSan test executable (no struct_rlimit64_sz reference remains). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The predefine-the-kernel-header-guard trick added in the previous commit only covered this one file. A different file (sanitizer_stoptheworld_linux_libcdep.cpp) hits the identical musl-vs-kernel-UAPI-header struct redefinition through the same headers in the opposite include order, so the same problem can surface in any translation unit that happens to pull in both sides. The general fix is a global compiler define applied to the whole aarch64-musl toolchain (see the ClickHouse repository's cmake/linux/toolchain-aarch64.cmake), making this per-file patch redundant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SI_STAT_LINUX gates the stat/lstat interceptors behind __GLIBC_PREREQ(2, 33), which selects glibc's unversioned stat/lstat ABI (before that, they were __xstat/__lxstat wrappers). __GLIBC_PREREQ is hardcoded to 0 for non-glibc via sanitizer_glibc_version.h, so this silently disabled the interceptors on musl even though musl has always exposed plain stat/lstat/fstat/fstatat. Without the interceptor, MSan sees stat's output buffer as uninitialized and reports false-positive use-of-uninitialized-value errors on any stat/lstat call - reproduced with protoc parsing orc_proto.proto (DiskSourceTree::OpenDiskFile calling stat), matching the arm_msan CI failure.
msan_interceptors.cpp has its own local SANITIZER_STAT_LINUX gate for the fstat/fstat64/fstatat/fstatat64 interceptors - a separate copy of the same broken pattern as SI_STAT_LINUX in sanitizer_platform_interceptors.h (already fixed for stat/lstat): __GLIBC_PREREQ is hardcoded to 0 for non-glibc via sanitizer_glibc_version.h, so the interceptors were silently compiled out on musl and MSan reported false-positive use-of-uninitialized-value on every fstat result. First hit in MSan-instrumented llvm-min-tblgen (llvm::sys::fs::status -> fillStatus -> typeForMode reading st_mode), which fails the arm_msan build while generating GenVT.inc - native tools get ambient instrumentation on arm because host == target. Enable plain fstat/fstatat for musl unconditionally, like stat/lstat. Keep the fstat64/fstatat64 variants disabled there: musl has no distinct LFS64 symbols (off_t is always 64-bit), and struct_stat64_sz is only defined for glibc, so enabling them would produce an undefined symbol the same way the getrlimit64/prlimit64 interceptors did. Verified with a minimal static MSan+musl binary calling fstat/fstatat and reading st_mode: reports use-of-uninitialized-value without this change, passes with it.
The interceptor picks between the POSIX (int return) and GNU (char* return) variants of strerror_r with a preprocessor check that treats "Linux without _POSIX_C_SOURCE/_XOPEN_SOURCE" as glibc. musl only ever provides the POSIX variant, regardless of feature-test macros, so the GNU-variant interceptor reinterprets the int return value as a pointer: a successful call returns 0, and the interceptor passes the resulting NULL to internal_strlen and crashes. Hit by every MSan aarch64 musl clickhouse-local run through libc++'s system_error machinery (do_strerror_r -> handle_strerror_r_return -> ___interceptor_strerror_r -> internal_strlen(NULL)); the same latent bug is linked into the ASan and TSan runtimes via this shared include. Add SANITIZER_MUSL to the POSIX-variant condition.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ClickHouse/ClickHouse#113503