| Message ID | 20240702234155.2106399-3-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Fix unwind from dc zva and FEAT_MOPS | expand |
On Wed, 3 Jul 2024 at 00:42, Richard Henderson <richard.henderson@linaro.org> wrote: > > Without this, qemu user will not unwind from the SIGSEGV > properly and die with > > qemu-aarch64: QEMU internal SIGSEGV {code=ACCERR, addr=0x7d1b36ec2000} > Segmentation fault > > Fill in the test case for ppc and s390x, which also use memset > from within a helper (but don't currently crash fwiw). > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/tcg/helper-a64.c | 10 ++-- > tests/tcg/multiarch/memset-fault.c | 77 ++++++++++++++++++++++++++++++ > 2 files changed, 82 insertions(+), 5 deletions(-) > create mode 100644 tests/tcg/multiarch/memset-fault.c > > diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c > index 0ea8668ab4..666bdb7c1a 100644 > --- a/target/arm/tcg/helper-a64.c > +++ b/target/arm/tcg/helper-a64.c > @@ -971,7 +971,7 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) > } > #endif > > - memset(mem, 0, blocklen); > + memset_ra(mem, 0, blocklen, GETPC()); > } > > void HELPER(unaligned_access)(CPUARMState *env, uint64_t addr, > @@ -1120,7 +1120,7 @@ static uint64_t set_step(CPUARMState *env, uint64_t toaddr, > } > #endif > /* Easy case: just memset the host memory */ > - memset(mem, data, setsize); > + memset_ra(mem, data, setsize, ra); > return setsize; > } I think strictly speaking since page_limit() and page_limit_rev() only look at the target page size and not the host page size, this will not quite behave correctly in the case where the host page size is smaller than the target page size, but that case doesn't work properly in any number of other situations already, so I don't really care about it. > @@ -1163,7 +1163,7 @@ static uint64_t set_step_tags(CPUARMState *env, uint64_t toaddr, > } > #endif > /* Easy case: just memset the host memory */ > - memset(mem, data, setsize); > + memset_ra(mem, data, setsize, ra); > mte_mops_set_tags(env, toaddr, setsize, *mtedesc); > return setsize; > } > @@ -1497,7 +1497,7 @@ static uint64_t copy_step(CPUARMState *env, uint64_t toaddr, uint64_t fromaddr, > } > #endif > /* Easy case: just memmove the host memory */ > - memmove(wmem, rmem, copysize); > + memmove_ra(wmem, rmem, copysize, ra); > return copysize; > } > > @@ -1572,7 +1572,7 @@ static uint64_t copy_step_rev(CPUARMState *env, uint64_t toaddr, > * Easy case: just memmove the host memory. Note that wmem and > * rmem here point to the *last* byte to copy. > */ > - memmove(wmem - (copysize - 1), rmem - (copysize - 1), copysize); > + memmove_ra(wmem - (copysize - 1), rmem - (copysize - 1), copysize, ra); > return copysize; > } > > diff --git a/tests/tcg/multiarch/memset-fault.c b/tests/tcg/multiarch/memset-fault.c > new file mode 100644 > index 0000000000..0e8258a88f > --- /dev/null > +++ b/tests/tcg/multiarch/memset-fault.c > @@ -0,0 +1,77 @@ > +#include <stdlib.h> > +#include <signal.h> > +#include <unistd.h> > +#include <sys/mman.h> > +#include <assert.h> Can we have a copyright-and-license header comment for all new files, please? > + > +#if defined(__powerpc64__) > +/* Needed for PT_* constants */ > +#include <asm/ptrace.h> > +#endif > + > +static void *ptr; > +static void *pc; > + > +static void test(void) > +{ > +#ifdef __aarch64__ > + void *t; > + asm("adr %0,1f; str %0,%1; 1: dc zva,%2" > + : "=&r"(t), "=m"(pc) : "r"(ptr)); > +#elif defined(__powerpc64__) > + void *t; > + asm("bl 0f; 0: mflr %0; addi %0,%0,1f-0b; std %0,%1; 1: dcbz 0,%2" > + : "=&r"(t), "=m"(pc) : "r"(ptr) : "lr"); > +#elif defined(__s390x__) > + void *t; > + asm("larl %0,1f; stg %0,%1; 1: xc 0(256,%2),0(%2)" > + : "=&r"(t), "=m"(pc) : "r"(ptr)); > +#else > + *(int *)ptr = 0; > +#endif > +} > + > +static void *host_signal_pc(ucontext_t *uc) > +{ > +#ifdef __aarch64__ > + return (void *)uc->uc_mcontext.pc; > +#elif defined(__powerpc64__) > + return (void *)uc->uc_mcontext.gp_regs[PT_NIP]; > +#elif defined(__s390x__) > + return (void *)uc->uc_mcontext.psw.addr; > +#else > + return NULL; > +#endif > +} > + > +static void sigsegv(int sig, siginfo_t *info, void *uc) > +{ > + assert(info->si_addr == ptr); > + assert(host_signal_pc(uc) == pc); > + exit(0); > +} > + > +int main(void) > +{ > + static const struct sigaction sa = { > + .sa_flags = SA_SIGINFO, > + .sa_sigaction = sigsegv > + }; > + size_t size; > + int r; > + > + size = getpagesize(); > + ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, > + MAP_ANON | MAP_PRIVATE, -1, 0); > + assert(ptr != MAP_FAILED); > + > + test(); > + > + r = sigaction(SIGSEGV, &sa, NULL); > + assert(r == 0); > + r = mprotect(ptr, size, PROT_NONE); > + assert(r == 0); > + > + test(); > + abort(); > +} A few comments in this test program to explain what it's doing would be helpful. Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c index 0ea8668ab4..666bdb7c1a 100644 --- a/target/arm/tcg/helper-a64.c +++ b/target/arm/tcg/helper-a64.c @@ -971,7 +971,7 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) } #endif - memset(mem, 0, blocklen); + memset_ra(mem, 0, blocklen, GETPC()); } void HELPER(unaligned_access)(CPUARMState *env, uint64_t addr, @@ -1120,7 +1120,7 @@ static uint64_t set_step(CPUARMState *env, uint64_t toaddr, } #endif /* Easy case: just memset the host memory */ - memset(mem, data, setsize); + memset_ra(mem, data, setsize, ra); return setsize; } @@ -1163,7 +1163,7 @@ static uint64_t set_step_tags(CPUARMState *env, uint64_t toaddr, } #endif /* Easy case: just memset the host memory */ - memset(mem, data, setsize); + memset_ra(mem, data, setsize, ra); mte_mops_set_tags(env, toaddr, setsize, *mtedesc); return setsize; } @@ -1497,7 +1497,7 @@ static uint64_t copy_step(CPUARMState *env, uint64_t toaddr, uint64_t fromaddr, } #endif /* Easy case: just memmove the host memory */ - memmove(wmem, rmem, copysize); + memmove_ra(wmem, rmem, copysize, ra); return copysize; } @@ -1572,7 +1572,7 @@ static uint64_t copy_step_rev(CPUARMState *env, uint64_t toaddr, * Easy case: just memmove the host memory. Note that wmem and * rmem here point to the *last* byte to copy. */ - memmove(wmem - (copysize - 1), rmem - (copysize - 1), copysize); + memmove_ra(wmem - (copysize - 1), rmem - (copysize - 1), copysize, ra); return copysize; } diff --git a/tests/tcg/multiarch/memset-fault.c b/tests/tcg/multiarch/memset-fault.c new file mode 100644 index 0000000000..0e8258a88f --- /dev/null +++ b/tests/tcg/multiarch/memset-fault.c @@ -0,0 +1,77 @@ +#include <stdlib.h> +#include <signal.h> +#include <unistd.h> +#include <sys/mman.h> +#include <assert.h> + +#if defined(__powerpc64__) +/* Needed for PT_* constants */ +#include <asm/ptrace.h> +#endif + +static void *ptr; +static void *pc; + +static void test(void) +{ +#ifdef __aarch64__ + void *t; + asm("adr %0,1f; str %0,%1; 1: dc zva,%2" + : "=&r"(t), "=m"(pc) : "r"(ptr)); +#elif defined(__powerpc64__) + void *t; + asm("bl 0f; 0: mflr %0; addi %0,%0,1f-0b; std %0,%1; 1: dcbz 0,%2" + : "=&r"(t), "=m"(pc) : "r"(ptr) : "lr"); +#elif defined(__s390x__) + void *t; + asm("larl %0,1f; stg %0,%1; 1: xc 0(256,%2),0(%2)" + : "=&r"(t), "=m"(pc) : "r"(ptr)); +#else + *(int *)ptr = 0; +#endif +} + +static void *host_signal_pc(ucontext_t *uc) +{ +#ifdef __aarch64__ + return (void *)uc->uc_mcontext.pc; +#elif defined(__powerpc64__) + return (void *)uc->uc_mcontext.gp_regs[PT_NIP]; +#elif defined(__s390x__) + return (void *)uc->uc_mcontext.psw.addr; +#else + return NULL; +#endif +} + +static void sigsegv(int sig, siginfo_t *info, void *uc) +{ + assert(info->si_addr == ptr); + assert(host_signal_pc(uc) == pc); + exit(0); +} + +int main(void) +{ + static const struct sigaction sa = { + .sa_flags = SA_SIGINFO, + .sa_sigaction = sigsegv + }; + size_t size; + int r; + + size = getpagesize(); + ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, + MAP_ANON | MAP_PRIVATE, -1, 0); + assert(ptr != MAP_FAILED); + + test(); + + r = sigaction(SIGSEGV, &sa, NULL); + assert(r == 0); + r = mprotect(ptr, size, PROT_NONE); + assert(r == 0); + + test(); + abort(); +}
Without this, qemu user will not unwind from the SIGSEGV properly and die with qemu-aarch64: QEMU internal SIGSEGV {code=ACCERR, addr=0x7d1b36ec2000} Segmentation fault Fill in the test case for ppc and s390x, which also use memset from within a helper (but don't currently crash fwiw). Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/tcg/helper-a64.c | 10 ++-- tests/tcg/multiarch/memset-fault.c | 77 ++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 tests/tcg/multiarch/memset-fault.c