| Message ID | 20210616011209.1446045-8-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Move signal trampolines to new page | expand |
> -----Original Message----- > From: Richard Henderson <richard.henderson@linaro.org> > Sent: Tuesday, June 15, 2021 7:12 PM > To: qemu-devel@nongnu.org > Cc: laurent@vivier.eu; alex.bennee@linaro.org; Taylor Simpson > <tsimpson@quicinc.com> > Subject: [PATCH 07/21] linux-user/hexagon: Implement setup_sigtramp > > Continue to initialize the words on the stack, as documented. > However, use the off-stack trampoline. > > Cc: Taylor Simpson <tsimpson@quicinc.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c index > 85eab5e943..bd0f9b1c85 100644 > --- a/linux-user/hexagon/signal.c > +++ b/linux-user/hexagon/signal.c > @@ -161,6 +161,11 @@ void setup_rt_frame(int sig, struct target_sigaction > *ka, > > setup_ucontext(&frame->uc, env, set); > tswap_siginfo(&frame->info, info); > + /* > + * The on-stack signal trampoline is no longer executed; > + * however, the libgcc signal frame unwinding code checks > + * for the presence of these two numeric magic values. > + */ Hexagon uses musl, not libgcc. So, I'm not sure if this is needed. The signals.c test passes for me without this change. Are you seeing it fail? > +void setup_sigtramp(abi_ulong sigtramp_page) { > + uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 4 * 2, 0); > + assert(tramp != NULL); > + > + default_rt_sigreturn = sigtramp_page; > + install_sigtramp(tramp); > + > + unlock_user(tramp, sigtramp_page, 4 * 2); } Put the closing curly on a new line. Thanks, Taylor
On 6/16/21 1:07 AM, Taylor Simpson wrote: >> + /* >> + * The on-stack signal trampoline is no longer executed; >> + * however, the libgcc signal frame unwinding code checks >> + * for the presence of these two numeric magic values. >> + */ > > Hexagon uses musl, not libgcc. So, I'm not sure if this is needed. The signals.c test passes for me without this change. Are you seeing it fail? I copied the comment from the kernel source. >> +void setup_sigtramp(abi_ulong sigtramp_page) { >> + uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 4 * 2, 0); >> + assert(tramp != NULL); >> + >> + default_rt_sigreturn = sigtramp_page; >> + install_sigtramp(tramp); >> + >> + unlock_user(tramp, sigtramp_page, 4 * 2); } > > Put the closing curly on a new line. That's your mailer. It's correct in the original. r~
On 6/16/21 8:05 AM, Richard Henderson wrote: > On 6/16/21 1:07 AM, Taylor Simpson wrote: >>> + /* >>> + * The on-stack signal trampoline is no longer executed; >>> + * however, the libgcc signal frame unwinding code checks >>> + * for the presence of these two numeric magic values. >>> + */ >> >> Hexagon uses musl, not libgcc. So, I'm not sure if this is needed. The signals.c test >> passes for me without this change. Are you seeing it fail? > > I copied the comment from the kernel source. Also, I think you're confusing libc and libgcc. r~
> -----Original Message----- > From: Richard Henderson <richard.henderson@linaro.org> > Sent: Wednesday, June 16, 2021 9:51 AM > To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org > Cc: laurent@vivier.eu; alex.bennee@linaro.org > Subject: Re: [PATCH 07/21] linux-user/hexagon: Implement setup_sigtramp > > On 6/16/21 8:05 AM, Richard Henderson wrote: > > On 6/16/21 1:07 AM, Taylor Simpson wrote: > >>> + /* > >>> + * The on-stack signal trampoline is no longer executed; > >>> + * however, the libgcc signal frame unwinding code checks > >>> + * for the presence of these two numeric magic values. > >>> + */ > >> > >> Hexagon uses musl, not libgcc. So, I'm not sure if this is needed. > >> The signals.c test passes for me without this change. Are you seeing it > fail? > > > > I copied the comment from the kernel source. > > Also, I think you're confusing libc and libgcc. Yes, I'm confused. Why is signal frame unwinding in libgcc? Also FWIW, we use LLVM's compiler-rt instead of libgcc.
On 6/16/21 2:37 PM, Taylor Simpson wrote:
> Yes, I'm confused. Why is signal frame unwinding in libgcc?
Because it's tied to the compiler, and it was a decent solution back in 1997.
I see llvm calls it libunwind, and only has special signal frame support for aarch64. I
wonder if ever other target is expecting unwind info in a vdso?
r~
diff --git a/linux-user/hexagon/target_signal.h b/linux-user/hexagon/target_signal.h index 345cf1cbb8..9e0223d322 100644 --- a/linux-user/hexagon/target_signal.h +++ b/linux-user/hexagon/target_signal.h @@ -31,4 +31,6 @@ typedef struct target_sigaltstack { #include "../generic/signal.h" +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 + #endif /* TARGET_SIGNAL_H */ diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c index 85eab5e943..bd0f9b1c85 100644 --- a/linux-user/hexagon/signal.c +++ b/linux-user/hexagon/signal.c @@ -161,6 +161,11 @@ void setup_rt_frame(int sig, struct target_sigaction *ka, setup_ucontext(&frame->uc, env, set); tswap_siginfo(&frame->info, info); + /* + * The on-stack signal trampoline is no longer executed; + * however, the libgcc signal frame unwinding code checks + * for the presence of these two numeric magic values. + */ install_sigtramp(frame->tramp); env->gpr[HEX_REG_PC] = ka->_sa_handler; @@ -170,8 +175,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka, frame_addr + offsetof(struct target_rt_sigframe, info); env->gpr[HEX_REG_R02] = frame_addr + offsetof(struct target_rt_sigframe, uc); - env->gpr[HEX_REG_LR] = - frame_addr + offsetof(struct target_rt_sigframe, tramp); + env->gpr[HEX_REG_LR] = default_rt_sigreturn; return; @@ -270,3 +274,14 @@ badframe: force_sig(TARGET_SIGSEGV); return 0; } + +void setup_sigtramp(abi_ulong sigtramp_page) +{ + uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 4 * 2, 0); + assert(tramp != NULL); + + default_rt_sigreturn = sigtramp_page; + install_sigtramp(tramp); + + unlock_user(tramp, sigtramp_page, 4 * 2); +}
Continue to initialize the words on the stack, as documented. However, use the off-stack trampoline. Cc: Taylor Simpson <tsimpson@quicinc.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/hexagon/target_signal.h | 2 ++ linux-user/hexagon/signal.c | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-)