A 16-bit near branch must truncate EIP to 16 bits. gen_jmp_rel() skips that when the target lands on the same guest page as the translation block, taking a branch that stayed on the page as proof the addition did not wrap.
That holds for the linear address, but EIP wraps at 0x10000, and the two only coincide when cs_base is page aligned. Otherwise two linear addresses can share a page while the EIP values either side of them straddle 0x10000, and a CALL or JMP rel16 runs with EIP untruncated. Compare the source and destination EIP pages instead. It has to stay a page test rather than a check on the EIP values: a CF_PCREL TB is not keyed on the virtual PC and records only its page offset, so the same TB can later run with EIP shifted by whole pages. One case is left over: a 16-bit segment with a limit above 0xffff (D/B clear, G set), where EIP can start out above the mask. The data16 guard keys on D/B and not the limit, so it does not catch that either. Closing it needs the limit in the TB key. 64-bit mode is unaffected, as no masking is applied there. Signed-off-by: Paul Galbraith <[email protected]> --- v1: https://patchew.org/QEMU/[email protected]/ Changes in v2: - Different approach. v1 required cs_base to be page aligned before allowing goto_tb, and dropped the data16-in-code32 guard on the assumption that the new condition subsumed it. It does not: that guard is there for a separate case, EIP starting out above the mask, so v1 would have swapped one problem for another. v2 compares source and destination EIP pages and leaves the guard alone. - Split into two patches; the regression test is now patch 2. target/i386/tcg/translate.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index d8de290acb..6d3c929eee 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -2025,12 +2025,24 @@ static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num) if (tb_cflags(s->base.tb) & CF_PCREL) { tcg_gen_addi_tl(cpu_eip, cpu_eip, new_pc - s->pc_save); + + /* + * True if a wrap cannot be ruled out: the source and destination EIP + * are in different EIP pages. If they share one, the addition cannot + * leave [0, mask] on any rerun of this TB, which only ever shifts EIP + * by whole pages. + */ + bool eip_may_wrap = !CODE64(s) && + (((s->pc_save - s->cs_base) ^ (new_pc - s->cs_base)) + & TARGET_PAGE_MASK) != 0; + /* - * If we can prove the branch does not leave the page and we have - * no extra masking to apply (data16 branch in code32, see above), - * then we have also proven that the addition does not wrap. + * If we can prove the branch does not leave the page, does not leave + * its EIP page, and we have no extra masking to apply (data16 branch + * in code32, see above), then the addition does not wrap. */ - if (!use_goto_tb || !translator_is_same_page(&s->base, new_pc)) { + if (!use_goto_tb || !translator_is_same_page(&s->base, new_pc) + || eip_may_wrap) { tcg_gen_andi_tl(cpu_eip, cpu_eip, mask); use_goto_tb = false; } -- 2.54.0.windows.1
