[Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer

[Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer

[chengang]

From: Chen Gang <chengang@emindsoft.com.cn>

The restorer needs the return code address which is frame->retcode, not
frame itself.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 linux-user/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index e487f9e..4157154 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -5455,7 +5455,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
                    &frame->retcode[1]);
         __put_user(INSN_CALLSYS, &frame->retcode[2]);
         /* imb(); */
-        r26 = frame_addr;
+        r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
     }
 
     if (err) {
-- 
1.9.3

Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer

[Laurent Vivier]

Le 29/03/2016 16:01, chengang@emindsoft.com.cn a écrit :
> From: Chen Gang <chengang@emindsoft.com.cn>
> 
> The restorer needs the return code address which is frame->retcode, not
> frame itself.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  linux-user/signal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index e487f9e..4157154 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -5455,7 +5455,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>                     &frame->retcode[1]);
>          __put_user(INSN_CALLSYS, &frame->retcode[2]);
>          /* imb(); */
> -        r26 = frame_addr;
> +        r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
>      }
>  
>      if (err) {
> 

If you change setup_rt_frame(), you must update setup_frame() too.

It seems correct.

Richard, as you have written the original code, could you check this is
correct?

Laurent

Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer

[Chen Gang]

On 3/29/16 22:25, Laurent Vivier wrote:
> Le 29/03/2016 16:01, chengang@emindsoft.com.cn a écrit :
>> The restorer needs the return code address which is frame->retcode, not
>> frame itself.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>  linux-user/signal.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index e487f9e..4157154 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -5455,7 +5455,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>>                     &frame->retcode[1]);
>>          __put_user(INSN_CALLSYS, &frame->retcode[2]);
>>          /* imb(); */
>> -        r26 = frame_addr;
>> +        r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
>>      }
>>  
>>      if (err) {
>>
> 
> If you change setup_rt_frame(), you must update setup_frame() too.
> 

Oh, yes, thanks.

> It seems correct.
> 
> Richard, as you have written the original code, could you check this is
> correct?
> 

Please give a check when you have time.

Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.

Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use frame->retcode instead of frame address for alpha target restorer

[Richard Henderson]

On 03/29/2016 07:25 AM, Laurent Vivier wrote:
> 
> 
> Le 29/03/2016 16:01, chengang@emindsoft.com.cn a écrit :
>> From: Chen Gang <chengang@emindsoft.com.cn>
>>
>> The restorer needs the return code address which is frame->retcode, not
>> frame itself.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>  linux-user/signal.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index e487f9e..4157154 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -5455,7 +5455,7 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>>                     &frame->retcode[1]);
>>          __put_user(INSN_CALLSYS, &frame->retcode[2]);
>>          /* imb(); */
>> -        r26 = frame_addr;
>> +        r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
>>      }
>>  
>>      if (err) {
>>
> 
> If you change setup_rt_frame(), you must update setup_frame() too.
> 
> It seems correct.
> 
> Richard, as you have written the original code, could you check this is
> correct?

Yes, it's correct, and yes both functions need fixing.

In practice, this path is historical and will never be used;
it was now a very old glibc that didn't supply a restorer.


r~