Restore r31 setting in powerpc32 swapcontext

Message ID alpine.DEB.2.21.1907300047220.1468@digraph.polyomino.org.uk
State Committed
Headers

Commit Message

Joseph Myers July 30, 2019, 12:48 a.m. UTC
  Commit ffe8a9a8318e1db225b22da8bc067408494bac5c, "powerpc: Remove
rt_sigreturn usage on context function", removed from powerpc32
swapcontext a setting of r31 that is relied upon in subsequent code.
I'm not sure why this didn't produce test failures in Adhemerval's
32-bit testing; in my (soft-float) testing in preparation for 2.30
release, I see several context-related failures

FAIL: stdlib/tst-makecontext2
FAIL: stdlib/tst-makecontext3
FAIL: stdlib/tst-setcontext
FAIL: stdlib/tst-setcontext2
FAIL: stdlib/tst-setcontext4
FAIL: stdlib/tst-setcontext7
FAIL: stdlib/tst-setcontext9
FAIL: stdlib/tst-swapcontext1

that did not appear in 2.29 testing.  This patch restores the removed
register setting in question, and thus fixes those failures.

Tested for powerpc (soft-float).

2019-07-30  Joseph Myers  <joseph@codesourcery.com>

	* sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
	(__CONTEXT_FUNC_NAME): Restore setting of r31.
  

Comments

Florian Weimer July 30, 2019, 7:17 a.m. UTC | #1
* Joseph Myers:

> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> index efebb10bba..6fa1ab7d6e 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> @@ -276,6 +276,9 @@ ENTRY(__CONTEXT_FUNC_NAME)
>  	cmpwi	r3,0
>  	bne	3f	/* L(error_exit) */
>  
> +	lwz	r4,_FRAME_PARM_SAVE2(r1)
> +	lwz	r31,_UC_REGS_PTR(r4)
> +
>  #ifdef __CONTEXT_ENABLE_FPRS
>  # ifdef __CONTEXT_ENABLE_VRS

Patch looks good to me.  Carlos will have to ack it as the RM, though.

Thanks,
Florian
  
Adhemerval Zanella July 30, 2019, 12:17 p.m. UTC | #2
On 29/07/2019 21:48, Joseph Myers wrote:
> Commit ffe8a9a8318e1db225b22da8bc067408494bac5c, "powerpc: Remove
> rt_sigreturn usage on context function", removed from powerpc32
> swapcontext a setting of r31 that is relied upon in subsequent code.
> I'm not sure why this didn't produce test failures in Adhemerval's
> 32-bit testing; in my (soft-float) testing in preparation for 2.30
> release, I see several context-related failures

Thanks for catching it, I am trying to understand why I didn't see it on the
environment I am using (gcc 8.1 on a POWER7). I just tested for hard-float,
I will try a soft-float to see if it is related.

I just tested with 1ff1373b3302 (which is the latest commit I checked on
the machine) and the tests do pass...

> 
> FAIL: stdlib/tst-makecontext2
> FAIL: stdlib/tst-makecontext3
> FAIL: stdlib/tst-setcontext
> FAIL: stdlib/tst-setcontext2
> FAIL: stdlib/tst-setcontext4
> FAIL: stdlib/tst-setcontext7
> FAIL: stdlib/tst-setcontext9
> FAIL: stdlib/tst-swapcontext1
> 
> that did not appear in 2.29 testing.  This patch restores the removed
> register setting in question, and thus fixes those failures.
> 
> Tested for powerpc (soft-float).
> 
> 2019-07-30  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> 	(__CONTEXT_FUNC_NAME): Restore setting of r31.
> 
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> index efebb10bba..6fa1ab7d6e 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
> @@ -276,6 +276,9 @@ ENTRY(__CONTEXT_FUNC_NAME)
>  	cmpwi	r3,0
>  	bne	3f	/* L(error_exit) */
>  
> +	lwz	r4,_FRAME_PARM_SAVE2(r1)
> +	lwz	r31,_UC_REGS_PTR(r4)
> +
>  #ifdef __CONTEXT_ENABLE_FPRS
>  # ifdef __CONTEXT_ENABLE_VRS
>  
>
  
Adhemerval Zanella July 30, 2019, 12:35 p.m. UTC | #3
On 30/07/2019 09:17, Adhemerval Zanella wrote:
> 
> 
> On 29/07/2019 21:48, Joseph Myers wrote:
>> Commit ffe8a9a8318e1db225b22da8bc067408494bac5c, "powerpc: Remove
>> rt_sigreturn usage on context function", removed from powerpc32
>> swapcontext a setting of r31 that is relied upon in subsequent code.
>> I'm not sure why this didn't produce test failures in Adhemerval's
>> 32-bit testing; in my (soft-float) testing in preparation for 2.30
>> release, I see several context-related failures
> 
> Thanks for catching it, I am trying to understand why I didn't see it on the
> environment I am using (gcc 8.1 on a POWER7). I just tested for hard-float,
> I will try a soft-float to see if it is related.
> 
> I just tested with 1ff1373b3302 (which is the latest commit I checked on
> the machine) and the tests do pass...

It does show for soft-float.

> 
>>
>> FAIL: stdlib/tst-makecontext2
>> FAIL: stdlib/tst-makecontext3
>> FAIL: stdlib/tst-setcontext
>> FAIL: stdlib/tst-setcontext2
>> FAIL: stdlib/tst-setcontext4
>> FAIL: stdlib/tst-setcontext7
>> FAIL: stdlib/tst-setcontext9
>> FAIL: stdlib/tst-swapcontext1
>>
>> that did not appear in 2.29 testing.  This patch restores the removed
>> register setting in question, and thus fixes those failures.
>>
>> Tested for powerpc (soft-float).
>>
>> 2019-07-30  Joseph Myers  <joseph@codesourcery.com>
>>
>> 	* sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> 	(__CONTEXT_FUNC_NAME): Restore setting of r31.
>>
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> index efebb10bba..6fa1ab7d6e 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> @@ -276,6 +276,9 @@ ENTRY(__CONTEXT_FUNC_NAME)
>>  	cmpwi	r3,0
>>  	bne	3f	/* L(error_exit) */
>>  
>> +	lwz	r4,_FRAME_PARM_SAVE2(r1)
>> +	lwz	r31,_UC_REGS_PTR(r4)
>> +
>>  #ifdef __CONTEXT_ENABLE_FPRS
>>  # ifdef __CONTEXT_ENABLE_VRS
>>  
>>
  
Carlos O'Donell July 30, 2019, 1:54 p.m. UTC | #4
On 7/30/19 3:17 AM, Florian Weimer wrote:
> * Joseph Myers:
> 
>> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> index efebb10bba..6fa1ab7d6e 100644
>> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
>> @@ -276,6 +276,9 @@ ENTRY(__CONTEXT_FUNC_NAME)
>>   	cmpwi	r3,0
>>   	bne	3f	/* L(error_exit) */
>>   
>> +	lwz	r4,_FRAME_PARM_SAVE2(r1)
>> +	lwz	r31,_UC_REGS_PTR(r4)
>> +
>>   #ifdef __CONTEXT_ENABLE_FPRS
>>   # ifdef __CONTEXT_ENABLE_VRS
> 
> Patch looks good to me.  Carlos will have to ack it as the RM, though.

Please commit to master.

Fixing release regressions like this is the highest priority.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
index efebb10bba..6fa1ab7d6e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/swapcontext-common.S
@@ -276,6 +276,9 @@  ENTRY(__CONTEXT_FUNC_NAME)
 	cmpwi	r3,0
 	bne	3f	/* L(error_exit) */
 
+	lwz	r4,_FRAME_PARM_SAVE2(r1)
+	lwz	r31,_UC_REGS_PTR(r4)
+
 #ifdef __CONTEXT_ENABLE_FPRS
 # ifdef __CONTEXT_ENABLE_VRS