[Qemu-devel] [PATCH] Avoid qemu SIGFPE for MIPS DIV

[Qemu-devel] [PATCH] Avoid qemu SIGFPE for MIPS DIV

[Richard Sandiford]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

Performing (-1 << 31) / -1 with a MIPS DIV instruction currently
causes the emulator to exit with a SIGFPE.  This wasn't a problem
before TCGification because we used 64-bit division instead of
32-bit division.  So I suppose there are two obvious fixes:
use 64-bit division once more, or add overflow checks in the
same way as we do for DDIV.  I've attached patches for both
approaches below.

BTW, sorry for the flurry of patches.  I tried to break things
up as much as possible, but it makes the situation look far
worse than it actually is...

Richard



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: div-overflow-fix.patch --]
[-- Type: text/x-diff, Size: 2581 bytes --]

Index: qemu/target-mips/translate.c
===================================================================
--- qemu.orig/target-mips/translate.c	2008-05-25 10:19:32.000000000 +0100
+++ qemu/target-mips/translate.c	2008-05-25 10:19:48.000000000 +0100
@@ -1985,21 +1985,38 @@ static void gen_muldiv (DisasContext *ct
 
             tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, l1);
             {
-                TCGv r_tmp1 = new_tmp();
-                TCGv r_tmp2 = new_tmp();
-                TCGv r_tmp3 = new_tmp();
+                int l2 = gen_new_label();
 
-                tcg_gen_trunc_tl_i32(r_tmp1, cpu_T[0]);
-                tcg_gen_trunc_tl_i32(r_tmp2, cpu_T[1]);
-                tcg_gen_div_i32(r_tmp3, r_tmp1, r_tmp2);
-                tcg_gen_rem_i32(r_tmp1, r_tmp1, r_tmp2);
-                tcg_gen_ext_i32_tl(cpu_T[0], r_tmp3);
-                tcg_gen_ext_i32_tl(cpu_T[1], r_tmp1);
-                gen_store_LO(cpu_T[0], 0);
-                gen_store_HI(cpu_T[1], 0);
-                dead_tmp(r_tmp1);
-                dead_tmp(r_tmp2);
-                dead_tmp(r_tmp3);
+                /* The handling of non-sign-extended values is unpredictable,
+                   but it still shouldn't trigger a SIGFPE in the emulator.  */
+                tcg_gen_ext32s_tl(cpu_T[0], cpu_T[0]);
+                tcg_gen_ext32s_tl(cpu_T[1], cpu_T[1]);
+                tcg_gen_brcondi_tl(TCG_COND_NE, cpu_T[0], -1 << 31, l2);
+                tcg_gen_brcondi_tl(TCG_COND_NE, cpu_T[1], -1, l2);
+                {
+                    tcg_gen_movi_tl(cpu_T[1], 0);
+                    gen_store_LO(cpu_T[0], 0);
+                    gen_store_HI(cpu_T[1], 0);
+                    tcg_gen_br(l1);
+                }
+                gen_set_label(l2);
+                {
+                    TCGv r_tmp1 = new_tmp();
+                    TCGv r_tmp2 = new_tmp();
+                    TCGv r_tmp3 = new_tmp();
+
+                    tcg_gen_trunc_tl_i32(r_tmp1, cpu_T[0]);
+                    tcg_gen_trunc_tl_i32(r_tmp2, cpu_T[1]);
+                    tcg_gen_div_i32(r_tmp3, r_tmp1, r_tmp2);
+                    tcg_gen_rem_i32(r_tmp1, r_tmp1, r_tmp2);
+                    tcg_gen_ext_i32_tl(cpu_T[0], r_tmp3);
+                    tcg_gen_ext_i32_tl(cpu_T[1], r_tmp1);
+                    gen_store_LO(cpu_T[0], 0);
+                    gen_store_HI(cpu_T[1], 0);
+                    dead_tmp(r_tmp1);
+                    dead_tmp(r_tmp2);
+                    dead_tmp(r_tmp3);
+                }
             }
             gen_set_label(l1);
         }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: div-overflow-fix-2.patch --]
[-- Type: text/x-diff, Size: 1653 bytes --]

Index: qemu/target-mips/translate.c
===================================================================
--- qemu.orig/target-mips/translate.c	2008-05-25 10:14:53.000000000 +0100
+++ qemu/target-mips/translate.c	2008-05-25 10:17:24.000000000 +0100
@@ -1985,21 +1985,17 @@ static void gen_muldiv (DisasContext *ct
 
             tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, l1);
             {
-                TCGv r_tmp1 = new_tmp();
-                TCGv r_tmp2 = new_tmp();
-                TCGv r_tmp3 = new_tmp();
+                TCGv r_tmp1 = tcg_temp_new(TCG_TYPE_I64);
+                TCGv r_tmp2 = tcg_temp_new(TCG_TYPE_I64);
 
-                tcg_gen_trunc_tl_i32(r_tmp1, cpu_T[0]);
-                tcg_gen_trunc_tl_i32(r_tmp2, cpu_T[1]);
-                tcg_gen_div_i32(r_tmp3, r_tmp1, r_tmp2);
-                tcg_gen_rem_i32(r_tmp1, r_tmp1, r_tmp2);
-                tcg_gen_ext_i32_tl(cpu_T[0], r_tmp3);
-                tcg_gen_ext_i32_tl(cpu_T[1], r_tmp1);
-                gen_store_LO(cpu_T[0], 0);
-                gen_store_HI(cpu_T[1], 0);
-                dead_tmp(r_tmp1);
-                dead_tmp(r_tmp2);
-                dead_tmp(r_tmp3);
+                tcg_gen_ext32s_tl(cpu_T[0], cpu_T[0]);
+                tcg_gen_ext32s_tl(cpu_T[1], cpu_T[1]);
+                tcg_gen_div_i64(r_tmp1, cpu_T[0], cpu_T[1]);
+                tcg_gen_rem_i64(r_tmp2, cpu_T[0], cpu_T[1]);
+                tcg_gen_ext32s_tl(r_tmp1, r_tmp1);
+                tcg_gen_ext32s_tl(r_tmp2, r_tmp2);
+                gen_store_LO(r_tmp1, 0);
+                gen_store_HI(r_tmp2, 0);
             }
             gen_set_label(l1);
         }