Skip to content

asort: replace old quicksort fallback with glibc 2.43 mergesort+heapsort#129

Open
nGoline wants to merge 1 commit into
rustyrussell:masterfrom
nGoline:update-asort-fallback
Open

asort: replace old quicksort fallback with glibc 2.43 mergesort+heapsort#129
nGoline wants to merge 1 commit into
rustyrussell:masterfrom
nGoline:update-asort-fallback

Conversation

@nGoline

@nGoline nGoline commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

The bug

The old fallback _asort (a copy of glibc's 2004 median-of-three quicksort) called cmp(a, a), the same pointer on both sides, when sorting arrays where all elements compare equal. In the inner loop, after the left and right scan pointers both stop on the pivot element, two comparisons of the form cmp(pivot, pivot) execute before the left_ptr == right_ptr guard fires and breaks.

This violates the implicit contract that comparators can assume a != b when called by a sort routine, which is a reasonable expectation since the C standard says comparator behaviour for equal-valued distinct elements is unspecified but never requires handling identical pointers.

The fix

Replace the old quicksort with the algorithm from glibc 2.43: a stable top-down mergesort that uses a stack buffer for small arrays and falls back to in-place heapsort if malloc fails. Mergesort always compares across two distinct sub-array halves, so self-comparisons cannot occur.

Portability

The glibc 2.43 source uses several glibc-internal headers and symbols that are not available outside glibc (memswap.h, pthreadP.h, __compar_d_fn_t, pthread_cleanup_combined_*, etc.). This PR replaces each with a portable equivalent; the algorithm itself is unchanged.

Tests

Added four new test cases:

  • all-equal array: sorts 100 identical elements — exercises the path the old quicksort mishandled, verifies the result is still sorted.
  • single-element array: verifies the early-exit path produces no comparator call and leaves the element untouched.
  • direct self-comparison: calls cmp(&val, &val, NULL) explicitly and asserts it returns 0, pinning the contract that comparators passed to asort must handle identical pointers gracefully.
  • self_compared flag: asserts the flag was set by the direct call, confirming the test machinery itself works correctly.

The diag line reports whether a self-comparison occurred anywhere during the test run, serving as a canary if a future implementation reintroduces them.

Signed-off-by: Nickolas Goline @nGoline

The old fallback (used when the native `qsort_r` lacks the glibc calling convention) was a copy of glibc's 2004 median-of-three quicksort.
It had a subtle self-comparison bug: when the left and right scan pointers both converged on the pivot with an all-equal array, the inner loop evaluated `cmp(pivot, pivot)` before detecting `left_ptr == right_ptr` and breaking. Comparators that use pointer identity to detect distinct elements would spuriously fire on this.

Replace it with glibc 2.43's stable mergesort (`heapsort` fallback on `malloc` failure).  Mergesort always compares from two distinct halves and never produces self-comparisons.

Portability adaptations to remove glibc-internal dependencies:
  - `__compar_d_fn_t` -> typedef from `_total_order_cb`
  - `__memswap` / `__mempcpy` -> local static inline helpers
  - `pthread_cleanup_*` -> removed (no thread cancellation needed)
  - `__set_errno` → `errno = …`
  - `libc_hidden_def` / `weak_alias` / `__qsort_r` -> removed / renamed to `_asort`

Add tests for all-equal and single-element arrays.
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