svn commit: r325765 - head/lib/libc/string

[Mark Millard]

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 = dst - src */
404	        subcs   r3, r1, r0      /* if (src > dsr) r3 = 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 = 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 
46	        eor     r1, r0, r1 
47	        eor     r0, r1, r0 
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 = dst - src */
55	        subcs   r3, r1, r0      /* if (src > dsr) r3 = 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.

===
Mark Millard
markmi at dsl-only.net