On 8/14/26 07:21, Ilya Chichkov wrote:
record_save() assumed that a target reads the bytes of an insn as a
strictly ascending sequence of adjacent chunks, and asserted that each
read begins exactly where the previous one ended.

That assumption no longer holds for riscv.  Since f9eaa1542b
("target/riscv: support atomic instruction fetch (Ziccif)"),
decode_opc() loads a full aligned word whenever pc is 4-byte aligned,
even when the insn turns out to be a 2-byte compressed one, so the
record may already hold bytes past the end of the insn being
translated.  When such a compressed insn sits at page offset 0xffc,
pc_next becomes 0xffe, which is within MAX_INSN_LEN of the end of the
page, and riscv_tr_translate_insn() probes the next insn to decide
whether it would cross the page boundary.  That probe reads at offset
2 while the record already covers [0,4), and the assert fires:

   qemu-system-riscv32: accel/tcg/translator.c:395: record_save:
   Assertion `offset == db->record_start + db->record_len' failed.

record_save() is only reached when the insn is fetched from MMIO, so
this is visible on boards that execute code from a region created with
memory_region_init_io(), such as an XIP flash window mapped over a
serial flash controller.

Both sides of the collision are correct: the wide fetch is required for
Ziccif atomicity, and the probe is required for correct fault reporting
at a page boundary, per 00c07344fa ("target/riscv: Make translator stop
before the end of a page").  Unlike a86d3352ab ("target/riscv: do not
use translator_ldl in opcode_at"), where a non-translation caller had
no business using translator_ld*, the probe here is a genuine
translation read whose bytes must be recorded.

Relax the invariant instead.  Keep requiring that a read neither moves
backwards nor leaves a gap, but let a read overlapping the recorded
range extend it only by the bytes past its end.

Fixes: f9eaa1542b ("target/riscv: support atomic instruction fetch (Ziccif)")
Signed-off-by: Ilya Chichkov <[email protected]>
---
  accel/tcg/translator.c | 14 +++++++++++---
  1 file changed, 11 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <[email protected]>

Applied to tcg-next.

r~


Reply via email to