Restore r31 setting in powerpc32 swapcontext
diff mbox series

Message ID alpine.DEB.2.21.1907300047220.1468@digraph.polyomino.org.uk
State New
Headers show
Series
  • Restore r31 setting in powerpc32 swapcontext
Related show

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 mbox series

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