Skip to content

Commit

Permalink
libc qsort(3): Eliminate ambiguous sign comparison
Browse files Browse the repository at this point in the history
The left side of the MIN() expression is the (signed) result of pointer
subtraction (ptrdiff_t).  The right hand side is the also the (signed)
result of pointer subtraction, additionally subtracting the element size
('es'), which is unsigned size_t.  This coerces the right-hand
expression into an unsigned value.  MIN(signed, unsigned) triggers
-Wsign-compare.

Sorting elements of size greater than SSIZE_MAX is nonsensical, so we
can instead treat the element size as ssize_t, leaving the right-hand
result the same signedness as the left.

Reviewed by:		arichardson, kib
Differential Revision:	https://reviews.freebsd.org/D31292
  • Loading branch information
cemeyer committed Jul 29, 2021
1 parent e370772 commit 7f8f79a
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
2 changes: 2 additions & 0 deletions lib/libc/stdlib/Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ MISRCS+=C99_Exit.c a64l.c abort.c abs.c atexit.c atof.c atoi.c atol.c atoll.c \
strtol.c strtold.c strtoll.c strtoq.c strtoul.c strtonum.c strtoull.c \
strtoumax.c strtouq.c system.c tdelete.c tfind.c tsearch.c twalk.c

CFLAGS.qsort.c+= -Wsign-compare

# Work around an issue on case-insensitive file systems.
# libc has both _Exit.c and _exit.s and they both yield
# _exit.o (case insensitively speaking).
Expand Down
7 changes: 6 additions & 1 deletion lib/libc/stdlib/qsort.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,12 @@ local_qsort(void *a, size_t n, size_t es, cmp_t *cmp, void *thunk)
pn = (char *)a + n * es;
d1 = MIN(pa - (char *)a, pb - pa);
vecswap(a, pb - d1, d1);
d1 = MIN(pd - pc, pn - pd - es);
/*
* Cast es to preserve signedness of right-hand side of MIN()
* expression, to avoid sign ambiguity in the implied comparison. es
* is safely within [0, SSIZE_MAX].
*/
d1 = MIN(pd - pc, pn - pd - (ssize_t)es);
vecswap(pb, pn - d1, d1);

d1 = pb - pa;
Expand Down

0 comments on commit 7f8f79a

Please sign in to comment.