Skip to content

Commit

Permalink
Fixed a bug where the translator was using an incorrect physical addr…
Browse files Browse the repository at this point in the history
…ess when the next instruction started at a page boundary
  • Loading branch information
ergo720 committed Nov 19, 2023
1 parent 74a1421 commit ab0d62b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 10 deletions.
2 changes: 1 addition & 1 deletion lib86cpu/core/emitter/x64/jit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ lc86_jit::gen_tc_epilogue()
{
// update the eip if we stopped decoding without a terminating instr
if (m_cpu->translate_next == 1) {
assert((DISAS_FLG_PAGE_CROSS | DISAS_FLG_ONE_INSTR) != 0);
assert((m_cpu->disas_ctx.flags & (DISAS_FLG_PAGE_CROSS | DISAS_FLG_PAGE_CROSS_NEXT | DISAS_FLG_ONE_INSTR)) != 0);
assert((m_cpu->tc->flags & TC_FLG_LINK_MASK) == 0);

MOV(MEMD32(RCX, CPU_CTX_EIP), m_cpu->virt_pc - m_cpu->cpu_ctx.regs.cs_hidden.base);
Expand Down
17 changes: 9 additions & 8 deletions lib86cpu/core/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ void JIT_API tlb_invalidate_(cpu_ctx_t *cpu_ctx, addr_t addr);
#define MMU_SET_CODE (1 << 4)

// disassembly context flags
#define DISAS_FLG_CS32 (1 << 0)
#define DISAS_FLG_SS32 (1 << 1)
#define DISAS_FLG_PAGE_CROSS (1 << 2)
#define DISAS_FLG_INHIBIT_INT (1 << 3)
#define DISAS_FLG_PE HFLG_PE_MODE // (1 << 4)
#define DISAS_FLG_FETCH_FAULT DISAS_FLG_PAGE_CROSS // (1 << 2)
#define DISAS_FLG_DBG_FAULT DISAS_FLG_PAGE_CROSS // (1 << 2)
#define DISAS_FLG_ONE_INSTR CPU_DISAS_ONE // (1 << 7)
#define DISAS_FLG_CS32 (1 << 0)
#define DISAS_FLG_SS32 (1 << 1)
#define DISAS_FLG_PAGE_CROSS (1 << 2)
#define DISAS_FLG_INHIBIT_INT (1 << 3)
#define DISAS_FLG_PAGE_CROSS_NEXT (1 << 5)
#define DISAS_FLG_PE HFLG_PE_MODE // (1 << 4)
#define DISAS_FLG_FETCH_FAULT DISAS_FLG_PAGE_CROSS // (1 << 2)
#define DISAS_FLG_DBG_FAULT DISAS_FLG_PAGE_CROSS // (1 << 2)
#define DISAS_FLG_ONE_INSTR CPU_DISAS_ONE // (1 << 7)

// tc struct flags/offsets
#define TC_JMP_DST_PC 0
Expand Down
6 changes: 5 additions & 1 deletion lib86cpu/core/translate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -909,8 +909,12 @@ cpu_translate(cpu_t *cpu)
if (ZYAN_SUCCESS(status)) {
// successfully decoded

// NOTE: the second OR for disas_ctx->flags is to handle the edge case where the last byte of the current instructions ends exactly at a page boundary. In this case,
// the current block can be added to the code cache (so DISAS_FLG_PAGE_CROSS should not be set), but the translation of this block must terminate now (so
// DISAS_FLG_PAGE_CROSS_NEXT should be set)
cpu->instr_bytes = instr.i.length;
disas_ctx->flags |= ((disas_ctx->virt_pc & ~PAGE_MASK) != ((disas_ctx->virt_pc + cpu->instr_bytes - 1) & ~PAGE_MASK)) << 2;
disas_ctx->flags |= ((disas_ctx->virt_pc & ~PAGE_MASK) != ((disas_ctx->virt_pc + cpu->instr_bytes) & ~PAGE_MASK)) << 5;

This comment has been minimized.

Copy link
@PatrickvL

PatrickvL Nov 20, 2023

If this check can be turned into a function (and obviously used everywhere possible), then that call might shorten these lines enough to allow mention of DISAS_FLG_PAGE_CROSS_NEXT (instead of the somewhat magic shift by 5)

disas_ctx->pc += cpu->instr_bytes;
disas_ctx->virt_pc += cpu->instr_bytes;

Expand Down Expand Up @@ -1572,7 +1576,7 @@ cpu_translate(cpu_t *cpu)
cpu->virt_pc += cpu->instr_bytes;
cpu->tc->size += cpu->instr_bytes;

} while ((cpu->translate_next | (disas_ctx->flags & (DISAS_FLG_PAGE_CROSS | DISAS_FLG_ONE_INSTR))) == 1);
} while ((cpu->translate_next | (disas_ctx->flags & (DISAS_FLG_PAGE_CROSS | DISAS_FLG_ONE_INSTR | DISAS_FLG_PAGE_CROSS_NEXT))) == 1);
}

uint32_t
Expand Down

0 comments on commit ab0d62b

Please sign in to comment.