[PATCH] target/i386: raise #DE for INT_MIN / -1 in IDIV helpers

Jiang Yuxuan posted 1 patch 1 week ago
target/i386/tcg/int_helper.c        |  4 +-
tests/tcg/i386/meson.build          |  1 +
tests/tcg/i386/test-idiv-overflow.c | 70 +++++++++++++++++++++++++++++
tests/tcg/x86_64/meson.build        |  1 +
4 files changed, 74 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/i386/test-idiv-overflow.c
[PATCH] target/i386: raise #DE for INT_MIN / -1 in IDIV helpers
Posted by Jiang Yuxuan 1 week ago
helper_idivw_AX() and helper_idivl_EAX() perform the division before
checking whether the quotient fits in the destination.  If the dividend
is the minimum value and the divisor is -1, the division overflows in
the host: INT32_MIN / -1 and INT64_MIN / -1 are undefined behaviour in C
and trap with SIGFPE on x86 hosts.  QEMU then dies of a host SIGFPE
instead of raising #DE in the guest, taking down the emulator (or the
whole VM under TCG).

Reject the two overflowing cases before dividing.  The 8-bit form is not
affected (it widens the dividend to int), and the 64-bit form already
checks first.

Add a regression test for both the 16-bit and 32-bit forms, run by the
i386 and x86_64 linux-user test suites.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4687
Signed-off-by: fujo <hooyan2024@gmail.com>
---
 target/i386/tcg/int_helper.c        |  4 +-
 tests/tcg/i386/meson.build          |  1 +
 tests/tcg/i386/test-idiv-overflow.c | 70 +++++++++++++++++++++++++++++
 tests/tcg/x86_64/meson.build        |  1 +
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/i386/test-idiv-overflow.c

diff --git a/target/i386/tcg/int_helper.c b/target/i386/tcg/int_helper.c
index 46741d9..45fa0a8 100644
--- a/target/i386/tcg/int_helper.c
+++ b/target/i386/tcg/int_helper.c
@@ -91,7 +91,7 @@ void helper_idivw_AX(CPUX86State *env, target_ulong t0)

     num = (env->regs[R_EAX] & 0xffff) | ((env->regs[R_EDX] & 0xffff) <<
16);
     den = (int16_t)t0;
-    if (den == 0) {
+    if (den == 0 || (num == INT32_MIN && den == -1)) {
         raise_exception_ra(env, EXCP00_DIVZ, GETPC());
     }
     q = (num / den);
@@ -130,7 +130,7 @@ void helper_idivl_EAX(CPUX86State *env, target_ulong t0)

     num = ((uint32_t)env->regs[R_EAX]) |
((uint64_t)((uint32_t)env->regs[R_EDX]) << 32);
     den = t0;
-    if (den == 0) {
+    if (den == 0 || (num == INT64_MIN && den == -1)) {
         raise_exception_ra(env, EXCP00_DIVZ, GETPC());
     }
     q = (num / den);
diff --git a/tests/tcg/i386/meson.build b/tests/tcg/i386/meson.build
index 84bd680..5d88858 100644
--- a/tests/tcg/i386/meson.build
+++ b/tests/tcg/i386/meson.build
@@ -38,6 +38,7 @@ tests += {
     'cflags': [files('test-i386-code16.S'), files('test-i386-vm86.S'),
                '-lm', '-fno-pie', '-Wl,--no-warn-rwx-segments'],
   },
+  'test-idiv-overflow.c': {},
   '../multiarch/sha512.c': {
      'exe_name': 'sha512-sse',
      'cflags': ['-msse4.1', '-O3'],
diff --git a/tests/tcg/i386/test-idiv-overflow.c
b/tests/tcg/i386/test-idiv-overflow.c
new file mode 100644
index 0000000..e8fc55a
--- /dev/null
+++ b/tests/tcg/i386/test-idiv-overflow.c
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Test that IDIV of the minimum dividend by -1 raises #DE, which Linux
+ * delivers to the guest as SIGFPE with si_code == FPE_INTDIV.
+ */
+#define _GNU_SOURCE
+#include <setjmp.h>
+#include <signal.h>
+#include <stdio.h>
+#include <string.h>
+
+static sigjmp_buf idiv_jmp;
+static volatile sig_atomic_t idiv_si_code;
+
+static void idiv_sig_handler(int sig, siginfo_t *info, void *puc)
+{
+    (void)sig;
+    (void)puc;
+    idiv_si_code = info->si_code;
+    siglongjmp(idiv_jmp, 1);
+}
+
+static int check_idiv_overflow(int width)
+{
+    struct sigaction sa, old;
+    int ret = 1;
+
+    memset(&sa, 0, sizeof(sa));
+    sa.sa_sigaction = idiv_sig_handler;
+    sa.sa_flags = SA_SIGINFO | SA_NODEFER;
+    sigemptyset(&sa.sa_mask);
+    sigaction(SIGFPE, &sa, &old);
+
+    idiv_si_code = 0;
+    if (sigsetjmp(idiv_jmp, 1) == 0) {
+        if (width == 16) {
+            /* DX:AX = 0x8000:0x0000, divisor -1 */
+            __asm__ volatile ("movl $0x8000, %%edx\n\t"
+                              "xorl %%eax, %%eax\n\t"
+                              "movl $-1, %%ecx\n\t"
+                              "idivw %%cx"
+                              : : : "eax", "ecx", "edx", "cc");
+        } else {
+            /* EDX:EAX = 0x80000000:0x00000000, divisor -1 */
+            __asm__ volatile ("movl $0x80000000, %%edx\n\t"
+                              "xorl %%eax, %%eax\n\t"
+                              "movl $-1, %%ecx\n\t"
+                              "idivl %%ecx"
+                              : : : "eax", "ecx", "edx", "cc");
+        }
+        printf("FAIL: idiv%d did not raise #DE\n", width);
+    } else if (idiv_si_code != FPE_INTDIV) {
+        printf("FAIL: idiv%d raised SIGFPE with si_code %d\n",
+               width, (int)idiv_si_code);
+    } else {
+        ret = 0;
+    }
+
+    sigaction(SIGFPE, &old, NULL);
+    return ret;
+}
+
+int main(void)
+{
+    int ret = 0;
+
+    ret |= check_idiv_overflow(16);
+    ret |= check_idiv_overflow(32);
+    return ret;
+}
diff --git a/tests/tcg/x86_64/meson.build b/tests/tcg/x86_64/meson.build
index 95c7ee6..4106d4d 100644
--- a/tests/tcg/x86_64/meson.build
+++ b/tests/tcg/x86_64/meson.build
@@ -52,6 +52,7 @@ tests += {
   },
   '../i386/test-i386-adcox.c': {'cflags': ['-O2']},
   '../i386/test-i386-bmi2.c': {'cflags': ['-O2', '-fwrapv']},
+  '../i386/test-idiv-overflow.c': {},
 }

 tests += {
-- 
2.53.0