Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does not build with musl libc -- qsort_r not available there #4311

Open
eschnett opened this issue Feb 21, 2025 · 7 comments · May be fixed by #4312
Open

Does not build with musl libc -- qsort_r not available there #4311

eschnett opened this issue Feb 21, 2025 · 7 comments · May be fixed by #4312

Comments

@eschnett
Copy link

Describe the bug
zstd 1.5.7 does not build on Linux musl system. The linker aborts because qsort_r is not defined.

Additional context
Add any other context about the problem here.

I am cross-building for Julia; see e.g. JuliaPackaging/Yggdrasil#10577.

musl intentionally does not provide qsort_r. zstd should use its qsort fallback instead, or use the C11 qsort_s function.

@Cyan4973
Copy link
Contributor

zstd is supposed to offer qsort as a fallback.
If it's not used, it means qsort_r detection could be improved.
And maybe we should have some sort of manual control via a compilation variable for cases that are not properly automated.

It would great to have a test case in CI that employs musl and would produce the same issue as the one observed here.

@terrelln
Copy link
Contributor

Here is where it is used:

static void stableSort(COVER_ctx_t *ctx) {
#if defined(__APPLE__)
qsort_r(ctx->suffix, ctx->suffixSize, sizeof(U32),
ctx,
(ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp));
#elif defined(_GNU_SOURCE)
qsort_r(ctx->suffix, ctx->suffixSize, sizeof(U32),
(ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp),
ctx);
#elif defined(_WIN32) && defined(_MSC_VER)
qsort_s(ctx->suffix, ctx->suffixSize, sizeof(U32),
(ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp),
ctx);
#elif defined(__OpenBSD__)
g_coverCtx = ctx;
mergesort(ctx->suffix, ctx->suffixSize, sizeof(U32),
(ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp));
#else /* C90 fallback.*/
g_coverCtx = ctx;
/* TODO(cavalcanti): implement a reentrant qsort() when is not available. */
qsort(ctx->suffix, ctx->suffixSize, sizeof(U32),
(ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp));
#endif
}

See also:

/* qsort_r is an extension. */
#if defined(__linux) || defined(__linux__) || defined(linux) || defined(__gnu_linux__) || \
defined(__CYGWIN__) || defined(__MSYS__)
#if !defined(_GNU_SOURCE) && !defined(__ANDROID__) /* NDK doesn't ship qsort_r(). */
#define _GNU_SOURCE
#endif
#endif

@Cyan4973
Copy link
Contributor

I just tried compilation with musl-gcc on a local Ubuntu desktop,
and it works fine, no issue reported.

I suspect the difference comes from employing different versions of musl.

Apparently, musl supports qsort_r() since v1.2.3,
and Ubuntu 24 provides musl v1.2.4.

So, to reproduce this issue I would need to find an older environment, that ships with an older version of musl.

@Cyan4973 Cyan4973 linked a pull request Feb 21, 2025 that will close this issue
@eschnett
Copy link
Author

eschnett commented Feb 21, 2025

I think we are using musl 1.2.5, and qsort_r should be supported there. I have also since found JuliaPackaging/Yggdrasil#4547, which discusses a related issue.

Correction: We're using 1.2.2.

@Cyan4973
Copy link
Contributor

Apparently, musl doesn't make it easy to determine that it's being linked to, nor with which version.
The author seems to have some unclear aversion against detection by macro:
https://www.openwall.com/lists/musl/2013/03/29/13

Searching over the internet noise, one can find solutions which employs side-effects (i.e. stuff that musl doesn't do and others do, that could be used as an indirect signal) but with no guarantee of continuity, so it's too dangerous to depend on that.

I don't see great solutions here.
Maybe the main one would be an option to manually enforce the C90 qsort() path with a build macro, for example ZSTD_USE_C90_QSORT, thus offering an option to musl users to complete compilation.

@Cyan4973
Copy link
Contributor

#4312 attempts to fix the situation by offering a new build macro ZSTD_USE_C90_QSORT.
It is tested successfully against musl v1.2.2 in this test.

@eschnett
Copy link
Author

You could use an autoconf test to see whether qsort_r is supported. However, a manual macro is fine.

Thanks!

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 a pull request may close this issue.

3 participants