swab: Fix implementation to support overlapping copies

A number of image processing packages assume that swab() can handle to
and from being the same. However, POSIX.1 states that overlapping
buffers produces undefined results. Our old implementation would produce
coherent results, but the recent change to the musl-inspired code does
not. Since there's complaints in the forums for these image processing
packages for musl and now FreeBSD, update the algorithm to just read a
word at a time and bswap16 the results. All FreeBSD's architecutres
support unaligned access in userland, and swab is not used in the kernel
(g_part_apm has its own copy), so opt for even simpler code that's
easier to understand. This makes the overlapping behavior match i386 again,
since its assembler routine for swab handles overlapping correctly.

PR: 283698
Sponsored by: Netflix
Reviewed by:	nwhitehorn
Differential Revision:	https://reviews.freebsd.org/D48259

1 files changed, 10 insertions, 7 deletions

diff --git a/lib/libc/string/swab.c b/lib/libc/string/swab.c
index 2b044d68ca46..ed4436a49810 100644
--- a/lib/libc/string/swab.c
+++ b/lib/libc/string/swab.c
@@ -4,19 +4,22 @@
  */
 
 #include <unistd.h>
+#include <sys/endian.h>
 
 void
 swab(const void * __restrict from, void * __restrict to, ssize_t len)
 {
-	const unsigned char *f = from;
-	unsigned char *t = to;
+	const uint16_t *f __aligned(1) = from;
+	uint16_t *t __aligned(1) = to;
 
+	/*
+	 * POSIX says overlapping copy behavior is undefined, however many
+	 * applications assume the old FreeBSD and current GNU libc behavior
+	 * that will swap the bytes correctly when from == to. Reading both bytes
+	 * and swapping them before writing them back accomplishes this.
+	 */
 	while (len > 1) {
-		t[0] = f[1];
-		t[1] = f[0];
-
-		f += 2;
-		t += 2;
+		*t++ = bswap16(*f++);
 		len -= 2;
 	}
 }