This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Fix MIPS64 memcpy regression
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Steve Ellcey <sellcey at imgtec dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Thu, 28 Jan 2016 01:53:05 +0000
- Subject: Re: Fix MIPS64 memcpy regression
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 2 dot 10 dot 1601212152150 dot 24424 at digraph dot polyomino dot org dot uk> <1453486476 dot 29343 dot 225 dot camel at ubuntu-sellcey>
On Fri, 22 Jan 2016, Steve Ellcey wrote:
> Thanks for catching this Joseph, I am not sure why my testing didn't hit
> it, I did run the tests on a 64 bit board running Debian but I am not
> sure what the kernel configuration is or if there are settings that
> would allow loads from unmapped pages. I wonder if it would be better
> to move the 'move a2,t8' instruction up after the beq instead of adding
> a nop. If the beq is true, the move would be harmless and shouldn't
> take any longer then the nop and if the beq is false then we need to
> execute that instruction anyway.
I've committed this patch version which does what you suggest.
Fix MIPS64 memcpy regression.
The MIPS memcpy optimizations at
<https://sourceware.org/ml/libc-alpha/2015-10/msg00597.html>
introduced a bug causing many string function tests to fail with
segfaults for n32 and n64:
FAIL: string/stratcliff
FAIL: string/test-bcopy
FAIL: string/test-memccpy
FAIL: string/test-memcmp
FAIL: string/test-memcpy
FAIL: string/test-memmove
FAIL: string/test-mempcpy
FAIL: string/test-stpncpy
FAIL: string/test-strncmp
FAIL: string/test-strncpy
(Some failures in other directories could also be caused by this bug.)
The problem is that after the check for whether a word of input is
left that can be copied as a word before moving to byte copies, a load
can occur in the branch delay slot, resulting in a segfault if we are
at the end of a page and the following page is unmapped. I don't see
how this would have passed the tests as reported in the original patch
posting (different kernel configurations affecting the code setting up
unmapped pages, maybe?), since the tests in question don't appear to
have changed recently.
This patch moves a later instruction into the delay slot, as suggested
at <https://sourceware.org/ml/libc-alpha/2016-01/msg00584.html>.
Tested for n32 and n64.
2016-01-28 Steve Ellcey <sellcey@imgtec.com>
Joseph Myers <joseph@codesourcery.com>
* sysdeps/mips/memcpy.S (MEMCPY_NAME) [USE_DOUBLE]: Avoid word
load in branch delay slot when less than a word of input left.
diff --git a/sysdeps/mips/memcpy.S b/sysdeps/mips/memcpy.S
index d79e144..9b072d7 100644
--- a/sysdeps/mips/memcpy.S
+++ b/sysdeps/mips/memcpy.S
@@ -565,11 +565,11 @@ L(lastw):
#ifdef USE_DOUBLE
andi t8,a2,3 /* a2 is the remainder past 4 byte chunks. */
beq t8,a2,L(lastb)
+ move a2,t8
lw REG3,0(a1)
sw REG3,0(a0)
PTR_ADDIU a0,a0,4
PTR_ADDIU a1,a1,4
- move a2,t8
#endif
/* Copy the last 8 (or 16) bytes */
--
Joseph S. Myers
joseph@codesourcery.com