Re: [PATCH v2 02/23] linux-user/aarch64: Implement setup_sigtramp

[Peter Maydell]

On Fri, 18 Jun 2021 at 20:33, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create and record the rt signal trampoline.
> Use it when the guest does not use SA_RESTORER.
>
> Cc: qemu-arm@nongnu.org
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/target_signal.h |  2 ++
>  linux-user/aarch64/signal.c        | 28 ++++++++++++++++++----------
>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/linux-user/aarch64/target_signal.h 
> b/linux-user/aarch64/target_signal.h
> index 18013e1b23..7580d99403 100644
> --- a/linux-user/aarch64/target_signal.h
> +++ b/linux-user/aarch64/target_signal.h
> @@ -25,4 +25,6 @@ typedef struct target_sigaltstack {
>  #define TARGET_SEGV_MTESERR  9  /* Synchronous ARM MTE exception */
>
>  #define TARGET_ARCH_HAS_SETUP_FRAME
> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
> +
>  #endif /* AARCH64_TARGET_SIGNAL_H */
> diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
> index 662bcd1c4e..65b84eb04e 100644
> --- a/linux-user/aarch64/signal.c
> +++ b/linux-user/aarch64/signal.c
> @@ -108,7 +108,6 @@ struct target_rt_sigframe {
>  struct target_rt_frame_record {
>      uint64_t fp;
>      uint64_t lr;
> -    uint32_t tramp[2];
>  };
>
>  static void target_setup_general_frame(struct target_rt_sigframe *sf,
> @@ -495,15 +494,7 @@ static void target_setup_frame(int usig, struct 
> target_sigaction *ka,


A little way up from here there's a comment:

    /* Reserve space for the return code.  On a real system this would
     * be within the VDSO.  So, despite the name this is not a "real"
     * record within the frame.
     */
    fr_ofs = layout.total_size;
    layout.total_size += sizeof(struct target_rt_frame_record);

That now needs updating, because we're no longer putting the return
code in that target_rt_frame_record.

I think the 'struct target_rt_frame_record' now does correspond
to the kernel's 'struct frame_record', right?

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM