Re: Fix MIPS64 memcpy regression

[Steve Ellcey <sellcey at imgtec dot com>]

On Thu, 2016-01-21 at 21:53 +0000, Joseph Myers wrote:

> 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 adds a nop in the delay slot.
> 
> Tested with string/ tests for n32.  Will commit subject to full
> testing for n32 and n64.

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.

Steve Ellcey
sellcey@imgtec.com


> 
> 2016-01-21  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..9ae0ba6 100644
> --- a/sysdeps/mips/memcpy.S
> +++ b/sysdeps/mips/memcpy.S
> @@ -565,6 +565,7 @@ L(lastw):
>  #ifdef USE_DOUBLE
>  	andi    t8,a2,3		/* a2 is the remainder past 4 byte chunks.  */
>  	beq	t8,a2,L(lastb)
> +	nop
>  	lw	REG3,0(a1)
>  	sw	REG3,0(a0)
>  	PTR_ADDIU a0,a0,4
>