diff mbox series

[v2,1/2] linux-user/hexagon: fix signal context save & restore

Message ID 20221229092006.10709-2-quic_mthiyaga@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Hexagon: fix signal context save & restore bug | expand

Commit Message

Mukilan Thiyagarajan (QUIC) Dec. 29, 2022, 9:20 a.m. UTC 
This patch fixes the issue originally reported in
this thread:

https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html

The root cause of the issue is a bug in the hexagon specific
logic for saving & restoring context during signal delivery.
The CPU state has two different representations for the
predicate registers. The current logic saves & restores only
the aliased HEX_REG_P3_O register, which is part of env->gpr[]
field in the CPU state, but not the individual byte-level
predicate registers (pO, p1, p2, p3) backed by env->pred[].

Since all predicated instructions refer only to the
indiviual registers, switching to and back from a signal handler
can clobber these registers if the signal handler writes to them
causing the normal application code to behave unpredictably when
context is restored.

In the reported issue with the 'signals' test, since the updated
hexagon toolchain had built musl with -O2, the functions called
from non_trivial_free were inlined. This meant that the code
emitted reused predicate P0 computed in the entry translation
block of the function non_trivial_free in one of the child TB
as part of an assertion. Since P0 is clobbered by the signal
handler in the signals test, the assertion in non_trivial_free
fails incorectly. Since musl for hexagon implements the 'abort'
function by deliberately writing to memory via null pointer,
this causes the test to fail with segmentation fault.

This patch modifies the signal context save & restore logic
to include the individual p0, p1, p2, p3 and excludes the
32b p3_0 register since its value is derived from the former
registers. It also adds a new test case that reliabily
reproduces the issue for all four predicate registers.

Buglink: https://github.com/quic/toolchain_for_hexagon/issues/6
Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
---
 linux-user/hexagon/signal.c        | 17 +++---
 tests/tcg/hexagon/Makefile.target  |  1 +
 tests/tcg/hexagon/signal_context.c | 84 ++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 6 deletions(-)
 create mode 100644 tests/tcg/hexagon/signal_context.c
diff mbox series

Patch

diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c
index ad4e3822d5..60fa7e1bce 100644
--- a/linux-user/hexagon/signal.c
+++ b/linux-user/hexagon/signal.c
@@ -39,15 +39,12 @@  struct target_sigcontext {
     target_ulong m0;
     target_ulong m1;
     target_ulong usr;
-    target_ulong p3_0;
     target_ulong gp;
     target_ulong ugp;
     target_ulong pc;
     target_ulong cause;
     target_ulong badva;
-    target_ulong pad1;
-    target_ulong pad2;
-    target_ulong pad3;
+    target_ulong pred[NUM_PREGS];
 };
 
 struct target_ucontext {
@@ -118,10 +115,14 @@  static void setup_sigcontext(struct target_sigcontext *sc, CPUHexagonState *env)
     __put_user(env->gpr[HEX_REG_M0], &sc->m0);
     __put_user(env->gpr[HEX_REG_M1], &sc->m1);
     __put_user(env->gpr[HEX_REG_USR], &sc->usr);
-    __put_user(env->gpr[HEX_REG_P3_0], &sc->p3_0);
     __put_user(env->gpr[HEX_REG_GP], &sc->gp);
     __put_user(env->gpr[HEX_REG_UGP], &sc->ugp);
     __put_user(env->gpr[HEX_REG_PC], &sc->pc);
+
+    int i;
+    for (i = 0; i < NUM_PREGS; i++) {
+        __put_user(env->pred[i], &(sc->pred[i]));
+    }
 }
 
 static void setup_ucontext(struct target_ucontext *uc,
@@ -230,10 +231,14 @@  static void restore_sigcontext(CPUHexagonState *env,
     __get_user(env->gpr[HEX_REG_M0], &sc->m0);
     __get_user(env->gpr[HEX_REG_M1], &sc->m1);
     __get_user(env->gpr[HEX_REG_USR], &sc->usr);
-    __get_user(env->gpr[HEX_REG_P3_0], &sc->p3_0);
     __get_user(env->gpr[HEX_REG_GP], &sc->gp);
     __get_user(env->gpr[HEX_REG_UGP], &sc->ugp);
     __get_user(env->gpr[HEX_REG_PC], &sc->pc);
+
+    int i;
+    for (i = 0; i < NUM_PREGS; i++) {
+        __get_user(env->pred[i], &(sc->pred[i]));
+    }
 }
 
 static void restore_ucontext(CPUHexagonState *env, struct target_ucontext *uc)
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index 9ee1faa1e1..f1378d86a9 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -43,6 +43,7 @@  HEX_TESTS += load_align
 HEX_TESTS += atomics
 HEX_TESTS += fpstuff
 HEX_TESTS += overflow
+HEX_TESTS += signal_context
 
 HEX_TESTS += test_abs
 HEX_TESTS += test_bitcnt
diff --git a/tests/tcg/hexagon/signal_context.c b/tests/tcg/hexagon/signal_context.c
new file mode 100644
index 0000000000..7202fa64b6
--- /dev/null
+++ b/tests/tcg/hexagon/signal_context.c
@@ -0,0 +1,84 @@ 
+/*
+ *  Copyright(c) 2022 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdio.h>
+#include <signal.h>
+#include <time.h>
+
+void sig_user(int sig, siginfo_t *info, void *puc)
+{
+    asm("r7 = #0\n\t"
+        "p0 = r7\n\t"
+        "p1 = r7\n\t"
+        "p2 = r7\n\t"
+        "p3 = r7\n\t"
+        : : : "r7", "p0", "p1", "p2", "p3");
+}
+
+int main()
+{
+    int err = 0;
+    unsigned int i = 100000;
+    struct sigaction act;
+    struct itimerspec it;
+    timer_t tid;
+    struct sigevent sev;
+
+    act.sa_sigaction = sig_user;
+    sigemptyset(&act.sa_mask);
+    act.sa_flags = SA_SIGINFO;
+    sigaction(SIGUSR1, &act, NULL);
+    sev.sigev_notify = SIGEV_SIGNAL;
+    sev.sigev_signo = SIGUSR1;
+    sev.sigev_value.sival_ptr = &tid;
+    timer_create(CLOCK_REALTIME, &sev, &tid);
+    it.it_interval.tv_sec = 0;
+    it.it_interval.tv_nsec = 100000;
+    it.it_value.tv_sec = 0;
+    it.it_value.tv_nsec = 100000;
+    timer_settime(tid, 0, &it, NULL);
+
+    asm("loop0(1f, %1)\n\t"
+        "1: r8 = #0xff\n\t"
+        "   p0 = r8\n\t"
+        "   p1 = r8\n\t"
+        "   p2 = r8\n\t"
+        "   p3 = r8\n\t"
+        "   jump 3f\n\t"
+        "2: memb(%0) = #1\n\t"
+        "   jump 4f\n\t"
+        "3:\n\t"
+        "   r8 = p0\n\t"
+        "   p0 = cmp.eq(r8, #0xff)\n\t"
+        "   if (!p0) jump 2b\n\t"
+        "   r8 = p1\n\t"
+        "   p0 = cmp.eq(r8, #0xff)\n\t"
+        "   if (!p0) jump 2b\n\t"
+        "   r8 = p2\n\t"
+        "   p0 = cmp.eq(r8, #0xff)\n\t"
+        "   if (!p0) jump 2b\n\t"
+        "   r8 = p3\n\t"
+        "   p0 = cmp.eq(r8, #0xff)\n\t"
+        "   if (!p0) jump 2b\n\t"
+        "4: {}: endloop0\n\t"
+        :
+        : "r"(&err), "r"(i)
+        : "memory", "r8", "p0", "p1", "p2", "p3");
+
+    puts(err ? "FAIL" : "PASS");
+    return err;
+}