[PATCH v3 0/2] SIGSEGV fixes

[PATCH v3 0/2] SIGSEGV fixes

[Taylor Simpson]

The Hexagon target was silently failing the SIGSEGV test because
the signal handler was not called.

Patch 1/2 fixes the Hexagon target
Patch 2/2 adds a check that the signal handler is called

**** Changes in v2 ****
Address feedback from Richard Henderson <richard.henderson@linaro.org>
- Replace put_user_* with cpu_st*_data_ra
- Replace get_user_* with cpu_ld*_data_ra
- Treat sig_segv_called as a counter

**** Changes in v3 ****
Address feedback from Richard Henderson <richard.henderson@linaro.org>
- Pass host PC as return address for cpu_st_*_data_ra/cpu_ld*_data_ra


Taylor Simpson (2):
  Hexagon (target/hexagon) remove put_user_*/get_user_*
  linux-test (tests/tcg/multiarch/linux-test.c) add check

 target/hexagon/op_helper.c       | 39 ++++++++++++++++++---------------------
 tests/tcg/multiarch/linux-test.c |  8 ++++++++
 2 files changed, 26 insertions(+), 21 deletions(-)

-- 
2.7.4

[PATCH v3 1/2] Hexagon (target/hexagon) remove put_user_*/get_user_*

[Taylor Simpson]

Replace put_user_* with cpu_st*_data_ra
Replace get_user_* with cpu_ld*_data_ra

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/op_helper.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 4595559..a959dba 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -17,6 +17,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu.h"
+#include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
 #include "fpu/softfloat.h"
 #include "cpu.h"
@@ -140,22 +141,22 @@ void HELPER(debug_check_store_width)(CPUHexagonState *env, int slot, int check)
 
 void HELPER(commit_store)(CPUHexagonState *env, int slot_num)
 {
-    switch (env->mem_log_stores[slot_num].width) {
+    uintptr_t ra = GETPC();
+    uint8_t width = env->mem_log_stores[slot_num].width;
+    target_ulong va = env->mem_log_stores[slot_num].va;
+
+    switch (width) {
     case 1:
-        put_user_u8(env->mem_log_stores[slot_num].data32,
-                    env->mem_log_stores[slot_num].va);
+        cpu_stb_data_ra(env, va, env->mem_log_stores[slot_num].data32, ra);
         break;
     case 2:
-        put_user_u16(env->mem_log_stores[slot_num].data32,
-                     env->mem_log_stores[slot_num].va);
+        cpu_stw_data_ra(env, va, env->mem_log_stores[slot_num].data32, ra);
         break;
     case 4:
-        put_user_u32(env->mem_log_stores[slot_num].data32,
-                     env->mem_log_stores[slot_num].va);
+        cpu_stl_data_ra(env, va, env->mem_log_stores[slot_num].data32, ra);
         break;
     case 8:
-        put_user_u64(env->mem_log_stores[slot_num].data64,
-                     env->mem_log_stores[slot_num].va);
+        cpu_stq_data_ra(env, va, env->mem_log_stores[slot_num].data64, ra);
         break;
     default:
         g_assert_not_reached();
@@ -393,37 +394,33 @@ static void check_noshuf(CPUHexagonState *env, uint32_t slot)
 static uint8_t mem_load1(CPUHexagonState *env, uint32_t slot,
                          target_ulong vaddr)
 {
-    uint8_t retval;
+    uintptr_t ra = GETPC();
     check_noshuf(env, slot);
-    get_user_u8(retval, vaddr);
-    return retval;
+    return cpu_ldub_data_ra(env, vaddr, ra);
 }
 
 static uint16_t mem_load2(CPUHexagonState *env, uint32_t slot,
                           target_ulong vaddr)
 {
-    uint16_t retval;
+    uintptr_t ra = GETPC();
     check_noshuf(env, slot);
-    get_user_u16(retval, vaddr);
-    return retval;
+    return cpu_lduw_data_ra(env, vaddr, ra);
 }
 
 static uint32_t mem_load4(CPUHexagonState *env, uint32_t slot,
                           target_ulong vaddr)
 {
-    uint32_t retval;
+    uintptr_t ra = GETPC();
     check_noshuf(env, slot);
-    get_user_u32(retval, vaddr);
-    return retval;
+    return cpu_ldl_data_ra(env, vaddr, ra);
 }
 
 static uint64_t mem_load8(CPUHexagonState *env, uint32_t slot,
                           target_ulong vaddr)
 {
-    uint64_t retval;
+    uintptr_t ra = GETPC();
     check_noshuf(env, slot);
-    get_user_u64(retval, vaddr);
-    return retval;
+    return cpu_ldq_data_ra(env, vaddr, ra);
 }
 
 /* Floating point */
-- 
2.7.4

[PATCH v3 2/2] linux-test (tests/tcg/multiarch/linux-test.c) add check

[Taylor Simpson]

Add a check that the SIGSEGV handler is called

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 tests/tcg/multiarch/linux-test.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
index c8c6aed..e2d88f8 100644
--- a/tests/tcg/multiarch/linux-test.c
+++ b/tests/tcg/multiarch/linux-test.c
@@ -439,10 +439,14 @@ static void sig_alarm(int sig)
     alarm_count++;
 }
 
+/* Count the number of times handler is called */
+static int sig_segv_called;
+
 static void sig_segv(int sig, siginfo_t *info, void *puc)
 {
     if (sig != SIGSEGV)
         error("signal");
+    sig_segv_called++;
     longjmp(jmp_env, 1);
 }
 
@@ -492,6 +496,10 @@ static void test_signal(void)
         *(volatile uint8_t *)0 = 0;
     }
 
+    if (sig_segv_called == 0) {
+        error("SIGSEGV handler not called");
+    }
+
     act.sa_handler = SIG_DFL;
     sigemptyset(&act.sa_mask);
     act.sa_flags = 0;
-- 
2.7.4

Re: [PATCH v3 1/2] Hexagon (target/hexagon) remove put_user_*/get_user_*

[Richard Henderson]

On 7/15/21 2:22 PM, Taylor Simpson wrote:
> Replace put_user_* with cpu_st*_data_ra
> Replace get_user_* with cpu_ld*_data_ra
> 
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> Signed-off-by: Taylor Simpson<tsimpson@quicinc.com>
> ---
>   target/hexagon/op_helper.c | 39 ++++++++++++++++++---------------------
>   1 file changed, 18 insertions(+), 21 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~