[PATCH v2 0/2] accel/tcg: fix crash on instruction straddling address-space end

[PATCH v2 0/2] accel/tcg: fix crash on instruction straddling address-space end

[Richard Henderson]

Handle address space wraparound during translation, with help from
the target translator.

Without cooperation of the target translator, unless this is a 64-bit
guest, this will no longer assert but is unlikely to function
correctly as generic code doesn't know the address space width.

Thanks to Tao Cui for a brilliant test case.


r~


Richard Henderson (1):
  accel/tcg: Allow some address space wraparound during translation

Tao Cui (1):
  tests/tcg/i386/system: Add regression test for translator_ld
    wraparound

 include/exec/translator.h          |  3 ++
 accel/tcg/translator.c             | 45 +++++++++++++++++++-----------
 tests/tcg/i386/system/meson.build  |  4 +++
 tests/tcg/i386/system/wraparound.S | 35 +++++++++++++++++++++++
 4 files changed, 70 insertions(+), 17 deletions(-)
 create mode 100644 tests/tcg/i386/system/wraparound.S

-- 
2.53.0

[PATCH v2 1/2] accel/tcg: Allow some address space wraparound during translation

[Richard Henderson]

Allow the two pages of translation to be non-contiguous.
As long as the target translator wraps pc properly, this
allows address space wraparound to function correctly.

This avoids an assert on a valid i386 system-mode test case.

Reported-by: Tao Cui <cui.tao@linux.dev>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/translator.h |  3 +++
 accel/tcg/translator.c    | 45 ++++++++++++++++++++++++---------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/exec/translator.h b/include/exec/translator.h
index 978dee25add..104e6d4f361 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -55,6 +55,8 @@ typedef enum DisasJumpType {
  * @pc_first: Address of first guest instruction in this TB.
  * @pc_next: Address of next guest instruction in this TB (current during
  *           disassembly).
+ * @pc_second_page: Address of the beginning of the second page of this TB,
+ *                  or -1 if the TB does not yet extend to a second page.
  * @is_jmp: What instruction to disassemble next.
  * @num_insns: Number of translated instructions (including current).
  * @max_insns: Maximum number of instructions to be translated in this TB.
@@ -69,6 +71,7 @@ struct DisasContextBase {
     TranslationBlock *tb;
     vaddr pc_first;
     vaddr pc_next;
+    vaddr pc_second_page;
     DisasJumpType is_jmp;
     int num_insns;
     int max_insns;
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 57daded60ff..52e605708f3 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -134,6 +134,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
     db->tb = tb;
     db->pc_first = pc;
     db->pc_next = pc;
+    db->pc_second_page = -1;
     db->is_jmp = DISAS_NEXT;
     db->num_insns = 0;
     db->max_insns = *max_insns;
@@ -285,16 +286,18 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db,
     /*
      * The read must conclude on the second page and not extend to a third.
      *
-     * TODO: We could allow the two pages to be virtually discontiguous,
-     * since we already allow the two pages to be physically discontiguous.
-     * The only reasonable use case would be executing an insn at the end
-     * of the address space wrapping around to the beginning.  For that,
-     * we would need to know the current width of the address space.
-     * In the meantime, assert.
+     * TODO: This doesn't handle address space wraparound properly for
+     * multi-byte reads, as we don't know the size of the address space here.
+     * But if the target translator wraps pc to 0 itself, and issues aligned
+     * reads, then this can work.
      */
-    base = (base & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
-    assert(((base ^ pc) & TARGET_PAGE_MASK) == 0);
-    assert(((base ^ last) & TARGET_PAGE_MASK) == 0);
+    if (db->pc_second_page == -1) {
+        db->pc_second_page = pc & TARGET_PAGE_MASK;
+    } else {
+        assert((pc & TARGET_PAGE_MASK) == db->pc_second_page);
+    }
+    assert((last & TARGET_PAGE_MASK) == db->pc_second_page);
+    base = db->pc_second_page;
     host = db->host_addr[1];
 
     if (host == NULL) {
@@ -372,16 +375,24 @@ static void record_save(DisasContextBase *db, vaddr pc,
 {
     int offset;
 
-    /* Do not record probes before the start of TB. */
-    if (pc < db->pc_first) {
-        return;
-    }
-
     /*
-     * In translator_access, we verified that pc is within 2 pages
-     * of pc_first, thus this will never overflow.
+     * In translator_ld, we verified that we touched no more than 2 pages,
+     * but we did not verify that they were virtually contiguous.
+     * Here, reimagine the two pages as virtually contiguous.
      */
-    offset = pc - db->pc_first;
+    if (likely(((db->pc_first ^ pc) & TARGET_PAGE_MASK) == 0)) {
+        /* first page */
+        /* Do not record probes before the start of TB. */
+        if (pc < db->pc_first) {
+            return;
+        }
+        offset = pc - db->pc_first;
+    } else {
+        int first_page_end_offset = -(db->pc_first | TARGET_PAGE_MASK);
+        assert(db->pc_second_page != -1);
+        assert((pc & TARGET_PAGE_MASK) == db->pc_second_page);
+        offset = pc - db->pc_second_page + first_page_end_offset;
+    }
 
     /*
      * Either the first or second page may be I/O.  If it is the second,
-- 
2.53.0

[PATCH v2 2/2] tests/tcg/i386/system: Add regression test for translator_ld wraparound

[Richard Henderson]

From: Tao Cui <cuitao@kylinos.cn>

Add a test case that reaches an instruction straddling the end of the 32-bit
address space (0xfffffffe).

The top page (0xfffff000) is SeaBIOS ROM, so the cross-boundary byte is
the ROM's own 0x00 (add r/m8, r8) at 0xffffffff, whose modrm is fetched
from [0x0]. A short exit stub is placed there.  The case runs on
qemu-system-i386 since the bug is 32-bit only.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Message-ID: <20260709020529.126652-3-cui.tao@linux.dev>
[rth: Simplify and convert to meson test harness]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/i386/system/meson.build  |  4 ++++
 tests/tcg/i386/system/wraparound.S | 35 ++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 tests/tcg/i386/system/wraparound.S

diff --git a/tests/tcg/i386/system/meson.build b/tests/tcg/i386/system/meson.build
index d3f73997c7f..c08563d0ab5 100644
--- a/tests/tcg/i386/system/meson.build
+++ b/tests/tcg/i386/system/meson.build
@@ -34,6 +34,10 @@ foreach t: tcg_tests['multiarch-softmmu']['tests']
   endforeach
 endforeach
 
+tests += {
+  'wraparound.S': { 'cflags': cflags, 'qemu_args': ['-m', '4G'] + qemu_def_args }
+}
+
 if 'qemu-system-i386' in emulators
   tcg_tests += {
     'i386-softmmu': {
diff --git a/tests/tcg/i386/system/wraparound.S b/tests/tcg/i386/system/wraparound.S
new file mode 100644
index 00000000000..9c77b3a8468
--- /dev/null
+++ b/tests/tcg/i386/system/wraparound.S
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Regression test for the translator_ld crash when an instruction
+ * straddles the end of the 32-bit address space (i386).
+ */
+
+        .code32
+        .section .text
+
+main:
+        /*
+         * The top page (0xfffff000) is SeaBIOS ROM and cannot be written.
+         * Its byte at 0xffffffff (0x00 = "add r/m8, r8") already crosses the
+         * page boundary into page1 at 0x0, which is exactly the case
+         * translator_ld must handle without aborting.  Reaching 0xfffffffe
+         * runs the ROM's cld, then that add; the add's modrm is fetched from
+         * [0x0], which is RAM, so build a short exit stub there:
+         *
+         *   [0x0] c0                 modrm -> "add al, al" (reg; EIP -> 1)
+         *   [0x1] c3                 ret
+         *
+         * Note: this relies on the SeaBIOS byte at 0xffffffff being 0x00
+         * (add r/m8, r8); if that ever changes, the stub below must move.
+         *
+         * Note that eax = 0 before and after the stub, so this becomes
+         * the exit code of the test.
+         */
+        xor     %eax, %eax
+        movw    $0xc3c0, (%eax)
+        movl    $0xfffffffe, %ecx
+        jmp     *%ecx
+
+        .globl  main
+        .type   main, @function
+        .size   main, . - main
-- 
2.53.0

Re: [PATCH v2 2/2] tests/tcg/i386/system: Add regression test for translator_ld wraparound

[Jim MacArthur]

On Wed, Sep 16, 2026 at 01:51:58PM -1000, Richard Henderson wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> Add a test case that reaches an instruction straddling the end of the 32-bit
> address space (0xfffffffe).
> 
> The top page (0xfffff000) is SeaBIOS ROM, so the cross-boundary byte is
> the ROM's own 0x00 (add r/m8, r8) at 0xffffffff, whose modrm is fetched
> from [0x0]. A short exit stub is placed there.  The case runs on
> qemu-system-i386 since the bug is 32-bit only.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> Message-ID: <20260709020529.126652-3-cui.tao@linux.dev>
> [rth: Simplify and convert to meson test harness]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/tcg/i386/system/meson.build  |  4 ++++
>  tests/tcg/i386/system/wraparound.S | 35 ++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>  create mode 100644 tests/tcg/i386/system/wraparound.S
> 
> diff --git a/tests/tcg/i386/system/meson.build b/tests/tcg/i386/system/meson.build
> index d3f73997c7f..c08563d0ab5 100644
> --- a/tests/tcg/i386/system/meson.build
> +++ b/tests/tcg/i386/system/meson.build
> @@ -34,6 +34,10 @@ foreach t: tcg_tests['multiarch-softmmu']['tests']
>    endforeach
>  endforeach
>  
> +tests += {
> +  'wraparound.S': { 'cflags': cflags, 'qemu_args': ['-m', '4G'] + qemu_def_args }
> +}
> +
>  if 'qemu-system-i386' in emulators
>    tcg_tests += {
>      'i386-softmmu': {
> diff --git a/tests/tcg/i386/system/wraparound.S b/tests/tcg/i386/system/wraparound.S
> new file mode 100644
> index 00000000000..9c77b3a8468
> --- /dev/null
> +++ b/tests/tcg/i386/system/wraparound.S
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Regression test for the translator_ld crash when an instruction
> + * straddles the end of the 32-bit address space (i386).
> + */
> +
> +        .code32
> +        .section .text
> +
> +main:
> +        /*
> +         * The top page (0xfffff000) is SeaBIOS ROM and cannot be written.
> +         * Its byte at 0xffffffff (0x00 = "add r/m8, r8") already crosses the
> +         * page boundary into page1 at 0x0, which is exactly the case
> +         * translator_ld must handle without aborting.  Reaching 0xfffffffe
> +         * runs the ROM's cld, then that add; the add's modrm is fetched from
> +         * [0x0], which is RAM, so build a short exit stub there:
> +         *
> +         *   [0x0] c0                 modrm -> "add al, al" (reg; EIP -> 1)
> +         *   [0x1] c3                 ret
> +         *
> +         * Note: this relies on the SeaBIOS byte at 0xffffffff being 0x00
> +         * (add r/m8, r8); if that ever changes, the stub below must move.
> +         *
> +         * Note that eax = 0 before and after the stub, so this becomes
> +         * the exit code of the test.
> +         */
> +        xor     %eax, %eax
> +        movw    $0xc3c0, (%eax)
> +        movl    $0xfffffffe, %ecx
> +        jmp     *%ecx
> +
> +        .globl  main
> +        .type   main, @function
> +        .size   main, . - main
> -- 
> 2.53.0

Nice test case!

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>

Re: [PATCH v2 1/2] accel/tcg: Allow some address space wraparound during translation

[Jim MacArthur]

On Wed, Sep 16, 2026 at 01:51:57PM -1000, Richard Henderson wrote:
> Allow the two pages of translation to be non-contiguous.
> As long as the target translator wraps pc properly, this
> allows address space wraparound to function correctly.
> 
> This avoids an assert on a valid i386 system-mode test case.
> 
> Reported-by: Tao Cui <cui.tao@linux.dev>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/translator.h |  3 +++
>  accel/tcg/translator.c    | 45 ++++++++++++++++++++++++---------------
>  2 files changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 978dee25add..104e6d4f361 100644

Reviewed-by: Jim MacArthur <jim.macarthur@linaro.org>