git: 7f8f79a5c444 - main - libc qsort(3): Eliminate ambiguous sign comparison

Conrad Meyer cem at FreeBSD.org
Thu Jul 29 04:01:07 UTC 2021


The branch main has been updated by cem:

URL: https://cgit.FreeBSD.org/src/commit/?id=7f8f79a5c444a565a32b0c6578b07f8d496f6c49

commit 7f8f79a5c444a565a32b0c6578b07f8d496f6c49
Author:     Conrad Meyer <cem at FreeBSD.org>
AuthorDate: 2021-07-23 18:04:21 +0000
Commit:     Conrad Meyer <cem at FreeBSD.org>
CommitDate: 2021-07-29 03:59:20 +0000

    libc qsort(3): Eliminate ambiguous sign comparison
    
    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
---
 lib/libc/stdlib/Makefile.inc | 2 ++
 lib/libc/stdlib/qsort.c      | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/libc/stdlib/Makefile.inc b/lib/libc/stdlib/Makefile.inc
index de8d8484e135..361761e73fdd 100644
--- a/lib/libc/stdlib/Makefile.inc
+++ b/lib/libc/stdlib/Makefile.inc
@@ -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).
diff --git a/lib/libc/stdlib/qsort.c b/lib/libc/stdlib/qsort.c
index cfd2d99025f0..5016fff7895f 100644
--- a/lib/libc/stdlib/qsort.c
+++ b/lib/libc/stdlib/qsort.c
@@ -171,7 +171,12 @@ loop:
 	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;


More information about the dev-commits-src-main mailing list