On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson <richard.hender...@linaro.org> wrote: > > Create a common helper for pc-relative branches. > The jmp jb insn was missing a mask for CODE32. > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
(Oops, my remark the previous patch should still have pointed to gen_jmp_tb). In gen_jz_ecx_string, in the translation for LOOPNZ/LOOPZ/LOOP/JECXZ and in i386_tr_tb_stop there is: > - gen_jmp_tb(s, s->pc - s->cs_base, 1); > + gen_jmp_rel(s, MO_32, 0, 1); What happens if the instruction's last byte is at 0xffff? Wraparound in the middle of an instruction is generally undefined, but I think it should work if the instruction does not cross the 64K/4G limit (and on real hardware, which obeys segment limits unlike TCG, said limit must be 64K/4G of course). In other words, why MO_32 and not "CODE32(s) ? MO_32 : MO_16"? Likewise, if you change that you need to change gen_repz/gen_repz2 too. Paolo > gen_set_label(l1); > return l2; > } > @@ -2756,6 +2757,18 @@ static void gen_jmp_tb(DisasContext *s, target_ulong > eip, int tb_num) > } > } > > +static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num) > +{ > + target_ulong dest = s->pc - s->cs_base + diff; > + > + if (ot == MO_16) { > + dest &= 0xffff; > + } else if (!CODE64(s)) { > + dest &= 0xffffffff; > + } > + gen_jmp_tb(s, dest, tb_num); > +} > + > static void gen_jmp(DisasContext *s, target_ulong eip) > { > gen_jmp_tb(s, eip, 0); > @@ -6816,20 +6829,12 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > break; > case 0xe8: /* call im */ > { > - if (dflag != MO_16) { > - tval = (int32_t)insn_get(env, s, MO_32); > - } else { > - tval = (int16_t)insn_get(env, s, MO_16); > - } > - tval += s->pc - s->cs_base; > - if (dflag == MO_16) { > - tval &= 0xffff; > - } else if (!CODE64(s)) { > - tval &= 0xffffffff; > - } > + int diff = (dflag != MO_16 > + ? (int32_t)insn_get(env, s, MO_32) > + : (int16_t)insn_get(env, s, MO_16)); > gen_push_v(s, eip_next_tl(s)); > gen_bnd_jmp(s); > - gen_jmp(s, tval); > + gen_jmp_rel(s, dflag, diff, 0); > } > break; > case 0x9a: /* lcall im */ > @@ -6847,19 +6852,13 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > } > goto do_lcall; > case 0xe9: /* jmp im */ > - if (dflag != MO_16) { > - tval = (int32_t)insn_get(env, s, MO_32); > - } else { > - tval = (int16_t)insn_get(env, s, MO_16); > + { > + int diff = (dflag != MO_16 > + ? (int32_t)insn_get(env, s, MO_32) > + : (int16_t)insn_get(env, s, MO_16)); > + gen_bnd_jmp(s); > + gen_jmp_rel(s, dflag, diff, 0); > } > - tval += s->pc - s->cs_base; > - if (dflag == MO_16) { > - tval &= 0xffff; > - } else if (!CODE64(s)) { > - tval &= 0xffffffff; > - } > - gen_bnd_jmp(s); > - gen_jmp(s, tval); > break; > case 0xea: /* ljmp im */ > { > @@ -6876,12 +6875,10 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) > } > goto do_ljmp; > case 0xeb: /* jmp Jb */ > - tval = (int8_t)insn_get(env, s, MO_8); > - tval += s->pc - s->cs_base; > - if (dflag == MO_16) { > - tval &= 0xffff; > + { > + int diff = (int8_t)insn_get(env, s, MO_8); > + gen_jmp_rel(s, dflag, diff, 0); > } > - gen_jmp(s, tval); > break; > case 0x70 ... 0x7f: /* jcc Jb */ > tval = (int8_t)insn_get(env, s, MO_8); > -- > 2.34.1 >