From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:35278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R02RH-00059o-Uy for qemu-devel@nongnu.org; Sat, 03 Sep 2011 22:28:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R02RG-0004WE-Cc for qemu-devel@nongnu.org; Sat, 03 Sep 2011 22:28:03 -0400 Received: from mail-pz0-f42.google.com ([209.85.210.42]:52270) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R02RG-0004W9-7r for qemu-devel@nongnu.org; Sat, 03 Sep 2011 22:28:02 -0400 Received: by pzk37 with SMTP id 37so7725510pzk.29 for ; Sat, 03 Sep 2011 19:28:00 -0700 (PDT) Sender: Richard Henderson Message-ID: <4E62E214.4080400@twiddle.net> Date: Sun, 04 Sep 2011 07:57:32 +0530 From: Richard Henderson MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] TCG sar UB (fwd) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: malc Cc: qemu-devel@nongnu.org On 09/03/2011 03:47 PM, malc wrote: > Doesn't make much sense to me, guest clearly asked for 0 and not -1, > besides -1 violates TCG's sar constraints and PPC obliges by emiting > illegal instruction in this case. The shift that the guest asked for was completely folded away. The -1 comes from gen_shift_rm_T1 in the computation of the new flags value. This could instead be moved inside the test for != 0, which is the only place that value is actually used anyway. Try this. Lightly tested. r~ diff --git a/target-i386/translate.c b/target-i386/translate.c index ccef381..b966762 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -1406,70 +1406,84 @@ static void gen_shift_rm_T1(DisasContext *s, int ot, int op1, { target_ulong mask; int shift_label; - TCGv t0, t1; + TCGv t0, t1, t2; - if (ot == OT_QUAD) + if (ot == OT_QUAD) { mask = 0x3f; - else + } else { mask = 0x1f; + } /* load */ - if (op1 == OR_TMP0) + if (op1 == OR_TMP0) { gen_op_ld_T0_A0(ot + s->mem_index); - else + } else { gen_op_mov_TN_reg(ot, 0, op1); + } - tcg_gen_andi_tl(cpu_T[1], cpu_T[1], mask); + t0 = tcg_temp_local_new(); + t1 = tcg_temp_local_new(); + t2 = tcg_temp_local_new(); - tcg_gen_addi_tl(cpu_tmp5, cpu_T[1], -1); + tcg_gen_andi_tl(t2, cpu_T[1], mask); if (is_right) { if (is_arith) { gen_exts(ot, cpu_T[0]); - tcg_gen_sar_tl(cpu_T3, cpu_T[0], cpu_tmp5); - tcg_gen_sar_tl(cpu_T[0], cpu_T[0], cpu_T[1]); + tcg_gen_mov_tl(t0, cpu_T[0]); + tcg_gen_sar_tl(cpu_T[0], cpu_T[0], t2); } else { gen_extu(ot, cpu_T[0]); - tcg_gen_shr_tl(cpu_T3, cpu_T[0], cpu_tmp5); - tcg_gen_shr_tl(cpu_T[0], cpu_T[0], cpu_T[1]); + tcg_gen_mov_tl(t0, cpu_T[0]); + tcg_gen_shr_tl(cpu_T[0], cpu_T[0], t2); } } else { - tcg_gen_shl_tl(cpu_T3, cpu_T[0], cpu_tmp5); - tcg_gen_shl_tl(cpu_T[0], cpu_T[0], cpu_T[1]); + tcg_gen_mov_tl(t0, cpu_T[0]); + tcg_gen_shl_tl(cpu_T[0], cpu_T[0], t2); } /* store */ - if (op1 == OR_TMP0) + if (op1 == OR_TMP0) { gen_op_st_T0_A0(ot + s->mem_index); - else + } else { gen_op_mov_reg_T0(ot, op1); - + } + /* update eflags if non zero shift */ - if (s->cc_op != CC_OP_DYNAMIC) + if (s->cc_op != CC_OP_DYNAMIC) { gen_op_set_cc_op(s->cc_op); + } - /* XXX: inefficient */ - t0 = tcg_temp_local_new(); - t1 = tcg_temp_local_new(); - - tcg_gen_mov_tl(t0, cpu_T[0]); - tcg_gen_mov_tl(t1, cpu_T3); + tcg_gen_mov_tl(t1, cpu_T[0]); shift_label = gen_new_label(); - tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, shift_label); + tcg_gen_brcondi_tl(TCG_COND_EQ, t2, 0, shift_label); - tcg_gen_mov_tl(cpu_cc_src, t1); - tcg_gen_mov_tl(cpu_cc_dst, t0); - if (is_right) + tcg_gen_addi_tl(t2, t2, -1); + tcg_gen_mov_tl(cpu_cc_dst, t1); + + if (is_right) { + if (is_arith) { + tcg_gen_sar_tl(cpu_cc_src, t0, t2); + } else { + tcg_gen_shr_tl(cpu_cc_src, t0, t2); + } + } else { + tcg_gen_shl_tl(cpu_cc_src, t0, t2); + } + + if (is_right) { tcg_gen_movi_i32(cpu_cc_op, CC_OP_SARB + ot); - else + } else { tcg_gen_movi_i32(cpu_cc_op, CC_OP_SHLB + ot); - + } + gen_set_label(shift_label); s->cc_op = CC_OP_DYNAMIC; /* cannot predict flags after */ tcg_temp_free(t0); tcg_temp_free(t1); + tcg_temp_free(t2); } static void gen_shift_rm_im(DisasContext *s, int ot, int op1, int op2,