| Message ID | 20221022061216.423098-1-huqi@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] target/i386: Fix caculation of LOCK NEG eflags | expand |
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);
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(-)
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(-) >
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);
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(-)