On 12/12/23 13:08, Paolo Bonzini wrote:
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.
Good catch, I'll squash it. Thanks.
r~