[v1] target/i386: Fix caculation of LOCK NEG eflags

[PATCH] target/i386: Fix caculation of LOCK NEG eflags

Qi Hu posted 1 patch 1 year, 6 months ago

Diff against v2
Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221021131744.337567-1-huqi@loongson.cn

Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>

There is a newer version of this series

target/i386/tcg/translate.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

Expand all Fold all

[PATCH] target/i386: Fix caculation of LOCK NEG eflags

Posted by Qi Hu 1 year, 6 months ago

In sequence:
---
lock negl -0x14(%rbp)
pushf
pop    %rax
---

%rax will obtain the wrong value becasue "lock neg" caculates the
wrong eflags. The "s->T0" is updated by the wrong value.

You can use this to do some test:
---
#include <assert.h>

int main()
{
  __volatile__ unsigned test = 0x2363a;
  __volatile__ char cond = 0;
  asm(
      "lock negl %0 \n\t"
      "sets %1"
      : "=m"(test), "=r"(cond)
      :
      :);
  assert(cond & 1);
}
---

Reported-by: Jinyang Shen <shenjinyang@loongson.cn>
Co-Developed-by: Xuehai Chen <chenxuehai@loongson.cn>
Signed-off-by: Xuehai Chen <chenxuehai@loongson.cn>
Signed-off-by: Qi Hu <huqi@loongson.cn>
---
 target/i386/tcg/translate.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index e19d5c1c64..e72c32827a 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -3282,24 +3282,25 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
                 }
                 a0 = tcg_temp_local_new();
                 t0 = tcg_temp_local_new();
+                t1 = tcg_temp_local_new();
                 label1 = gen_new_label();
 
                 tcg_gen_mov_tl(a0, s->A0);
                 tcg_gen_mov_tl(t0, s->T0);
 
                 gen_set_label(label1);
-                t1 = tcg_temp_new();
                 t2 = tcg_temp_new();
                 tcg_gen_mov_tl(t2, t0);
                 tcg_gen_neg_tl(t1, t0);
                 tcg_gen_atomic_cmpxchg_tl(t0, a0, t0, t1,
                                           s->mem_index, ot | MO_LE);
-                tcg_temp_free(t1);
                 tcg_gen_brcond_tl(TCG_COND_NE, t0, t2, label1);
 
                 tcg_temp_free(t2);
+
+                tcg_gen_mov_tl(s->T0, t1);
+                tcg_temp_free(t1);
                 tcg_temp_free(a0);
-                tcg_gen_mov_tl(s->T0, t0);
                 tcg_temp_free(t0);
             } else {
                 tcg_gen_neg_tl(s->T0, s->T0);
-- 
2.38.0

Re: [PATCH] target/i386: Fix caculation of LOCK NEG eflags

Posted by Richard Henderson 1 year, 6 months ago

On 10/21/22 23:17, Qi Hu wrote:
>                   t0 = tcg_temp_local_new();
> +                t1 = tcg_temp_local_new();
>                   label1 = gen_new_label();
>   
>                   tcg_gen_mov_tl(a0, s->A0);
>                   tcg_gen_mov_tl(t0, s->T0);
>   
>                   gen_set_label(label1);
> -                t1 = tcg_temp_new();
>                   t2 = tcg_temp_new();
>                   tcg_gen_mov_tl(t2, t0);
>                   tcg_gen_neg_tl(t1, t0);
>                   tcg_gen_atomic_cmpxchg_tl(t0, a0, t0, t1,
>                                             s->mem_index, ot | MO_LE);
> -                tcg_temp_free(t1);
>                   tcg_gen_brcond_tl(TCG_COND_NE, t0, t2, label1);
>   
>                   tcg_temp_free(t2);
> +
> +                tcg_gen_mov_tl(s->T0, t1);
> +                tcg_temp_free(t1);
>                   tcg_temp_free(a0);
> -                tcg_gen_mov_tl(s->T0, t0);

Instead of creating another temporary stored across the branch,
simply replace this final mov with a neg.


r~