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

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

[Qi Hu]

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

%rax will obtain the wrong value becasue the "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);
  return 0;
}
---

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>
---
V1 -> V2:
Following Richard's suggestion, just change mov to neg instead of using
local_tmp.
---
 target/i386/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index e19d5c1c64..cec2182080 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -3299,7 +3299,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
 
                 tcg_temp_free(t2);
                 tcg_temp_free(a0);
-                tcg_gen_mov_tl(s->T0, t0);
+                tcg_gen_neg_tl(s->T0, t0);
                 tcg_temp_free(t0);
             } else {
                 tcg_gen_neg_tl(s->T0, s->T0);
-- 
2.38.0

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

[Richard Henderson]

On 10/22/22 16:12, Qi Hu wrote:
> In sequence:
> ---
> lock negl -0x14(%rbp)
> pushf
> pop    %rax
> ---
> 
> %rax will obtain the wrong value becasue the "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);
>    return 0;
> }
> ---
> 
> 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>
> ---
> V1 -> V2:
> Following Richard's suggestion, just change mov to neg instead of using
> local_tmp.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> ---
>   target/i386/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index e19d5c1c64..cec2182080 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -3299,7 +3299,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>   
>                   tcg_temp_free(t2);
>                   tcg_temp_free(a0);
> -                tcg_gen_mov_tl(s->T0, t0);
> +                tcg_gen_neg_tl(s->T0, t0);
>                   tcg_temp_free(t0);
>               } else {
>                   tcg_gen_neg_tl(s->T0, s->T0);

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

[Philippe Mathieu-Daudé]

Typo "calculation" in subject.

On 22/10/22 08:12, Qi Hu wrote:
> In sequence:
> ---
> lock negl -0x14(%rbp)
> pushf
> pop    %rax
> ---
> 
> %rax will obtain the wrong value becasue the "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);
>    return 0;
> }
> ---
> 
> 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>
> ---
> V1 -> V2:
> Following Richard's suggestion, just change mov to neg instead of using
> local_tmp.
> ---
>   target/i386/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

[Qi Hu]

On 2022/10/24 05:02, Philippe Mathieu-Daudé wrote:
> Typo "calculation" in subject.

Thanks for the reminder. It's my fault. I will send V3 to fix this typo.


Qi

>
> On 22/10/22 08:12, Qi Hu wrote:
>> In sequence:
>> ---
>> lock negl -0x14(%rbp)
>> pushf
>> pop    %rax
>> ---
>>
>> %rax will obtain the wrong value becasue the "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);
>>    return 0;
>> }
>> ---
>>
>> 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>
>> ---
>> V1 -> V2:
>> Following Richard's suggestion, just change mov to neg instead of using
>> local_tmp.
>> ---
>>   target/i386/tcg/translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>