From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K0D9D-0000lh-8U for qemu-devel@nongnu.org; Sun, 25 May 2008 06:08:16 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K0D9A-0000lA-1M for qemu-devel@nongnu.org; Sun, 25 May 2008 06:08:12 -0400 Received: from [199.232.76.173] (port=60122 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K0D97-0000km-7e for qemu-devel@nongnu.org; Sun, 25 May 2008 06:08:10 -0400 Received: from nf-out-0910.google.com ([64.233.182.189]:50104) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1K0D96-0003f2-Nb for qemu-devel@nongnu.org; Sun, 25 May 2008 06:08:09 -0400 Received: by nf-out-0910.google.com with SMTP id b2so770767nfb.12 for ; Sun, 25 May 2008 03:08:07 -0700 (PDT) From: Richard Sandiford Date: Sun, 25 May 2008 11:08:00 +0100 Message-ID: <87fxs666in.fsf@firetop.home> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Subject: [Qemu-devel] [PATCH] Avoid qemu SIGFPE for MIPS DIV Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org --=-=-= 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 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=div-overflow-fix.patch 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); } --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=div-overflow-fix-2.patch 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); } --=-=-=--