[PATCH v2] m68k: use conventional function parameters for do_sigreturn

[PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Greg Ungerer]

Create conventional stack parameters for the calls to do_sigreturn and
do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
dig into the stack to create local pointers to the saved switch stack
and the pt_regs structs.

The motivation for this change is a problem with non-MMU targets that
have broken signal return paths on newer versions of gcc. It appears as
though gcc has determined that the pointers into the saved stack structs,
and the saved structs themselves, are function parameters and updates to
them will be lost on function return, so they are optimized away. This
results in large parts of restore_sigcontext() and mangle_kernel_stack()
functions being removed. Of course this results in non-functional code
causing kernel oops. This problem has been observed with gcc version
5.2 and 5.3, and probably exists in earlier versions as well.

Using conventional stack parameter pointers passed to these functions has
the advantage of the code here not needing to know the exact details of
how the underlying entry handler layed these structs out on the stack.
So the rather ugly pointer setup casting and arg referencing can be
removed.

The resulting code after this change is a few bytes larger (due to the
overhead of creating the stack args and their tear down). Not being hot
paths I don't think this is too much of a problem here.

An alternative solution is to put a barrier() in the do_sigreturn() code,
but this doesn't feel quite as clean as this solution.

This change has been compile tested on all defconfigs, and run tested on
Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
no-MMU (QEMU and M5208EVB).

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
v2: reworded the commit log message with better description of problem

 arch/m68k/kernel/entry.S  | 6 ++++++
 arch/m68k/kernel/signal.c | 8 ++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
index b54ac7a..97cd3ea 100644
--- a/arch/m68k/kernel/entry.S
+++ b/arch/m68k/kernel/entry.S
@@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
 
 ENTRY(sys_sigreturn)
 	SAVE_SWITCH_STACK
+	movel	%sp,%sp@-		  | switch_stack pointer
+	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
 	jbsr	do_sigreturn
+	addql	#8,%sp
 	RESTORE_SWITCH_STACK
 	rts
 
 ENTRY(sys_rt_sigreturn)
 	SAVE_SWITCH_STACK
+	movel	%sp,%sp@-		  | switch_stack pointer
+	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
 	jbsr	do_rt_sigreturn
+	addql	#8,%sp
 	RESTORE_SWITCH_STACK
 	rts
 
diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
index af1c4f3..2dcee3a 100644
--- a/arch/m68k/kernel/signal.c
+++ b/arch/m68k/kernel/signal.c
@@ -737,10 +737,8 @@ badframe:
 	return 1;
 }
 
-asmlinkage int do_sigreturn(unsigned long __unused)
+asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
 {
-	struct switch_stack *sw = (struct switch_stack *) &__unused;
-	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
 	unsigned long usp = rdusp();
 	struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
 	sigset_t set;
@@ -764,10 +762,8 @@ badframe:
 	return 0;
 }
 
-asmlinkage int do_rt_sigreturn(unsigned long __unused)
+asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
 {
-	struct switch_stack *sw = (struct switch_stack *) &__unused;
-	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
 	unsigned long usp = rdusp();
 	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
 	sigset_t set;
-- 
1.9.1

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Philippe De Muyter]

Hi Greg,

On Mon, Feb 15, 2016 at 04:36:29PM +1000, Greg Ungerer wrote:
> Create conventional stack parameters for the calls to do_sigreturn and
> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
> dig into the stack to create local pointers to the saved switch stack
> and the pt_regs structs.
> 
> The motivation for this change is a problem with non-MMU targets that
> have broken signal return paths on newer versions of gcc. It appears as
> though gcc has determined that the pointers into the saved stack structs,
> and the saved structs themselves, are function parameters and updates to
> them will be lost on function return, so they are optimized away. This
> results in large parts of restore_sigcontext() and mangle_kernel_stack()
> functions being removed. Of course this results in non-functional code
> causing kernel oops. This problem has been observed with gcc version
> 5.2 and 5.3, and probably exists in earlier versions as well.
> 
> Using conventional stack parameter pointers passed to these functions has
> the advantage of the code here not needing to know the exact details of
> how the underlying entry handler layed these structs out on the stack.
> So the rather ugly pointer setup casting and arg referencing can be
> removed.
> 
> The resulting code after this change is a few bytes larger (due to the
> overhead of creating the stack args and their tear down). Not being hot
> paths I don't think this is too much of a problem here.

As an intermediate solution, I think that you can avoid part of that overhead ...

> 
> An alternative solution is to put a barrier() in the do_sigreturn() code,
> but this doesn't feel quite as clean as this solution.
> 
> This change has been compile tested on all defconfigs, and run tested on
> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
> no-MMU (QEMU and M5208EVB).
> 
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> ---
> v2: reworded the commit log message with better description of problem
> 
>  arch/m68k/kernel/entry.S  | 6 ++++++
>  arch/m68k/kernel/signal.c | 8 ++------
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
> index b54ac7a..97cd3ea 100644
> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>  
>  ENTRY(sys_sigreturn)
>  	SAVE_SWITCH_STACK
> +	movel	%sp,%sp@-		  | switch_stack pointer
> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer

if you remove the above asm instruction ...

>  	jbsr	do_sigreturn
> +	addql	#8,%sp

change the 8 by 4 here ...

>  	RESTORE_SWITCH_STACK
>  	rts
>  
>  ENTRY(sys_rt_sigreturn)
>  	SAVE_SWITCH_STACK
> +	movel	%sp,%sp@-		  | switch_stack pointer
> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer

ditto here ...

>  	jbsr	do_rt_sigreturn
> +	addql	#8,%sp

and here ...

>  	RESTORE_SWITCH_STACK
>  	rts
>  
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index af1c4f3..2dcee3a 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -737,10 +737,8 @@ badframe:
>  	return 1;
>  }
>  
> -asmlinkage int do_sigreturn(unsigned long __unused)
> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)

and if you use the following prototype :
 asmlinkage int do_sigreturn(struct switch_stack *sw) ...

>  {
> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);

and keep the 'C' instruction above

>  	unsigned long usp = rdusp();
>  	struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
>  	sigset_t set;
> @@ -764,10 +762,8 @@ badframe:
>  	return 0;
>  }
>  
> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
 same here
>  {
> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
 and here
>  	unsigned long usp = rdusp();
>  	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
>  	sigset_t set;

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Greg Ungerer]

Hi Philippe,

On 15/02/16 20:03, Philippe De Muyter wrote:
> On Mon, Feb 15, 2016 at 04:36:29PM +1000, Greg Ungerer wrote:
>> Create conventional stack parameters for the calls to do_sigreturn and
>> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
>> dig into the stack to create local pointers to the saved switch stack
>> and the pt_regs structs.
>>
>> The motivation for this change is a problem with non-MMU targets that
>> have broken signal return paths on newer versions of gcc. It appears as
>> though gcc has determined that the pointers into the saved stack structs,
>> and the saved structs themselves, are function parameters and updates to
>> them will be lost on function return, so they are optimized away. This
>> results in large parts of restore_sigcontext() and mangle_kernel_stack()
>> functions being removed. Of course this results in non-functional code
>> causing kernel oops. This problem has been observed with gcc version
>> 5.2 and 5.3, and probably exists in earlier versions as well.
>>
>> Using conventional stack parameter pointers passed to these functions has
>> the advantage of the code here not needing to know the exact details of
>> how the underlying entry handler layed these structs out on the stack.
>> So the rather ugly pointer setup casting and arg referencing can be
>> removed.
>>
>> The resulting code after this change is a few bytes larger (due to the
>> overhead of creating the stack args and their tear down). Not being hot
>> paths I don't think this is too much of a problem here.
>
> As an intermediate solution, I think that you can avoid part of that overhead ...
>
>>
>> An alternative solution is to put a barrier() in the do_sigreturn() code,
>> but this doesn't feel quite as clean as this solution.
>>
>> This change has been compile tested on all defconfigs, and run tested on
>> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
>> no-MMU (QEMU and M5208EVB).
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>> ---
>> v2: reworded the commit log message with better description of problem
>>
>>   arch/m68k/kernel/entry.S  | 6 ++++++
>>   arch/m68k/kernel/signal.c | 8 ++------
>>   2 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
>> index b54ac7a..97cd3ea 100644
>> --- a/arch/m68k/kernel/entry.S
>> +++ b/arch/m68k/kernel/entry.S
>> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>>
>>   ENTRY(sys_sigreturn)
>>   	SAVE_SWITCH_STACK
>> +	movel	%sp,%sp@-		  | switch_stack pointer
>> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>
> if you remove the above asm instruction ...
>
>>   	jbsr	do_sigreturn
>> +	addql	#8,%sp
>
> change the 8 by 4 here ...
>
>>   	RESTORE_SWITCH_STACK
>>   	rts
>>
>>   ENTRY(sys_rt_sigreturn)
>>   	SAVE_SWITCH_STACK
>> +	movel	%sp,%sp@-		  | switch_stack pointer
>> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>
> ditto here ...
>
>>   	jbsr	do_rt_sigreturn
>> +	addql	#8,%sp
>
> and here ...
>
>>   	RESTORE_SWITCH_STACK
>>   	rts
>>
>> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
>> index af1c4f3..2dcee3a 100644
>> --- a/arch/m68k/kernel/signal.c
>> +++ b/arch/m68k/kernel/signal.c
>> @@ -737,10 +737,8 @@ badframe:
>>   	return 1;
>>   }
>>
>> -asmlinkage int do_sigreturn(unsigned long __unused)
>> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
>
> and if you use the following prototype :
>   asmlinkage int do_sigreturn(struct switch_stack *sw) ...
>
>>   {
>> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
>> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>
> and keep the 'C' instruction above
>
>>   	unsigned long usp = rdusp();
>>   	struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
>>   	sigset_t set;
>> @@ -764,10 +762,8 @@ badframe:
>>   	return 0;
>>   }
>>
>> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
>> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
>   same here
>>   {
>> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
>> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>   and here
>>   	unsigned long usp = rdusp();
>>   	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
>>   	sigset_t set;

Those changes ultimately make no difference to code size.
Although the entry point code is smaller the resulting
generated code for do_sigreturn() and do_rt_sigreturn()
is slightly larger. (At least that is the case with the gcc-5.3
compiler I am currently using).

Personally  I don't like having to keep that ugly stack referencing
definition of regs based on sw either. I would like to remove that.

"sw" isn't actually used in do_sigreturn() at all, so that could
be removed from the its parameters. I chose to keep it there in this
patch for consistency with do_rt_sigreturn().

Regards
Greg

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Philippe De Muyter]

Hi Greg,

On Mon, Feb 15, 2016 at 11:06:15PM +1000, Greg Ungerer wrote:
> Hi Philippe,
>
> On 15/02/16 20:03, Philippe De Muyter wrote:
>> On Mon, Feb 15, 2016 at 04:36:29PM +1000, Greg Ungerer wrote:
>>> Create conventional stack parameters for the calls to do_sigreturn and
>>> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
>>> dig into the stack to create local pointers to the saved switch stack
>>> and the pt_regs structs.
>>>
>>> The motivation for this change is a problem with non-MMU targets that
>>> have broken signal return paths on newer versions of gcc. It appears as
>>> though gcc has determined that the pointers into the saved stack structs,
>>> and the saved structs themselves, are function parameters and updates to
>>> them will be lost on function return, so they are optimized away. This
>>> results in large parts of restore_sigcontext() and mangle_kernel_stack()
>>> functions being removed. Of course this results in non-functional code
>>> causing kernel oops. This problem has been observed with gcc version
>>> 5.2 and 5.3, and probably exists in earlier versions as well.
>>>
>>> Using conventional stack parameter pointers passed to these functions has
>>> the advantage of the code here not needing to know the exact details of
>>> how the underlying entry handler layed these structs out on the stack.
>>> So the rather ugly pointer setup casting and arg referencing can be
>>> removed.
>>>
>>> The resulting code after this change is a few bytes larger (due to the
>>> overhead of creating the stack args and their tear down). Not being hot
>>> paths I don't think this is too much of a problem here.
>>
>> As an intermediate solution, I think that you can avoid part of that 
>> overhead ...
>>
>>>
>>> An alternative solution is to put a barrier() in the do_sigreturn() code,
>>> but this doesn't feel quite as clean as this solution.
>>>
>>> This change has been compile tested on all defconfigs, and run tested on
>>> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
>>> no-MMU (QEMU and M5208EVB).
>>>
>>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>>> ---
>>> v2: reworded the commit log message with better description of problem
>>>
>>>   arch/m68k/kernel/entry.S  | 6 ++++++
>>>   arch/m68k/kernel/signal.c | 8 ++------
>>>   2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
>>> index b54ac7a..97cd3ea 100644
>>> --- a/arch/m68k/kernel/entry.S
>>> +++ b/arch/m68k/kernel/entry.S
>>> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>>>
>>>   ENTRY(sys_sigreturn)
>>>   	SAVE_SWITCH_STACK
>>> +	movel	%sp,%sp@-		  | switch_stack pointer
>>> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>>
>> if you remove the above asm instruction ...
>>
>>>   	jbsr	do_sigreturn
>>> +	addql	#8,%sp
>>
>> change the 8 by 4 here ...
>>
>>>   	RESTORE_SWITCH_STACK
>>>   	rts
>>>
>>>   ENTRY(sys_rt_sigreturn)
>>>   	SAVE_SWITCH_STACK
>>> +	movel	%sp,%sp@-		  | switch_stack pointer
>>> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>>
>> ditto here ...
>>
>>>   	jbsr	do_rt_sigreturn
>>> +	addql	#8,%sp
>>
>> and here ...
>>
>>>   	RESTORE_SWITCH_STACK
>>>   	rts
>>>
>>> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
>>> index af1c4f3..2dcee3a 100644
>>> --- a/arch/m68k/kernel/signal.c
>>> +++ b/arch/m68k/kernel/signal.c
>>> @@ -737,10 +737,8 @@ badframe:
>>>   	return 1;
>>>   }
>>>
>>> -asmlinkage int do_sigreturn(unsigned long __unused)
>>> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack 
>>> *sw)
>>
>> and if you use the following prototype :
>>   asmlinkage int do_sigreturn(struct switch_stack *sw) ...
>>
>>>   {
>>> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
>>> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>>
>> and keep the 'C' instruction above
>>
>>>   	unsigned long usp = rdusp();
>>>   	struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
>>>   	sigset_t set;
>>> @@ -764,10 +762,8 @@ badframe:
>>>   	return 0;
>>>   }
>>>
>>> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
>>> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack 
>>> *sw)
>>   same here
>>>   {
>>> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
>>> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>>   and here
>>>   	unsigned long usp = rdusp();
>>>   	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 
>>> 4);
>>>   	sigset_t set;
>
> Those changes ultimately make no difference to code size.
> Although the entry point code is smaller the resulting
> generated code for do_sigreturn() and do_rt_sigreturn()
> is slightly larger. (At least that is the case with the gcc-5.3
> compiler I am currently using).
>
> Personally  I don't like having to keep that ugly stack referencing
> definition of regs based on sw either. I would like to remove that.
>
> "sw" isn't actually used in do_sigreturn() at all, so that could
> be removed from the its parameters. I chose to keep it there in this
> patch for consistency with do_rt_sigreturn().

OK.  Thanks for the attention and explanation.

>
> Regards
> Greg

Best regards

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Greg Ungerer]

Hi Geert,

Do you have any objections or issues with this change?

Andreas and Philippe kindly gave feedback, which I have addressed
(I think to everyone's satisfaction?)

I would very much like to see this applied to resolve the compilation
issue for non-MMU with modern versions of gcc. I don't mind pushing
through my tree. I just want to make sure first that nobody objects
to this change or has any remaining issues with it.

Regards
Greg


On 15/02/16 16:36, Greg Ungerer wrote:
> Create conventional stack parameters for the calls to do_sigreturn and
> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
> dig into the stack to create local pointers to the saved switch stack
> and the pt_regs structs.
> 
> The motivation for this change is a problem with non-MMU targets that
> have broken signal return paths on newer versions of gcc. It appears as
> though gcc has determined that the pointers into the saved stack structs,
> and the saved structs themselves, are function parameters and updates to
> them will be lost on function return, so they are optimized away. This
> results in large parts of restore_sigcontext() and mangle_kernel_stack()
> functions being removed. Of course this results in non-functional code
> causing kernel oops. This problem has been observed with gcc version
> 5.2 and 5.3, and probably exists in earlier versions as well.
> 
> Using conventional stack parameter pointers passed to these functions has
> the advantage of the code here not needing to know the exact details of
> how the underlying entry handler layed these structs out on the stack.
> So the rather ugly pointer setup casting and arg referencing can be
> removed.
> 
> The resulting code after this change is a few bytes larger (due to the
> overhead of creating the stack args and their tear down). Not being hot
> paths I don't think this is too much of a problem here.
> 
> An alternative solution is to put a barrier() in the do_sigreturn() code,
> but this doesn't feel quite as clean as this solution.
> 
> This change has been compile tested on all defconfigs, and run tested on
> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
> no-MMU (QEMU and M5208EVB).
> 
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> ---
> v2: reworded the commit log message with better description of problem
> 
>  arch/m68k/kernel/entry.S  | 6 ++++++
>  arch/m68k/kernel/signal.c | 8 ++------
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
> index b54ac7a..97cd3ea 100644
> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -71,13 +71,19 @@ ENTRY(__sys_vfork)
>  
>  ENTRY(sys_sigreturn)
>  	SAVE_SWITCH_STACK
> +	movel	%sp,%sp@-		  | switch_stack pointer
> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>  	jbsr	do_sigreturn
> +	addql	#8,%sp
>  	RESTORE_SWITCH_STACK
>  	rts
>  
>  ENTRY(sys_rt_sigreturn)
>  	SAVE_SWITCH_STACK
> +	movel	%sp,%sp@-		  | switch_stack pointer
> +	pea	%sp@(SWITCH_STACK_SIZE+4) | pt_regs pointer
>  	jbsr	do_rt_sigreturn
> +	addql	#8,%sp
>  	RESTORE_SWITCH_STACK
>  	rts
>  
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index af1c4f3..2dcee3a 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -737,10 +737,8 @@ badframe:
>  	return 1;
>  }
>  
> -asmlinkage int do_sigreturn(unsigned long __unused)
> +asmlinkage int do_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
>  {
> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>  	unsigned long usp = rdusp();
>  	struct sigframe __user *frame = (struct sigframe __user *)(usp - 4);
>  	sigset_t set;
> @@ -764,10 +762,8 @@ badframe:
>  	return 0;
>  }
>  
> -asmlinkage int do_rt_sigreturn(unsigned long __unused)
> +asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
>  {
> -	struct switch_stack *sw = (struct switch_stack *) &__unused;
> -	struct pt_regs *regs = (struct pt_regs *) (sw + 1);
>  	unsigned long usp = rdusp();
>  	struct rt_sigframe __user *frame = (struct rt_sigframe __user *)(usp - 4);
>  	sigset_t set;
> 

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Geert Uytterhoeven]

Hi Greg,

On Wed, Feb 24, 2016 at 2:10 AM, Greg Ungerer <gerg@uclinux.org> wrote:
> Do you have any objections or issues with this change?
>
> Andreas and Philippe kindly gave feedback, which I have addressed
> (I think to everyone's satisfaction?)
>
> I would very much like to see this applied to resolve the compilation
> issue for non-MMU with modern versions of gcc. I don't mind pushing
> through my tree. I just want to make sure first that nobody objects
> to this change or has any remaining issues with it.

If Andreas is happy with it, I will apply it.
Andreas?

> On 15/02/16 16:36, Greg Ungerer wrote:
>> Create conventional stack parameters for the calls to do_sigreturn and
>> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
>> dig into the stack to create local pointers to the saved switch stack
>> and the pt_regs structs.
>>
>> The motivation for this change is a problem with non-MMU targets that
>> have broken signal return paths on newer versions of gcc. It appears as
>> though gcc has determined that the pointers into the saved stack structs,
>> and the saved structs themselves, are function parameters and updates to
>> them will be lost on function return, so they are optimized away. This
>> results in large parts of restore_sigcontext() and mangle_kernel_stack()
>> functions being removed. Of course this results in non-functional code
>> causing kernel oops. This problem has been observed with gcc version
>> 5.2 and 5.3, and probably exists in earlier versions as well.
>>
>> Using conventional stack parameter pointers passed to these functions has
>> the advantage of the code here not needing to know the exact details of
>> how the underlying entry handler layed these structs out on the stack.
>> So the rather ugly pointer setup casting and arg referencing can be
>> removed.
>>
>> The resulting code after this change is a few bytes larger (due to the
>> overhead of creating the stack args and their tear down). Not being hot
>> paths I don't think this is too much of a problem here.
>>
>> An alternative solution is to put a barrier() in the do_sigreturn() code,
>> but this doesn't feel quite as clean as this solution.
>>
>> This change has been compile tested on all defconfigs, and run tested on
>> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
>> no-MMU (QEMU and M5208EVB).
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Andreas Schwab]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Greg,
>
> On Wed, Feb 24, 2016 at 2:10 AM, Greg Ungerer <gerg@uclinux.org> wrote:
>> Do you have any objections or issues with this change?
>>
>> Andreas and Philippe kindly gave feedback, which I have addressed
>> (I think to everyone's satisfaction?)
>>
>> I would very much like to see this applied to resolve the compilation
>> issue for non-MMU with modern versions of gcc. I don't mind pushing
>> through my tree. I just want to make sure first that nobody objects
>> to this change or has any remaining issues with it.
>
> If Andreas is happy with it, I will apply it.

No objections.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v2] m68k: use conventional function parameters for do_sigreturn

[Geert Uytterhoeven]

On Mon, Feb 15, 2016 at 7:36 AM, Greg Ungerer <gerg@uclinux.org> wrote:
> Create conventional stack parameters for the calls to do_sigreturn and
> do_rt_sigreturn. The current C code for do_sigreturn and do_rt_sigreturn
> dig into the stack to create local pointers to the saved switch stack
> and the pt_regs structs.
>
> The motivation for this change is a problem with non-MMU targets that
> have broken signal return paths on newer versions of gcc. It appears as
> though gcc has determined that the pointers into the saved stack structs,
> and the saved structs themselves, are function parameters and updates to
> them will be lost on function return, so they are optimized away. This
> results in large parts of restore_sigcontext() and mangle_kernel_stack()
> functions being removed. Of course this results in non-functional code
> causing kernel oops. This problem has been observed with gcc version
> 5.2 and 5.3, and probably exists in earlier versions as well.
>
> Using conventional stack parameter pointers passed to these functions has
> the advantage of the code here not needing to know the exact details of
> how the underlying entry handler layed these structs out on the stack.
> So the rather ugly pointer setup casting and arg referencing can be
> removed.
>
> The resulting code after this change is a few bytes larger (due to the
> overhead of creating the stack args and their tear down). Not being hot
> paths I don't think this is too much of a problem here.
>
> An alternative solution is to put a barrier() in the do_sigreturn() code,
> but this doesn't feel quite as clean as this solution.
>
> This change has been compile tested on all defconfigs, and run tested on
> Atari (through aranym), ColdFire with MMU (M5407EVB) and ColdFire with
> no-MMU (QEMU and M5208EVB).
>
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>

Thanks,.applied and queued for v4.6 with Andreas' Ack.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds