Date: Wed, 15 Nov 2017 15:52:32 -0800 From: Mark Millard <markmi@dsl-only.net> To: Bruce Evans <brde@optusnet.com.au>, svn-src-head@freebsd.org Subject: Re: svn commit: r325765 - head/lib/libc/string Message-ID: <37F2AD18-1B69-4559-8BD3-B92E01A067BF@dsl-only.net>
next in thread | raw e-mail | index | archive | help
Bruce Evans brde at optusnet.com.au wrote on Tue Nov 14 12:41:50 UTC 2017 : > - memcpy.3. It was already documented in the BUGS section that = memcpy() is > implemented as bcopy() and therefore the "strings" "may overlap" = [then > portability considerations due to this]. This is buggier than = before: > - old bug: only the MI implementation of memcpy() clearly = implements > memcpy() as bcopy(). The amd64, i386 and arm MD implementations > do the same. head/sys/arm/arm/support.S ( -r283366 ) has 394 ENTRY(bcopy) 395 /* switch the source and destination registers */ 396 eor r0, r1, r0 397 eor r1, r0, r1 398 eor r0, r1, r0 399 EENTRY(memmove) 400 /* Do the buffers overlap? */ 401 cmp r0, r1 402 RETeq /* Bail now if src/dst are the same */ 403 subcc r3, r0, r1 /* if (dst > src) r3 =3D dst - = src */ 404 subcs r3, r1, r0 /* if (src > dsr) r3 =3D src - = dst */ 405 cmp r3, r2 /* if (r3 < len) we have an = overlap */ 406 bcc PIC_SYM(_C_LABEL(memcpy), PLT) . . . head/lib/libc/arm/string/memmove.S ( -r288373 ) has: 37 #ifndef _BCOPY 38 /* LINTSTUB: Func: void *memmove(void *, const void *, size_t) = */ 39 ENTRY(memmove) 40 #else 41 /* bcopy =3D memcpy/memmove with arguments reversed. */ 42 /* LINTSTUB: Func: void bcopy(void *, void *, size_t) */ 43 ENTRY(bcopy) 44 /* switch the source and destination registers */ 45 eor r0, r1, r0=20 46 eor r1, r0, r1=20 47 eor r0, r1, r0=20 48 #endif 49 /* Do the buffers overlap? */ 50 cmp r0, r1 51 it eq 52 RETeq /* Bail now if src/dst are the same */ 53 ite cc 54 subcc r3, r0, r1 /* if (dst > src) r3 =3D dst - = src */ 55 subcs r3, r1, r0 /* if (src > dsr) r3 =3D src - = dst */ 56 cmp r3, r2 /* if (r3 < len) we have an = overlap */ 57 bcc PIC_SYM(_C_LABEL(memcpy), PLT) . . . It looks to me like bcopy and memmove call memcpy under some conditions, not the other way around. I did not notice any code in memcpy going back to bcopy (or memmove) in either area. (That might have lead to recursion as things are.) Also netbsd "fixed" the above code for picking when to use memcpy a few years ago (without making the comments accurately track the details as I remember). By my understanding the new netbsd code was the correct variant but others commenting for bugzilla 217065 disagreed. (My comments for 217065 were clearly as I was figuring things out for unfamiliar material. So they are not simple/clean. But I still reach the same conclusions as I eventually did there.) Another issue is if compilers (clang, gcc) are well controlled for code substitutions to preserve supposed FreeBSD rules about where overlaps are well defined. The library code might not be all there is to it as far as behavior goes for compiled C/C++ source code. head/sys/arm64/arm64/ is similar for bcopy/memmove calling memcpy (but head/lib/libc/amd64/string/ is not): head/sys/arm64/arm64/memmove.S ( -r307909 ) has: 87 ENTRY(bcopy) 88 /* Switch the input pointers when called as bcopy */ 89 mov x3, x1 90 mov x1, x0 91 mov x0, x3 92 EENTRY(memmove) 93 sub tmp1, dstin, src 94 cmp count, 96 95 ccmp tmp1, count, 2, hi 96 b.hs memcpy . . . head/lib/libc/amd64/string/bcopy.S has: 44 #ifdef MEMCOPY 45 ENTRY(memcpy) 46 #else 47 #ifdef MEMMOVE 48 ENTRY(memmove) 49 #else 50 ENTRY(bcopy) 51 #endif 52 #endif 53 #if defined(MEMCOPY) || defined(MEMMOVE) 54 movq %rdi,%rax /* return dst */ 55 #else 56 xchgq %rdi,%rsi 57 #endif . . . (the rest being the same for the 3 routines) . . . > mips is backwards for bcopy() and implements it as > memmove(). mips has 2 separate memcpy*.S files, one for plain = arm > and one for xscale. The plain arm one is smaller than memmove.S, > so presumably doesn't support overlapped copies. The xscale one > is much larger, so has space for overlapped copies and many = optimizations > that no longer work. I don't know what it does. The mix of "mips" and "arm"/"xscale" above confused me. Looking around this seems to be referencing head/lib/libc/arm/string/ materials instead of head/lib/libc/mips/string/ or head/sys/mips/mips/ materials. =3D=3D=3D Mark Millard markmi at dsl-only.net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?37F2AD18-1B69-4559-8BD3-B92E01A067BF>