Re: [PATCH 1/1] linux-user/s390x: Apply h2g to address of sigreturn stub

From: Richard Henderson <richard.henderson@linaro.org>
To: Andreas Krebbel <krebbel@linux.ibm.com>,
	Laurent Vivier <laurent@vivier.eu>
Cc: qemu-s390x@nongnu.org, qemu-devel@nongnu.org,
	David Hildenbrand <david@redhat.com>
Subject: Re: [PATCH 1/1] linux-user/s390x: Apply h2g to address of sigreturn stub
Date: Wed, 24 Mar 2021 08:34:48 -0600	[thread overview]
Message-ID: <17bfd572-623d-6bc4-fd9f-4c9b7d372156@linaro.org> (raw)
In-Reply-To: <4fadf180-24d5-01f2-8345-14d52f098db4@linux.ibm.com>

On 3/24/21 8:14 AM, Andreas Krebbel wrote:
> The part I actually needed to fix was in setup_rt_frame and there the fix is correct I think since
> here we do use 'frame' which is the host address.
> 
> While doing that change I also stumbled upon the other location in setup_frame. There it is using
> frame_addr for doing the same thing. There as you say adding h2g is wrong.
> 
> Here just the change which I think is needed:
> 
> diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
> index ecfa2a14a9..7fba1c7999 100644
> --- a/linux-user/s390x/signal.c
> +++ b/linux-user/s390x/signal.c
> @@ -213,7 +213,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>       if (ka->sa_flags & TARGET_SA_RESTORER) {
>           env->regs[14] = (unsigned long) ka->sa_restorer | PSW_ADDR_AMODE;
>       } else {
> -        env->regs[14] = (unsigned long) frame->retcode | PSW_ADDR_AMODE;
> +        env->regs[14] = (unsigned long) h2g(frame->retcode) | PSW_ADDR_AMODE;

Correct, though I think the formulation using frame_addr is more obvious.

Unrelated, but all the uses of "unsigned long" are wrong -- they should be 
target_ulong.

r~