On Tue, Dec 12, 2023 at 6:25 PM Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> In 32-bit mode, pc = eip + cs_base is also 32-bit, and must wrap.
> Failure to do so results in incorrect memory exceptions to the guest.
> Before 732d548732ed, this was implicitly done via truncation to
> target_ulong but only in qemu-system-i386, not qemu-system-x86_64.
>
> To fix this, we must add conditional zero-extensions.
> Since we have to test for 32 vs 64-bit anyway, note that cs_base
> is always zero in 64-bit mode.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2022
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
>
> This may be too late for 8.2; if not, then 8.2.1 and 8.1.next.
> I think I have found all forms of pc <-> eip, but another set
> of eyes would be appreciated.

Looks good, but perhaps you could also squash the following?

diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index 2c6a12c8350..83ee89579b8 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -52,7 +52,11 @@ static void x86_cpu_synchronize_from_tb(CPUState *cs,
     /* The instruction pointer is always up to date with CF_PCREL. */
     if (!(tb_cflags(tb) & CF_PCREL)) {
         CPUX86State *env = cpu_env(cs);
-        env->eip = tb->pc - tb->cs_base;
+        if (tb->flags & HF_CS64_MASK) {
+            env->eip = tb->pc;
+        } else {
+            env->eip = (uint32_t) (tb->pc - tb->cs_base);
+        }
     }
 }


It wouldn't be the same bug as 2022 (it wouldn't be new with the vaddr
change) so it's okay to sort out this extra case after release.

Paolo


Reply via email to