Re: [Qemu-devel] [PATCH 3/5] target/hppa: fix log conditions

[Richard Henderson]

On 2/11/19 10:19 AM, Sven Schnelle wrote:
>  switch (cf >> 1) {
> -case 4: case 5: case 6:
> -cf &= 1;
> +case 0: /* never */
> +cond = cond_make_f();
> +break;
> +case 1: /* = all bits are zero */
> +cond = cond_make_0(TCG_COND_EQ, res);
> +break;
> +case 2: /* < leftmost bit is 1 */
> +cond = cond_make_0(TCG_COND_LT, res);
> +break;
> +case 3: /* <= leftmost bit is 1 or all bits 0 */
> +cond = cond_make_0(TCG_COND_LE, res);
> +break;
> +case 7: /* OD rightmost bit is 1 */
> +tmp = tcg_temp_new();
> +tcg_gen_andi_reg(tmp, res, 1);
> +cond = cond_make_0(TCG_COND_NE, tmp);
> +tcg_temp_free(tmp);
> +break;
> +default:
>  break;
>  }

You can't do nothing for cases 4,5,6.  That lets a bad guest crash qemu, since
cond will be uninitialized.  Also, this patch has to be sorted before the
previous, as otherwise you introduce a regression during bisection.

I've fixed this up locally.


r~

[Qemu-devel] [PATCH 3/5] target/hppa: fix log conditions

[Sven Schnelle]

Now that do_cond() uses sign overflow for some condition matches we
need to roll our own version without sign overflow checks.

Signed-off-by: Sven Schnelle 
---
 target/hppa/translate.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 0e8cc8117a..bce8773b1a 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -996,12 +996,35 @@ static DisasCond do_sub_cond(unsigned cf, TCGv_reg res,
 
 static DisasCond do_log_cond(unsigned cf, TCGv_reg res)
 {
+DisasCond cond;
+TCGv_reg tmp;
+
 switch (cf >> 1) {
-case 4: case 5: case 6:
-cf &= 1;
+case 0: /* never */
+cond = cond_make_f();
+break;
+case 1: /* = all bits are zero */
+cond = cond_make_0(TCG_COND_EQ, res);
+break;
+case 2: /* < leftmost bit is 1 */
+cond = cond_make_0(TCG_COND_LT, res);
+break;
+case 3: /* <= leftmost bit is 1 or all bits 0 */
+cond = cond_make_0(TCG_COND_LE, res);
+break;
+case 7: /* OD rightmost bit is 1 */
+tmp = tcg_temp_new();
+tcg_gen_andi_reg(tmp, res, 1);
+cond = cond_make_0(TCG_COND_NE, tmp);
+tcg_temp_free(tmp);
+break;
+default:
 break;
 }
-return do_cond(cf, res, res, res);
+if (cf & 1) {
+cond.c = tcg_invert_cond(cond.c);
+}
+return cond;
 }
 
 /* Similar, but for shift/extract/deposit conditions.  */
-- 
2.20.1